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

[rust-installer] Allow long link names in tar files #109577

Merged
merged 1 commit into from Mar 28, 2023

Conversation

jonhoo
Copy link
Contributor

@jonhoo jonhoo commented Mar 24, 2023

Without this, users trying to run x.py dist under a sufficiently long path run into problems when we build the resulting tarballs due to length limits in the original tar spec. The error looks like:

    Finished release [optimized + debuginfo] target(s) in 0.34s
Copying stage0 std from stage0 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-musl)
Building stage0 tool rust-installer (x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.35s
Dist rust-std-1.67.1-x86_64-unknown-linux-musl
Error: failed to generate installer

Caused by:
    0: failed to tar file '/home/AAAAAAAAAAAAAA/BBBBBB/CCCC/DDD/EEEEE/FFFFFFFFFFFF/GGGGGGGGGGGGGGGG/HHHHHHHHHH/IIIIIIIIIIIIIII/JJJJJ/KKKKKKK/src/build/tmp/tarball/rust-std/x86_64-unknown-linux-musl/rust-std-1.67.1-x86_64-unknown-linux-musl/rust-std-x86_64-unknown-linux-musl/lib/rustlib/x86_64-unknown-linux-musl/lib/self-contained/libc.a'
    1: provided value is too long when setting link name for
Build completed unsuccessfully in 0:00:03

The fix is to make use of the widely-supported GNU tar extensions which lift this restriction. Switching to tar::Builder::append_link takes care of that for us. See also alexcrichton/tar-rs#273.

@rustbot
Copy link
Collaborator

rustbot commented Mar 24, 2023

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Mar 24, 2023
Without this, users trying to run `x.py dist` under a sufficiently long
path run into problems when we build the resulting tarballs due to
length limits in the original tar spec. The error looks like:

        Finished release [optimized + debuginfo] target(s) in 0.34s
    Copying stage0 std from stage0 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-musl)
    Building stage0 tool rust-installer (x86_64-unknown-linux-gnu)
        Finished release [optimized] target(s) in 0.35s
    Dist rust-std-1.67.1-x86_64-unknown-linux-musl
    Error: failed to generate installer

    Caused by:
        0: failed to tar file '/home/AAAAAAAAAAAAAA/BBBBBB/CCCC/DDD/EEEEE/FFFFFFFFFFFF/GGGGGGGGGGGGGGGG/HHHHHHHHHH/IIIIIIIIIIIIIII/JJJJJ/KKKKKKK/src/build/tmp/tarball/rust-std/x86_64-unknown-linux-musl/rust-std-1.67.1-x86_64-unknown-linux-musl/rust-std-x86_64-unknown-linux-musl/lib/rustlib/x86_64-unknown-linux-musl/lib/self-contained/libc.a'
        1: provided value is too long when setting link name for
    Build completed unsuccessfully in 0:00:03

The fix is to make use of the widely-supported GNU tar extensions which
lift this restriction. Switching to [`tar::Builder::append_link`] takes
care of that for us. See also alexcrichton/tar-rs#273.

[`tar::Builder::append_link`]: https://docs.rs/tar/0.4.38/tar/struct.Builder.html#method.append_link
@jonhoo
Copy link
Contributor Author

jonhoo commented Mar 25, 2023

Given this affects rust-installer, I think it could use a @bors try to vet the change.

@bors
Copy link
Contributor

bors commented Mar 25, 2023

@jonhoo: 🔑 Insufficient privileges: not in try users

@tmiasko
Copy link
Contributor

tmiasko commented Mar 25, 2023

@bors try

@bors
Copy link
Contributor

bors commented Mar 25, 2023

⌛ Trying commit 4d55010 with merge 90e5ae8fc74c5db264562012bde753d653c13623...

@bors
Copy link
Contributor

bors commented Mar 26, 2023

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

@Mark-Simulacrum
Copy link
Member

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented Mar 26, 2023

📌 Commit 4d55010 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Mar 26, 2023

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

@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 Mar 26, 2023
@bors
Copy link
Contributor

bors commented Mar 28, 2023

⌛ Testing commit 4d55010 with merge cbc064b...

@bors
Copy link
Contributor

bors commented Mar 28, 2023

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 28, 2023
@bors bors merged commit cbc064b into rust-lang:master Mar 28, 2023
12 checks passed
@rustbot rustbot added this to the 1.70.0 milestone Mar 28, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (cbc064b): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.6% [-4.7%, -2.4%] 2
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

@jonhoo jonhoo deleted the long-tar-names branch March 28, 2023 16:19
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. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants