cc-wrapper: More intelligent sierra hack #29096
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shlevy
approved these changes
Sep 7, 2017
| @@ -2,67 +2,113 @@ | |||
|
|
|||
| set -eu -o pipefail | |||
|
|
|||
| # For cmd | while; read; do ...; done | |||
There was a problem hiding this comment.
This is needed for the block after the pipe (the while loop here) to set Bash variables. Otherwise it runs in a subshell and the variables it sets are not visible outside that block.
There was a problem hiding this comment.
I wouldn't have though of this reason without the comment, so it was useful for me.
There was a problem hiding this comment.
Yeah it's not clear to my lastpipe is always an improvement, unlike the other extensions I used, so I wanted to motivate it.
I should remove the extra semicolon, however :D
Ericson2314
force-pushed
the
appease-sierra-linker
branch
from
bc87860
to
ff214ff
Compare
Patch is drawn from [1], a back-port of [2]. Eventually, we might consider doing something for GNU binutils too, in order that we switch (the normal) ld-wrapper to always use this to leverage ld to resolve libraries, rather than faking it in bash. [1]: https://github.com/obsidiansystems/cctools-port/tree/libs-normalize-17.03 [2]: tpoechtrager/cctools-port#34
Ericson2314
force-pushed
the
appease-sierra-linker
branch
3 times, most recently
from
86316fb
to
ba053dc
Compare
Also fix numberous bugs, such as: - Not getting confused on more flags taking file arguments. - Ensuring children reexport their children, but the original binary/library doesn't. - Not spawning children when it turns out we just dynamically link under the threshold but our total number of inputs exceeeds it. - Children were always named `libunnamed-*`, when that name was supposed to be the last resort only. In addition to the script, we also patch ld-wrapper to respect `.dylib` and `.so` alike. In a future version of nixpkgs, this can be so enabled by defaut. Newer nixpkgs will probably do this by default.
Ericson2314
force-pushed
the
appease-sierra-linker
branch
from
ba053dc
to
c5b655a
Compare
|
Ok! I tested this on a big haskell project ;) and with my existing C test. It passes! |
Ericson2314
added
the
9.needs: port to stable
8 tasks
8 tasks
samueldr
removed
the
9.needs: port to stable
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation for this change
I made a patch for cctool's ld in order to get it to split out flags for all inputs in a fully-resolved normal form. I use this to avoid trying to
-reexport-lstatic libraries. Linking them directly will reexport them just fine.@TaktInc needs this for 17.03, so I'm starting here and then forward porting to master (and probably
17.09if all goes well).The patch itself uses
\0as a delimiter, but perhapsexpandResponseParamscould be used to parse a more human readable output. I just bring this up if it would help upstream the patch.N.B. as I mentioned in #24844 (comment), if we likewise patch GNU Binutils, we can dramatically simplify ld-wrapper (with and without the sierra hack) by not trying to resolve linker scripts on our own.
Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)CC @orivej