Skip to content

Fix check for existing crate when using --extern #17189

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

Merged
merged 3 commits into from
Sep 14, 2014

Conversation

bkoropoff
Copy link
Contributor

When checking for an existing crate, compare against the crate_metadata::name field, which is the crate name which was requested during resolution, rather than the result of the crate_metadata::name() method, which is the crate name within the crate metadata, as these may not match when using the --extern option to rustc.

This fixes spurious "multiple crate version" warnings under the following scenario:

  • The crate foo, is referenced multiple times
  • --extern foo=./path/to/libbar.rlib is specified to rustc
  • The internal crate name of libbar.rlib is not foo

The behavior surrounding Context::should_match_name and the comments in loader.rs both lead me to believe that this scenario is intended to work.

Fixes #17186

When checking for an existing crate, compare against the
`crate_metadata::name` field, which is the crate name which
was requested during resolution, rather than the result of the
`crate_metadata::name()` method, which is the crate name within
the crate metadata, as these may not match when using the --extern
option to `rustc`.

This fixes spurious "multiple crate version" warnings under the
following scenario:

- The crate `foo`, is referenced multiple times
- `--extern foo=./path/to/libbar.rlib` is specified to rustc
- The internal crate name of `libbar.rlib` is not `foo`

The behavior surrounding `Context::should_match_name` and the
comments in `loader.rs` both lead me to believe that this scenario
is intended to work.

Fixes rust-lang#17186
@bkoropoff
Copy link
Contributor Author

cc @alexcrichton

@alexcrichton
Copy link
Member

Could you add a test for this as well? This seems pretty subtle and it'd be good to not regress on it.

@bkoropoff
Copy link
Contributor Author

I added a run-make test.

@bkoropoff bkoropoff force-pushed the extern-existing-crate branch from a02b117 to 3da255d Compare September 13, 2014 01:10
@bkoropoff
Copy link
Contributor Author

I didn't have time this morning to run a full make check after adding the regression test, but doing so now I noticed that the source files didn't have a license. I've force-pushed a fixed commit, but I assume bors will need to be told to requeue the PR.

bors added a commit that referenced this pull request Sep 14, 2014
…ichton

When checking for an existing crate, compare against the `crate_metadata::name` field, which is the crate name which was requested during resolution, rather than the result of the `crate_metadata::name()` method, which is the crate name within the crate metadata, as these may not match when using the --extern option to `rustc`.

This fixes spurious "multiple crate version" warnings under the following scenario:

- The crate `foo`, is referenced multiple times
- `--extern foo=./path/to/libbar.rlib` is specified to rustc
- The internal crate name of `libbar.rlib` is not `foo`

The behavior surrounding `Context::should_match_name` and the comments in `loader.rs` both lead me to believe that this scenario is intended to work.

Fixes #17186
@bors bors closed this Sep 14, 2014
@bors bors merged commit 3da255d into rust-lang:master Sep 14, 2014
@bkoropoff bkoropoff deleted the extern-existing-crate branch September 20, 2014 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spurious multiple crate version warning when using --extern
3 participants