Update crucible version #262
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Created using spr 1.3.4
This was referenced Dec 2, 2022
gjcolombo
approved these changes
Dec 2, 2022
|
Thanks! |
|
My pleasure--thanks for the changes! (I've made basically the same edits locally for some other work I'm doing and can confirm that all this seems to work fine with the Crucible integration tests I'm writing.) |
sunshowers
added a commit
to oxidecomputer/omicron
that referenced
this pull request
Dec 5, 2022
This is PR 3 of N in this series. For the earlier ones, see: * oxidecomputer/crucible#552 * oxidecomputer/propolis#262 Currently, we specify the progenitor dependency as both: ```toml progenitor-client = { git = "https://github.com/oxidecomputer/progenitor", branch = "main" } ``` And ```toml progenitor-client = { git = "https://github.com/oxidecomputer/progenitor" } ``` Cargo (due to rust-lang/cargo#7497) doesn't dedup these dependencies even though they resolve to the same branch and hash. This causes issues with `cargo doc` colliding with itself, as it tries to document both: ``` Documenting progenitor-impl v0.2.1-dev (https://github.com/oxidecomputer/progenitor#4b5cef4b) Documenting progenitor-impl v0.2.1-dev (https://github.com/oxidecomputer/progenitor?branch=main#4b5cef4b) ``` Fix this by always using the branch name. (We choose this option over not specifying the branch so to maintain uniformity in cases where a non-default branch must be used.) This also requires a crucible update, since that didn't specify a branch either. And the crucible update necessitates a propolis update to get the versions aligned. In the future it would be nice to have a lint which ensures that git dependencies always have the branch name in them.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is PR 2 of N to make sure that omicron doesn't get two separate
dependencies, one for progenitor with main and one for progenitor
without main.
For PR 1, see oxidecomputer/crucible#552.
Without this fix, omicron ends up with two separate versions of
crucible, which causes build failures in omicron.
The update caused some breakage from
oxidecomputer/crucible#516, which I've tried to
address (cc @leftwo to confirm that my changes look good).