[cfe-dev] Renaming the install-clang-headers target

Shoaib Meenai via cfe-dev cfe-dev at lists.llvm.org
Wed Feb 27 12:55:52 PST 2019


Any other opinions here? So far, Tom and Petr (in the original review) are in favor of renaming the existing behavior of the install-clang-headers target and repurposing that name for installing clang's API headers, to have better consistency with LLVM's targets, and Dimitry is in favor of adding a new target for the new behavior, since it's less disruptive.

I'll note that this target is present in a bunch of the cache files in clang/cmake/caches (specifically BaremetalARM.cmake, Fuchsia-stage2.cmake, DistributionExample-stage2.cmake, and Apple-stage2.cmake), but I can just update those myself if I change the target, so I'm not too concerned. It doesn't appear to be explicitly referenced by any of the zorg builders though.

From: Shoaib Meenai <smeenai at fb.com>
Date: Monday, February 25, 2019 at 10:53 AM
To: Dimitry Andric <dimitry at andric.com>, Tom Stellard <tstellar at redhat.com>
Cc: cfe-dev <cfe-dev at lists.llvm.org>
Subject: Re: [cfe-dev] Renaming the install-clang-headers target

The name we were floating around was install-clang-library-headers (to match with install-clang-libraries), although I like install-clang-api-headers too. I agree that adding a new target would be least disruptive, but the existing inconsistency between install-llvm-headers (which does install LLVM's API headers) and install-clang-headers is also unfortunate.

From: Dimitry Andric <dimitry at andric.com>
Date: Monday, February 25, 2019 at 12:42 PM
To: Tom Stellard <tstellar at redhat.com>
Cc: Shoaib Meenai <smeenai at fb.com>, cfe-dev <cfe-dev at lists.llvm.org>
Subject: Re: [cfe-dev] Renaming the install-clang-headers target

On 25 Feb 2019, at 16:13, Tom Stellard <tstellar at redhat.com<mailto:tstellar at redhat.com>> wrote:
On 02/21/2019 11:24 PM, Dimitry Andric wrote:
On 22 Feb 2019, at 03:44, Tom Stellard via cfe-dev <cfe-dev at lists.llvm.org<mailto:cfe-dev at lists.llvm.org>> wrote:
On 02/21/2019 03:56 PM, Shoaib Meenai wrote:
Thanks. What sort of transition plan would be appropriate here? Again, if this happened, we'd be changing the existing install-clang-headers target to install clang's library headers instead of its resource directory headers, and adding a new install-clang-resource-headers to install the resource directory headers, so users of the existing install-clang-headers target would have to change their builds to use install-clang-resource-headers. Some ideas I can think of:
* Just note the target name change in a PSA to cfe-dev and/or the release notes and let users take it from there.
I would do both, announce it to the list and add an entry to the
release notes.  I don't think anything else is required,
because this target is probably not used that much anyway.
I use it, to create minimal versions of clang, e.g. just the clang executable and the required resource headers.  That is very convenient for bisecting, and way less bloated than a full installation.  That said, I don't have any strong feelings either way: a new target would be easier for me, but it is indeed less consistent.
Does the clang target depend on install-clang-headers?  It seems like it
should.

Well, the "clang" target is only for building the actual clang executable, while "install-clang" installs it.  This is similar for other targets, like "lld" and "install-lld", so I wouldn't change it.

I guess "install-clang-headers" was always a bit of an exceptional case, because it does not install any clang API headers, but the set of internal compiler headers (which is fairly tightly matched to the exact revision of the clang executable).  It would seem to be least disruptive to just add a new install-xxx target for installing the clang API headers, for example "install-clang-api-headers".

-Dimitry


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20190227/f84512e8/attachment.html>


More information about the cfe-dev mailing list