Skip to content

Don’t ICE if fs::canonicalise fails in meta-load #26044

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 1 commit into from
Jun 8, 2015

Conversation

nagisa
Copy link
Member

@nagisa nagisa commented Jun 5, 2015

This might fail when --extern library is a symlink to an invalid location. Instead just pretend it
doesn’t exist at all.

Fixes #26006

@rust-highfive
Copy link
Contributor

r? @nrc

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

@alexcrichton
Copy link
Member

Thanks! Could you add a test for this as well? (probably a run-make test, perhaps only enabled on Unix)

@nagisa nagisa force-pushed the canonicalize-metadata-loader branch 2 times, most recently from f5299ba to 5ca401c Compare June 6, 2015 09:08
@nagisa
Copy link
Member Author

nagisa commented Jun 6, 2015

Unsure how to make run-make tests run only on specified platforms.

@alexcrichton
Copy link
Member

You can take a look at some existing run-make test, but there should be a IS_WINDOWS macro available to you in the Makefile which should allow this to perform differently on Windows.

This might fail when --extern library is a symlink to an invalid location. Instead just pretend it
doesn’t exist at all.
@nagisa nagisa force-pushed the canonicalize-metadata-loader branch from 5ca401c to ab86fac Compare June 7, 2015 17:34
@nagisa
Copy link
Member Author

nagisa commented Jun 7, 2015

Updated.

time: libc
mkdir -p out/time out/time/deps
ln -sf out/libc/liblibc.rlib out/time/deps/
$(RUSTC) in/time/lib.rs -Ldependency=out/time/deps/
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this command expected to fail without an ICE error message? (e.g, shouldn't there be a piping or assertion of some form here?)

Copy link
Member Author

Choose a reason for hiding this comment

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

As far as I understand -Ldependency= adds some path to a list of directories in which to look for library files, right? Then this fails to use symlinked libc and proceeds to find one in rustc’s own distribution, which is why this doesn’t fail.

If this specified --extern libc=, though, then it would be a different problem.

Copy link
Member

Choose a reason for hiding this comment

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

Ah right, I see what's going on here now, nevermind then!

@nrc nrc assigned alexcrichton and unassigned nrc Jun 7, 2015
@alexcrichton
Copy link
Member

@bors: r+ ab86fac

@bors
Copy link
Collaborator

bors commented Jun 8, 2015

⌛ Testing commit ab86fac with merge 295ef21...

@bors
Copy link
Collaborator

bors commented Jun 8, 2015

💔 Test failed - auto-mac-64-opt

@alexcrichton
Copy link
Member

@bors: retry

On Mon, Jun 8, 2015 at 12:35 AM, bors [email protected] wrote:

[image: 💔] Test failed - auto-mac-64-opt
http://buildbot.rust-lang.org/builders/auto-mac-64-opt/builds/5251


Reply to this email directly or view it on GitHub
#26044 (comment).

bors added a commit that referenced this pull request Jun 8, 2015
…chton

This might fail when --extern library is a symlink to an invalid location. Instead just pretend it
doesn’t exist at all.

Fixes #26006
@bors
Copy link
Collaborator

bors commented Jun 8, 2015

⌛ Testing commit ab86fac with merge 4e14ef0...

@bors bors merged commit ab86fac into rust-lang:master Jun 8, 2015
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.

5 participants