This is identical to the install-distribution target, except that it
strips the installed binaries.
Details
Diff Detail
- Build Status
Buildable 12700 Build 12700: arc lint + arc unit
Event Timeline
CMakeLists.txt | ||
---|---|---|
352 | I don't think there is any reason for this to be gated on an option. Once your other patches adding stripped targets have fully gone through you should be able to always add this. |
CMakeLists.txt | ||
---|---|---|
352 | Like I said in the description, I added the install-*-distribution targets for pretty much everything I could think of, but it wouldn't surprise me at all if I missed something, and I'd rather not break people's cmake configures because of an oversight on my part. One alternative would be not have this option, and instead unconditionally try to create the install-distribution-stripped target, but give a warning (not an error) if any of the individual requisite install-*-stripped targets don't exist. It might be slightly noisy, but the warnings should be pretty easy to address, and eventually they can be upgraded to an error. I'd also still announce the change on llvm-dev, so people understand why they're suddenly getting these warnings during their configure. What do you think? |
CMakeLists.txt | ||
---|---|---|
352 | I think that the LLVM_DISTRIBUTION_COMPONENTS option is used infrequently enough that it probably is fine to have as an error by default and not gated by an option. The only place I think you're missing install-*-stripped targets is the llvm/runtimes directory's extra targets. I've added @phosek who has done a lot of the work in that area. I may have time to take a stab at it this afternoon. |
CMakeLists.txt | ||
---|---|---|
352 | libc++'s is awaiting review (https://reviews.llvm.org/D40680). I'd missed the runtimes one. I can throw up a patch for that if you haven't started working on that already. I'm happy to make this unconditional then. |
I'd be happy to review the runtimes part. Also, how is this supposed to interact with LLVM_EXTERNALIZE_DEBUGINFO?
The two are pretty orthogonal IMO. LLVM_EXTERNALIZE_DEBUGINFO controls the stripping of binaries in the build tree; this controls the stripping of binaries in the install tree. Since the binaries in the build tree are what get installed, LLVM_EXTERNALIZE_DEBUGINFO would also result in the installed binaries being stripped, but kinda indirectly. LLVM_EXTERNALIZE_DEBUGINFO also doesn't strip global symbols (it performs strip -gx) whereas this does, and the difference can be substantial in some cases (e.g. on my clang build strip -s saves 3 MB on top of strip -gx).
I don't think there is any reason for this to be gated on an option. Once your other patches adding stripped targets have fully gone through you should be able to always add this.