[PATCH] D40689: [llvm] Add install-distribution-stripped

Chris Bieneman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 1 15:03:02 PST 2017


beanz added a reviewer: phosek.
beanz added a subscriber: phosek.
beanz added inline comments.


================
Comment at: CMakeLists.txt:352
 
+option(LLVM_ENABLE_INSTALL_DISTRIBUTION_STRIPPED
+       "Enable the install-distribution-stripped target, to install a stripped distribution."
----------------
smeenai wrote:
> beanz wrote:
> > 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.
> 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?
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.


https://reviews.llvm.org/D40689





More information about the llvm-commits mailing list