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

Single profile for build dependencies #1774

Open
jdub opened this issue Jun 30, 2015 · 4 comments
Open

Single profile for build dependencies #1774

jdub opened this issue Jun 30, 2015 · 4 comments
Labels
A-build-dependencies Area: [build-dependencies] A-profiles Area: profiles S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@jdub
Copy link
Contributor

jdub commented Jun 30, 2015

I've just noticed that build dependencies are currently rebuilt for each profile (or at least debug and release, I haven't noticed what happens for the others).

That's fine for a little build helper like gcc, but painful for bindgen (mainly due to syntex_syntax build time).

Are there circumstances in which the build dependency build profile is relevant to the main project build? I could very well be missing something here.

If it's never relevant, then having to rebuild all your build dependencies for each profile is a (minor) pain.

Potential solutions:

Only build the build dependencies once…

  • Optimise for performance: … with the release profile.
  • Optimise for failure: … with the debug profile.
  • Optimise for control: … with a new build profile, configured in the project's Cargo.toml like the others (and choose defaults from the above, I guess).

Thoughts?

@alexcrichton
Copy link
Member

Yeah I've often wondered this in the past as well. I feel like choosing debug is the right default (most build scripts are not cpu bound) with the ability to tweak it (e.g. profile.build) would be the best way forward.

@jdub
Copy link
Contributor Author

jdub commented Jul 2, 2015

I discovered an inter-related problem. It could be regarded as a different bug. Let me know.

I'm pretty sure that build dependencies should disregard both the profile (as above) and the target.

An example failure follows. As you can see, cargo rustc is attempting to depend on std, which is not available for the custom target. I double-checked this behaviour with cargo build.

This kind of surprises me, and makes me wonder if I'm doing something wrong. Have people simply not been using cargo for cross-compiles with build dependencies? In this case, it's just x86 on x86_64, but how would an ARM build work on x86_64 with build dependencies built for ARM?

Oh, by the way, in this case libc is a build dependency of a dependency.

cargo rustc --verbose --target=i686-unknown-linux-gnu.json -- -C soft-float -C lto -C opt-level=1 -Z no-landing-pads
...
/home/jdub/.cargo/registry/src/github.com-0a35038f75765ae4/libc-0.1.8/rust/src/liblibc/lib.rs:1:1: 1:1 error: can't find crate for `std`
/home/jdub/.cargo/registry/src/github.com-0a35038f75765ae4/libc-0.1.8/rust/src/liblibc/lib.rs:1 // Copyright 2012-2015 The Rust Project Developers. See the COPYRIGHT
                                                                                                ^
error: aborting due to previous error
Could not compile `libc`.

Caused by:

  Process didn't exit successfully: `rustc /home/jdub/.cargo/registry/src/github.com-0a35038f75765ae4/libc-0.1.8/rust/src/liblibc/lib.rs --crate-name libc --crate-type lib -g --cfg feature="default" --cfg feature="cargo-build" -C metadata=ef5cbad4ef5c7a1e -C extra-filename=-ef5cbad4ef5c7a1e --out-dir /home/jdub/src/derp/derp/target/i686-unknown-linux-gnu/debug/deps --emit=dep-info,link --target i686-unknown-linux-gnu.json -C linker=util/linker.sh -L dependency=/home/jdub/src/derp/derp/target/i686-unknown-linux-gnu/debug/deps -L dependency=/home/jdub/src/derp/derp/target/i686-unknown-linux-gnu/debug/deps -Awarnings` (exit code: 101)

@alexcrichton
Copy link
Member

I think this problem is orthogonal to build dependencies, but the problem you're seeing is that your compiler cannot target 32-bit windows because you're missing the standard library. Each Rust installation comes with a set of folders for the targets it has libraries for, and if these aren't present then there truly is no standard library to link against.

For any cross compilation scenario the first step is to ensure that you have those dependencies (e.g. std and everything under the facade). We plan on making this much easier to do in the future, so stay tuned!

@jdub
Copy link
Contributor Author

jdub commented Jul 2, 2015

That makes total sense for dependencies, but shouldn't build dependencies be built for the current machine (i.e. without --target)?

Aha! Now that I look more closely, I notice that immediate build dependencies are built without --target. In the following log, core is built with --target, but filetime and gcc are not. Perfect.

But in the previous log, libc was only being built for the benefit of...

🥚 👉 😵

Damn it. I didn't disable the non-build dependency. Sorry.

   Compiling core v0.0.0 (file:///home/jdub/src/derp/derp)
     Running `rustc vendor/libcore/src/lib.rs --crate-name core --crate-type lib -g -C metadata=7eddcbb0a532076f -C extra-filename=-7eddcbb0a532076f --out-dir /home/jdub/src/derp/derp/target/i686-unknown-linux-gnu/debug/deps --emit=dep-info,link --target i686-unknown-linux-gnu.json -C linker=util/linker.sh -L dependency=/home/jdub/src/derp/derp/target/i686-unknown-linux-gnu/debug/deps -L dependency=/home/jdub/src/derp/derp/target/i686-unknown-linux-gnu/debug/deps`
   Compiling filetime v0.1.4
     Running `rustc /home/jdub/.cargo/registry/src/github.com-0a35038f75765ae4/filetime-0.1.4/src/lib.rs --crate-name filetime --crate-type lib -g -C metadata=98fab2a8e902dde6 -C extra-filename=-98fab2a8e902dde6 --out-dir /home/jdub/src/derp/derp/target/debug/deps --emit=dep-info,link -L dependency=/home/jdub/src/derp/derp/target/debug/deps -L dependency=/home/jdub/src/derp/derp/target/debug/deps -Awarnings`
   Compiling gcc v0.3.8 (file:///home/jdub/src/derp/derp)
     Running `rustc /home/jdub/src/derp/gcc-rs/src/lib.rs --crate-name gcc --crate-type lib -g -C metadata=d4b5f7b7ec599315 -C extra-filename=-d4b5f7b7ec599315 --out-dir /home/jdub/src/derp/derp/target/debug/deps --emit=dep-info,link -L dependency=/home/jdub/src/derp/derp/target/debug/deps -L dependency=/home/jdub/src/derp/derp/target/debug/deps --extern filetime=/home/jdub/src/derp/derp/target/debug/deps/libfiletime-98fab2a8e902dde6.rlib`

@carols10cents carols10cents added the A-configuration Area: cargo config files and env vars label May 10, 2017
bors added a commit that referenced this issue Apr 27, 2018
Profile Overrides (RFC #2282 Part 1)

Profile Overrides (RFC #2282 Part 1)

WIP: Putting this up before I dig into writing tests, but should be mostly complete.  I also have a variety of questions below.

This implements the ability to override profiles for dependencies and build scripts.  This includes a general rework of how profiles work internally. Closes #5298.

Profile overrides are available with `profile-overrides` set in `cargo-features` in the manifest.

Part 2 is to implement profiles in config files (to be in a separate PR).

General overview of changes:

- `Profiles` moved to `core/profiles.rs`. All profile selection is centralized there.
- Removed Profile flags `test`, `doc`, `run_custom_build`, and `check`.
- Removed `Profile` from `Unit` and replaced it with two enums: `CompileMode` and `ProfileFor`.  This is the minimum information needed to compute profiles at a later stage.
- Also removed `rustc_args`/`rustdoc_args` from `Profile` and place them in `Context`.  This is currently not very elegant because it is a special case, but it works. An alternate solution I considered was to leave them in the `Profile` and add a special uber-override layer.  Let me know if you think it should change.
- Did some general cleanup in `generate_targets`.

## Misc Fixes
- `cargo check` now honors the `--release` flag.  Fixes #5218.
- `cargo build --test` will set `panic` correctly for dependences. Fixes #5369.
- `cargo check --tests` will no longer include bins twice (once as a normal check, once as a `--test` check).  It only does `--test` check now.
    - Similarly, `cargo check --test name` no longer implicitly checks bins.
- Examples are no longer considered a "test".  (See #5397). Consequences:
    - `cargo test` will continue to build examples as a regular build (no change).
    - `cargo test --tests` will no longer build examples at all.
    - `cargo test --all-targets` will no longer build examples as tests, but instead build them as a regular build (now matches `cargo test` behavior).
    - `cargo check --all-targets` will no longer check examples twice (once as
      normal, once as `--test`).  It now only checks it once as a normal
      target.

## Questions
- Thumbs up/down on the general approach?
- The method to detect if a package is a member of a workspace should probably be redone.  I'm uncertain of the best approach.  Maybe `Workspace.members` could be a set?
- `Hash` and `PartialEq` are implemented manually for `Profile` only to avoid matching on the `name` field.  The `name` field is only there for debug purposes. Is it worth it to keep `name`?  Maybe useful for future use (like #4140)?
- I'm unhappy with the `Finished` line summary that displays `[unoptimized + debuginfo]`.  It doesn't actually show what was compiled.  Currently it just picks the base "dev" or "release" profile.  I'm not sure what a good solution is (to be accurate it would need to potentially display a list of different options).  Is it ok?  (See also #4140 for the wrong profile name being printed.)
- Build-dependencies use different profiles based on whether or not `--release` flag is given.  This means that if you want build-dependencies to always use a specific set of settings, you have to specify both `[profile.dev.build_override]` and `[profile.release.build_override]`.  Is that reasonable (for now)?  I've noticed some issues (like #1774, #2234, #2424) discussing having more control over how build-dependencies are handled.
- `build --bench xxx` or `--benches` builds dependencies with dev profile, which may be surprising.  `--release` does the correct thing.  Perhaps print a warning when using `cargo build` that builds benchmark deps in dev mode?
- Should it warn/error if you have an override for a package that does not exist?
- Should it warn/error if you attempt to set `panic` on the `test` or `bench` profile?

## TODO
- I have a long list of tests to add.
- Address a few "TODO" comments left behind.
@ehuss ehuss added the A-profiles Area: profiles label Oct 12, 2018
@ehuss ehuss removed the A-configuration Area: cargo config files and env vars label May 20, 2020
@weihanglo weihanglo added the S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. label May 15, 2023
@epage epage added the A-build-dependencies Area: [build-dependencies] label Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-dependencies Area: [build-dependencies] A-profiles Area: profiles S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Development

No branches or pull requests

6 participants