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

Include all shipped files in the manifest #22

Closed
pietroalbini opened this issue Oct 27, 2020 · 6 comments · Fixed by rust-lang/rust#78486
Closed

Include all shipped files in the manifest #22

pietroalbini opened this issue Oct 27, 2020 · 6 comments · Fixed by rust-lang/rust#78486
Labels
A-optimization Issue that affects the speed or resource usage of promote-release C-new-feature A new feature that we could implement

Comments

@pietroalbini
Copy link
Member

pietroalbini commented Oct 27, 2020

Currently, the manifests don't include all the files available in the release, meaning that promote-release has to whitelist extra paths to preserve in its pruning code (introduced by rust-lang/rust#78196 and #21).

Namely, they don't include the rustc-{channel}-src.{ext} tarballs. Those tarballs are problematic since they are not installable through rustup, as they don't follow the standard format (for example here is the rust-src component.

rust-src-1.47.0.tar.xz
├── components
├── install.sh
├── rust-installer-version
└── rust-src
    ├── lib
    │   └── rustlib
    │       └── src
    │           └── rust
    │               ├── Cargo.lock
    │               └── library/
    └── manifest.in
rustc-1.47.0-src.tar.xz
├── Cargo.lock
├── Cargo.toml
├── config.toml.example
├── configure
├── CONTRIBUTING.md
├── COPYRIGHT
├── git-commit-hash
├── library/
├── LICENSE-APACHE
├── LICENSE-MIT
├── README.md
├── RELEASES.md
├── src/
├── vendor/
├── version
└── x.py

A way to solve this issue cleanly would be to figure out a way to include the rustc-{channel}-src tarballs in the manifest, thus making sure that all files we ship are referenced in a single manifest file. The best way to include those files is still an open question.

Once this is fixed this snippet can be removed.

@pietroalbini
Copy link
Member Author

@Mark-Simulacrum proposed to treat rustc-{channel}-src as a component with a new installable = false key, but that might mess up old rustup versions (cc @kinnison). Another idea I had was to create an artifacts section of the manifest, which would be currently ignored by rustup (logic for it could be added later if people need it).

@Mark-Simulacrum
Copy link
Member

Hm, so the artifacts idea seems even better, but I do worry a bit that it loses the nice-ish ability to provide signatures and various compression formats for a single artifact without listing it as distinct artifacts. Maybe that's not too important though, and in practice given we just have the src tarballs today... seems fine to just have a table for now.

@pietroalbini
Copy link
Member Author

@Mark-Simulacrum oh, I was thinking of using the same schema we have right now, but in a different top-level map:

[artifacts.rustc-src.target."*"]
available = true
url = "https://static.rust-lang.org/dist/2020-10-27/rustc-dev-nightly-x86_64-pc-windows-msvc.tar.gz"
hash = "4b2ed13b88ba98b853c7f46034f3c7424e05caf7fe0c9b52ca93d90894d39285"
xz_url = "https://static.rust-lang.org/dist/2020-10-27/rustc-dev-nightly-x86_64-pc-windows-msvc.tar.xz"
xz_hash = "731b8c0d37dfadad3ac7187e3832ee51c25a0cd40caa1faf45297b3960abbfe0"

...but that is less flexible, and doesn't support uncompressed stuff.

@kinnison
Copy link

Rustup determines if a component is available based on whether it has the url/hash pairs and the available key. I'm not sure what it'd do with a component which has urls but had available = false. The toml is parsed here: https://github.com/rust-lang/rustup/blob/master/src/dist/manifest.rs#L419 which suggests it'd just ignore the url etc. if the available key is set to false, but I'd not stake a bet on it without some proper testing which I'm not in a position to do right now.

If, on the other hand, you add a new table then that'll simply be ignored by rustup until such time as we decide if/how we want to handle them.

If you do, then I strongly suggest you use a better scheme than the above though, something like:

[[artifacts.rustc-src.target."*"]]
url = "https://...."
hash = "439438209489032..."

Might be better, since compression type can be detected at runtime easily enough, and that way we don't need to change the channel format for different kinds of artifact.

@pietroalbini
Copy link
Member Author

@kinnison that list approach is really interesting, especially for files that are not tarballs: in #23 I discovered that we also need to whitelist .msi and .pkg files for the Windows and macOS installers.

@pietroalbini pietroalbini added A-optimization Issue that affects the speed or resource usage of promote-release C-new-feature A new feature that we could implement labels Oct 28, 2020
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 29, 2020
…rk-Simulacrum

Include non-rustup artifacts in the manifest

This PR fixes rust-lang/promote-release#22 by including all the files we ship in the generated manifests, even the ones that are not installable through rustup. In practice this adds the following "artifacts":

* `source-code`: the tarball containing the full source code used to build the release (`rustc-{channel}-src.tar.xz`)
* `installer-msi`: the MSI installer for Windows systems (`rust-{channel}-{target}.msi`)
* `installer-pkg`: the PKG installer for macOS systems (`rust-{channel}-{target}.pkg`)

These files are included in a new `artifacts` table of the manifest, like so:

```toml
[[artifacts.installer-msi.target.aarch64-pc-windows-msvc]]
url = "https://example.com/2020-10-28/rust-nightly-aarch64-pc-windows-msvc.msi"
hash-sha256 = "6b41d5b829d20834c5d93628d008ec618f8914ee79303363bd13a86fd5f305dd"

[[artifacts.installer-msi.target.i686-pc-windows-gnu]]
url = "https://example.com/2020-10-28/rust-nightly-i686-pc-windows-gnu.msi"
hash-sha256 = "83f020de6e180c155add9fce1cea2ac6e5f744edbd6dc1581e24de8f56b2ca7a"

[[artifacts.installer-msi.target.i686-pc-windows-msvc]]
url = "https://example.com/2020-10-28/rust-nightly-i686-pc-windows-msvc.msi"
hash-sha256 = "dbc80c24e9d5df01616c6f216114b4351f51a94218e2368b5cebe4165b270702"

[[artifacts.installer-msi.target.x86_64-pc-windows-gnu]]
url = "https://example.com/2020-10-28/rust-nightly-x86_64-pc-windows-gnu.msi"
hash-sha256 = "8196eca3f02d72d4c8776ad4fcc72897125e2cf6404ae933e31c07e197e3c9fa"

[[artifacts.installer-msi.target.x86_64-pc-windows-msvc]]
url = "https://example.com/2020-10-28/rust-nightly-x86_64-pc-windows-msvc.msi"
hash-sha256 = "b2e7fd6463790732fcf9c726b9448068712341943199cb40fc11d1138b8a207b"

[[artifacts.installer-pkg.target.aarch64-apple-darwin]]
url = "https://example.com/2020-10-28/rust-nightly-aarch64-apple-darwin.pkg"
hash-sha256 = "70421c191752fb33886f8033b029e634bcc993b72308cef52a38405840e91f5c"

[[artifacts.installer-pkg.target.x86_64-apple-darwin]]
url = "https://example.com/2020-10-28/rust-nightly-x86_64-apple-darwin.pkg"
hash-sha256 = "ebd7a5acb61e82d85e855146cc9bd856f32228ee7f40dd94c659b00614ed4f1f"

[[artifacts.source-code.target."*"]]
url = "https://example.com/2020-10-28/rustc-nightly-src.tar.gz"
hash-sha256 = "5fcc487ee4c15c689de8ddf7daac7ff6a65c80498197b9aea58622dc2b3bca10"

[[artifacts.source-code.target."*"]]
url = "https://example.com/2020-10-28/rustc-nightly-src.tar.xz"
hash-sha256 = "0c618ef0ec5f64da1801e9d0df6c755f6ed1a8780ec5c8ee75e55614be51d42c"

```

Each artifact can be available for multiple targets, and each target can have multiple versions of the same file (for example, a `gz`-compressed one and a `xz`-compressed one). In the future rustup might add functionality to let users retrieve the artifacts, but that's not needed to land this PR, and whether to do the implementation is up to the rustup maintainers.

r? `@kinnison`
cc `@Mark-Simulacrum`
@pietroalbini
Copy link
Member Author

Fixed by rust-lang/rust#78486.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-optimization Issue that affects the speed or resource usage of promote-release C-new-feature A new feature that we could implement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants