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

Unify fetching of SHAs between rustfmt and other tools. #492

Merged
merged 2 commits into from
Nov 21, 2020

Conversation

PiotrSikora
Copy link
Contributor

Signed-off-by: Piotr Sikora piotrsikora@google.com

@google-cla google-cla bot added the cla: yes label Nov 11, 2020
@dfreese
Copy link
Collaborator

dfreese commented Nov 13, 2020

I'm fine with cleaning up some of the versioning in the rustfmt versions file, but I don't think we gain a lot of clarity by trying to combine them into a single list. It removes the distinction that they're running off of different versioning schemes.

@PiotrSikora
Copy link
Contributor Author

I'm fine with cleaning up some of the versioning in the rustfmt versions file, but I don't think we gain a lot of clarity by trying to combine them into a single list. It removes the distinction that they're running off of different versioning schemes.

Which list do you mean? fetch_shas_VERSIONS.txt and fetch_shas_RUSTFMT_VERSIONS.txt are still separate lists used for stable releases, so I don't think that distinction is removed.

If you mean combining enumerate_keys() with enumerate_rustfmt_keys(), then the reason for doing that is making sure that the same targets and dates for nightly and beta releases are used for all the tools, otherwise we were missing rustfmt SHAs for various platforms and for nightly and beta releases, since rustfmt follows the same naming convention (YYYY-MM-DD/TOOL-nightly-PLATFORM) as other tools.

I removed 2019-11-07/rustfmt-1.4.8 because it doesn't seem that it could be used anyway, e.g.

rust_repositories(version = "nightly", rustfmt_version = "nightly", iso_date = "2020-11-13", edition="2018")

will fetch:

https://static.rust-lang.org/dist/2020-11-13/rust-nightly-x86_64-unknown-linux-gnu.tar.gz
https://static.rust-lang.org/dist/2020-11-13/rustfmt-nightly-x86_64-unknown-linux-gnu.tar.gz
https://static.rust-lang.org/dist/2020-11-13/llvm-tools-nightly-x86_64-unknown-linux-gnu.tar.gz
https://static.rust-lang.org/dist/2020-11-13/rust-std-nightly-x86_64-unknown-linux-gnu.tar.gz
https://static.rust-lang.org/dist/2020-11-13/rust-std-nightly-wasm32-unknown-unknown.tar.gz
https://static.rust-lang.org/dist/2020-11-13/rust-std-nightly-wasm32-wasi.tar.gz

I could add all that logic to enumerate_rustfmt_keys() instead of combining both functions, but that a lot of repetition that could easily diverge over time.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
@PiotrSikora
Copy link
Contributor Author

(rebased on top of current master)

@PiotrSikora
Copy link
Contributor Author

Ping.

@PiotrSikora
Copy link
Contributor Author

btw: it looks that rustfmt versioning changed in 1.48.0 and it matches other tools now (see: #497)

@mfarrugi mfarrugi merged commit e18f90d into bazelbuild:master Nov 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants