Skip to content

Comments

Fix test_rlib_dep_reader_call failure edge case#2298

Merged
sylvestre merged 1 commit intomozilla:mainfrom
trxcllnt:fix/rust-rlib-dep-test
Dec 22, 2024
Merged

Fix test_rlib_dep_reader_call failure edge case#2298
sylvestre merged 1 commit intomozilla:mainfrom
trxcllnt:fix/rust-rlib-dep-test

Conversation

@trxcllnt
Copy link
Contributor

@trxcllnt trxcllnt commented Dec 21, 2024

This PR fixes a failure in the test_rlib_dep_reader_call test when $CARGO_HOME isn't in the user's $HOME.

I develop in a container based on messense/rust-musl-cross:x86_64-musl. The $CARGO_HOME dir is at /root/.cargo, and I create a non-root user and mount in the sccache source.

For some reason, rustc fails when run with an empty env fails (like in the test):

$ echo $HOME
/home/user
$ echo $CARGO_HOME 
/root/.cargo
$ env -i $CARGO_HOME/bin/rustc --crate-type=rlib -o /tmp/x.rlib - <<< ""
error: rustup could not choose a version of rustc to run, because one wasn't specified explicitly, and no default is configured.
help: run 'rustup default stable' to download the latest stable release of Rust and set it as your default toolchain.

However, defining the RUSTUP_HOME envvar succeeds:

$ env -i RUSTUP_HOME=$RUSTUP_HOME $CARGO_HOME/bin/rustc --crate-type=rlib -o /tmp/x.rlib - <<< ""

@sylvestre
Copy link
Collaborator

sylvestre commented Dec 21, 2024

would it be possible to add a test here to make sure we don't regress?
thanks

@trxcllnt
Copy link
Contributor Author

trxcllnt commented Dec 22, 2024

@sylvestre I can't think of how to test this in sccache. It seems to occur when rustc is run in an empty environment and $RUSTUP_HOME isn't in the current user's $HOME, and I'm not sure how we'd be able to recreate this in a test.

The fact that the test isn't failing in CI or for other devs sounds like a possible bug in rustc, as if it's using $HOME/.cargo as the fallback when when $RUSTUP_HOME is unset? That would explain why this test fails for me, but not others.

@trxcllnt
Copy link
Contributor Author

trxcllnt commented Dec 22, 2024

I guess one way would be to run all ci.yaml tests as a non-root user in the messense/rust-musl-cross containers, but that seems heavy-handed...

@sylvestre
Copy link
Collaborator

sylvestre commented Dec 22, 2024

ok, thanks

@sylvestre sylvestre merged commit 9a5b681 into mozilla:main Dec 22, 2024
59 checks passed
@codecov-commenter
Copy link

codecov-commenter commented Dec 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (0cc0c62) to head (7559ead).
Report is 124 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #2298       +/-   ##
==========================================
- Coverage   30.91%       0   -30.92%     
==========================================
  Files          53       0       -53     
  Lines       20112       0    -20112     
  Branches     9755       0     -9755     
==========================================
- Hits         6217       0     -6217     
+ Misses       7922       0     -7922     
+ Partials     5973       0     -5973

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nc7s nc7s mentioned this pull request Dec 25, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@trxcllnt @sylvestre @codecov-commenter