Page MenuHomePhabricator

[Clang][WIP] Allow renaming of "clang"
AbandonedPublic

Authored by nemanjai on May 25 2021, 4:22 PM.

Details

Summary

It is likely that at least some vendors would like the ability to rename the clang executable to a proprietary name. That would allow a clear differentiation between the upstream clang and a proprietary downstream version of clang on the same system.

I am not sure if there are vendors that already do this downstream but it seems like a useful capability to allow that to be parameterized in the CMake configuration and an upstream buildbot to be added that builds with a different name. We at IBM certainly desire this capability and are willing to convert one of our upstream buildbots to such a build.

This patch provides a new CMake macro CLANG_EXE_NAME that can be used to specify the name of the clang executable. Then the symlinks to it (i.e. clang-cl and clang++ will use that name and append the appropriate suffix). Furthermore, this patch updates test cases and utilities that use clang to use this name. The name is added to llvm-config so that it can be queried using llvm-config --clang-exe (which will return the full path with the correct name of the clang executable).

This is a work in progress because it is incomplete. Depending on the feedback on this review, the name change can affect the various clang-xx utilities, libclang... libraries, etc.

Please provide feedback if you think this is useful, not useful, a good/bad idea, good/bad technical direction etc.

Diff Detail

Unit TestsFailed

TimeTest
970 msx64 windows > Clang.utils/update_cc_test_checks::basic-cplusplus.test
Script: -- : 'RUN: at line 3'; cp C:\ws\w16-1\llvm-project\premerge-checks\clang\test\utils\update_cc_test_checks/Inputs/basic-cplusplus.cpp C:\ws\w16-1\llvm-project\premerge-checks\build\tools\clang\test\utils\update_cc_test_checks\Output\basic-cplusplus.test.tmp.cpp && C:/Python39/python.exe 'c:\ws\w16-1\llvm-project\premerge-checks\build\tools\clang\test\..\..\..\..\llvm\utils\update_cc_test_checks.py' --clang 'c:\ws\w16-1\llvm-project\premerge-checks\build\tools\clang\test\..\..\..\bin\clang' --opt 'c:\ws\w16-1\llvm-project\premerge-checks\build\tools\clang\test\..\..\..\bin\opt' C:\ws\w16-1\llvm-project\premerge-checks\build\tools\clang\test\utils\update_cc_test_checks\Output\basic-cplusplus.test.tmp.cpp

Event Timeline

nemanjai created this revision.May 25 2021, 4:22 PM
nemanjai requested review of this revision.May 25 2021, 4:22 PM
Herald added projects: Restricted Project, Restricted Project, Restricted Project, Restricted Project. · View Herald TranscriptMay 25 2021, 4:22 PM
Herald added subscribers: llvm-commits, Restricted Project, wdng. · View Herald Transcript

Can't say I'm super enthusiastic about this (I assume the build already supports prefixes and suffixes, which I'd hope would be adequate - but presumably are not for your use case), though there's some, I think, related prior art: Sony folks (@probinson @jhenderson) have (or had at some point) different C++ language standard/version defaults than upstream and have maintained/made changes to upstream test cases that assume the upstream default version to not make that assumption (to have it explicit). So having some costs/changes upstream for downstream differences like this seems at least vaguely plausible to me.

I guess I might use this if it existed to have shorter names for my development build (rather than clang-tot maybe I could name it clot or something... ).

Can't say I'm super enthusiastic about this (I assume the build already supports prefixes and suffixes, which I'd hope would be adequate - but presumably are not for your use case), though there's some, I think, related prior art: Sony folks (@probinson @jhenderson) have (or had at some point) different C++ language standard/version defaults than upstream and have maintained/made changes to upstream test cases that assume the upstream default version to not make that assumption (to have it explicit). So having some costs/changes upstream for downstream differences like this seems at least vaguely plausible to me.

We do our executable renaming post build and lit testing. We do the renaming to nearly all our built tools, not just the clang (clang++ etc) family, e.g. the LLVM binutils like llvm-objdump becomes xxx-llvm-objdump, so unless the scope of this increases to include those, I'm not sure how useful it would be to us (and expanding the scope to other tools becomes problematic because there isn't a %clang equivalent for testing purposes for those other tools, so presumably would require significantly more updates?).

@probinson may have more thoughts on this though.

Can't say I'm super enthusiastic about this (I assume the build already supports prefixes and suffixes, which I'd hope would be adequate - but presumably are not for your use case), though there's some, I think, related prior art: Sony folks (@probinson @jhenderson) have (or had at some point) different C++ language standard/version defaults than upstream and have maintained/made changes to upstream test cases that assume the upstream default version to not make that assumption (to have it explicit). So having some costs/changes upstream for downstream differences like this seems at least vaguely plausible to me.

We do our executable renaming post build and lit testing. We do the renaming to nearly all our built tools, not just the clang (clang++ etc) family, e.g. the LLVM binutils like llvm-objdump becomes xxx-llvm-objdump, so unless the scope of this increases to include those, I'm not sure how useful it would be to us (and expanding the scope to other tools becomes problematic because there isn't a %clang equivalent for testing purposes for those other tools, so presumably would require significantly more updates?).

@probinson may have more thoughts on this though.

No, you've stated my thoughts almost exactly. If this became a generic prefixing thing, we'd want to apply it to clang, llvm tools, lld, etc.

Re the %clang equivalent, actually there is already tool-name substitution without the % in order to add the build-dir path to the tool name, so I *think* that can be made to work without having to update every test in existence. But, if the prefix isn't added universally, it's not really useful for us.

I'm also not a fan of this change. From a project perspective the binary is clang and while people may wish to change the name for their own product teams it seems like that onus should be on them.

I'm also not a fan of this change. From a project perspective the binary is clang and while people may wish to change the name for their own product teams it seems like that onus should be on them.

It occurs to me that there's precedent regarding prefixes, which is that you can rename/soft-link clang to have a triple embedded in the name (e.g., x86_64-pc-linux-clang) and then the driver will factor the prefix into default-target computations. But AFAIK there's no support for directly doing that renaming/linking in the build system. I agree with @echristo this properly belongs to the vendors not the project.

nemanjai abandoned this revision.May 28 2021, 5:54 AM

Thanks everyone for providing feedback on this. I posted this to gauge interest in the community for such a change. As it appears, the consensus seems to be that this isn't desired so I will abandon this change and vendors will continue with their homegrown solutions to this problem.