Skip to content

Avoid ICE if environment variable is not set #31248

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 29, 2016
Merged

Avoid ICE if environment variable is not set #31248

merged 1 commit into from
Jan 29, 2016

Conversation

ruuda
Copy link
Contributor

@ruuda ruuda commented Jan 27, 2016

This fixes #31238.

r? @michaelsproul

@michaelsproul
Copy link
Contributor

Looks good, but probably best to at least spit a warning rather than silently failing to emit metadata. Usually, crates with __build_diagnostics_arrray are compiled via the build system, where that environment variable is defined. The case you found should be the only problematic one.

@ruuda
Copy link
Contributor Author

ruuda commented Jan 28, 2016

Fixed, please take another look.

@michaelsproul
Copy link
Contributor

Looks perfectly reasonable but I won't have time to test it for a few days (and I also can't r+).

cc @brson @nrc ?

@nrc
Copy link
Member

nrc commented Jan 28, 2016

I would go further and emit an error, rather than a warning - not emitting metadata is pretty bad

@ruuda
Copy link
Contributor Author

ruuda commented Jan 28, 2016

Done. This now makes rustdoc fail with an error when I run x86_64-unknown-linux-gnu/stage2/bin/rustdoc src/librustc/lib.rs though. (Which at least is better than an ICE.)

@ruuda
Avoid ICE if environment variable is not set
a7f1d12 
Rustdoc could trigger a code path that relied on the
$CFG_COMPILER_HOST_TRIPLE environment variable being
set, causing an ICE if it was not. This fixes that,
emitting an error instead of crashing.
@nrc
Copy link
Member

nrc commented Jan 29, 2016

@bors: r+

@bors
Copy link
Collaborator

bors commented Jan 29, 2016

📌 Commit a7f1d12 has been approved by nrc

bors added a commit that referenced this pull request Jan 29, 2016
@bors
Auto merge of #31248 - ruud-v-a:fix-triple-ice, r=nrc
0f196bc 
This fixes #31238.

r? @michaelsproul
@bors
Copy link
Collaborator

bors commented Jan 29, 2016

⌛ Testing commit a7f1d12 with merge 0f196bc...

@bors
Copy link
Collaborator

bors commented Jan 29, 2016

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt

@bors bors merged commit a7f1d12 into rust-lang:master Jan 29, 2016
@michaelsproul
Copy link
Contributor

@ruud-v-a: To make your use case work you can always manually export the environment variable. I don't think the value is even significant as it's just used for naming, e.g. use "linux" instead of "x86_64-gnu-linux-unknown" (or whatever)

@ruuda ruuda deleted the fix-triple-ice branch November 30, 2016 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

rustdoc: unable to determine target arch from $CFG_COMPILER_HOST_TRIPLE
4 participants
@ruuda @michaelsproul @nrc @bors