Skip to content
This repository was archived by the owner on Apr 13, 2024. It is now read-only.

driver: Enable llvm-objcopy and llvm-strip #253

Merged

Conversation

nathanchance
Copy link
Member

except for s390, which does not appear to have any support for it in
the LLVM tools.

Tested locally with build-all.sh.

Closes: #212
Closes: #249

except for s390, which does not appear to have any support for it in
the LLVM tools.

Tested locally with build-all.sh.

[skip ci]

Closes: ClangBuiltLinux#212
Closes: ClangBuiltLinux#249
Signed-off-by: Nathan Chancellor <[email protected]>
@tpimh
Copy link
Contributor

tpimh commented Apr 2, 2020

For s390 you set OBJDUMP variable, not STRIP. Is it intentional?

@nathanchance
Copy link
Member Author

Yes and no. I intentionally left it out because it does not appear to be used during a build (I have not seen an error). I can add it if you feel it necessary.

@tpimh
Copy link
Contributor

tpimh commented Apr 2, 2020

If it's not used during the build, no need to add it. I was just concerned because this pull request doesn't mention objdump anywhere else.

@nathanchance
Copy link
Member Author

Ah, gotcha. The objdump one is ClangBuiltLinux/linux#859 and it has been there since we added s390 support.

Copy link
Member

@nickdesaulniers nickdesaulniers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool, so we'll still need to turn this on in the travis yaml? Also, I might prefer to just fail the build if we're using an old binutils. Kbuild doesn't have that fallback behavior, and I worry that things might be broken with the llvm utils and we don't notice because no alarms go off, because we've just been falling back. Basically, it's a vote against supporting older versions. But that doesn't need to block this patch. 🚅

@nathanchance
Copy link
Member Author

cool, so we'll still need to turn this on in the travis yaml?

Nope, it is enabled by default now.

Basically, it's a vote against supporting older versions.

Sure, I can push a patch after I merge this.

@nathanchance nathanchance merged commit df9300f into ClangBuiltLinux:master Apr 2, 2020
@nathanchance nathanchance deleted the llvm-objcopy-strip branch April 2, 2020 17:49
@nickdesaulniers
Copy link
Member

Nope, it is enabled by default now.

rereads code

got it, just not s390, I see.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

enable llvm-objcopy llvm-strip
3 participants