BaseTools: allow users to override CC and CXX on the make command line #4162
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 3 commits
March 23, 2023 09:24
In https://bugzilla.tianocore.org/show_bug.cgi?id=2842 clang support was added by having users specify "make CXX=llvm" when building BaseTools. The Makefile then sees that and sets CC=$(CLANG_BIN)clang and CXX=$(CLANG_BIN)clang++. That requires that the executables 'clang' and 'clang++' exist and for example aren't named 'clang-17' and 'clang++-17'. Also, it's an unusual way of specifying the compiler, since many users will expect to be able to override CC and CXX on the make command line. Rework the BaseTools Makefiles removing the 'BUILD_' prefix (BUILD_CC and BUILD_CXX) and using the standard name 'LDFLAGS' instead of 'LFLAGS'. This allows clang to be used by running 'make -C BaseTools CC=clang CXX=clang++'. This also requires reworking the gcc support, since $(CC) has a default value of 'cc', so the '?=' syntax won't override it. Fix this by checking if $(CC) has its original value, and if so overriding the environment. Signed-off-by: Rebecca Cran <rebecca@quicinc.com> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
In https://bugzilla.tianocore.org/show_bug.cgi?id=2842 clang support was added by having users specify "make CXX=llvm" when building BaseTools. Improve the detection of when a user wants to use the clang toolchain: instead of checking if CXX=llvm (which in most cases doesn't make sense, because the C++ compiler won't be run via an 'llvm' command), run '$(CC) --version | grep clang' to see if the compiler's version string contains 'clang', and if so configure the environment. This provides flexibility to specify for example CC=clang-17 CXX=clang++-17 if multiple versions are installed. Signed-off-by: Rebecca Cran <rebecca@quicinc.com> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
clang 17 defaults to C++17, where the 'register' keyword is deprecated and the warning changed to an error. To avoid build errors, compile against C++14 by specifying '-std=c++14' in CXXFLAGS. Signed-off-by: Rebecca Cran <rebecca@quicinc.com> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
|
@lgao4 Patch check is failing. Removing push label until CI check failures are resolved. |
|
PR can not be merged due to conflict. Please rebase and resubmit |
1 similar comment
|
PR can not be merged due to conflict. Please rebase and resubmit |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, the BaseTools Makefiles use BUILD_CC and BUILD_CXX, which
doesn't allow users to override the compiler to use in the expected way
by running e.g. "make CC=clang-17 CXX=clang++-17". clang/llvm support
was added in https://bugzilla.tianocore.org/show_bug.cgi?id=2842 in a
way that required users to run "make CXX=llvm" and have clang and clang++
executables under $(CLANG_BIN). As far as I know this isn't a standard
way of telling a build system to use clang, and so is likely difficult
to discover by users.
This patch series fixes that, and as a side effect allows the clang
analyzer to run via "scan-build make".
Since clang 17 defaults to C++17 or newer where the 'register' keyword
is deprecated and the warning turned into an error, override the
version used when building C++ code via "-std=c++14".