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

Should publishes with versions only differing in metadata be allowed? #1059

Closed
sfackler opened this issue Sep 15, 2017 · 15 comments · Fixed by #6518
Closed

Should publishes with versions only differing in metadata be allowed? #1059

sfackler opened this issue Sep 15, 2017 · 15 comments · Fixed by #6518
Labels
A-backend ⚙️ C-bug 🐞 Category: unintended, undesired behavior

Comments

@sfackler
Copy link
Member

For example, https://crates.io/crates/openssl-src has 110.0.0 and 110.0.0+1.1.0f, which are semver-equivalent since build metadata doesn't affect the actual version. It seems like this probably shouldn't be allowed?

@mbrubeck
Copy link
Contributor

Some previous discussion in dtolnay/semver#107

@carols10cents
Copy link
Member

Sure sounds like we should be disallowing versions like these that only differ in build metadata....

@carols10cents carols10cents added A-backend ⚙️ A-versions C-bug 🐞 Category: unintended, undesired behavior labels Sep 20, 2017
@jeikabu
Copy link

jeikabu commented Dec 6, 2018

Looks like I fell victim to this.

I'd been using semver metadata only to discover it doesn't work as expected. Rather than getting rid of metadata support it would make sense to let them be semver equivalent. Whether some attempt is made to parse and make sense of the metadata is a convention that could be developed separately. Until then taking the most recent would be the "least surprising".

To me the primary use case is crates that have some external, non-crate dependency and you want to mirror it's semver version. "sys" crates fall in this category. I have a crate that corresponds to NNG 1.1.1. Being able to version my crate as 1.1.1 instead of some unrelated version scheme that requires mapping between the two is the simplest. If I need to alter crate contents in some way (e.g. packaging or documentation- some non-API-breaking change) 1.1.1+SOMETHING would make the most sense.

As it is, if you have a crate versioned 0.1.0+XYZ and you then publish/yank 0.1.0 the crate is "yanked" even when there's a semver-equivalent crate available, 0.1.0+3.

bors added a commit to rust-lang/cargo that referenced this issue Apr 1, 2019
Warn on version req with metadata.

Metadata in a version requirement (such as `1.0.0+1234`) is ignored. This adds a warning that it will be ignored.

On crates.io I found about 5 crates, plus a few dozen google-* crates (presumably all created by the same person) which have dependencies of this form.

See discussion at #6504 (comment). cc rust-lang/crates.io#1059 for ongoing discussion about what to do about publishing such versions.
@sgrif
Copy link
Contributor

sgrif commented Aug 15, 2019

I really do think this just boils down to a bug in semver. Build metadata is ignored for precedence, but that does not mean it's ignored for equality. To me this implies that if two versions differ only in metadata, we may select one at random to treat as the "latest version", but they are still different versions, and should be treated as such both by crates.io and Cargo. I laid out some further reasoning in dtolnay/semver#107 (comment)

@pietroalbini
Copy link
Member

The ordering being the same is not going to be changed as far as I can see, and that makes any possible use of releases with the same version but different metadata practically impossible.

If the contents of the two releases with different metadata are the same there is not really a point of having two releases, and if it's different users that don't check specifically for either version are going to get a random release, which is plain bad and unexpected for everyone.

Based on that I'd like to see publishing releases with the same version but different metadata blocked on new uploads, regardless of what Cargo does. Blocking metadata completly is not needed, as there are still valid uses for it (for example the google-* crates use metadata to let users know at a glance when the bindings were automatically generated).

Maybe we should look for active crates that are consistently using metadata today and ask them for feeback?

@jackgerrits
Copy link

jackgerrits commented Jan 30, 2020

I've just hit this, I have a sys crate and the version tracks the version of the bound library. I had to do a follow up release to fix an issue with the docs.rs build. I am not able to increment the version number as then the versions would be out of sync. I used the metadata to indicate it was an updated version. However, now I am not able to depend on the lib:

error: failed to verify the checksum of `vowpalwabbit-sys v8.8.0`

I tried yanking the original versions and only kept the latest post release but this did not seem to help.

https://crates.io/crates/vowpalwabbit-sys

@jtgeibel
Copy link
Member

@jackgerrits So I don't think this is a long-term solution, but I have an idea of something that may be interesting to test. Would you try pushing several versions of your crate to staging.crates.io in the opposite order?

It would be interesting to see what happens server and client side. In particular, I expect that cargo will pick the first matching row in the index, and that by publishing +post3 first cargo will always grab it. It's probably still not possible to access the other versions, but at least it would give us a way to modify the index without really removing any metadata tracked by the index.

So that's my short term idea. We could reorder the lines in the index, an act that in theory is a noop but in practice will hopefully fix things for your crate. A bit longer term, we could error or warn in cargo when using a +. But in your case, any such warning would be too late, as you had already published 8.8.0. Long term, I'm not really sure what the solution is. I haven't yet had a chance to re-read this thread and the linked semver thread.

@carols10cents
Copy link
Member

I'm going to lay out my current thoughts of what the actual behavior of interacting with build metadata should be, ideally:

  1. Publishing of crate versions that differ only in build metadata should be allowed, to support the sys crate use case. That is, I should be able to publish a-sys v1.0.0-whatever and a-sys v1.0.0-somethingelse.

  2. When Cargo.lock contains a version with build metadata, it should always download the exact same version, including the same build metadata. That is, if I have a Cargo.lock that contains a-sys v1.0.0-whatever, I should always get a-sys v1.0.0-whatever even if a-sys v1.0.0-somethingelse also exists.

  3. When Cargo is resolving versions from Cargo.toml without a Cargo.lock or when doing a cargo update, if a version is specified with build metadata in Cargo.toml it should always get that exact version? That is, if I specify a-sys = { version = "1.0.0-whatever" }, I should always get exactly 1.0.0-whatever? (I'm not sure this is expected behavior or not, I could also see expecting the behavior in the next item)

  4. When Cargo is resolving versions from Cargo.toml without a Cargo.lock or when doing a cargo update, if a version is specified WITHOUT build metadata in Cargo.toml it should always get the semver compatible version published most recently. That is, if I specify a = { version = "1.0.0" } and a-sys v1.0.0-whatever was published and then a-sys v1.0.0-somethingelse was published, I should always get a-sys v1.0.0-somethingelse.

I think the biggest problem right now is that 2. is not happening, as detailed in rust-lang/cargo#7180. I don't think this is a problem in crates.io, it's only a problem in cargo.

So I think this issue can be closed because the answer to the title is "yes, publishes with versions only differing in metadata should be allowed".

Am I missing anything?

@carols10cents
Copy link
Member

When we discussed this in the crates.io team meeting, we were wondering what other package managers do. I just tested npm, and it strips build metadata completely on publish.

Experiment 1:

  • Publish a package with version 1.0.0
  • Attempt to publish version 1.0.0+something
  • Publish is rejected because +something is removed and 1.0.0 already exists

Experiment 2:

  • Publish a package with version 1.0.1+something
  • Publish succeeds with version 1.0.1 because +something is removed

https://www.npmjs.com/package/@carols10cents/foo

@carols10cents
Copy link
Member

Rubygems doesn't support specifying build metadata at all: rubygems/bundler#6972

@ehuss
Copy link
Contributor

ehuss commented Jan 31, 2020

Overall, I think my preference would be to have crates.io not allow multiple versions that differ only in metadata. Whether or not the metadata is displayed on crates.io is another question, though I don't know if it matters too much.


Some other behaviors:

NuGet seems to display, but otherwise ignore metadata (https://github.com/NuGet/Home/wiki/SemVer-2.0.0-support#package-identity).

Go modules seem to mostly ignore metadata? When I use a requirement with metadata, it just seems to get removed.

Julia's Pkg doesn't error, but it looks like it might randomly pick based on metadata (or the latest one?). Regardless, it does not appear to be working correctly.

Python doesn't follow semver, but has a similar concept (https://www.python.org/dev/peps/pep-0440/#local-version-identifiers). They are not intended for public repositories, and it looks like cannot be used in a "version specifier" (semver version requirement).

@sgrif
Copy link
Contributor

sgrif commented Jan 31, 2020

I can certainly see why so many other package managers would ignore or forbid metadata, since it could definitely cause problems based on the semver spec

Build metadata MUST be ignored when determining version precedence. Thus two versions that differ only in the build metadata, have the same precedence. Examples: 1.0.0-alpha+001, 1.0.0+20130313144700, 1.0.0-beta+exp.sha.5114f85.

This implies that if versions 1.0.0, 1.0.0+foo and 1.0.0+bar exist, the versionreq 1.0.0 can match any of them. The spec is unfortunately never mentions metadata WRT equality, but I would assume that defaults to two versions with differing metadata are not equal.

If we do decide to forbid publishing versions that differ only in metadata, we still have the issue of what should we do with those versions that exist today?

@dwijnand
Copy link
Member

The spec is unfortunately never mentions metadata WRT equality, but I would assume that defaults to two versions with differing metadata are not equal.

We explored this in dtolnay/semver#108 (comment) 😄

I have a sys crate and the version tracks the version of the bound library. I had to do a follow up release to fix an issue with the docs.rs build. I am not able to increment the version number as then the versions would be out of sync. I used the metadata to indicate it was an updated version. However, now I am not able to depend on the lib

Btw, the closest thing to an "official" answer about this use case is:

how transitive dependencies are tracked is a matter for tooling, not semver. Semver provides a standard means for labeling nodes in a dependency graph, not how to mange the entire graph.

(semver/semver#169 (comment))

😕

bors added a commit to rust-lang/cargo that referenced this issue May 10, 2021
Bump index cache version to deal with semver metadata version mismatch.

#9467 has uncovered an issue with how versions are handled in the index cache.  When using a debug build of Cargo, it may panic due to the [cache contents changing](https://github.com/rust-lang/cargo/blob/5c455130b6001c7f54e872e161c27f6e996aff1f/src/cargo/sources/registry/index.rs#L606-L619).  The particular problem I am running into is that the index has an entry for `openssl-src 110.0.0` and `110.0.0+1.1.0f`. This is due to an issue with crates.io where it allows publishing multiple versions with the same metadata (rust-lang/crates.io#1059).  Cargos before #9467 would populate the index cache with two entries, both with version `110.0.0`.  Afterwards, there are two separate entries (`110.0.0` and `110.0.0+1.1.0f`).

The change here is to bump the index cache version so that new versions of cargo will clear the cache, and won't trigger the assertion.

This may be a bit of a heavy-handed solution, as I think this only affects debug builds of cargo.  However, I instantly started running into this problem, so I suspect it will be a real annoyance for anyone developing cargo. Happy to discuss other possible solutions.
@Turbo87
Copy link
Member

Turbo87 commented May 23, 2023

I've played around a bit with the database dump and came up with a query to find versions in the database with the same version number but conflicting build metadata:

SELECT crates.name, split_part(num, '+', 1) as num_no_build, COUNT(num) as count
FROM versions
LEFT JOIN crates ON crate_id = crates.id
WHERE position('+' in num) != 0
GROUP BY crates.name, num_no_build
HAVING COUNT(num) > 1
ORDER BY COUNT(num) DESC, crates.name, num_no_build;

This results in 614 problematic versions, spread out across 261 crates.

List of all crates with conflicting versions:
  • almel: 1
  • async-sea-orm-session: 1
  • boomphf-patched: 1
  • cargo_crates-io_docs-rs_test: 1
  • chromaprint-sys-next: 1
  • cifra: 2
  • ckb-system-scripts: 1
  • cursed-trying-to-break-cargo: 1
  • eddn: 1
  • elite_journal: 1
  • fbthrift-git: 1
  • ffav-sys: 1
  • flatc: 1
  • gerb: 1
  • googapis: 3
  • google-abusiveexperiencereport1: 1
  • google-acceleratedmobilepageurl1: 1
  • google-adexchangebuyer1d3: 6
  • google-adexchangebuyer1d3-cli: 3
  • google-adexchangebuyer1d4: 5
  • google-adexchangebuyer1d4-cli: 2
  • google-adexchangebuyer2_v2_beta1: 1
  • google-adexchangeseller2: 3
  • google-adexperiencereport1: 2
  • google-admin1_directory: 6
  • google-admin1_directory-cli: 1
  • google-admin1_reports: 5
  • google-admin1_reports-cli: 2
  • google-adsense1d4: 6
  • google-adsense1d4-cli: 3
  • google-adsensehost4d1: 6
  • google-adsensehost4d1-cli: 3
  • google-analytics3: 6
  • google-analytics3-cli: 2
  • google-analyticsreporting4: 1
  • google-androiddeviceprovisioning1: 2
  • google-androidenterprise1: 6
  • google-androidenterprise1-cli: 1
  • google-androidmanagement1: 2
  • google-androidpublisher2: 6
  • google-androidpublisher2-cli: 2
  • google-appengine1: 4
  • google-appengine1-cli: 1
  • google-appengine1_beta4: 5
  • google-appengine1_beta4-cli: 2
  • google-appengine1_beta5: 5
  • google-appengine1_beta5-cli: 2
  • google-appsactivity1: 5
  • google-appsactivity1-cli: 3
  • google-appstate1: 6
  • google-appstate1-cli: 3
  • google-bigquery2: 6
  • google-bigquery2-cli: 1
  • google-bigquerydatatransfer1: 2
  • google-books1: 5
  • google-books1-cli: 1
  • google-calendar3: 6
  • google-calendar3-cli: 3
  • google-civicinfo2: 2
  • google-classroom1: 5
  • google-cloudbilling1: 3
  • google-cloudbilling1-cli: 1
  • google-cloudbuild1: 1
  • google-clouddebugger2: 4
  • google-clouddebugger2-cli: 2
  • google-clouderrorreporting1_beta1: 1
  • google-cloudfunctions1: 2
  • google-cloudiot1: 1
  • google-cloudkms1: 2
  • google-cloudmonitoring2_beta2: 4
  • google-cloudmonitoring2_beta2-cli: 2
  • google-cloudresourcemanager1: 5
  • google-cloudresourcemanager1-cli: 1
  • google-cloudresourcemanager1_beta1: 6
  • google-cloudresourcemanager1_beta1-cli: 2
  • google-cloudtasks2_beta2: 2
  • google-cloudtrace1: 2
  • google-cloudtrace2: 1
  • google-compute1: 2
  • google-container1: 4
  • google-container1_beta1: 1
  • google-content2: 6
  • google-content2-cli: 1
  • google-content2_sandbox: 5
  • google-content2_sandbox-cli: 3
  • google-coordinate1: 1
  • google-customsearch1: 3
  • google-customsearch1-cli: 1
  • google-dataproc1: 5
  • google-dataproc1-cli: 1
  • google-datastore1: 1
  • google-datastore1_beta2: 1
  • google-datastore1_beta3: 1
  • google-deploymentmanager2: 5
  • google-deploymentmanager2-cli: 2
  • google-deploymentmanager2_beta2: 1
  • google-dfareporting2d1: 1
  • google-dfareporting2d2: 1
  • google-dfareporting2d3: 1
  • google-dfareporting2d4: 1
  • google-dfareporting2d5: 2
  • google-dfareporting2d5-cli: 2
  • google-dfareporting2d6: 2
  • google-dfareporting2d6-cli: 2
  • google-dfareporting2d7: 2
  • google-dfareporting2d7-cli: 2
  • google-dfareporting2d8: 2
  • google-dfareporting3: 1
  • google-dialogflow2_beta1: 1
  • google-discovery1: 1
  • google-dlp2_beta1: 1
  • google-dns1: 6
  • google-dns1-cli: 3
  • google-doubleclickbidmanager1: 6
  • google-doubleclickbidmanager1-cli: 1
  • google-doubleclicksearch2: 6
  • google-doubleclicksearch2-cli: 3
  • google-drive2: 6
  • google-drive2-cli: 1
  • google-drive3: 6
  • google-drive3-cli: 1
  • google-fcm1: 1
  • google-firebasedynamiclinks1: 3
  • google-firebasedynamiclinks1-cli: 1
  • google-firebaseremoteconfig1: 1
  • google-firestore1_beta1: 1
  • google-fitness1: 3
  • google-fitness1-cli: 1
  • google-freebase1: 1
  • google-fusiontables2: 3
  • google-fusiontables2-cli: 1
  • google-games1: 6
  • google-games1-cli: 3
  • google-gamesconfiguration1_configuration: 6
  • google-gamesconfiguration1_configuration-cli: 3
  • google-gamesmanagement1_management: 6
  • google-gamesmanagement1_management-cli: 3
  • google-genomics1: 6
  • google-genomics1-cli: 3
  • google-gmail1: 5
  • google-gmail1-cli: 1
  • google-groupsmigration1: 2
  • google-groupsmigration1-cli: 1
  • google-groupssettings1: 3
  • google-groupssettings1-cli: 1
  • google-iam1: 4
  • google-identitytoolkit3: 6
  • google-identitytoolkit3-cli: 1
  • google-kgsearch1-cli: 1
  • google-language1: 1
  • google-language1_beta1: 1
  • google-licensing1: 1
  • google-licensing1-cli: 1
  • google-logging1_beta3: 1
  • google-logging2: 3
  • google-logging2-cli: 1
  • google-logging2_beta1: 4
  • google-logging2_beta1-cli: 1
  • google-manufacturers1: 3
  • google-manufacturers1-cli: 1
  • google-mirror1: 4
  • google-mirror1-cli: 1
  • google-ml1: 2
  • google-ml1_beta1: 1
  • google-monitoring3: 5
  • google-monitoring3-cli: 3
  • google-oauth2_v2: 1
  • google-oslogin1: 1
  • google-oslogin1_beta: 2
  • google-pagespeedonline2: 3
  • google-pagespeedonline2-cli: 1
  • google-partners2: 3
  • google-partners2-cli: 1
  • google-people1: 3
  • google-playmoviespartner1: 3
  • google-playmoviespartner1-cli: 2
  • google-plus1: 6
  • google-plus1-cli: 3
  • google-plusdomains1: 6
  • google-plusdomains1-cli: 3
  • google-prediction1d6: 1
  • google-proximitybeacon1_beta1: 4
  • google-proximitybeacon1_beta1-cli: 1
  • google-pubsub1: 4
  • google-pubsub1-cli: 2
  • google-pubsub1_beta2: 5
  • google-pubsub1_beta2-cli: 3
  • google-qpxexpress1: 1
  • google-replicapool1_beta2: 1
  • google-replicapoolupdater1_beta1: 2
  • google-replicapoolupdater1_beta1-cli: 1
  • google-resourceviews1_beta2: 1
  • google-runtimeconfig1: 1
  • google-runtimeconfig1_beta1: 1
  • google-safebrowsing4: 3
  • google-safebrowsing4-cli: 1
  • google-searchconsole1: 2
  • google-searchconsole1-cli: 1
  • google-servicecontrol1: 1
  • google-sheets4: 1
  • google-siteverification1: 3
  • google-siteverification1-cli: 1
  • google-sourcerepo1: 2
  • google-spanner1: 2
  • google-spectrum1_explorer: 4
  • google-spectrum1_explorer-cli: 2
  • google-speech1: 1
  • google-speech1_beta1: 1
  • google-sqladmin1_beta4: 6
  • google-sqladmin1_beta4-cli: 1
  • google-storage1: 6
  • google-storage1-cli: 1
  • google-storagetransfer1: 3
  • google-storagetransfer1-cli: 2
  • google-surveys2: 2
  • google-surveys2-cli: 1
  • google-tagmanager1: 2
  • google-tagmanager2: 1
  • google-taskqueue1_beta2: 1
  • google-testing1: 2
  • google-tpu1_alpha1: 1
  • google-translate2: 2
  • google-vault1: 2
  • google-videointelligence1_beta1: 1
  • google-vision1: 1
  • google-webmasters3: 4
  • google-webmasters3-cli: 2
  • google-youtube3: 7
  • google-youtubeanalytics1: 6
  • google-youtubeanalytics1-cli: 2
  • google-youtubereporting1: 4
  • google-youtubereporting1-cli: 2
  • jpegxl-rs: 1
  • jpegxl-sys: 1
  • knet-bindings: 1
  • libaeron-sys: 1
  • libaeron_driver-sys: 1
  • libbpf-sys: 1
  • lightpanel: 1
  • lnpbp: 2
  • lnpbp_derive: 1
  • lnpbp_services: 1
  • lz4-sys: 1
  • mylibrary: 1
  • opencc-sys: 1
  • qpl: 1
  • router-bridge: 1
  • runng: 1
  • runng-sys: 2
  • sep-sys: 1
  • serum-borsh: 1
  • solana-accountsdb-plugin-kafka: 1
  • spdlog-src: 2
  • spng: 1
  • spng-sys: 1
  • strict_encoding: 1
  • thinker: 3
  • tonic_datastore_v1: 1
  • vowpalwabbit-sys: 1
  • zabrze: 1
  • zeromq-src: 1

@Turbo87
Copy link
Member

Turbo87 commented May 30, 2023

#6518 has now been deployed. in other words: crates.io no longer allows "publishes with versions only differing in metadata"

we are keeping the existing versions for now, simply because we haven't yet decided what to do with them. given that cargo is not able to handle them properly it might be best to delete them, but we will need consensus among the team(s) and potentially crate owners before moving forward with something like that.

ahl added a commit to ahl/cargo that referenced this issue Jul 8, 2023
bors added a commit to rust-lang/cargo that referenced this issue Jul 9, 2023
update re: multiple versions that differ only in the metadata tag

Resolved by rust-lang/crates.io#1059 a few weeks ago
trvswgnr added a commit to crablang/crabgo that referenced this issue Jul 9, 2023
* refactor: rename `location` to `path` for `GitCheckout`

For being consistent with `GitDatabase`.

* refactor: rename arg `orig_url` to `remote_url` for `fetch()`

`orig_url` was unclear. Whose origin? `remote_url` is way more
self-explanatory under a `git fetch` context.

* refactor: new const `CHECKOUT_READY_LOCK` for `.cargo-ok`

The explanation is moved to be public from an inline comment.

* refactor: remove unused function `GitRemote::rev_for`

Although this is only a few lines of code, I think it is still worth
keeping it clean if it's not used by Cargo internally. Not to mention
this apparently has an alternative solution at this moment, for example:

```diff
- let oid = git_remote.rev_for(db_path, git_ref)?;
+ let oid = git_remote.db_at(db_path)?.resolve(git_ref)?;
```

From a naive research on GitHub, there is only one usage[^1] and it
seems to be active but still experimental. Pardon my abrupt change
breaking your project :(

Only when we start considering what we want to expose can Cargo has
a clearer boundaries between its subcrates.

[^1]: https://github.com/trustification/source-distributed/blob/124ef26081a41330a553483441fca544bedbb473/src/git.rs#L34

* refactor: dont derive `Serialize` for git database/checkout

It doesn't seem that they need to derive `Serialize`. Is there any other
usage I am not aware of?

* refactor: deduce path from repo for `GitCheckout`

It was confusing and may lead to inconsistency that people need to
pass both `path` and `repo` and ensure they are in sync.

* refactor: deduce `parent_remote_url` from `database.remote.url()`

This should be less controversial as the current logic share the
same `GitRemote`. You can see the callsite of `GitDatabase::copy_to`
use `self.url()`, which then calls into `self.remote.url(). When a
`GitDatabase` is created, Cargo also uses `self.remote` as the
remote of that `GitDatabase`.

* test(z-flags): Verify `-Z` flags list is sorted

* Add message on reusing previous temporary path on failed cargo installs

* Add additional info on a new line

* refactor: extract proxy detection to `cargo::util`

* Upgrade to `gix` v0.45 for multi-round pack negotiations.

Previously, fetches and clones would routinely fail with a panic
that indicated that pack-negotiation can't take longer than 1 round
with our previous `Naive` approach.

With this version of `gitoxide` there is now faithful support for both
the `consecutive` and the `skipping` algorithm and multiple rounds of
negotiations, which should make all clones and fetches possible.

* docs: add link to nightly config doc for `SourceConfigMap`

* doc: point to nightly cargo doc

Nightly doc contains more information and rationale for each item.
It's a better fit for those want to extend Cargo's functionality.

* doc: doc comments for registry module

* doc: doc comments for registry index

* refactor: move constants closer to where they are used

This is quite straightforward.

* refactor: use the term "index schema" instead of "index format"

We had index format version and index file version. Both sometimes
are written as "index version". This is so confusing that I need to
check to source code back-and-forth.

Today we make "index format version", which refers to the exact
representation of a index file, rename to "index schema version".
Other index file version of index version should just mean the opaque
version of an index file for cache invalidation use.

* refactor: move `RegistryPackage` to `index` module

`RegistryPackage` is a line in a raw registyr index file. This should
belong to `index.rs` module.

* refactor: use `str::to_lowercase`

This piece of code was written before 1.2 and `str::to_lowercase`
is an API came out in Rust 1.2.

* fix: disable multiplexing for bad curl

In certain versions of libcurl when proxy is in use with HTTP/2
multiplexing, connections will continue stacking up. This was
fixed in libcurl 8.0.0 in curl/curl@821f6e2

However, Cargo can still link against old system libcurl if it is from a
custom built one or on macOS. For those cases, multiplexing needs to be
disabled when those versions are detected.

* test: loose overly matches for git cli output

The output format should be stable I believe, but it turns out not.
This is how `git fetch` man page says [1]:

```
<flag> <summary> <from> -> <to> [<reason>]
```

In Git 2.41 they've changed the fetch output a bit [2].

I think let's just loose it to prevent future breakages.

[1]: https://git-scm.com/docs/git-fetch#_output
[2]: https://github.blog/2023-06-01-highlights-from-git-2-41/

* refactor: rename `RegistryPackage` to `IndexPackage`

This is a better name to reflect it is from "index" files.

* add test case

* rename `url` to `child_remote_url`

* make `child_remote_url` a `Url` and pass it to `update_submodules`

* remove `match` expr

* use implicit arg capture and wrap line

* add `Url::parse` context

* remove escapes and leading whitespace from string matches

* feat: Add -Zscript

* refactor(cli): Allow use of both args/sub_args

* feat(cli): Interpret some subcommands as manifest-commands

* chore(ci): Ensure bors is blocked on clippy

* chore: Migrate print-ban from test to clippy

This should be more resilient to false positives like in #12245 where a
string contains `println`.

* docs: docs for arguments of `RegistrySource::new`

* docs: fix the wrong cache directory path

* docs: add a link to Cargo book for download URL template

* docs: doc comments for registry downloads

* docs: doc comments for local registry

* docs: doc comments for remote registry

* docs: doc comments for http registry

* test(cli): Verify precedence over external subcommands

* feat(cli): Define precedence among subcommands

I decided to start things off fairly simple.  It either looks like the user is
specifying a manifest or they aren't.  This is assuming we only need to
handle `cargo foo.rs` and `cargo ./foo.rs` and not `cargo foo`.
I think in most shebang cases, path multiple components will present which makes
that a dead giveaway and likely to not overlap with aliases and
subcommands.

For reference, dlang's dub goes a lot further
1. Checks for the subcommand name being `-`
2. Checks if subcommand name ends with `.d`
3. Checks if subcommand name is built-in
4. Checks if a file with the subcommand name exists
5. Checks if a file with the subcommand name + `.d` exists

This would allow a `run.d` to override `dub-run` which doesn't seem
good.

* refactor: unify .crate tarball name constrution

* refactor: remove leftover of #6880

The relevant part was removed in 1daff03
LAST_UPDATED_FILE was never used even before #6880.
They were just leftover during the PR updates.

* refactor: inline `make_dep_prefix`

It is not used anywhere other than in module `registry::download`.

* refactor: extract `config.json` as a constant

* refactor: reuse calls to try_old_curl macro

At least doc duplicate can be eliminiated

* fix(cli): Warn on stable for precedence changes

This will give us a window to collect feedback on if this affects
anyone.

* refactor(cli): Pull out run error handling

* refactor(cli): Align the two run's params

* feat(cli): Pull in cargo-script-mvs core logic

This is no where near the implementation we want but I think we should
develop incrementally on top of what we already have.

See https://github.com/epage/cargo-script-mvs/tree/main

* docs(unstable): Expand on manifest commands so far

This is written to reflect the current implementation though some parts
might read a little weird because I didn't want to write throw-away
documentation for when we change this.  For example, single-file
packages are currently only supported in `cargo <command>` and not as
manifest paths but this will change.

* Record results from testing

Update specifying-dependencies.md to show not specifying the branch for git doesn't always use the main branch.

* changelog: add 1.71 backport fix for libcurl

* changelog: update for 1.72

* Update specifying-dependencies.md

* Update dependencies

Hand-picked some dependencies to update. Looks pretty safe?

- anyhow
- base64
- clap@4.2.1
- flate2
- git2
- libgit2-sys
- log
- openssl
- pkg-config
- proptest
- pulldown-cmark
- serde
- serde_json
- snapbox
- tempfile
- time
- toml
- toml_edit
- url

* refactor(embedded): Flatten parsing

This is a step towards making it easier to implement multiple fixes /
features without conflicts.

* refactor(embedded): Decouple package name sanitization

* refactor(embedded): Hide unused items

* refactor(embedded): Centralize separator choice

* Show a better error when container tests fail.

* Update triagebot links.

* Fix version requirement syntax example

* fix(embedded): Don't append hash to bin names

Background: the hash existed for sharing a target directory.  That code isn't
implemented yet and a per-user build cache might remove the need for it,
so let's remove it for now and more carefully weigh adding it back in.

Immediate: This reduces the chance of hitting file length issues on Windows.

Generally: This is a bit hacky and for an official solution, we should
probably try to find a better way.  This could become more important as
single-file packages are allowed in workspaces.

* refactor(embedded): Switch to `syn` for parsing doc comments

The hope is this will result in more resilient comment handling, being
more consistent with rustdoc.

I also hoped for less code but `syn` is doing less than I had expected,
requiring us to copy code over from other parts of rust.  It seems every
proc macro has to do this but there is no guide to it, so they all do it
differently, only covering the cases they thought to test for.

Note that this still won't support `include_str!()`.

* fix(embedded): Be consistent with existing style when sanitizing

* refactor(embedded): Centralize package name rules

* fix(embedded): Sanitize like we validate package names

* Clarify the default behavior of cargo-install.

* fix(embedded): Extend sanitization rules to cover cargo-new validation

* refactor(embedded): Centralize placeholder

This was originally split out because before #12269, it was needed
elsewhere.

* refactor(embedded): Be accurate in variable name

* fix(toml)!: Prevent people parsing manifests from strings

* test(embedded): Verify no autobin behavior is running

* fix(embedded): Don't create an intermediate manifest

To parse the manifest, we have to write it out so our regular manifest
loading code could handle it.  This updates the manifest parsing code to
handle it.

This doesn't mean this will work everywhere in all cases though.  For
example, ephemeral workspaces parses a manifest from the SourceId and
these won't have valid SourceIds.

As a consequence, `Cargo.lock` and `CARGO_TARGET_DIR` are changing from being next to
the temp manifest to being next to the script.  This still isn't the
desired behavior but stepping stones.

This also exposes the fact that we didn't disable `autobins` like the
documentation says we should.

* fix(embedded): Ensure we don't auto-discover any targets

* fix(cli): Make 'cargo foo.rs' behave like --manifest-path

This mirrors the logic `ArgMatchesExt::root_manifest`

* test(cli): Verify you can't use Cargo.toml

* feat(cli): Accept 'cargo Cargo.toml'

This wasn't in the original Pre-RFC but in terms of consistently
accepting manifests everything, I think this makes sense.

* refactor(core): Split out target-dir defaulting

* fix(embedded): Keep target dir in cargo home

This was broken in #12268 when we stopped using an intermediate
`Cargo.toml` file.

Unlike pre-#12268,
- We are hashing the path, rather than the content, with the assumption
  that people change content more frequently than the path
- We are using a simpler hash than `blake3` in the hopes that we can get
  away with it

Unlike the Pre-RFC demo
- We are not forcing a single target dir for all scripts in the hopes
  that we get #5931

* feat: prepare for the next lockfile bump

The `-Znext-lockfile-bump` is added, so we can prepare for all
lockfile format changes and then stabilize then all at once.

`-Znext-lockfile-bump` is not intended for using outside our test
suite and development. Hence it's hidden.

* test(script): Verify existing build.rs behavior

* fix(embedded): Don't auto-discover build.rs files

With #12268, we moved the manifest root to be the scripts parent
directory, making it so auto-discovery might pick some things up.

We previously ensured `auto*` don't pick things up but missed `build.rs`
This is now addressed.

* Enable `doctest-in-workspace` by default

This stabilizes and enables the `-Z doctest-in-workspace` flag by default.

Also adds another testcase to make sure that the `include!()` and `file!()` macros interact well together.

* test(embedded): Verify existing lockfile behavior

* fix(embedded): Don't pollute script dir with lockfile

This puts the lockfile back into a target directory in the users home,
like before #12268.

Another idea that came up was to move the workspace root to be in the
target directory (which would effectively be like pre-#12268) but I
think that is a bit hacky / misleading.

This does mean that the lockfile is buried away from the user and they
can't pass it along with their script.  In most cases I've dealt with,
this would be fine.  When the lockfile is needed, they will also most
likely have a workspace, so it shoud have a local lockfile in that case.
The inbetween case is something that needs further evaluation for
whether we should handle it and how.

* fix: remove `-Zjobserver-per-rustc` again

It was accidentally added back in
rust-lang/cargo@cfffda9

* docs(cargo-test): missing apostrophe for possessive nouns

* docs(cargo-test): clarify the cwd is for running tests

* docs(unstable): remove tracking for `-Zdoctest-in-workspace`

* test(embedded): Verify existing behavior on basic commands

* docs(embedded): Cross-link impls

* refactor: rename registry auth module to auth/mod.rs

To be honest I don't even feel like they belong here.
Maybe we should have a dedicated directory for registry-related code.

* fix: Allow embedded manifests in all commands

I originally centralized the error reporting until I realized it likely
is intentionally not centralized so we report errors in terms of the
arguments the user provided.

* fix(cli): Unify manifest path error handling

* fix(cli): Ensure we don't accidentally let embedded manifests on stable

There should be a later check when parsing but just in case, let's have
a check here as well.

* refactor: extract asymmetric to its own module

* refactor: rename registry module to registry/mod.rs

* refactor: extract publish to its own module

* refactor: extract search to its own module

* refactor: extract login to its own module

* refactor: extract logout to its own module

* refactor: extract yank to its own module

* refactor: extract owner to its own module

* refactor: move http handle constructor to its own module

* chore: update auto-label trigger files

* Convert valid feature name warning to an error.

* test(cli): Verify directory-manifest behaivor

* test(cli): Verify missing-manifest behavior

* fix(cli): Improve error on manifest dir

* feat: check if `config-include` file ends with `.toml`

This is to avoid possible name collisions. For example, a user
creates a file called `.cargo/cache`, and then in the future
cargo wants to create a directory called `.cargo/cache/`, that
would collide with what the user specified. Restricting to `.toml`
extensions would avoid that since we won’t make a directory named
with a `.toml` extension.

* test(config-include): assert config merged in DFS

* doc(unstable): merge precedence of `-Zconfig-include`

* docs(unstable): Point stable-unstable docs to nightly docs

* build(deps): bump openssl from 0.10.54 to 0.10.55

Bumps [openssl](https://github.com/sfackler/rust-openssl) from 0.10.54 to 0.10.55.
- [Release notes](https://github.com/sfackler/rust-openssl/releases)
- [Commits](sfackler/rust-openssl@openssl-v0.10.54...openssl-v0.10.55)

---
updated-dependencies:
- dependency-name: openssl
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>

* -Znext-lockfile-bump: Don't suggest using -Z on stable

* test(script): Verify existing config behavior

* fix(script): Process config relative to script, not CWD

* cargo script example needs nightly -Zscript feature

* refactor(cli): Decouple exec lookup from doing it

* refactor(cli): Infer the command before configuring

This will let the command have some sway in how we configure

* test(script): Verify verbosity

* fix(script): Be quiet on programmatic output

The goal is that we shouldn't interefere with end-user output when
"cargo script"s are used programmatically.  The only way to detect this
is when piping.  CI will also look like this.

My thought is that if someone does want to do `#!/usr/bin/env -S cargo -v`, it
should have a consistent meaning between local development
(`cargo run --manifest-path`) and "script mode" (`cargo`), so I
effectively added a new verbosity level in these cases.  To get normal
output in all cases, add a `-v` like the tests do.  Do `-vv` if you want
the normal `-v` mode.  If you want it always quiet, do `--quiet`.

I want to see the default verbosity for interactive "script mode" a bit
quieter to the point that all normal output cargo makes is cleared before
running the built binary.  I am holding off on that now as that could
tie into bigger conversations / refactors
(see https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Re-thinking.20cargo's.20output).

* docs(unstable): Update script documentation

See #12207 for what all is merged thatt his is trying to cover.

* refactor(toml): use macro to reduce boilerplates

* refactor(toml): remove unnecessary clone on inheritable fields

* refactor: move getters method together

* doc: should be `.cargo-ok`

* Add some more documentation for Source download functions.

* Don't try to compile cargo-credential-gnome-secret on non-Linux platforms.

* Add READMEs for the credential helpers.

* docs(ref): Provide guidance on version requirements

I've been dealing with these situations as either the package author,
depending on such a package, or supporting users who run into problems
that I figure documenting this guidance in a central place means I won't
have to repeat myself as often and have to re-find all of the relevant
links each time.

Alternatives to how this was documented
- Use a regular header.  All of sections in this document are flat and
  its hard to see association between them.  This also feels like its
  more on the level of the "note"s.
- Put this in a central Recommendations page.  I think we should do
  something more for these when we have more (nothing else in my quick
  scan stood out as "recommendations" like this).  At that point we can
  have a better idea of how it would work (much like the rule of 3 for
  generalizing code).  I also suspect a "Recommendations" index might be
  better.
- Put this in the FAQ.  This can easily be framed as a question and we
  put the `Cargo.lock` policy in there.

I left out talking about alternative proc-macro designs as I feel like
treading new ground in community practices is out of the scope of this.

See also https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Version.20Requirements.20documentation

* doc: state `PackageId`/`SourceId` string is opaque

Cargo tries to maintain the stability of every output it emits.
However, for things like `PackageId` and `SourceId` users should
see them as opaque strings. We generally don't change it but we
want to reserve the right to change when it is required.

* update re: multiple versions that differ only in the metadata tag

Resolved by rust-lang/crates.io#1059 a few weeks ago

* add another bin named "crabgo" with the same path

changing the main bin name means modifying a lot of tests
i don't see a reason to do that at this point

* "cargo" -> "crabgo"

* update readme

* Squashed commit of the following:

commit f86b54d
Merge: 8505fe0 550ccf0
Author: Travis A. Wagner <trvswgnr@gmail.com>
Date:   Fri Jun 23 13:53:42 2023 -0400

    Merge pull request #6 from crablang/update-readme

    update readme

commit 8505fe0
Merge: 5889303 e181e93
Author: Sammi Turner <ms.sammi.turner@gmail.com>
Date:   Fri Jun 23 12:07:50 2023 +0100

    Merge pull request #5 from crablang/trav/rename

    rename

commit 550ccf0
Author: Travis Wagner <travis.wagner@umusic.com>
Date:   Fri Jun 23 05:36:15 2023 -0400

    update readme

commit e181e93
Author: Travis Wagner <travis.wagner@umusic.com>
Date:   Fri Jun 23 05:03:22 2023 -0400

    "cargo" -> "crabgo"

commit 01ab24a
Author: Travis Wagner <travis.wagner@umusic.com>
Date:   Fri Jun 23 04:58:34 2023 -0400

    add another bin named "crabgo" with the same path

    changing the main bin name means modifying a lot of tests
    i don't see a reason to do that at this point

commit 5889303
Merge: c9b89c6 57326f5
Author: Travis A. Wagner <trvswgnr@gmail.com>
Date:   Mon Jun 5 03:13:07 2023 -0400

    Merge pull request #3 from crablang/synced

    synced

* rm extra line

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Weihang Lo <me@weihanglo.tw>
Co-authored-by: Scott Schafer <schaferjscott@gmail.com>
Co-authored-by: MarcusGrass <marcus.grass@protonmail.com>
Co-authored-by: bors <bors@rust-lang.org>
Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
Co-authored-by: christian <christian.crespoo.98@gmail.com>
Co-authored-by: Ed Page <eopage@gmail.com>
Co-authored-by: c-git <43485962+c-git@users.noreply.github.com>
Co-authored-by: Eric Huss <eric@huss.org>
Co-authored-by: Artem Tokarev <tokarev28.art@gmail.com>
Co-authored-by: Michael Diamond <dimo414@gmail.com>
Co-authored-by: Arpad Borsos <swatinem@swatinem.de>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Yerkebulan Tulibergenov <yerkebulan@gmail.com>
Co-authored-by: Adam Leventhal <ahl@oxide.computer>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend ⚙️ C-bug 🐞 Category: unintended, undesired behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.