Change upstream via $RUSTUP_DIST_SERVER

[knight42]

The downloads from the original https://static.rust-lang.org/ are rather slow in some regions, e.g. China. It is reasonable to enable users to choose other mirror sites.

Besides, I doubt that setting RUSTUP_DIST_ROOT is able to alter the root URL for downloading Rust toolchains and updates, since the urls written in the manifest files are still something like https://static.rust-lang.org/dist/2016-05-24/rust-1.9.0-aarch64-unknown-linux-gnu.tar.gz, which makes no difference.

[brson]

Thanks for the PR. This is an important consideration.

I'm a little worried about the proliferation of these env vars that modify the download. It seems to me that most people would expect setting RUSTUP_DIST_ROOT to do this kind of replacement of the manifest URLs. How would you feel about doing this same thing but using the dist_root value from DownloadCfg? Off-hand I can't think of any reason that would be undesirable.

cc @Diggsey @alexcrichton

[Diggsey]

I agree we should be using DIST_ROOT for this. It also might be worth making it a setting (like host triple), to reduce the need to set environment variables (while still allowing the environment variable to act as an override)

[knight42]

@brson @Diggsey
At present, only toolchains and its updates could be downloaded from RUSTUP_DIST_ROOT, what if I want to download cargo, which is located at https://static.rust-lang.org/cargo-dist/?

Is it acceptable to change the value of RUSTUP_DIST_ROOT to 'https://static.rust-lang.org' and append the 'dist' in the program?

[alexcrichton]

I agree with @brson that this may be one too may be one too many config variables, I'd expect solutions here along the lines of:

• When mirroring, the manifest is updated with altered URLs
• Locally changing the resolution of static.rust-lang.org

[knight42]

@alexcrichton
Option A breaks the consistency with upstream and option B requires extra work on client side. They both seem kind of inelegant.

IMO, this is a case that should be handled by rustup which is a "toolchain installer".

What do you think?

[alexcrichton]

In my mind the purpose of the manifest is just metadata for the distribution, so it's fine for that to get modified somewhat arbitrarily. (i.e. I'd probably still personally go towards that route)

[Diggsey]

It does make it more difficult to create a mirror though. Couldn't we just avoid the problem by using relative paths in the manifest files?

[brson]

At present, only toolchains and its updates could be downloaded from RUSTUP_DIST_ROOT, what if I want to download cargo, which is located at https://static.rust-lang.org/cargo-dist/?

This is a good point. The manifest already contains cargo-dist URLs. multirust used a variable called RUSTUP_DIST_SERVER (I think) that just contained the server name and not the dist directory. We could go back to that scheme (probably put RUSTUP_DIST_ROOT on the path to deprecation, which would allow the replacement to work as long as all the paths were on the same server.

It's worth noting that I expect there to eventually be functionality to merge external manifests into the manifest definition, so e.g. @steveklabnik can publish intermezzos bins that work with the normal toolchain. Those manifests would point to resources on a different servo. I consider that use case pretty niche though and don't anticipate problematic interactions with this URL-replacement feature.

[brson]

It does make it more difficult to create a mirror though. Couldn't we just avoid the problem by using relative paths in the manifest files?

Yes. Possibly. I recall that @alexcrichton and I discussed this and made the conscious decision to use full paths, but don't recall why. Using relative paths would have strange interactions with the above feature I mentioned where we might merge two different manifests from two different sources.

[alexcrichton]

I wouldn't mind switching to a different variable as the one source of configuration, I largely just wouldn't want to have quite a large myriad of ways to configure rustup.

[knight42]

@brson

I have updated my code, replacing the urls with RUSTUP_DIST_SERVER.

[knight42]

@Diggsey @alexcrichton @brson
Any further suggestion?

[knight42]

Any progress？

[brson]

Thanks for the updates @knight42. I'm glad you left the backwards-compatibility code in. Good idea.

I'd prefer if the loading of RUSTUP_DIST_SERVER wasn't duplicated 3 times. Would it be possible instead to change the dist_root_url value in Config to dist_root_server, then get the two cases in Manifestation from the Config type? It would require making the compatibility code remove any trailing dist in the path.

Would you also change the use of RUSTUP_DIST_ROOT in the test suite to use this new value instead?

I'd still like to see what @Diggsey thinks of this solution since he created the RUSTUP_DIST_ROOT variable.

[Diggsey]

I'm for this: the deviation from rustup.sh was from the initial port to rust, when it seemed like everything was under the /dist folder, to make life a little easier for people wanting to set up a mirror.

[brson]

This patch looks great now, but is it possible to remove this dist_root_url field completely? Looks like it's mainly used by rustup_dist::dist, so that module can just be changed to append "/dist".

[knight42]

@brson I am afraid not.

The dist_root_url field is accessed in rustup::toolchain(see here), which is used to construct the DownloadCfg.

Because the type of DownloadCfg.dist_root is &str, it is impossible to assign a modified value.

[knight42]

@brson
All tests passed! 😃

BTW, AppVeyor is goddamn slow 💔

[Diggsey]

@knight42 Yeah, free accounts have zero parallelism, so the builds have to run sequentially.

[knight42]

Could this be merged now?

[brson]

Thanks so much @knight42!