-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Template a registry's dl field #4836
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
Conversation
Previously, crate files were always downloaded from `/{crate}/{version}/download`. However, if the backing crate store for a custom registry is a raw file server rather than an API endpoint that requires every file to be named `download` which is a bit weird. Now a registry's dl URL can be templated with `{crate}` and `{version}` to have more control over the resulting path. For backwards compatibility, we append the default template suffix onto the dl URL if neither of the template parameters are present for backwards compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall, just a tiny nit about an allocation.
src/cargo/sources/registry/remote.rs
Outdated
.push("download"); | ||
let mut url = config.dl.clone(); | ||
if !url.contains(CRATE_TEMPLATE) && !url.contains(VERSION_TEMPLATE) { | ||
let suffix = format!("/{}/{}/download", CRATE_TEMPLATE, VERSION_TEMPLATE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Total nit but this intermediate allocation bothers me, can't we do write!(url, "/{}/{}/downloads", ...)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep that'd also work.
Note: we cannot change crates.io to use this syntax because the registry needs to support older versions of cargo. So the backwards compatible form will be used by the registry indefinitely. |
Updated! |
@bors r+ |
📌 Commit 52f099b has been approved by |
Template a registry's dl field Previously, crate files were always downloaded from `/{crate}/{version}/download`. However, if the backing crate store for a custom registry is a raw file server rather than an API endpoint that requires every file to be named `download` which is a bit weird. Now a registry's dl URL can be templated with `{crate}` and `{version}` to have more control over the resulting path. For backwards compatibility, we append the default template suffix onto the dl URL if neither of the template parameters are present for backwards compatibility. r? @alexcrichton cc @withoutboats
☀️ Test successful - status-appveyor, status-travis |
Previously, crate files were always downloaded from
/{crate}/{version}/download
. However, if the backing crate store for acustom registry is a raw file server rather than an API endpoint that
requires every file to be named
download
which is a bit weird. Now aregistry's dl URL can be templated with
{crate}
and{version}
tohave more control over the resulting path.
For backwards compatibility, we append the default template suffix onto
the dl URL if neither of the template parameters are present for
backwards compatibility.
r? @alexcrichton
cc @withoutboats