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

Refactor dist tarballs generation #79788

Merged
merged 23 commits into from
Dec 23, 2020
Merged

Conversation

pietroalbini
Copy link
Member

Before this PR, each tarball we ship as part of a release was generated by manually creating the directory structure and invoking rust-installer generate. This means each tarball was slightly different, adding new ones meant copy-pasting the code generating another tarball and removing the useless parts, and more importantly refactoring how tarballs are generated is extremely time-consuming.

This PR introduces a new abstraction in rustbuild, Tarball. The Tarball struct provides a trivial API to generate simple tarballs, and can get out of the way when more complex tarballs have to generate. For example, the whole code to generate the build-manifest tarball is now the following:

let tarball = Tarball::new(builder, "build-manifest", &self.target.triple);
tarball.add_file(&build_manifest, "bin", 0o755);
tarball.generate()

One notable change between the old tarballs and the new ones is that the "overlay" (README.md, COPYRIGHT, LICENSE-APACHE and LICENSE-MIT) is now available in every produced tarball, while before each tarball inconsistently had or didn't have those files. Tarballs that need a different overlay have a way to change which files to include (with the set_overlay method):

let mut tarball = Tarball::new(builder, "rustfmt", &target.triple);
tarball.set_overlay(OverlayKind::Rustfmt);
tarball.add_file(rustfmt, "bin", 0o755);
tarball.add_file(cargofmt, "bin", 0o755);
tarball.add_legal_and_readme_to("share/doc/rustfmt");
Some(tarball.generate())

The PR should be reviewed commit-by-commit, as each commit migrated a separate tarball to use Tarball. During development i made sure every tarball can still be generated, and for the most compex tarballs I manually ensured the list of files between the old and new tarballs did not have unexpected changes.

r? @Mark-Simulacrum

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 7, 2020
src/bootstrap/tarball.rs Outdated Show resolved Hide resolved
@Mark-Simulacrum
Copy link
Member

I'm probably not going to get a chance to review this until the weekend at the earliest -- is this blocking some work? If so I could potentially fit it in but it'd be a bit rough.

@pietroalbini
Copy link
Member Author

It's not blocking any work, the next task for me on this project is to change rust-installer, and that can happen in parallel of this PR. Also I'm probably as busy as you :)

@Mark-Simulacrum
Copy link
Member

This looks amazing to me -- I noted one typo, and I'm sure I might've missed others, but I don't think that should block (we can patch things up if we notice them later). It seems good to get this landed quickly so we're not landing it around the release (so we can fix things on master without worrying about backports).

r=me with typo fixed -- up to you on @Keruspe's comment.

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 12, 2020
@pietroalbini
Copy link
Member Author

@bors r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Dec 14, 2020

📌 Commit 272838e3f60487a40e25363e6f681dfd011234dc has been approved by Mark-Simulacrum

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 14, 2020
@bors
Copy link
Contributor

bors commented Dec 14, 2020

⌛ Testing commit 272838e3f60487a40e25363e6f681dfd011234dc with merge de7ce5c3f8869f3288d1fb9480fdcd985362f15a...

@bors
Copy link
Contributor

bors commented Dec 14, 2020

💔 Test failed - checks-actions

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

@bors try

@bors
Copy link
Contributor

bors commented Dec 14, 2020

⌛ Trying commit 272838e3f60487a40e25363e6f681dfd011234dc with merge 80acf4fa64fccb397fb4cec1d663dd473f492507...

@Mark-Simulacrum
Copy link
Member

FWIW I think git absorb or similar should make it easy to avoid the "fix review comments" commit, but since this would still have 21 commits - which seems right - I am not going to block on that.

@bors
Copy link
Contributor

bors commented Dec 14, 2020

💔 Test failed - checks-actions

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 14, 2020
@pietroalbini
Copy link
Member Author

Ok should've fixed the CI failure (I also amended the commit history).

@bors try

@bors
Copy link
Contributor

bors commented Dec 14, 2020

⌛ Trying commit 6a66aa4a263a3cd7586b723ec589c2f739e233ef with merge bf52e3fae8d78a18529a0c726ce78238217c20e8...

@bors
Copy link
Contributor

bors commented Dec 14, 2020

☀️ Try build successful - checks-actions
Build commit: bf52e3fae8d78a18529a0c726ce78238217c20e8 (bf52e3fae8d78a18529a0c726ce78238217c20e8)

@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Dec 23, 2020

📌 Commit 8736731 has been approved by Mark-Simulacrum

@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 Dec 23, 2020
@bors
Copy link
Contributor

bors commented Dec 23, 2020

⌛ Testing commit 8736731 with merge 7f9c43c...

@bors
Copy link
Contributor

bors commented Dec 23, 2020

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 7f9c43c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 23, 2020
@bors bors merged commit 7f9c43c into rust-lang:master Dec 23, 2020
@rustbot rustbot added this to the 1.50.0 milestone Dec 23, 2020
@pietroalbini pietroalbini deleted the bootstrap-tarball branch December 23, 2020 22:47
@semarie
Copy link
Contributor

semarie commented Dec 26, 2020

if I didn't mess myself, with this PR, the current nightly tarball extracts inside image/ directory instead of previously rustc-nightly-src/ directory.

does such name will stick on release too (instead of rustc-1.50-src/) ?

@Mark-Simulacrum
Copy link
Member

Yeah, at least rustc-nightly-src changed. Looking into a fix now.

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Dec 26, 2020
@Mark-Simulacrum
Copy link
Member

Fix should be up in #80397, and seems to work locally.

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 28, 2020
…ietroalbini

Use package name for top-level directory in bare tarballs

This fixes a bug introduced by rust-lang#79788.

r? `@pietroalbini`
@pietroalbini
Copy link
Member Author

Sorry about that!

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 31, 2020
…mulacrum

Fix broken ./x.py install

During my tarball refactorings in rust-lang#79788 I changed the directory layout used by the tarball generation code, and that broke the other parts of rustbuild which hardcoded the paths of those directories. Namely, `./x.py install` relied on the uncompressed copy of the tarball left behind by `fabricate`/`rust-installer`, causing rust-lang#80494.

While the easy fix for rust-lang#80494 would've been to just update the hardcoded paths to match the new structure, that fix would leave us in the same situation if we were to change the directory layout again in the future. Instead I refactored the code to return a `GeneratedTarball` struct as the output of all the dist steps, and I put all the paths the rest of rustbuild needs to care about in its fields. That way, future changes to `src/bootstrap/tarball.rs` will not break other stuff.

This PR is best reviewed commit-by-commit.
r? `@Mark-Simulacrum`
`@rustbot` modify labels: beta-nominated beta-accepted T-release
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Dec 31, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 31, 2020
…mulacrum

Fix broken ./x.py install

During my tarball refactorings in rust-lang#79788 I changed the directory layout used by the tarball generation code, and that broke the other parts of rustbuild which hardcoded the paths of those directories. Namely, `./x.py install` relied on the uncompressed copy of the tarball left behind by `fabricate`/`rust-installer`, causing rust-lang#80494.

While the easy fix for rust-lang#80494 would've been to just update the hardcoded paths to match the new structure, that fix would leave us in the same situation if we were to change the directory layout again in the future. Instead I refactored the code to return a `GeneratedTarball` struct as the output of all the dist steps, and I put all the paths the rest of rustbuild needs to care about in its fields. That way, future changes to `src/bootstrap/tarball.rs` will not break other stuff.

This PR is best reviewed commit-by-commit.
r? `@Mark-Simulacrum`
`@rustbot` modify labels: beta-nominated beta-accepted T-release
andersk added a commit to andersk/rust that referenced this pull request Apr 18, 2021
The --bulk-dirs argument was removed for rust-docs in commit
c768ce1 and rustc-docs in commit
8ca46fc (rust-lang#79788), presumably by
mistake; that slowed down installation of rust-docs from under a
second to some twenty *minutes*.  Restoring --bulk-dirs reverses this
slowdown.

Fixes rust-lang#80684.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 22, 2021
…mulacrum

bootstrap: Restore missing --bulk-dirs for rust-docs, rustc-docs

The `--bulk-dirs` argument was removed for rust-docs in commit c768ce1 and rustc-docs in commit 8ca46fc (rust-lang#79788), presumably by mistake; that slowed down installation of rust-docs from under a second to some twenty *minutes*.  Restoring `--bulk-dirs` reverses this slowdown.

Fixes rust-lang#80684.

Cc `@pietroalbini.`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants