Skip to content

Set correct build directory for codegen-units - #61807

Closed
mati865 wants to merge 1 commit into
rust-lang:masterfrom
mati865:double-tmp
Closed

mati865 wants to merge 1 commit into
rust-lang:masterfrom
mati865:double-tmp

Conversation

@mati865

@mati865 mati865 commented Jun 13, 2019

Copy link
Copy Markdown
Member

Fixes #34586

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 13, 2019
if let Mode::CodegenUnits = config.mode {
// codegen-units tests use wrong directory, for more info see
// https://github.com/rust-lang/rust/issues/34586#issuecomment-501681886
if let Ok(build_dir) = env::var("BUILD_DIR") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm, if the problem is that we are passing -Zincremental=tmp/foo, maybe it would be better to have some way to alter the command-line flags to include the correct directory?

I'm imagining:

// compile-flags:-Zincremental=$BUILD_DIR/tmp/...

for example? (We'd have to replace that $BUILD_DIR of course.)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm a bit worried about side-effects from changing the current directory in a narrow range of cases.

@mati865 mati865 Jun 14, 2019 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Environment variables doesn't seem to be expanded in compile-flags and that results in $VARIABLE/tmp/... directory.

I'm a bit worried about side-effects from changing the current directory in a narrow range of cases.

When I enabled it for everything only 3 rustdoc tests broke(I was building with --no-fail-fast), I haven't investigated further but they should be fixable.

@Dylan-DPC-zz

Copy link
Copy Markdown

ping from triage @nikomatsakis @mati865 any updates?

@mati865 mati865 closed this Jul 1, 2019
@Dylan-DPC-zz Dylan-DPC-zz added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 1, 2019
@mati865
mati865 deleted the double-tmp branch November 3, 2022 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rustbuild: There are two tmp/ directories

4 participants