Renaming the install-clang-headers target

The install-clang-headers target in Clang today installs Clang’s resource directory headers (the contents of lib/clang/$VERSION/include). This is inconsistent with LLVM’s install-llvm-headers target, which installs the headers corresponding to LLVM’s libraries.

Clang does install its library headers when you perform a full install, but there’s currently no target to specifically just install these headers (e.g. as part of a distribution). I’m attempting to add one in https://reviews.llvm.org/D58317, and it would be natural to name this target install-clang-headers to mirror the corresponding LLVM target, but of course that name is already taken by the resource headers install target.

One option would be to rename the existing install-clang-headers target to something like install-clang-resource-headers, and then switch install-clang-headers to installing the library headers. This would require end users to update their build scripts, and anyone who didn’t update their build scripts would just silently install the wrong thing (though that would presumably lead to errors later on when actually trying to use the installed toolchain). I’m posting here to ask if such a switch would be acceptable to users of this target, and what I can do to announce and ease the transition (besides just a PSA to the mailing lists).

Another option would be to accept the inconsistency between LLVM and Clang’s target names, leave the existing install-clang-headers target alone, and come up with a new name for the clang target to install its library headers. I’m not completely happy with any of the names I’ve been able to come up with, though install-clang-library-headers seems like an okay option.

The install-clang-headers target in Clang today installs Clang's resource directory headers (the contents of lib/clang/$VERSION/include). This is inconsistent with LLVM's install-llvm-headers target, which installs the headers corresponding to LLVM's libraries.

Clang does install its library headers when you perform a full install, but there's currently no target to specifically just install these headers (e.g. as part of a distribution). I'm attempting to add one in https://reviews.llvm.org/D58317, and it would be natural to name this target install-clang-headers to mirror the corresponding LLVM target, but of course that name is already taken by the resource headers install target.

One option would be to rename the existing install-clang-headers target to something like install-clang-resource-headers, and then switch install-clang-headers to installing the library headers. This would require end users to update their build scripts, and anyone who didn't update their build scripts would just silently install the wrong thing (though that would presumably lead to errors later on when actually trying to use the installed toolchain). I'm posting here to ask if such a switch would be acceptable to users of this target, and what I can do to announce and ease the transition (besides just a PSA to the mailing lists).

I think it's better to have consistent naming between clang and llvm, so
I am in favor of renaming the existing install-clang-headers target
to something else.

-Tom

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.
  • Have the install-clang-headers target give a warning (which might be lost in build noise) or an error (which would be much more noticeable) informing them of the change. Users who want the new behavior for the install-clang-headers target could pass a CMake flag to enable it (this is similar to how the soft errors for old toolchains are handled). We could drop the error and make the new behavior the default after a release (when it’s reasonable to expect everyone to have updated their builds already).

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.

-Tom

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.

-Dimitry

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.

-Tom

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

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.

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.

I’ll join Tom and Petr on the renaming. I don’t think this name is so widely used that we can’t rename it.