Skip to content

rustc: Swap link order of native libs/rust deps#28605

Merged
bors merged 1 commit into
rust-lang:masterfrom
alexcrichton:link-native-first
Oct 1, 2015
Merged

rustc: Swap link order of native libs/rust deps#28605
bors merged 1 commit into
rust-lang:masterfrom
alexcrichton:link-native-first

Conversation

@alexcrichton

Copy link
Copy Markdown
Member

This commit swaps the order of linking local native libraries and upstream
native libraries on the linker command line. Detail of bugs this can cause can
be found in #28595, and this change also invalidates the test case that was
added for #12446 which is now considered a bug because the downstream dependency
would need to declare that it depends on the native library somehow.

Closes #28595
[breaking-change]

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @pcwalton

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

@alexcrichton

Copy link
Copy Markdown
Member Author

r? @brson

To be clear, this is going to be a breaking change (as evidenced by the test case that needed to be removed), but I also believe this is a bug fix to correct the test case that was added (e.g. that should definitely work). If we decide to move forward then this should certainly be mentioned in the release notes, and I can write up a longer explanation about what errors might show up and how to fix them.

@rust-highfive rust-highfive assigned brson and unassigned pcwalton Sep 23, 2015
@brson brson added the relnotes Marks issues that should be documented in the release notes of the next release. label Sep 28, 2015
@brson

brson commented Sep 28, 2015

Copy link
Copy Markdown
Contributor

Should we crater this? Sounds frightening.

@brson

brson commented Sep 28, 2015

Copy link
Copy Markdown
Contributor

@brson Yes. Ok, I'll do it.

@brson

brson commented Sep 29, 2015

Copy link
Copy Markdown
Contributor

Still running.

@brson

brson commented Sep 30, 2015

Copy link
Copy Markdown
Contributor

Crater found nothing

@brson

brson commented Sep 30, 2015

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Sep 30, 2015

Copy link
Copy Markdown
Collaborator

📌 Commit 0acd11e has been approved by brson

@bors

bors commented Sep 30, 2015

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 0acd11e with merge baf8021...

@bors

bors commented Sep 30, 2015

Copy link
Copy Markdown
Collaborator

💔 Test failed - auto-linux-64-x-android-t

@alexcrichton

Copy link
Copy Markdown
Member Author

@bors: r=brson 27f0c78

@bors

bors commented Oct 1, 2015

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 27f0c78 with merge 26a1df3...

@bors

bors commented Oct 1, 2015

Copy link
Copy Markdown
Collaborator

💔 Test failed - auto-linux-64-x-android-t

This commit swaps the order of linking local native libraries and upstream
native libraries on the linker command line. Detail of bugs this can cause can
be found in rust-lang#28595, and this change also invalidates the test case that was
added for rust-lang#12446 which is now considered a bug because the downstream dependency
would need to declare that it depends on the native library somehow.

Closes rust-lang#28595
@alexcrichton

Copy link
Copy Markdown
Member Author

@bors: r=brson 9502df5

@bors

bors commented Oct 1, 2015

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 9502df5 with merge 587be42...

bors added a commit that referenced this pull request Oct 1, 2015
This commit swaps the order of linking local native libraries and upstream
native libraries on the linker command line. Detail of bugs this can cause can
be found in #28595, and this change also invalidates the test case that was
added for #12446 which is now considered a bug because the downstream dependency
would need to declare that it depends on the native library somehow.

Closes #28595
[breaking-change]
@bors bors merged commit 9502df5 into rust-lang:master Oct 1, 2015
@Ms2ger

Ms2ger commented Oct 1, 2015

Copy link
Copy Markdown
Contributor

@larsbergstrom fyi

@alexcrichton

Copy link
Copy Markdown
Member Author

Indeed thanks @Ms2ger! I'll also cc @metajack, and I've created a post on internals about this change to help get ahead of any surprising breakage.

@alexcrichton alexcrichton deleted the link-native-first branch October 1, 2015 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

relnotes Marks issues that should be documented in the release notes of the next release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants