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

Introduce Cargo schema versioning #1953

Closed

Conversation

joshtriplett
Copy link
Member

@joshtriplett joshtriplett commented Mar 16, 2017

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 #1707 and #1709, and provides a basis
for other Cargo feature RFCs to build on.

Co-authored by Josh Triplett (#1707), @Ericson2314 (#1709), and Alex
Elsayed (@eternaleye).

Rendered

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).
@joshtriplett joshtriplett added the T-dev-tools Relevant to the development tools team, which will review and decide on the RFC. label Mar 16, 2017
@bjadamson
Copy link

Can you post rendered for us mobilers?

@joshtriplett
Copy link
Member Author

@bjadamson Done.

@steveklabnik That link points to the version in the specific commit; I edited the original post to include a link to the latest version on the branch, in case we update that branch.

```ebnf
<pkg-meta> ::= { name = ..., version = ..., ... }
<package> ::= <pkg-meta>
| { <number> = <pkg-meta> }
Copy link
Contributor

@Ericson2314 Ericson2314 Mar 16, 2017

Choose a reason for hiding this comment

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

nit: should align | with second :, and add spaces after so the non-terminals line up too. [I just bullshitted something consistent in etherpad cause variable-width font.]

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

<package> ::= <pkg-meta>
| { <number> = <pkg-meta> }
| { <number> = { <number> = <pkg-meta> } }
<Cargofile> ::= { package = <package>, ... }
Copy link
Contributor

@Ericson2314 Ericson2314 Mar 16, 2017

Choose a reason for hiding this comment

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

Maybe add

            |   { project = <pkg-meta>, ... }

to make clear that the legacy [project] syntax does not work with explicit versions?

Copy link
Member Author

Choose a reason for hiding this comment

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

Rather than attempt to do that in the grammar, I'd rather just add a note in this section mentioning the historical section name and that schema versioning requires package.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

# Detailed design
[design]: #detailed-design

Introduce a new Cargo schema version, initially `1.0.0`. Cargo will increase
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how much we gain if anything from having a full semver-style version here. docker-compose just uses a single integer, for example. (I haven't read the RFC in detail yet, so this may have been covered elsewhere already 😄 )

Copy link
Contributor

@Ericson2314 Ericson2314 Mar 16, 2017

Choose a reason for hiding this comment

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

@sfackler I forget if it is, so let me add this: it conveys to the user that their old syntax is still valid (if they are copying and pasting, or editing an existing Cargo.toml).

One could likewise ask, "what's the point of Rust language backwards compat if crates written in different versions of rust can be used together?". I think a proposal like that for Rust would elicit a big reaction :).

Copy link
Member Author

Choose a reason for hiding this comment

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

@sfackler We really don't need the last component of a semver, the "patch version", since that only changes with implementation, and this schema version shouldn't take implementation into account. But I do see some value in having a major.minor version. That said, if we decided that the major version would never increase past 1 (meaning Cargo will never drop support for older schemas), then we could just have a single number.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well the matter of "what interfaces does Cargo support" is distinct from "does bumping this version break my existing Cargo.toml / build.rs".

The syntaxes used in these code blocks resembles EBNF and JSON,
respectively, but don't quite follow them entirely; remove the language
names to avoid incorrect highlighting.
@withoutboats
Copy link
Contributor

Two questions:

  • First, cargo is released in sync with Rust, at least today. There are additional motivations for specifying the minimum version of Rust this project supports, why not just fold this into that sort of proposal and make the version of Rust / version of cargo the same version numbers?

  • Second, the syntax seems quite odd, why not have a key under package like cargo-version (or rust-version after the previous point) which takes a semver value?

@joshtriplett
Copy link
Member Author

joshtriplett commented Mar 23, 2017

@withoutboats

First, cargo is released in sync with Rust, at least today. There are additional motivations for specifying the minimum version of Rust this project supports, why not just fold this into that sort of proposal and make the version of Rust / version of cargo the same version numbers?

Three reasons. First, note that this is not a Cargo version number; this is a Cargo schema version number. It won't necessarily change with every version of Cargo, and in fact shouldn't unless Cargo semantics change in some way. Two successive versions of Cargo may have identical schema versions, or might even have the schema version change by more than one (for convenience of intermediate development). So it has nothing to do with the version number of of either the Cargo tool or rustc.

Second, requiring that rustc and Cargo remain in perfect synchronization just doesn't work in Linux distributions. It would introduce serious bootstrapping issues. We may well want to (or need to) upload a newer Cargo first, or a newer rustc first. (For instance, we might want to package a newer cargo because it has patches we need to support packaging infrastructure.)

And third, because almost anything we'd do to introduce a dependency on the Rust language version required needs a Cargo schema upgrade to support; see below for why. So we started with Cargo rather than Rust, and a subsequent RFC will specify dependencies on Rust.

Second, the syntax seems quite odd, why not have a key under package like cargo-version (or rust-version after the previous point) which takes a semver value?

We documented that in the RFC; see the last couple of paragraphs of the Alternatives section. Current Cargo just ignores that key and proceeds, whereas the approach we used ensures that current Cargo stops and doesn't proceed on a schema it doesn't understand. That's exactly the goal here.

We extensively tested the behavior of current Cargo with various potential extensions to the Cargo.toml format, and this seemed like the least invasive syntax with the behavior we needed.

@withoutboats
Copy link
Contributor

Current Cargo just ignores that key and proceeds, whereas the approach we used ensures that current Cargo stops and doesn't proceed on a schema it doesn't understand. That's exactly the goal here.

This seems like one of the most fundamental questions of this RFC. The oddness of the syntax, the requirement that users understand a distinction between a "cargo schema," a "cargo version," and a "rust version" - all of this seems to be in order to support this goal that cargo doesn't procede on a crate using a schema it doesn't understand.

Why is this so important? As far as I can tell, the reason is to improve diagnostics when an old version of cargo fails to understand a Cargo.toml. Are there other reasons I don't understand?

I am very dubious about this motivation. First, as soon as you do add this feature, versions from that point on can behave appropriately. So even if you chose a "more normal" syntax, eventually the number of versions of cargo which can't behave responsibly in the face of schema upgrades would shrink toward 0.

Moreover, existing versions of cargo won't fail in a responsible way when they see a [package.1.0], they'll fail noisly and confusedly, just as they would if they ignore the schema key and tried to parse a section they don't understand. This doesn't seem like strong motivation to not just do the expected thing.

Now as to the question of whether this should be tied to the Rust version:

First, note that this is not a Cargo version number; this is a Cargo schema version number. It won't necessarily change with every version of Cargo, and in fact shouldn't unless Cargo semantics change in some way. Two successive versions of Cargo may have identical schema versions, or might even have the schema version change by more than one (for convenience of intermediate development). o it has nothing to do with the version number of of either the Cargo tool or rustc.

This isn't really correct: the schema version is a montonic function of the cargo version, because it only every increases when the cargo version increases. Because of the way semver works, this makes the cargo version an adequate if not perfect signal for the schema version. The only thing that could happen is you could require a cargo version higher than strictly necessary because there was no update on that version, but we can design the tooling to reduce the likelihood of that.

Second, requiring that rustc and Cargo remain in perfect synchronization just doesn't work in Linux distributions. It would introduce serious bootstrapping issues. We may well want to (or need to) upload a newer Cargo first, or a newer rustc first. (For instance, we might want to package a newer cargo because it has patches we need to support packaging infrastructure.)

Can you expand on this? Distinguishing between the Rust version and the cargo version doubles the overhead for users to track both which version of Rust they require and which version of cargo they require. I would strongly prefer to make this one version number for users. I know that distributions have their own cycles, but most users use rustup and are on the most recent stable release & we need to balance the costs to the users against the issues for distributions.

@Ericson2314
Copy link
Contributor

@withoutboats I'll respond in more detail later, but for now consider if you are on an old version of Rust and Cargo and solving for deps, Cargo may well fail to find a solution it would have found before because it's blowing up on newer versions of libraries using Cargo features it cannot understand.

@withoutboats
Copy link
Contributor

@Ericson2314 how does setting the version with the syntax proposed fix that? Existing versions will still fail to parse the manifest.

@Ericson2314
Copy link
Contributor

@withoutboats A single version for everything may seem appealing now, but I think will loose some elegance if/when Cargo gains unstable features. Either they live in one namespace, a layer violation, or we distinguish between the two anyways.

Also, minor quibble, note conflating the schema and tool version does preclude teaching Cargo multiple schemas. E.g. I'd loke to tesch it a new breaking schema (while having it remember the old for backwards compat). The new schema now is unnameable.

@Ericson2314
Copy link
Contributor

Ericson2314 commented Mar 23, 2017

how does setting the version with the syntax proposed fix that? Existing versions will still fail to parse the manifest.

We would, as one-time move, make a new index with the schema version. In the local case, yes things would blow up, but that's less bad than them succeeding when they shouldn't. Sorry it's this false solution which is even more insidious, and which I ought to have brought up from the get-go instead.

I wouldn't mind however just declaring that old versions of Cargo are broken and people must upgrade this one last time (going forward the version field in the index avoids this). After all, prior new features have so broken Cargo. But, let's not arrive here by accident.

@joshtriplett
Copy link
Member Author

@withoutboats

Why is this so important? As far as I can tell, the reason is to improve diagnostics when an old version of cargo fails to understand a Cargo.toml. Are there other reasons I don't understand?

In addition to the several reasons @Ericson2314 mentioned about improving dependency resolution: this change ensures that every future change to Cargo doesn't have to scrutinize exactly how all past versions of Cargo will interpret the new data, and whether they'll fail, crash, or just silently ignore new information and think they can proceed anyway. The silently ignore case represents the biggest concern: some potential Cargo features (such as new sections) will not cause old versions of Cargo to stop.

As an example, suppose we later add a new dependency type that handles cross-compilation better, or tool dependencies better. The developer adds a dependency, and they have current Cargo and Rust so it just works. The CI system tests with the earliest version that satisfies those dependencies, and it works, so changes pass CI. Then someone running locally with an older version of the tool and an older version of Cargo attempts to build. The old Cargo ignores the new tool dependencies, so the build proceeds, and if they're lucky it fails mysteriously (and produces support and debugging adventures for them or the crate developers). If they're not lucky, the build succeeds but silently misbehaves, perhaps because the developers added the dependency specifically because the version they depend on fixes some critical bug.

Or, with a Cargo schema version, the old version of Cargo stops and says "I don't know how to build this crate", and then after upgrading Cargo, the new Cargo can handle the new dependencies and say "hey, you need a newer version of this tool".

Moreover, existing versions of cargo won't fail in a responsible way when they see a [package.1.0], they'll fail noisly and confusedly, just as they would if they ignore the schema key and tried to parse a section they don't understand

Existing versions of Cargo won't typically try to parse sections they don't understand; they'll ignore them and blithely proceed anyway.

Also, with the approach in this RFC, Cargo (including existing versions with no schema version support) will never actually download crates from crates.io that have a schema version it doesn't understand. (However, we still want it to fail if it does, due to things like directory registries that bypass the crates.io index.)

Distinguishing between the Rust version and the cargo version doubles the overhead for users to track both which version of Rust they require and which version of cargo they require. I would strongly prefer to make this one version number for users.

Systems will continue to exist that have (for instance) rustc 1.15.1 and Cargo 0.16 installed; what behavior would you have Cargo exhibit on such systems? (Note that it already isn't one version number; Cargo already has its own semver.) In addition, rustc itself builds with Cargo, and Cargo builds with Cargo, with a whole tree of dependency crates that themselves may have Cargo and Rust dependencies. Bootstrapping and upgrading in a distribution environment requires needs the ability to move them forward independently.

@withoutboats
Copy link
Contributor

I think there's been some conflation between the value of versioning cargo in general & the specific aspects of this proposal I'm disagreeing with: versioning it distinctly from Rust and versioning it in a syntactically unusual way to ensure that pre-RFC versions of cargo can't compile the crate. I agree that versioning cargo is in general a good idea, but I disagree that a) having a distinct version from Rust, and b) pre-RFC versions of cargo not accidentally compiling a crate incorrectly are worth the costs.

I'd like to try to thread out each of these aspects of the proposal and deal with them individually to avoid this sort of conflation.


The syntax:

[package.1.2]

Is a surprising, confusing, and unclear syntax. No one will know these numbers correspond to a "schema" version (I would expect, if anything, they correspond to the version of the package). This will be something users will have to learn about forever.

For what? To guarantee that pre-RFC versions of cargo don't accidentally build a package incorrectly? This is not compelling to me. We have the problem of accidentally building a package incorrectly today, and it is not a very significant problem. Pre-RFC versions of cargo will eventually fall out of use, this syntax will last forever. Once those versions of cargo have vanished, whatever syntax we use, we will not have this problem (because they will all check the version).

If we really feel its so important to guarantee breakage in pre-versioned cargos, we should require you to rename package to some new synonym when you add the key using the normal syntax. But I don't think that's a good idea either.


The distinction between a Cargo.toml schema version and the version of Rust (and possibly the version of cargo) leads to a situation in which every Cargo.toml contains multiple toolchain versions:

[package]
schema = "1.2"
rust = "1.18"
cargo = "1.7"

This is a lot of knobs to expose to every single user in every one of their packages, not only as an option but as something they are required to specify & maintain.

What does it buy us?

A single version for everything may seem appealing now, but I think will loose some elegance if/when Cargo gains unstable features. Either they live in one namespace, a layer violation, or we distinguish between the two anyways.

I don't understand. You just have to be on nightly to use unstable cargo features, they don't have to exist within a single namespace. This isn't a layering violation, you already would have to set the version to nightly in your toml to use unstable features.

Also, minor quibble, note conflating the schema and tool version does preclude teaching Cargo multiple schemas. E.g. I'd love to teach it a new breaking schema (while having it remember the old for backwards compat). The new schema now is unnameable.

This seems like a fairly unlikely thing to happen & not something we should optimize for. If we actually wanted to break the schema version, we could introduce a new key at the time (cargo would assume versions without a schema version specified use the old schema). The schema is not unnameable, its just unnamed.

Systems will continue to exist that have (for instance) rustc 1.15.1 and Cargo 0.16 installed; what behavior would you have Cargo exhibit on such systems?

Do you have a more specific concern? We're talking about what version you're able to specify you need.

(Note that it already isn't one version number; Cargo already has its own semver.)

This semver is pretty meaningless though. (If it tracks anything, it tracks the library API cargo exposes). It gets bumped at the same time Rust's does, and we consider the CLI and manifest format to be versioned with Rust (that is, we can't break backcompat even though cargo's versioning is pre-1.0).

In addition, rustc itself builds with Cargo, and Cargo builds with Cargo, with a whole tree of dependency crates that themselves may have Cargo and Rust dependencies. Bootstrapping and upgrading in a distribution environment requires needs the ability to move them forward independently.

This is certainly not a standard use case and I don't want every user to have to pay the cost for the distribution bootstrapping environment. I'm not sure I understand you, but I believe the concern is that one of them may specify a Rust version that is higher than you can bootstrap from, the solution here seems to be for these projects to avoid that as part of a "bootstrap compatibility" guarantee, which does not mean they need to independently specify their version requirements.

@eternaleye
Copy link

@withoutboats: Several of the "more specifc concerns" you're asking about are already quite thoroughly addressed in the "Alternatives" section.

One that I am particularly concerned about is Cargo's handling of cross-compilation - it currently conflates "things needed for build.rs", "things needed at build time", and "things needed on the build architecture". Resolving this would almost certainly involve introducing new dependency sections, which are ignored at present, and could easily lead to silent miscompiles.

This is exacerbated by Macros 1.1 (build-arch deps in the [dependencies] map), and Macros 2.0 would worsen it further in all likelihood.

Similarly, using the Cargo version number is notably hostile to non-cargo tools. This includes generating Cargo-based distro packages, parsing Cargo.toml in other build systems (as there's currently work on for Google's Bazel), automatically working with Cargo.toml in IDEs, externally-defined Cargo subcommands, and many others. It would significantly encourage overly-tight bounds, with the result of gratuitously breaking out-of-tree tools.

Finally, any change which does not change the index format (adding a new file, which puts the schema version in a prominent place, so as to avoid breaking old cargo) can easily result in Cargo producing a dependency solution, downloading the .crate files, and realizing too late that it is not capable of actually building the crates it decided on.

@sfackler
Copy link
Member

Is there a context in which "silent miscompile" is something other than "crate failed to build"?

@eternaleye
Copy link

@sfackler: I've never seen it mean anything other than "compile, but produce an incorrect program". Failing to build isn't silent in any sense.

@sfackler
Copy link
Member

I guess I'm confused as to how a build dependency could be missing and the build would still succeed.

@eternaleye
Copy link

@sfackler: The issue isn't even so much a build dependency being missing, as it having incorrect constraints. Consider, for example, target.foo.dependencies vs. the conflation of build-arch and host-arch deps in [dependencies] with Macros 1.1.

@sfackler
Copy link
Member

How can the use of macros 1.1 dependencies cause silent miscompiles with older versions of Cargo? The macro crate would either fail to compile straight up, or it will not be loaded in as a procedural macro when the main crate is build so it'll fail to look up whatever derives or macros are being pulled from that dependency. I am confused in a similar way with respect to target specific dependencies.

@withoutboats
Copy link
Contributor

withoutboats commented Mar 23, 2017

@eternaleye I don't agree with what the RFC's alternative section says.

It might be possible to introduce versioning of tools like cargo and rustc without preventing old Cargo from parsing the non-dependency information of a crate, such as by introducing a namespace for tool names. RFC 1707 took this approach, with a tool: prefix. However, this would require case-by-case evaluation of every feature with old Cargo to observe its behavior, and would make it more difficult to modify the semantics of such dependencies, such as to handle cross-compilation robustly.

We don't provide this sort of forward compatibility guarantee today. For some time we would, for pragmatic reasons, need to do this 'case-by-case evaluation,' but after some time pre-RFC versions of cargo would fade out of use and we would be able to discount them. And I think this time is roughly a year, maybe less. As far as I'm concerned, this is totally acceptable & significantly preferable to the syntax this RFC proposes.

@eternaleye
Copy link

eternaleye commented Mar 23, 2017

@withoutboats: Several of the follow-ups @joshtriplett, @Ericson2314, and I discussed when drafting this would likely need exactly such a fail-stop anyway; postponing the definition of a fail-stop would then require each come up with their own. That's... not a complexity win.

Also, I was responding to you asking for "more specific concerns." The Alternatives section lists several, in good detail.

@sfackler: Yeah, sorry - you got an incomplete thought from me there.

The smallest expansion is "if you depend on a generator, but conditionally depend on features of the generator, then conflating kinds of dependency makes the model much more problematic"

The issue is that, for any kind of dependency manager, there are basically two axes.
The first is when you need the dep; the second is how you use the dep.

It basically boils down to (build, run) and (exec, read, import, link).

The "when"s are self-explanatory, so I'll focus on the "how"s.

  • "exec" means "I need to invoke a tool"
  • "read" means "I will read a file and make decisions based on it" (things like pkg-config files)
  • "import" means "I will read a file and incorporate it into myself" (really only valid at build time; covers headers, as well as monomorphized code)
  • "link" means "I will incorporate a binding reference to this into myself" (dynamic libs)

At build-time, exec and read refer to build-arch, while import and link refer to host-arch.

At run-time, exec and read refer to host-arch, link must be satisfied "compatibly" to at build, and import is not especially meaningful.

Historically, Cargo only supported (build, import) and (build, exec). Due to the desire to avoid being
a system package manager, it does no management of runtime dependencies (the distinction
between import and link), and didn't support "read".

At first, those were well-separated - dev-dependencies was (build, exec) while dependencies was (build, import).

As a result, Rust has gained a reputation for (relatively) painless cross-compilation.

However, recent work has been adding conflations of these, which I feel runs a heavy risk of losing that.

For example:

  • Macros 1.1 adds (build, exec) in dependencies
  • RFC 1707 added more (build, exec) in dependencies, as well as adding a distinct kind of (build, exec) in dev-dependencies
  • The pkg-config stuff @joshtriplett has been working on adds both (build, import) dependencies on headers and pkg-config files, as well as (build, link) and (run, link) dependencies on system libraries

In order to handle both the things that are being added to Cargo, and the (reasonable) desire to carry the necessary information for system package mangers and pkg-config-using build.rs scripts to do their jobs in Cargo.toml, stuff like this needs to actually follow some systematic scheme.

@joshtriplett
Copy link
Member Author

@withoutboats

For some time we would, for pragmatic reasons, need to do this 'case-by-case evaluation,' but after some time pre-RFC versions of cargo would fade out of use and we would be able to discount them. And I think this time is roughly a year, maybe less.

2-3 years, at least, based on how long it'll take users of long-term stable distributions to move forward. One of the many motivating factors for this effort came from people attempting to build a crate on a stable distribution and getting mysterious results.

If we really feel its so important to guarantee breakage in pre-versioned cargos, we should require you to rename package to some new synonym when you add the key using the normal syntax.

That is a workable solution. We could, instead of putting the version number in the section heading, instead rename the section just once (e.g. package2 or similar) and add a new key for the schema version at the same time. That would work, though in practice that would add more text to the Cargo.toml file.

I do understand your concern about the version number looking like one for the package rather than one for the schema; that hadn't occurred to me. If we can agree on the goal of ensuring that old versions of Cargo (including pre-schema versions) stop and don't attempt to build a crate with a schema version they don't understand, then changing the specific format used to do so seems fine, as long as we test that that format has the desired effect.

The distinction between a Cargo.toml schema version and the version of Rust (and possibly the version of cargo) leads to a situation in which every Cargo.toml contains multiple toolchain versions:

[package]
schema = "1.2"
rust = "1.18"
cargo = "1.7"

First of all, you almost certainly don't need both a version of Cargo and a schema version. The schema version covers anything a dependency on Cargo would cover (unless the crate itself needed the Cargo library API). We're only talking about two version numbers: Cargo and Rust. One would be the schema version, which you'd increase your dependency on if you use a feature of Cargo that requires it (and in many cases new Cargo can help you with that more automatically, by not accepting such a feature without a dependency on the appropriate schema version). The other would be the Rust version, which you'd increase your dependency on if you need new Rust features. (And for the latter, I suspect we'll end up with a syntax that looks more like a dependency.)

Do you have a more specific concern? We're talking about what version you're able to specify you need.

Yes, exactly: what version number would a system with version X of Rust and Y of Cargo be able to support? What version number would Cargo check dependencies against? It can't be the version of Rust, because then old-cargo-with-new-rust will break. It can't be the version of Cargo, because then new-cargo-with-old-rust will break. And "both" would be painful when they get out of sync.

@sfackler Suppose you have a new mechanism for build tool dependencies, for a tool you need to shell out to during your build process. Users might already have the tool installed, but with an insufficient version. If versions of Cargo that don't understand "tool dependencies" just proceed with the build anyway, then the build could invoke that tool. And having bumped dependencies in my Cargo.toml before to ensure I had a sufficiently new version of some crate to avoid buggy behavior, I can easily imagine having a tool dependency because older versions of the tool generate code that compiles but has runtime bugs.

@alexcrichton alexcrichton added T-cargo Relevant to the Cargo team, which will review and decide on the RFC. and removed T-dev-tools Relevant to the development tools team, which will review and decide on the RFC. labels May 22, 2017
@withoutboats
Copy link
Contributor

withoutboats commented Jun 16, 2017

We talked about this RFC in the cargo team this week. Only @alexcrichton, @matklad and myself were in the meeting, so other team members may have different thoughts.


The big question this RFC raises is about trade offs. Obviously, it would be great to have a good experience (avoiding miscompilations and nonsensical errors) for users on too old versions of Rust/cargo, but this has to be balanced against the user experience for everyone else (the vast majority of Rust users use the latest version of stable after all), as well as the implementation complexity.

For example, we think asking every user to track separate Rust and cargo versions isn't a good trade off. We'd like to have users track as few versions as possible. We've put in a lot of work to enable everyone to be on the latest stable, we don't want to burden people with a lot of complexity to support scenarios that are very rare.

We also felt it would also be too much implementation & organizational overhead to actually check that you depend on the correct version with every new feature you use. That is, we can't guarantee that your version declaration accurately reflects the minimum version you rely on unless you test against that version in CI.


We felt that this would be the right way to strike a balance between these concerns:

  • We add an (optional) release version to the Cargo.toml. If you fill this in, you are responsible for guaranteeing its accuracy.
  • When you publish to crates.io, if you've provided a minimum version we record it, otherwise we record the toolchain version used to perform the cargo package (which we know it will compile on, because we test that as a part of packaging).
  • We add a warning whenever cargo builds a package which depends on a higher version than it is, and if the build fails we print additional advisories recommending you upgrade to a latest version.
  • We also add a flag to cargo build that will take release versions into account when resolving dependencies, and filter out any dependency versions which have a release version higher than the current version of cargo.

In other words, we want to implement this system in a very "opt-in" fashion.

Since we can't guarantee correctness, we don't want a situation where crates.io is full of dishonestly low version constraints, since that helps no one. But since this means version constraints will often be dishonestly high, we only want to warn by default, and not refuse to compile a crate with an older version than it allows.

If users are committed to pinning to a particular version of Rust, we also want to support them in ignoring crate releases that they can't compile. This is what the flag would be for. However, we think most users on older versions of Rust just haven't thought to upgrade, which is why the default experience would be to recommend that they upgrade when their build fails.

@Ericson2314
Copy link
Contributor

Ericson2314 commented Jun 16, 2017

@withoutboats it's crucial that the solver try to avoid plans with bad versions, or frankly versions of any sort aren't worth anything. Not saying it can't go ahead if it doesn't find a plan satisfying those constraints, or that a lockfile can't overrule them, but extra information guiding the solver is the most important purpose.

@Ericson2314
Copy link
Contributor

Ericson2314 commented Jun 16, 2017

We also felt it would also be too much implementation & organizational overhead to actually check that you depend on the correct version with every new feature you use. That is, we can't guarantee that your version declaration accurately reflects the minimum version you rely on unless you test against that version in CI.

That would be completeness. Soundness is much easier---build with the min version and see if it works!

@Ericson2314
Copy link
Contributor

Ericson2314 commented Jun 16, 2017

Anyways, I'm back so hopefully all three authors are available for discussion as @aturon long before proposed. Based on prior conversation there's a lot to be discussed, so talking about the RFC as-written probably isn't worth it.

@withoutboats
Copy link
Contributor

@withoutboats it's crucial that the solver try to avoid plans with bad versions, or frankly versions of any sort aren't worth anything. Not saying it can't go ahead if it doesn't find a plan satisfying those constraints, or that a lockfile can't overrule them, but extra information guiding the solver is the most important purpose.

If I understand you correctly, you're saying that you think its very important that cargo's version resolution exclude crate versions with higher minimum versions than your current Rust version.

As I described in my post, we want there to be a flag that enables exactly this behavior. The reason we don't think it would be a good idea by default is that since people who don't opt in to tracking their version are going to use "their current version" always, and that version will likely be higher than necessary. So limiting resolution will keep people from getting new features and bug fixes that they can actually compile.

At the same time, most people would be happy to upgrade to the latest stable to keep their dependencies working, since most people on old stable are "rustup users who have forgotten to upgrade." So just warning is a great default experience, since then they will be notified that the way to solve their problem is to upgrade. For people who don't want to upgrade, though, a flag will enable exactly the behavior you want.

That would be completeness. Soundness is much easier---build with the min version and see if it works!

Yes, and anyone who wants to guarantee compatibility would be encouraged to do exactly that, automatically, with their CI. But its not easy for cargo to do that when you publish, cargo can't control what version of cargo you're using.

@Ericson2314
Copy link
Contributor

@withoutboats Wow, I'm really sorry. I just missed that last bullet entirely.

@Ericson2314
Copy link
Contributor

@withoutboats I still don't see the problem with making that flag opt-in by default. If most users are fine to be on the latest stable, than the flag should be fine as on the latest stable it won't have an affect either way. Default-off would only benefit those intentionally on old stables, depending in crates with the default-inferred overly-conservative min version. That seems like just the niche demographic that you don't want to optimize for. [I imagine most min-version-conscious users will use crates written by those with a similar approach. They of course will want the flag on]

@withoutboats
Copy link
Contributor

withoutboats commented Jun 16, 2017

@Ericson2314 But our belief is the average user who is not on the latest stable would rather have their build fail and be told to upgrade than to miss out on bug fixes that newer versions of the crate would contain.

What I think is very normal is not to be "on the latest stable" but to be "on the latest stable I've been given a reason to upgrade to." If no new features come out for a few cycles that your code needs, but they do for your dependencies, you want to be reminded that you should upgrade, not just miss out on whatever's been added to the dependencies. That is, I would posit many users take a very passive interest in upgrading, and need to be prompted to take action.

(For example, I personally still haven't downloaded the most recent stable.)

@Ericson2314
Copy link
Contributor

Say we just print out warnings during solving about skipping plans due to a too-low min version? I personally am always annoyed when build plans fail, so that strikes me as poor UX. Plus, this gauarantees the user knows they aremissing something, rather than them only learning based on whether the solver's plan happens to include a unbuildable dep.

Furthermore, I'm thinking users woukd signal intentionally sticking with an old version by putting a min version on the workspace root:

  • by default Cargo will issue those warnings and use the toolchain version

  • with the root min version, it will skip the warnings without an explicit flag to keep them, and additionally ensure the toolchain respects the root min version.

(For efficiency, we probably don't want to construct plans regardless of version, but could warn instead if any compatible dep with a too-tight min version is discovered)

@aturon
Copy link
Member

aturon commented Sep 6, 2017

@joshtriplett I believe you plan to post a new RFC on this topic -- do you want to close this one out?

@aturon
Copy link
Member

aturon commented Sep 20, 2017

I'm going to go ahead and close this out, in an effort to clean up the queue. i believe that @joshtriplett plans a fresh RFC on this topic at some point soon.

@joshtriplett
Copy link
Member Author

joshtriplett commented Oct 19, 2017

I've posted #2182 to address this. Unlike this RFC, #2182 makes the versioning automatic, without having to list a version number explicitly.

@newpavlov
Copy link
Contributor

newpavlov commented Jan 25, 2018

#2182 does not cover specifying minimum rustc version. Does someone have plans for writing an RFC which will include it? Preferably it must be a proposal which will cover how to handle versions of rustc, cargo and Cargo.toml schema (with potential case of having cargo older than rustc), as well as interaction with epoch keyword, especially considering "marketing" aspect of the latter. (which I think is a bit messy concept, but oh well...)

I like @withoutboats arguments. (although note that I haven't read this thread thoroughly) Ideally I would like to just specify something like rust = "1.21" in my Cargo.toml files, add rust: 1.21.0 to CI configuration, see if tooling detects any problems (unsolvable dependency requirements, contradicting Rust version and epoch, etc.), and be done with it. Also it is important to have an ability to indicate that crate requires nightly (e.g. rust = "nightly"). As for that should cargo new include into Cargo.toml, I think it should take the current tooling version or epoch (when we'll get those), over-restriction risk IMHO is a manageable cost. Also the new RFC ideally should cover the following questions:

  • Should cargo somehow check minimum Rust version? If yes how it's better to do? (by just checking stabilization attributes of used features or by compiling crate with this version on cargo publish?)
  • Should new cargo versions refuse to upload crate without specified rust or epoch field? (I think yes, with an ability to disable this check just in case)
  • How to handle crates with unspecified rust or epoch field? (i.e. all currently published crate versions)
  • Is it possible to split implementation to smaller subtasks to make it easier to introduce this feature bit by bit? (I think that this feature is a long overdue and sooner we get it the better, even if it will be just mostly declarative with minimal amount of checks)

@est31
Copy link
Member

est31 commented Jan 25, 2018

Should cargo somehow check minimum Rust version? If yes how it's better to do? (by just checking stabilization attributes of used features or by compiling crate with this version on cargo publish?)

I think the second option is the safest one. Sometimes compilers have breaking changes or other subtle changes that are not in a feature gate (mostly talking about language features here). Also, compiler devs rather would prefer to remove feature gating if something is stabilized because the more gates, the more complicated the compiler will get. For some features, gating is super easy but for others it is harder. It will be already a large enough challenge to support all the epochs to come. I mean most contributors can't even get the version on a #[stable(since=version)] attribute right, and that includes me :). And language features are considerably more complex to gate :p.

From an user perspective, verification without actually using the version will suck anyway due to the bugs so I would still recommend everyone to use the actual compiler version they target.

One of the advantages of having rustup integrated into cargo (which apparently will happen) is that if the targeted version doesn't match the one that cargo is currently using, cargo can automatically download the targeted version and use that to run its tests on (maybe in addition to the currently used version to prevent any bug reports?). Without cargo integration one could error on a mismatch but that behaviour is suboptimal.

The number of users who are on an outdated version are a minority and I actually want them to stay a minority because the Rust ecosystem should stay vibrant. But as they are a minority they need extra protection: you can't just let them be on their own and file bugs for various crates because something broke without the authors noticing.

What's also important is that one needs to ensure that the dependency graph can actually be resolved in a way where the versions work out. It is no use if you claim to work on rustc 1.20 but one of your dependencies you added since the last release always has needed rustc 1.24. Maybe an earlier version of a dependency of which the latest version needs 1.24 works though. If it doesn't work out however, cargo should give an error during cargo publish suggesting to increase the minimum supported version.

Should new cargo versions refuse to upload crate without specified rust or epoch field?

No, but instead we should IMO:

  • make cargo init add a rustc-version = <version of currently active rustc> field to Cargo.toml
  • on cargo package if there is no such field in Cargo.toml, we should do the same, adding a rustc-version = <version of currently active rustc> to Cargo.toml. Maybe also emitting a warning cargo automatically added a version = .. field, please add a rust-version = ... field to Cargo.toml to indicate your targeted version or something like that. The details probably get a bit hairy with nightly/beta because the crate might rely on nightly features... for this we maybe need to ask rustc whether the code opted into any feature gates or not.

This combination will get most acceptance amongst users who don't care about these things, who are I imagine quite a large chunk. If you don't care about these things and cargo yells at you, you will very likely dislike the feature and that's not what I want people to do!

How to handle crates with unspecified rust or epoch field?

If the suggestions above are followed, most new publishments will obtain a specified field. Those publishments that don't have such a field will all be from older rustc versions. As the compilers that know about the existence of a version field already publish using a version field, they are safe to assume that all crates without such a field are compatible to them :).

@kornelski
Copy link
Contributor

Cargo automatically testing on supported stable version sounds awesome. I often use nightly only because of incremental compilation or bug fixes, and forget to test on stable before publishing crates.

@mathstuf
Copy link

I don't know about testing on publish as a requirement (i.e., basically, don't download the toolchain if it isn't already available). There's no testing with other platforms or of feature flags on upload either. IMO, it's something best left to CI.

@kornelski
Copy link
Contributor

kornelski commented Jan 26, 2018

Sorry, by testing I've meant testing if it compiles at all with cargo build (that is done already), not cargo test.

@mathstuf
Copy link

mathstuf commented Jan 26, 2018

Same thing. As long as it's built on just one target platform and with default flags, just testing the minimum toolchain version isn't enough IMO.

@newpavlov
Copy link
Contributor

On internals I've published pre-pre-RFC with a new min supported Rust version proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-cargo Relevant to the Cargo team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.