Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cargo version pinning #1255

Merged
merged 2 commits into from
Nov 28, 2023
Merged

Cargo version pinning #1255

merged 2 commits into from
Nov 28, 2023

Conversation

graydon
Copy link
Contributor

@graydon graydon commented Nov 28, 2023

This does the env side of #1222 -- there are still core-side things to do but this is a step.

No packages are changed in the main lockfile here, just the one in the fuzzer sub-workspace which was out of sync.

@graydon graydon requested review from sisuresh, dmkozh and a team as code owners November 28, 2023 05:31
Copy link
Member

@leighmcculloch leighmcculloch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

Questions that don't need to impede merging:

I don't see us adding locking in the publish commands. Is that necessary?

Do we plan to pin transitive deps too some way? This change is only pinning direct deps.

@graydon graydon added this pull request to the merge queue Nov 28, 2023
Merged via the queue into main with commit b357bda Nov 28, 2023
10 checks passed
@graydon graydon deleted the cargo-version-pinning branch November 28, 2023 07:03
@graydon
Copy link
Contributor Author

graydon commented Nov 28, 2023

Transitive deps get pinned in the trees baked into core in the core build. We don't have anything doing that at the plain env layer. It might be nice, but the core build is the level that's most worried about minor deviation.

I don't know if locking is necessary for publishing, that's an interesting question. I assumed not. It looks like maybe it does support --locked also. I guess I will open another PR with that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants