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

Support {prefix} and {lowerprefix} markers in config.json dl key #8267

Merged
merged 2 commits into from
Jun 8, 2020
Merged

Support {prefix} and {lowerprefix} markers in config.json dl key #8267

merged 2 commits into from
Jun 8, 2020

Conversation

drmikehenry
Copy link
Contributor

Hello,

The crates.io-index Git repository uses a nice directory structure to keep individual directory sizes under control.

When mirroring crates.io, it's useful to store crate files in a similar directory structure for the same reasons.

Cargo provides "markers" for use in the dl key of the config.json file in crates.io-index to allow flexibility in mapping a crate's name and version into a URL for the crate. The marker {crate} is replaced by the crate's name, and the marker {version} is replaced with the crate's version. The default URL template is https://crates.io/api/v1/crates/{crate}/{version}/download.

Currently, if a mirror of crates.io stores crates in a directory structure similar to that of crates.io-index, it's up to the server to construct the directory name from the crate name. This eliminates trivial web servers and file: URLs from hosting such a tree of crates.

This pull requests adds two new markers for the dl key in config.json, allowing Cargo to supply the directory name as part of the URL. The marker {lowerprefix} is the same directory name used within crates.io-index; it is calculated from the crate name converted to lowercase. The marker {prefix} is similar, but it uses the crate name as-is (without case conversion), which is useful for supporting older versions of Cargo that lack these markers; for example, nginx rewrite rules can easily construct {prefix} but can't perform case-conversion to construct {lowerprefix}. These new markers will provide implementation flexibility and simplicity for crate mirror servers.

…key.

These new markers allow Cargo to supply a directory name (similar to
that used in crates.io-index) as part of a crate's download URL,
enabling simpler hosting of crates.  Previously, a `file` URL would need
to put all crates into a single huge directory (such as `/srv/crates/`),
e.g.:

    file:///srv/crates/{crate}/{crate}-{version}.crate

With the `{prefix}` marker, a more efficient directory structure may be
used, e.g.:

    file:///srv/crates/{prefix}/{crate}/{crate}-{version}.crate

An example crate of `cargo-0.44.1.crate` would map to the path:

    /srv/crates/ca/rg/cargo/cargo-0.44.1.crate
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 22, 2020
@alexcrichton alexcrichton added the T-cargo Team: Cargo label May 26, 2020
@alexcrichton
Copy link
Member

Thanks for the PR and seems reasonable to me! I'm not sure that {prefix} and {lowerprefix} are the best names here but we can always bikeshed. In any case this is some new stable surface area, so I'm going to canvas the team:

@rfcbot fcp merge

@rfcbot
Copy link
Collaborator

rfcbot commented May 26, 2020

Team member @alexcrichton has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period An FCP proposal has started, but not yet signed off. disposition-merge FCP with intent to merge labels May 26, 2020
@joshtriplett
Copy link
Member

I've had several discussions with some of the folks involved with creating the original directory structure with these prefix subdirectories, and each time, they've expressed regret at the use of those directories; they're much less relevant on modern filesystems. That shouldn't stop us from supporting the directory structure we have; I'm only mentioning it because anyone creating a new registry should seriously evaluate the directory structure and figure out what works best, rather than assuming that "Rust's upstream registry did it so it's probably a good idea". 😄

That said, having support for this seems reasonable:
@rfcbot reviewed

@drmikehenry
Copy link
Contributor Author

Thanks for considering this pull request. As always (for me, anyway :-)), naming is hard. I'm happy to change {prefix} and {lowerprefix} to whatever is preferred.

@rfcbot rfcbot added the final-comment-period FCP — a period for last comments before action is taken label May 26, 2020
@rfcbot
Copy link
Collaborator

rfcbot commented May 26, 2020

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period An FCP proposal has started, but not yet signed off. label May 26, 2020
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

A agree, the prefix/lowerprefix names are a little vague, but I don't have any better suggestions.

Comment on lines 129 to 135
- `dl`: This is the URL for downloading crates listed in the index. The value
may have the markers `{crate}` and `{version}` which are replaced with the
name and version of the crate to download. If the markers are not present,
then the value `/{crate}/{version}/download` is appended to the end.
name and version of the crate to download, or the marker `{prefix}` which is
replaced with the crate's prefix, or the marker `{lowerprefix}` which is
replaced with the crate's prefix converted to lowercase. If none of the
markers are present, then the value `/{crate}/{version}/download` is appended
to the end. See below for more about crate prefixes.
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be a little clearer to list each substitution separately, maybe something like this:

- `dl`: This is the URL for downloading crates listed in the index. The value
  may have the following markers which will be replaced with their
  corresponding value:

  - `{crate}`: The name of crate.
  - `{version}`: The crate version.
  - `{prefix}`: A directory prefix computed from the crate name. For example,
    a crate named `cargo` has a prefix of `ca/rg`. See below for details.
  - `{lowerprefix}`: Lowercase variant of `{prefix}`.

  If none of the markers are present, then the value
  `/{crate}/{version}/download` is appended to the end.

Alternatively, maybe the entire discussion of the dl format could be moved into a separate section. At first reading, it seemed a little confusing because there are two concepts (the index directory layout, and the download directory layout).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks; I've incorporated your wording improvements into the pull request.

@rfcbot
Copy link
Collaborator

rfcbot commented Jun 5, 2020

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@rfcbot rfcbot removed the final-comment-period FCP — a period for last comments before action is taken label Jun 5, 2020
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Jun 8, 2020

📌 Commit b375bea has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 8, 2020
@bors
Copy link
Contributor

bors commented Jun 8, 2020

⌛ Testing commit b375bea with merge 22a112b...

@bors
Copy link
Contributor

bors commented Jun 8, 2020

☀️ Test successful - checks-azure
Approved by: alexcrichton
Pushing 22a112b to master...

@bors bors merged commit 22a112b into rust-lang:master Jun 8, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 11, 2020
Update cargo

15 commits in 40ebd52206e25c7a576ee42c137cc06a745a167a..1ec223effbbbf9fddd3453cdcae3a96a967608eb
2020-06-01 22:35:00 +0000 to 2020-06-09 20:03:14 +0000
- Default values for `readme` if not specified (rust-lang/cargo#8277)
- Fix tree completions. (rust-lang/cargo#8342)
- Support `{prefix}` and `{lowerprefix}` markers in `config.json` `dl` key (rust-lang/cargo#8267)
- Add environment variables to identify the binary and crate name (rust-lang/cargo#8270)
- Bump to 0.47.0, update changelog (rust-lang/cargo#8336)
- Nits: Remove unneeded mut and loop (rust-lang/cargo#8334)
- 1.45 beta backports (rust-lang/cargo#8331)
- Better error message when passing in relative path to Workspace::new (rust-lang/cargo#8321)
- Don't hash executable filenames on apple platforms. (rust-lang/cargo#8329)
- fix clippy warnings (rust-lang/cargo#8324)
- Require latest libgit2 to pull in bugfixes (rust-lang/cargo#8320)
- Fix an accidental raw access of field (rust-lang/cargo#8319)
- Use mem::take to replace with Default values (rust-lang/cargo#8314)
- Allow Windows dylibs without dll suffix. (rust-lang/cargo#8310)
- Show alias in help message (rust-lang/cargo#8307)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge FCP with intent to merge finished-final-comment-period FCP complete S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-cargo Team: Cargo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants