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

Fix setting of PATH for make check on windows #27553

Merged
merged 1 commit into from
Aug 11, 2015

Conversation

Diggsey
Copy link
Contributor

@Diggsey Diggsey commented Aug 6, 2015

I have no idea how bors keeps working without this - I can only assume it's some peculiarity of how windows searches for DLLs.

Without this change, running make check on windows will not correctly set PATH to include eg. x86_64-pc-windows-gnu\stage1\bin\rustlib\x86_64-pc-windows-gnu\lib, and when it tries to run eg. stage1/test/stdtest-x86_64-pc-windows-gnu.exe, it will fail because windows can't find the DLLs on which it relies.

It seems to be just a mistake: when the equivalent was added for the branch that deals with unix-like platforms, the windows branch was left unchanged.

@rust-highfive
Copy link
Collaborator

r? @pcwalton

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

@brson
Copy link
Contributor

brson commented Aug 6, 2015

Let's think carefully before merging this. The configuration here is tricky and it may be intentional.

It seems possible to me that we intentionally configured the tests on windows to load the 'host' libs and not the 'target' libs. I do vaguely recall stage1 tests not working on windows for known reasons (known years ago and now forgotten).

@alexcrichton what do you think of this? cc @vadimcn

@vadimcn
Copy link
Contributor

vadimcn commented Aug 7, 2015

@Diggsey, any idea which dlls is it not finding? For me, make check just works.

@Diggsey
Copy link
Contributor Author

Diggsey commented Aug 7, 2015

@vadimcn When running make check it will specifically mention test-xxx.dll from x86_64-pc-windows-gnu\stage1\bin\rustlib\x86_64-pc-windows-gnu\lib. However, that's just the first one it looks for - running depends.exe on the executable shows that it has missing dependencies on most of the DLLs from that folder. (additionally, I've tested by copying just the test dll over, and it does not fix the problem).

How is the executable supposed to find the DLLs? I've checked, and nothing in the makefile rules triggered by make check attempts to modify PATH on windows, and there's no attempt to copy or build the DLLs into the same directory, or a parent directory.

The only other ways I know of how windows could find the DLLs were if PATH already included a directory containing the DLLs, or if the DLLs happened to be loaded by another process at the time (and I'm still not sure about that one, it's not clear from msdn when that applies).

@alexcrichton
Copy link
Member

Yeah I think this was an accidental omission (unix already does this). Depending on the ordering of what make check decides to build the test-xxx.dll may not be in place in the host directory because it's only there as rustdoc links to it (which isn't required for some run-pass tests, for example).

@bors: r+ 6203d97

@bors
Copy link
Contributor

bors commented Aug 11, 2015

⌛ Testing commit 6203d97 with merge 60fe30e...

bors added a commit that referenced this pull request Aug 11, 2015
I have no idea how bors keeps working without this - I can only assume it's some peculiarity of how windows searches for DLLs.

Without this change, running `make check` on windows will not correctly set PATH to include eg. `x86_64-pc-windows-gnu\stage1\bin\rustlib\x86_64-pc-windows-gnu\lib`, and when it tries to run eg. `stage1/test/stdtest-x86_64-pc-windows-gnu.exe`, it will fail because windows can't find the DLLs on which it relies.

It seems to be just a mistake: when the equivalent was added for the branch that deals with unix-like platforms, the windows branch was left unchanged.
@bors
Copy link
Contributor

bors commented Aug 11, 2015

⛄ The build was interrupted to prioritize another pull request.

bors added a commit that referenced this pull request Aug 11, 2015
I have no idea how bors keeps working without this - I can only assume it's some peculiarity of how windows searches for DLLs.

Without this change, running `make check` on windows will not correctly set PATH to include eg. `x86_64-pc-windows-gnu\stage1\bin\rustlib\x86_64-pc-windows-gnu\lib`, and when it tries to run eg. `stage1/test/stdtest-x86_64-pc-windows-gnu.exe`, it will fail because windows can't find the DLLs on which it relies.

It seems to be just a mistake: when the equivalent was added for the branch that deals with unix-like platforms, the windows branch was left unchanged.
@bors
Copy link
Contributor

bors commented Aug 11, 2015

⌛ Testing commit 6203d97 with merge 1af31d4...

@bors bors merged commit 6203d97 into rust-lang:master Aug 11, 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.

7 participants