Skip to content

Conversation

@compnerd
Copy link
Member

@compnerd compnerd commented Dec 1, 2017

This change broke the Windows builds for all the variants. -z defs cannot be
used on all targets, particularly in environments where the stdlib is being
compiled for a foreign host. This needs to be handled more carefully as I
mentioned on the PR itself. In the mean time, revert it to get Windows building
again.

Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.

Resolves SR-NNNN.

This change broke the Windows builds for all the variants.  `-z defs` cannot be
used on all targets, particularly in environments where the stdlib is being
compiled for a foreign host.  This needs to be handled more carefully as I
mentioned on the PR itself.  In the mean time, revert it to get Windows building
again.
@compnerd
Copy link
Member Author

compnerd commented Dec 1, 2017

CC @davezarzycki

@compnerd
Copy link
Member Author

compnerd commented Dec 1, 2017

@swift-ci please test and merge

@troughton
Copy link
Contributor

@compnerd Have you had a chance to look at #13140? I have a more comprehensive solution there.

@swift-ci swift-ci merged commit 5495392 into swiftlang:master Dec 1, 2017
@davezarzycki
Copy link
Contributor

Hi @compnerd – Thank you for selectively reverting just the part doesn't work for you instead of reverting the whole patch. That leaves the change I made to SwiftRemoteMirror in place that I need.

That being said, I think it is unfortunate to strip -Wl,-z,defs from the entire Linux build. Doing so allows for bugs and regressions to creep in. Can we not contain the project wide -Wl,-z,defs hack to just Windows somehow?

@troughton
Copy link
Contributor

@davezarzycki #13140 should take care of that, although I’m not sure if there’s a more elegant way.

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.

4 participants