Skip to content
This repository was archived by the owner on Dec 29, 2022. It is now read-only.

cargo update, pin "cargo" to 6a8eb71f6 #680

Merged
merged 3 commits into from
Jan 29, 2018

Conversation

alexheretic
Copy link
Member

Lots of our dependencies are somewhat behind on patch versions. Some of these do affect us, like having env_logger 0.5.3. I would have thought the lock file was for ensuring one developers experience is the same as anothers & as the release - rather than longer term freezing of dependencies.

If the reason for the lack of wholesale semver-compatible updates is some specific ones we want to carefully control, would it be better to declaratively control these in the Cargo.toml file?

For this pr I've assumed cargo is the only such crate & have put a reference to the commit hash we're locked to & updated the rest.

[dependencies]
cargo = { git = "https://github.com/rust-lang/cargo", rev = "848eb156" }
# ...

Are they're others we want to control? Or perhaps I am missing a good reason to rarely update in this way?

@alexheretic
Copy link
Member Author

alexheretic commented Jan 24, 2018

On the point of cargo maybe we should consider generally using the commit hash that matches the current nightly. After all this hash will actually be sure to compile with the toolchain it was delivered with.

# cargo +nightly -vV
cargo 0.26.0-nightly (6a8eb71f6 2018-01-13)
release: 0.26.0
commit-hash: 6a8eb71f6d226f9ac869dbacd5ff6aa76deef1c4
commit-date: 2018-01-13

As such I'll update the cargo hash to this one. We could even test / warn that the cargo version is outdated to help us keep track of it.

@alexheretic alexheretic changed the title cargo update, pin "cargo" to 848eb156 cargo update, pin "cargo" to 6a8eb71f6 Jan 24, 2018
@nrc
Copy link
Member

nrc commented Jan 29, 2018

The point of the committed lock file is to have predictable breakage. I don't think we should put off doing updates, ideally we would do them frequently. Cargo in particular can cause problems when we update because of breaking changes in its API.

@nrc nrc merged commit 4beff21 into rust-lang:master Jan 29, 2018
@nrc
Copy link
Member

nrc commented Jan 29, 2018

Thanks for the PR!

@alexheretic alexheretic deleted the update-pin-cargo branch January 29, 2018 11:39
@alexheretic
Copy link
Member Author

Ok thanks! The explicit rev = HASH for cargo should allow cargo update to be less of an issue then.

What do you think of trying to keep the cargo version in line with cargo +nightly version, perhaps we could warn on build if the build cargo version is newer than the rls/Cargo.toml one?

@nrc
Copy link
Member

nrc commented Feb 1, 2018

Given that this has not been too much of a problem in practice, and when it has it has required a non-trivial amount of effort to fix, I don't think it is worth expending too much effort on this kind of solution.

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

Successfully merging this pull request may close these issues.

2 participants