-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
RFC: Support versioned dependencies on build tools, such as cargo and rustc #1707
Conversation
Heh, I was just about to write something like this. I think rather than working on tools, this should work on languages, i.e. the Since Cargo is pre 1.0 (officially), we don't have much leeway to leverage Cargo's own semver (no non-breaking minor changes) But since crates.io is live it's definitely safe to say w are on >=1.0 for the Cargo.toml language. Likewise, other Rust compilers may want to be versioned differently, so it's best to separate language and compiler version. See #1133 for how @brson wanted to have compilers specify the language version. |
My motivation, and initial idea of implementation, were sufficiently different that I wrote a separate RFC #1709 . Feel free to take the motivation and adapt it for this one however. |
How about some crate that simplifies the task of checking that in build.rs? |
@Ericson2314 I like the concept of RFC #1709, but I'd suggest using the dependency syntax as this RFC does, for the same reason discussed in the alternatives section here: doing so allows it to appear anywhere a dependency can appear, such as in dev-dependencies, or platform-specific dependencies, or named-feature-specific dependencies. So, what about |
@KalitaAlexey I'd like something completely declarative that Cargo can check in advance, rather than something checked by build.rs at build time. A declarative dependency on |
Document the advantages of declarative dependencies over build-time detection.
ping @rust-lang/tools - nominating for discussion at next meeting since there's been no movement since August. |
Thanks for the PR! We discussed this at the tools meeting this week. We agree there is merit in being able to tie projects to working versions of tools; this has been brought up in the past particularly in the case of nightly, where crates will sometimes only build with certain versions of Rust. However, we saw several downsides to this approach:
Therefore we prefer the approach in #1709 and I propose closing this PR in favour of that one. @rfcbot: fcp close |
Team member @nrc has proposed to close this. The next step is review by the rest of the tagged teams: No concerns currently listed. Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
@rfcbot reviewed |
@nrc I'd very much like to have support for specifying the version of Rust as well. However, the Rust version does not inherently imply a specific Cargo version, as distributions package them separately. So, we still need separate versions for Rust and for Cargo. In addition, I'm not arguing for just "showing an error message"; while that'd be useful, I'd primarily like this metadata to 1) provide appropriate minimum dependencies, 2) document and test minimum version requirements, and most importantly 3) allow older versions of Cargo/Rust (after this point) to detect that they can't handle a certain version of a library, and go looking for an older version that supports that version of Cargo/Rust. On top of that, the approach in #1709 fails to parse with existing Cargo (unlike this approach), doesn't support any other tool, and doesn't integrate with all the existing Cargo facilities to make dependencies optional. For instance, consider a crate that needs Rust 1.14 with a given feature enabled, but Rust 1.10 with that feature disabled. The approach in #1709 can't support that; the approach in this RFC can. If the primary concern is that you care more about specifying the version of Rust than the version of Cargo, how about if I withdraw this RFC from consideration for now, and submit a new version of it that specifies how to handle Rust, Cargo, and potentially other tools? |
I think that dismissing this approach as a complex way of just showing an error message is missing the key advantage this provides. One of the big unresolved questions in the Rust ecosystem right now is how to deal with increasing the required version of Rust in crates. For example, @BurntSushi spent a lot of effort ensuring that regex 0.1.x continued to compile with Rust 1.3, as updating the language required would potentially break users; if you were using an older compiler, and did With this in place, Cargo should essentially treat it as if you have an implicit exact version dependency on the tools that you are running. That would conflict with the greater version dependency in the newer version of regex, which means the resolver should be able to pick the highest version of regex that still supports the compiler you are running. That would make updating the compiler version requirement in regex no longer a breaking change;
I think that the same is true for versioned dependencies on libraries; you may not actually test against the minimum versions that you specify, or newer versions may turn out to be accidentally incompatible. For applications, this is helped by lockfiles, but it's still possible for libraries to make mistakes and someone using the library to be broken by a I don't think that this is a reason to reject this feature, as the problem already exists for version ranges specified by libraries, it's just an impetus for other tooling improvements to, say, do CI testing that tries out various different sets of packages that all satisfy the version dependencies, both for tools and libraries. This feature would give the ability to conscientious maintainers, like @BurntSushi, to specify the version of I agree with @joshtriplett that this proposal seems more compatible with existing versions of Cargo, and more flexible than the one in #1709. I think that this proposal should probably be rewritten to cover its use for specifying the version of Rust required, and in particular, flesh out whether that's done by a |
@lambda Thank you for your detailed comment. I agree that this could make it much easier to maintain support for older versions of Rust and Cargo. For instance, a tool could detect if a crate that declares compatibility with a given version has a dependency (direct or indirect) on a crate that doesn't. We could also teach Travis and similar CI to understand these declarations, to build and test with the declared minimum versions supported, making sure the declared versions work. At the moment, versions of the standard library and rustc go hand-in-hand, and using a newer feature from either one means you depend on that version of Rust/rustc, so I don't think it makes sense to declare versions on them separately. However, the same framework would work for some future mechanism to declare an explicit dependency on "std" or parts of it, such as with xargo. So, for now I'd opt for |
OK, I've updated the RFC to include support for Given the new version of the RFC, would it make sense to defer FCP to let people re-review the new version, particularly given the overlap with the holiday season? |
Something I've run in to plenty in the nodejs world is packages trying to force people to update their node or npm version, even if not using any new features. Usually library developers who cannot fathom there being a cost to upgrading. Thankfully, npm only views these directives as warnings. I sincerely hope cargo only warns, but doesn't fail when tool versions don't match. |
@seanmonstar Good call. I suspect that the Rust community will do a better job with that (given that many Rust developers consciously pay attention to what they depend on and aim to support older versions for many crates), but I'll add something to the RFC about that. |
Based on feedback from Sean McArthur.
@manuel-woelker Yeah, I had rustup in mind when hinting at other tools doing something more with the version information. And yes, explicit declarations like "clippy requires nightly" would help massively. |
@rfcbot reviewed |
Another similar request I've made in Rustup: rust-lang/rustup#460 FWIW, I would also prefer for this information to be Cargo.toml |
I think version pinning and version requirements are something that will happen either officially or non-officially at some point. I believe the std-aware cargo RFC also effectively adds language-version requirements to cargo, and that would need to be taken into account before merging. I'm afraid I'm not up to speed on the details of this particular RFC because it's not a priority feature for me and since it seemed like it was going to be closed, but as with many rfcs, there has been a lot of discussion post-fcp nomination continuing to argue for it. This feature will probably require more focused attention from @alexcrichton or maybe myself and other tools maintainers to get it through. |
@brson I've read the std-aware cargo RFC; it helps declare versions on libraries, but not on rustc or cargo themselves. For instance, crates using Regarding tools development: I will happily write a patch for Cargo to implement this, if that helps. |
Looking over this again, @nrc proposed awhile ago to close this RFC in favor of #1709, which is an RFC for language version requirements rather than tool version requirements. I personally still feel like this is the correct approach if we were to implement this. @joshtriplett you mention that we need separate versions of Rust and Cargo, but I disagree with this. Effectively Rust and Cargo have the same various as they progress forward in history. Currently Cargo bends over backwards to support multiple rustc compilers, but we're very unlikely to continue doing this into the future. Instead it will be the case that given a Rust compiler there's only one version of Cargo guaranteed to work with that compiler (the one that it was released with). With that in mind it means that while functionally there are two versions in practice there's only one, the compiler. That aspect is covered in #1709 which is why I'm still in favor of closing this in favor of that RFC (in terms of the idea at least). @lambda yeah you're definitely correct that it's more than just an error message if this affects Cargo's dependency resolution as well. That, however, is a whole other can of worms, but I'm ok continuing the conversation on #1709. |
The most important difference between #1707 and #1709 isn't whether we have one version number or two, or whether one of the version numbers refers to "rust the language" or "rustc the compiler". (Though for Linux distributions it's still important to not assume tight coupling between rustc and cargo.) The most important difference between the two is how to handle the version number and dependencies on it: via the existing Cargo dependency mechanism that can vary with feature flags, dev-dependencies, and similar (#1707), or via a completely separate field with no ability to vary with any of those (#1709). (And, as a side effect, whether old versions of Cargo treat the version requirement as a dependency failure (#1707) or silently ignore it and then fail later in more mysterious ways (#1709).) If #1709 used the existing Cargo dependency mechanism, and supported integration with feature flags, dev-dependencies, and similar, then I'd withdraw this RFC. That one difference is why I'll keep poking at this. |
@alexcrichton I see the benefits of making sure that Cargo and rustc will released together, but that to me is even more version to track the @joshtriplett I don't think we actually disagree on much, principle-wise :), at least that's my take-away from both these threads. There's just the specifics of how to make sure old Cargo treats the new syntax correctly. I'm not wild about mashing up tools and library depends together, but I absolutely agree that integration with our existing conditionals (features, targets, etc) is a must. Main thing is I've not gotten around to updating #1709. [To be honest, with the way my other Cargo RFC has sat idle, I haven't felt much of a rush to do so. But if somebody pings me resolving either RFC is "on deck", I'll hop-to.] |
@Ericson2314 To make things easier for the tools team to resolve, would you be interested in collaborating on a single revised RFC, incorporating the key learnings and feedback from both #1707 and #1709? I'd be happy to work with you on that; perhaps we could set up a time for a real-time conversation, and then perhaps put together an RFC in Etherpad? I've also done a fair bit of testing with Cargo to evaluate how it handles various kinds of syntax extensions; we could work out what behavior we want, and make sure that whatever syntax we come up with matches that behavior (since we can't retroactively make existing versions of Cargo behave differently). |
@joshtriplett sure! |
@eternaleye I, at least, would appreciate if you could join too :). |
@Ericson2314 I'm definitely interested |
@Ericson2314 Looks like you're on US/Eastern time? I'm in US/Pacific. Let's meet on IRC #rust some time compatible with those, and coordinate from there. Would tomorrow (the 12th) work for you? If so, pick any time you like. If not, we can coordinate something during the following week. |
@joshtriplett sounds good! Any time not too earlier should be good for me, but with the time difference that should fall out for free. I'll be loitering on IRC. |
@Ericson2314 Sounds great. I'll stay logged in; ping me via /msg on irc.mozilla.org whenever you're ready. IRC nick JoshTriplett. |
Summarizing the discussions today: @Ericson2314, @eternaleye, and I met on IRC, talked through the problem, and ended up drafting a successor RFC via Etherpad for both #1707 and #1709. We'll submit that RFC in the near future; that will serve as a baseline to solve the most critical core problem, and then we can build further work on top of that. |
This RFC makes it possible to introduce new Cargo features that older versions of Cargo would otherwise misunderstand, such as new types of dependencies or dependency semantics; the new schema version prevents those versions of Cargo from silently ignoring those features and misbehaving, and allows dependency resolution to work appropriately for both old and new Cargo. This serves as a successor to both rust-lang#1707 and rust-lang#1709, and provides a basis for other Cargo feature RFCs to build on. Co-authored by Josh Triplett (rust-lang#1707), @Ericson2314 (rust-lang#1709), and Alex Elsayed (@eternaleye).
As the original proposer of this RFC, I'd like to formally withdraw it in favor of #1953. |
@joshtriplett don't we technically need to do another on top of that, for language and tool dependencies? |
@Ericson2314 We need another one on top for Rust language and/or rustc version, and I'd like to help with that RFC as well. But my original goal with #1707 was to handle dependencies on Cargo, and #1953 does that. |
@joshtriplett Oh right, I missed/forget that during our discussion :). I guess I might keep #1709 open for the moment then? Just don't want to come off as duplicitous doing so :). I will certainly amend it to use/require #1953. |
I'm going to close this out, given that its author, @joshtriplett, has said that #1953 subsumes it. |
Rendered