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

Always build libraries into the same location #2919

Merged
merged 1 commit into from
Jul 27, 2016

Conversation

alexcrichton
Copy link
Member

Previously Cargo would compile a library into a different location depending on
whether it was the "root crate" or not. In the ongoing saga of reducing Cargo's
reliance on the idea of a "root crate" this PR is the next step. With workspaces
the root crate of a compliation changes all the time, so the output needs to be
the same whether a crate is at the root or not.

Fixing this inconsistence in turn fixes bugs like #2855 and #2897 which arise
due to this discrepancy. Additionally, Cargo will no longer recompile a library
when it's used as a "root crate" or not.

This is fixed by taking a few steps:

  • Everything is now compiled into the deps directory, regardless of whether
    it's a root output or not.
  • If a "root crate" is being compiled, then the relevant outputs are hard-linked
    up one level to where they are today. This means that your binaries, dylibs,
    staticlibs, etc, will all show up where they used to.
  • The -C metadata flag is always omitted for path dependencies now. These
    dependencies are always path dependencies and already all have unique crate
    names. Additionally, they're the only crates in the DAG without metadata, so
    there's no need to provide additional metadata. This in turn means that none
    of the file names of the generated crates are mangled.

Closes #2855

@rust-highfive
Copy link

@alexcrichton: no appropriate reviewer found, use r? to override

@alexcrichton
Copy link
Member Author

r? @brson

@brson
Copy link
Contributor

brson commented Jul 26, 2016

The -C metadata flag is always omitted for path dependencies now. These dependencies are always path dependencies and already all have unique crate names. Additionally, they're the only crates in the DAG without metadata, so there's no need to provide additional metadata. This in turn means that none of the file names of the generated crates are mangled.

Besides making the file names prettier, does this accomplish anything else?

let src_dir = src.parent().unwrap();
if !src_dir.ends_with("deps") {
continue
}
Copy link
Contributor

@brson brson Jul 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would these two negative conditions exist (the continues)? Is the data bogus? Can you add a comment?

@brson
Copy link
Contributor

brson commented Jul 26, 2016

r=me

@alexcrichton
Copy link
Member Author

Besides making the file names prettier, does this accomplish anything else?

Yeah whenever you build the "main crate" you don't want these hashes. For example you don't want your main binary called foo-42892524824. This ends up also meaning that we generate libfoo.rlib for the main crate as well, but depending on your point of view it may not always be the main crate. To solve this we just never pass this extra filename so it has the same output name regardless of what's the 'root crate'

We can add -C metadata if we really need to (without -C extra-filename), but I don't think we'll need to just yet.

@alexcrichton alexcrichton force-pushed the workspace-rebuild branch 2 times, most recently from f908a52 to 05a0197 Compare July 26, 2016 23:26
@alexcrichton
Copy link
Member Author

@bors: r=brson

@bors
Copy link
Contributor

bors commented Jul 26, 2016

📌 Commit 05a0197 has been approved by brson

@alexcrichton
Copy link
Member Author

@bors: r=brson e6d0ecb

@bors
Copy link
Contributor

bors commented Jul 26, 2016

⌛ Testing commit e6d0ecb with merge e5ff7e9...

@bors
Copy link
Contributor

bors commented Jul 26, 2016

💔 Test failed - cargo-win-msvc-64

@alexcrichton
Copy link
Member Author

@bors: r=brson

@bors
Copy link
Contributor

bors commented Jul 26, 2016

📌 Commit fa649df has been approved by brson

@bors
Copy link
Contributor

bors commented Jul 27, 2016

⌛ Testing commit fa649df with merge fabb42a...

@bors
Copy link
Contributor

bors commented Jul 27, 2016

💔 Test failed - cargo-win-msvc-64

Previously Cargo would compile a library into a different location depending on
whether it was the "root crate" or not. In the ongoing saga of reducing Cargo's
reliance on the idea of a "root crate" this PR is the next step. With workspaces
the root crate of a compliation changes all the time, so the output needs to be
the same whether a crate is at the root or not.

Fixing this inconsistence in turn fixes bugs like rust-lang#2855 and rust-lang#2897 which arise
due to this discrepancy. Additionally, Cargo will no longer recompile a library
when it's used as a "root crate" or not.

This is fixed by taking a few steps:

* Everything is now compiled into the `deps` directory, regardless of whether
  it's a root output or not.
* If a "root crate" is being compiled, then the relevant outputs are hard-linked
  up one level to where they are today. This means that your binaries, dylibs,
  staticlibs, etc, will all show up where they used to.
* The `-C metadata` flag is always omitted for path dependencies now. These
  dependencies are always path dependencies and already all have unique crate
  names. Additionally, they're the only crates in the DAG without metadata, so
  there's no need to provide additional metadata. This in turn means that none
  of the file names of the generated crates are mangled.

Closes rust-lang#2855
@alexcrichton
Copy link
Member Author

@bors: r=brson

@bors
Copy link
Contributor

bors commented Jul 27, 2016

📌 Commit 0863469 has been approved by brson

@bors
Copy link
Contributor

bors commented Jul 27, 2016

⌛ Testing commit 0863469 with merge 4404c62...

bors added a commit that referenced this pull request Jul 27, 2016
Always build libraries into the same location

Previously Cargo would compile a library into a different location depending on
whether it was the "root crate" or not. In the ongoing saga of reducing Cargo's
reliance on the idea of a "root crate" this PR is the next step. With workspaces
the root crate of a compliation changes all the time, so the output needs to be
the same whether a crate is at the root or not.

Fixing this inconsistence in turn fixes bugs like #2855 and #2897 which arise
due to this discrepancy. Additionally, Cargo will no longer recompile a library
when it's used as a "root crate" or not.

This is fixed by taking a few steps:

* Everything is now compiled into the `deps` directory, regardless of whether
  it's a root output or not.
* If a "root crate" is being compiled, then the relevant outputs are hard-linked
  up one level to where they are today. This means that your binaries, dylibs,
  staticlibs, etc, will all show up where they used to.
* The `-C metadata` flag is always omitted for path dependencies now. These
  dependencies are always path dependencies and already all have unique crate
  names. Additionally, they're the only crates in the DAG without metadata, so
  there's no need to provide additional metadata. This in turn means that none
  of the file names of the generated crates are mangled.

Closes #2855
@bors
Copy link
Contributor

bors commented Jul 27, 2016

☀️ Test successful - cargo-cross-linux, cargo-linux-32, cargo-linux-64, cargo-mac-32, cargo-mac-64, cargo-win-gnu-32, cargo-win-gnu-64, cargo-win-msvc-32, cargo-win-msvc-64
Approved by: brson
Pushing 4404c62 to master...

@bors bors merged commit 0863469 into rust-lang:master Jul 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants