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

Add regression test for resolving --extern libc=test.rlib #114166

Merged
merged 1 commit into from
Aug 2, 2023

Conversation

Enselic
Copy link
Member

@Enselic Enselic commented Jul 28, 2023

Closes #26043

I could not find a test for this particular use case. The closest I got was tests/ui/imports/issue-37887.rs, but that is a regression test for a different use case (see #37887).

@rustbot
Copy link
Collaborator

rustbot commented Jul 28, 2023

r? @eholk

(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-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 28, 2023
@Noratrieb
Copy link
Member

This doesn't seem to actually test the issue. You probably want to make a run-make test instead.

@Enselic
Copy link
Member Author

Enselic commented Jul 28, 2023

Oops, looks like you're right, thank you for pointing it out. Sorry for a bad PR, I read the original issue too sloppily :(

@rustbot author

@rustbot rustbot 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 Jul 28, 2023
@Enselic
Copy link
Member Author

Enselic commented Jul 28, 2023

Sorry for the bad initial PR :( Retake:

Issue #26043 is about that rustc invalidly will consider its own libc as a candidate for --extern libc=test.rlib.

That was accidentally fixed in nightly-2020-07-31. With nightly-2020-07-30 (the nightly before the fix) we see this:

$ RUSTC_LOG=debug rustc +bisector-nightly-2020-07-30-x86_64-unknown-linux-gnu test.rs --extern libc=test.rlib
[...]
[INFO  rustc_metadata::creader] resolving crate `libc`
[...]
[INFO  rustc_metadata::creader] resolved crates:
[...]
[INFO  rustc_metadata::creader]   name: libc
[INFO  rustc_metadata::creader]   cnum: 6
[INFO  rustc_metadata::creader]   hash: 5cb655619dfdc51f
[INFO  rustc_metadata::creader]   reqd: Explicit
[INFO  rustc_metadata::creader]    rlib: /home/martin/.rustup/toolchains/bisector-nightly-2020-07-30-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/liblibc-34294dcda38da0eb.rlib
[...]
error[E0658]: use of unstable library feature 'rustc_private': this crate is being loaded from the sysroot, an unstable location; did you mean to load this crate from crates.io via `Cargo.toml` instead?
 --> test.rs:1:1
  |
1 | extern crate libc;
  | ^^^^^^^^^^^^^^^^^^
  |
  = note: see issue #27812 <https://github.com/rust-lang/rust/issues/27812> for more information
  = help: add `#![feature(rustc_private)]` to the crate attributes to enable

error: aborting due to previous error

For more information about this error, try `rustc --explain E0658`.

It is clear that libc is resolved wrongly. Both based on the debug log and based on the emitted user-facing error message. With nightly-2020-07-31 we instead see this:

$ RUSTC_LOG=debug rustc +bisector-nightly-2020-07-31-x86_64-unknown-linux-gnu test.rs --extern libc=test.rlib
[...]
[INFO  rustc_metadata::creader] resolving crate `libc`
[INFO  rustc_metadata::creader] falling back to a load
error: extern location for libc does not exist: test.rlib
 --> test.rs:1:1
  |
1 | extern crate libc;
  | ^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

We can see both that the error changed, and that the resolver failed to resolve libc at all. Given that we know in what nightly this changed (commit range is git log --no-merges --oneline db0492ace429cfeb3567e..cfc572cae2d1fc381cc), it most likely is #74915 that accidentally fixed #26043.

For maintainability reasons, I think we should avoid writing a test that interprets RUSTC_LOG output. Instead, let's make use of that the error message changed. We can write a ui test that asserts on the "fixed" error.

I have pushed such a test now. Thank you for taking a look before. Would you mind taking another look please? Thanks!

r? Nilstrieb

@rustbot ready

@rustbot rustbot assigned Noratrieb and unassigned eholk Jul 28, 2023
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 28, 2023
@Enselic Enselic changed the title Add regression test for extern crate libc Add regression test for resolving --extern libc=test.rlib Jul 28, 2023
I could not find a test for this particular use case. The closest I got
was `tests/ui/imports/issue-37887.rs`, but that is a regression test
for a different use case.
@Noratrieb
Copy link
Member

That's a fun PR to accidentally fix it, but I can see how it happens. An invalid path like test.rlib would of course fail to canonicalize since it doesn't exist. Previously, it would turn that into a FileDoesntMatch result, which then continued searching (I assume) and found the sysroot candidate. But afterwards, the canonicalization failure was ignored and test.rlib was kept to proceed, which later made it so that it fails to open it and emits a hard error.
Thanks for the detailed analysis!
I don't think the compiler will ever not depend on libc, so that seems like a safe crate for the test to actually test what it's supposed to test. Testing the error is also correct here, since that's the problem.
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Aug 2, 2023

📌 Commit 7dd5e3c has been approved by Nilstrieb

It is now in the queue for this repository.

@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 Aug 2, 2023
Noratrieb added a commit to Noratrieb/rust that referenced this pull request Aug 2, 2023
Add regression test for resolving `--extern libc=test.rlib`

Closes rust-lang#26043

I could not find a test for this particular use case. The closest I got was [`tests/ui/imports/issue-37887.rs`](https://github.com/rust-lang/rust/blob/master/tests/ui/imports/issue-37887.rs), but that is a regression test for a different use case (see rust-lang#37887).
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 2, 2023
Rollup of 5 pull requests

Successful merges:

 - rust-lang#114079 (Use `upvar_tys` in more places, make it return a list)
 - rust-lang#114166 (Add regression test for resolving `--extern libc=test.rlib`)
 - rust-lang#114321 (get auto traits for parallel rustc)
 - rust-lang#114335 (fix and extend ptr_comparison test)
 - rust-lang#114347 (x.py print more detailed format files and untracked files count)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ddda3fa into rust-lang:master Aug 2, 2023
@rustbot rustbot added this to the 1.73.0 milestone Aug 2, 2023
@Enselic Enselic deleted the libc-unavailable branch August 17, 2023 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustc will fallback to libraries distributed with the compiler in presence of --extern
5 participants