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

The gold linker should not be used on Windows #9499

Closed
UK992 opened this issue Feb 2, 2016 · 5 comments
Closed

The gold linker should not be used on Windows #9499

UK992 opened this issue Feb 2, 2016 · 5 comments

Comments

@UK992
Copy link
Contributor

@UK992 UK992 commented Feb 2, 2016

After #9449 building on Windows failed:

$ ./mach build -r
Could not execute process `C:/msys64/home/Urban/servo/etc/rustc-with-gold -vV` (never executed)

Caused by:
  %1 is not a valid Win32 application. (os error 193)
Build completed in 0.14s
@UK992 UK992 changed the title Windows build failed after PR#9449 After #9449 Windows build failed Feb 2, 2016
@jdm
Copy link
Member

@jdm jdm commented Feb 2, 2016

@larsbergstrom This seems difficult to get right!

@larsbergstrom
Copy link
Contributor

@larsbergstrom larsbergstrom commented Feb 2, 2016

@jdm Agreed :-)

Though a large part of it is also that we don't have CI up yet for all of our platforms. While I can say I should certainly know better, as we expand our platform matrix, it's pretty unreasonable to expect every reviewer to know the impact of small python changes on, say, the AARCH64 cross-build.

@larsbergstrom larsbergstrom changed the title After #9449 Windows build failed The gold linker should not be used on Windows Feb 2, 2016
@larsbergstrom
Copy link
Contributor

@larsbergstrom larsbergstrom commented Feb 2, 2016

The change required here is that the python code that chooses to use the gold linker:

if self.config["tools"]["rustc-with-gold"]:

Should not do so if the sys.platform == 'win32'.

@UK992
Copy link
Contributor Author

@UK992 UK992 commented Feb 2, 2016

@larsbergstrom in MINGW64 shell, sys.platform return 'msys', not 'win32'.

@jasonwilliams
Copy link
Contributor

@jasonwilliams jasonwilliams commented Feb 4, 2016

So would we check both msys and win32?
In the long term it would be good not to be relying so heavily on the mingw64 stuff at all

bors-servo added a commit that referenced this issue Feb 13, 2016
add check for win32 or msys before running rustc-with-gold fixes #9499

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9588)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.