public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/3] BaseTools: allow users to override CC and CXX on the make command line
@ 2023-02-17  3:50 Rebecca Cran
  2023-02-17  3:50 ` [PATCH 1/3] BaseTools: Allow users to specify compiler to use with make CC= CXX= Rebecca Cran
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Rebecca Cran @ 2023-02-17  3:50 UTC (permalink / raw)
  To: devel, Andrew Fish, Leif Lindholm, Michael D Kinney, Bob Feng,
	Liming Gao, Yuwei Chen
  Cc: Rebecca Cran

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".

Rebecca Cran (3):
  BaseTools: Allow users to specify compiler to use with make CC= CXX=
  BaseTools: Improve detection of users wanting to build using clang
  BaseTools: Build against C++14 when building with clang

 BaseTools/Source/C/DevicePath/GNUmakefile          |  7 ++-
 BaseTools/Source/C/LzmaCompress/GNUmakefile        |  2 +-
 BaseTools/Source/C/Makefiles/app.makefile          |  2 +-
 BaseTools/Source/C/Makefiles/footer.makefile       |  6 +-
 BaseTools/Source/C/Makefiles/header.makefile       | 59 ++++++++++----------
 BaseTools/Source/C/VfrCompile/GNUmakefile          | 19 ++++---
 BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 20 +++----
 BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile   | 31 +++++-----
 BaseTools/Source/Python/Workspace/DscBuildData.py  |  2 +-
 9 files changed, 76 insertions(+), 72 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/3] BaseTools: Allow users to specify compiler to use with make CC= CXX=
  2023-02-17  3:50 [PATCH 0/3] BaseTools: allow users to override CC and CXX on the make command line Rebecca Cran
@ 2023-02-17  3:50 ` Rebecca Cran
  2023-03-15 10:07   ` [edk2-devel] " Gerd Hoffmann
  2023-02-17  3:51 ` [PATCH 2/3] BaseTools: Improve detection of users wanting to build using clang Rebecca Cran
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Rebecca Cran @ 2023-02-17  3:50 UTC (permalink / raw)
  To: devel, Andrew Fish, Leif Lindholm, Michael D Kinney, Bob Feng,
	Liming Gao, Yuwei Chen
  Cc: Rebecca Cran

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>
---
 BaseTools/Source/C/DevicePath/GNUmakefile          |  4 +-
 BaseTools/Source/C/LzmaCompress/GNUmakefile        |  2 +-
 BaseTools/Source/C/Makefiles/app.makefile          |  2 +-
 BaseTools/Source/C/Makefiles/footer.makefile       |  6 +--
 BaseTools/Source/C/Makefiles/header.makefile       | 52 ++++++++++----------
 BaseTools/Source/C/VfrCompile/GNUmakefile          | 14 +++---
 BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 18 +++----
 BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile   | 28 +++++------
 BaseTools/Source/Python/Workspace/DscBuildData.py  |  2 +-
 9 files changed, 64 insertions(+), 64 deletions(-)

diff --git a/BaseTools/Source/C/DevicePath/GNUmakefile b/BaseTools/Source/C/DevicePath/GNUmakefile
index 13b54ead65ac..3afc7fc0504e 100644
--- a/BaseTools/Source/C/DevicePath/GNUmakefile
+++ b/BaseTools/Source/C/DevicePath/GNUmakefile
@@ -13,12 +13,12 @@ OBJECTS = DevicePath.o UefiDevicePathLib.o DevicePathFromText.o  DevicePathUtili
 
 include $(MAKEROOT)/Makefiles/app.makefile
 
-GCCVERSION = $(shell $(BUILD_CC) -dumpversion | awk -F'.' '{print $$1}')
+GCCVERSION = $(shell $(CC) -dumpversion | awk -F'.' '{print $$1}')
 ifneq ("$(GCCVERSION)", "5")
 ifneq ($(CXX), llvm)
 ifneq ($(DARWIN),Darwin)
 # gcc 12 trips over device path handling
-BUILD_CFLAGS += -Wno-error=stringop-overflow
+CFLAGS += -Wno-error=stringop-overflow
 endif
 endif
 endif
diff --git a/BaseTools/Source/C/LzmaCompress/GNUmakefile b/BaseTools/Source/C/LzmaCompress/GNUmakefile
index c837e7782373..a00ef4bc8061 100644
--- a/BaseTools/Source/C/LzmaCompress/GNUmakefile
+++ b/BaseTools/Source/C/LzmaCompress/GNUmakefile
@@ -24,4 +24,4 @@ OBJECTS = \
 
 include $(MAKEROOT)/Makefiles/app.makefile
 
-BUILD_CFLAGS += -D_7ZIP_ST
+CFLAGS += -D_7ZIP_ST
diff --git a/BaseTools/Source/C/Makefiles/app.makefile b/BaseTools/Source/C/Makefiles/app.makefile
index 6a2a8f5e8a0e..506343a6e0b4 100644
--- a/BaseTools/Source/C/Makefiles/app.makefile
+++ b/BaseTools/Source/C/Makefiles/app.makefile
@@ -15,7 +15,7 @@ APPLICATION = $(MAKEROOT)/bin/$(APPNAME)
 all: $(MAKEROOT)/bin $(APPLICATION)
 
 $(APPLICATION): $(OBJECTS)
-	$(LINKER) -o $(APPLICATION) $(BUILD_LFLAGS) $(OBJECTS) -L$(MAKEROOT)/libs $(LIBS)
+	$(LINKER) -o $(APPLICATION) $(LDFLAGS) $(OBJECTS) -L$(MAKEROOT)/libs $(LIBS)
 
 $(OBJECTS): $(MAKEROOT)/Include/Common/BuildVersion.h
 
diff --git a/BaseTools/Source/C/Makefiles/footer.makefile b/BaseTools/Source/C/Makefiles/footer.makefile
index 85c3374224f2..7546da8cf5a0 100644
--- a/BaseTools/Source/C/Makefiles/footer.makefile
+++ b/BaseTools/Source/C/Makefiles/footer.makefile
@@ -15,13 +15,13 @@ install: $(MAKEROOT)/libs-$(HOST_ARCH) $(LIBRARY)
 	cp $(LIBRARY) $(MAKEROOT)/libs-$(HOST_ARCH)
 
 $(LIBRARY): $(OBJECTS)
-	$(BUILD_AR) crs $@ $^
+	$(AR) crs $@ $^
 
 %.o : %.c
-	$(BUILD_CC)  -c $(BUILD_CPPFLAGS) $(BUILD_CFLAGS) $< -o $@
+	$(CC)  -c $(CPPFLAGS) $(CFLAGS) $< -o $@
 
 %.o : %.cpp
-	$(BUILD_CXX) -c $(BUILD_CPPFLAGS) $(BUILD_CXXFLAGS) $< -o $@
+	$(CXX) -c $(CPPFLAGS) $(CXXFLAGS) $< -o $@
 
 .PHONY: clean
 clean:
diff --git a/BaseTools/Source/C/Makefiles/header.makefile b/BaseTools/Source/C/Makefiles/header.makefile
index 4e88a4fbd86b..347918c7d4fa 100644
--- a/BaseTools/Source/C/Makefiles/header.makefile
+++ b/BaseTools/Source/C/Makefiles/header.makefile
@@ -45,19 +45,19 @@ CYGWIN:=$(findstring CYGWIN, $(shell uname -s))
 LINUX:=$(findstring Linux, $(shell uname -s))
 DARWIN:=$(findstring Darwin, $(shell uname -s))
 ifeq ($(CXX), llvm)
-BUILD_CC ?= $(CLANG_BIN)clang
-BUILD_CXX ?= $(CLANG_BIN)clang++
-BUILD_AS ?= $(CLANG_BIN)clang
-BUILD_AR ?= $(CLANG_BIN)llvm-ar
-BUILD_LD ?= $(CLANG_BIN)llvm-ld
-else
-BUILD_CC ?= gcc
-BUILD_CXX ?= g++
-BUILD_AS ?= gcc
-BUILD_AR ?= ar
-BUILD_LD ?= ld
+CC ?= $(CLANG_BIN)clang
+CXX ?= $(CLANG_BIN)clang++
+AS ?= $(CLANG_BIN)clang
+AR ?= $(CLANG_BIN)llvm-ar
+LD ?= $(CLANG_BIN)llvm-ld
+else ifeq ($(origin CC),default)
+CC = gcc
+CXX = g++
+AS = gcc
+AR = ar
+LD = ld
 endif
-LINKER ?= $(BUILD_CC)
+LINKER ?= $(CC)
 ifeq ($(HOST_ARCH), IA32)
 ARCH_INCLUDE = -I $(MAKEROOT)/Include/Ia32/
 
@@ -81,34 +81,34 @@ $(error Bad HOST_ARCH)
 endif
 
 INCLUDE = $(TOOL_INCLUDE) -I $(MAKEROOT) -I $(MAKEROOT)/Include/Common -I $(MAKEROOT)/Include/ -I $(MAKEROOT)/Include/IndustryStandard -I $(MAKEROOT)/Common/ -I .. -I . $(ARCH_INCLUDE)
-BUILD_CPPFLAGS = $(INCLUDE)
+CPPFLAGS = $(INCLUDE)
 
 # keep EXTRA_OPTFLAGS last
 BUILD_OPTFLAGS = -O2 $(EXTRA_OPTFLAGS)
 
 ifeq ($(DARWIN),Darwin)
 # assume clang or clang compatible flags on OS X
-BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror \
+CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror \
 -Wno-deprecated-declarations -Wno-self-assign -Wno-unused-result -nostdlib -g
 else
 ifeq ($(CXX), llvm)
-BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -fwrapv \
+CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -fwrapv \
 -fno-delete-null-pointer-checks -Wall -Werror \
 -Wno-deprecated-declarations -Wno-self-assign \
 -Wno-unused-result -nostdlib -g
 else
-BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -fwrapv \
+CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -fwrapv \
 -fno-delete-null-pointer-checks -Wall -Werror \
 -Wno-deprecated-declarations -Wno-stringop-truncation -Wno-restrict \
 -Wno-unused-result -nostdlib -g
 endif
 endif
 ifeq ($(CXX), llvm)
-BUILD_LFLAGS =
-BUILD_CXXFLAGS = -Wno-deprecated-register -Wno-unused-result
+LDFLAGS =
+CXXFLAGS = -Wno-deprecated-register -Wno-unused-result
 else
-BUILD_LFLAGS =
-BUILD_CXXFLAGS = -Wno-unused-result
+LDFLAGS =
+CXXFLAGS = -Wno-unused-result
 endif
 ifeq ($(HOST_ARCH), IA32)
 #
@@ -117,18 +117,18 @@ ifeq ($(HOST_ARCH), IA32)
 #  so only do this is uname -m returns i386.
 #
 ifeq ($(DARWIN),Darwin)
-  BUILD_CFLAGS   += -arch i386
-  BUILD_CPPFLAGS += -arch i386
-  BUILD_LFLAGS   += -arch i386
+  CFLAGS   += -arch i386
+  CPPFLAGS += -arch i386
+  LDFLAGS  += -arch i386
 endif
 endif
 
 # keep BUILD_OPTFLAGS last
-BUILD_CFLAGS   += $(BUILD_OPTFLAGS)
-BUILD_CXXFLAGS += $(BUILD_OPTFLAGS)
+CFLAGS   += $(BUILD_OPTFLAGS)
+CXXFLAGS += $(BUILD_OPTFLAGS)
 
 # keep EXTRA_LDFLAGS last
-BUILD_LFLAGS += $(EXTRA_LDFLAGS)
+LDFLAGS += $(EXTRA_LDFLAGS)
 
 .PHONY: all
 .PHONY: install
diff --git a/BaseTools/Source/C/VfrCompile/GNUmakefile b/BaseTools/Source/C/VfrCompile/GNUmakefile
index fc329944b992..9fbaaaba21d7 100644
--- a/BaseTools/Source/C/VfrCompile/GNUmakefile
+++ b/BaseTools/Source/C/VfrCompile/GNUmakefile
@@ -17,9 +17,9 @@ TOOL_INCLUDE = -I Pccts/h
 OBJECTS = AParser.o DLexerBase.o ATokenBuffer.o EfiVfrParser.o VfrLexer.o VfrSyntax.o \
           VfrFormPkg.o VfrError.o VfrUtilityLib.o VfrCompiler.o
 ifeq ($(CXX), llvm)
-VFR_CPPFLAGS = -Wno-deprecated-register -DPCCTS_USE_NAMESPACE_STD $(BUILD_CPPFLAGS)
+VFR_CPPFLAGS = -Wno-deprecated-register -DPCCTS_USE_NAMESPACE_STD $(CPPFLAGS)
 else
-VFR_CPPFLAGS = -DPCCTS_USE_NAMESPACE_STD $(BUILD_CPPFLAGS)
+VFR_CPPFLAGS = -DPCCTS_USE_NAMESPACE_STD $(CPPFLAGS)
 endif
 # keep BUILD_OPTFLAGS last
 VFR_CXXFLAGS = $(BUILD_OPTFLAGS)
@@ -27,7 +27,7 @@ VFR_CXXFLAGS = $(BUILD_OPTFLAGS)
 # keep EXTRA_LDFLAGS last
 VFR_LFLAGS = $(EXTRA_LDFLAGS)
 
-LINKER = $(BUILD_CXX)
+LINKER = $(CXX)
 
 EXTRA_CLEAN_OBJECTS = EfiVfrParser.cpp EfiVfrParser.h VfrParser.dlg VfrTokens.h VfrLexer.cpp VfrLexer.h VfrSyntax.cpp tokens.h
 
@@ -60,16 +60,16 @@ Pccts/dlg/dlg:
 	BIN_DIR='.' $(MAKE) -C Pccts/dlg
 
 ATokenBuffer.o: Pccts/h/ATokenBuffer.cpp
-	$(BUILD_CXX) -c $(VFR_CPPFLAGS) $(INC) $(VFR_CXXFLAGS) $? -o $@
+	$(CXX) -c $(VFR_CPPFLAGS) $(INC) $(VFR_CXXFLAGS) $? -o $@
 
 DLexerBase.o: Pccts/h/DLexerBase.cpp
-	$(BUILD_CXX) -c $(VFR_CPPFLAGS) $(INC) $(VFR_CXXFLAGS) $? -o $@
+	$(CXX) -c $(VFR_CPPFLAGS) $(INC) $(VFR_CXXFLAGS) $? -o $@
 
 AParser.o: Pccts/h/AParser.cpp
-	$(BUILD_CXX) -c $(VFR_CPPFLAGS) $(INC) $(VFR_CXXFLAGS) $? -o $@
+	$(CXX) -c $(VFR_CPPFLAGS) $(INC) $(VFR_CXXFLAGS) $? -o $@
 
 VfrSyntax.o: VfrSyntax.cpp
-	$(BUILD_CXX) -c $(VFR_CPPFLAGS) $(INC) $(VFR_CXXFLAGS) $? -o $@
+	$(CXX) -c $(VFR_CPPFLAGS) $(INC) $(VFR_CXXFLAGS) $? -o $@
 
 clean: localClean
 
diff --git a/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile b/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
index 559b1c99f1ef..558d2f7b0111 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
+++ b/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
@@ -157,7 +157,7 @@ PCCTS_H=../h
 #	$(DLG) -C2 parser.dlg scan.c
 #
 #set.$(OBJ_EXT): $(SET)/set.c
-#	$(BUILD_CC) $(BUILD_CFLAGS) -c $(OUT_OBJ)set.$(OBJ_EXT) $(SET)/set.c
+#	$(CC) $(CFLAGS) -c $(OUT_OBJ)set.$(OBJ_EXT) $(SET)/set.c
 
 
 
@@ -165,17 +165,17 @@ PCCTS_H=../h
 #   UNIX  (default)
 #
 ifeq ($(CXX), llvm)
-BUILD_CC?=$(CLANG_BIN)clang
-else
-BUILD_CC?=gcc
+CC?=$(CLANG_BIN)clang
+else ifeq ($(origin CC),default)
+CC=gcc
 endif
 COPT=-O
 ANTLR=${BIN_DIR}/antlr
 DLG=${BIN_DIR}/dlg
 OBJ_EXT=o
 OUT_OBJ = -o
-BUILD_CFLAGS= $(COPT) -I. -I$(SET) -I$(PCCTS_H) -DUSER_ZZSYN $(COTHER) -DZZLEXBUFSIZE=65536
-BUILD_CPPFLAGS=
+CFLAGS= $(COPT) -I. -I$(SET) -I$(PCCTS_H) -DUSER_ZZSYN $(COTHER) -DZZLEXBUFSIZE=65536
+CPPFLAGS=
 #
 # SGI Users, use this CFLAGS
 #
@@ -184,7 +184,7 @@ OBJ=antlr.o scan.o err.o bits.o build.o fset2.o fset.o gen.o  \
         globals.o hash.o lex.o main.o misc.o set.o pred.o egman.o mrhoist.o fcache.o
 
 $(BIN_DIR)/antlr : $(OBJ) $(SRC)
-		$(BUILD_CC) $(BUILD_CFLAGS) -o $(BIN_DIR)/antlr $(OBJ)
+		$(CC) $(CFLAGS) -o $(BIN_DIR)/antlr $(OBJ)
 
 # what files does PCCTS generate (both ANTLR and DLG)
 PCCTS_GEN=antlr.c scan.c err.c tokens.h mode.h parser.dlg stdpccts.h remap.h
@@ -207,10 +207,10 @@ scan.o : scan.c mode.h tokens.h
 #	$(DLG) -C2 parser.dlg scan.c
 
 set.o : $(SET)/set.c
-	$(BUILD_CC) $(BUILD_CFLAGS) -c -o set.o $(SET)/set.c
+	$(CC) $(CFLAGS) -c -o set.o $(SET)/set.c
 
 %.o : %.c 
-	$(BUILD_CC) -c $(BUILD_CFLAGS) $(BUILD_CPPFLAGS) $< -o $@
+	$(CC) -c $(CFLAGS) $(CPPFLAGS) $< -o $@
 
 #
 # ****** These next targets are common to UNIX and PC world ********
diff --git a/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile b/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
index 5a3561edecd4..e214b35ab5e1 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
+++ b/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
@@ -35,7 +35,7 @@ PCCTS_H=../h
 #
 #$(LIBS: = +^
 #)
-#$(DEF_FILE) $(LFLAGS) ;
+#$(DEF_FILE) $(LDFLAGS) ;
 #<<
 #        bind $@ c:\os2\doscalls.lib
 #        copy *.exe ..\bin
@@ -59,7 +59,7 @@ PCCTS_H=../h
 #$@ /Tde /c
 #
 #$(LIBS)
-#$(DEF_FILE) $(LFLAGS) ;
+#$(DEF_FILE) $(LDFLAGS) ;
 #|
 #        copy *.exe ..\bin
 #
@@ -83,7 +83,7 @@ PCCTS_H=../h
 #
 #$(LIBS: = +^
 #)
-#$(DEF_FILE) $(LFLAGS) ;
+#$(DEF_FILE) $(LDFLAGS) ;
 #<<
 #        copy *.exe ..\bin
 #
@@ -115,22 +115,22 @@ PCCTS_H=../h
 #   UNIX
 #
 ifeq ($(CXX), llvm)
-BUILD_CC?=$(CLANG_BIN)clang
-else
-BUILD_CC?=cc
+CC?=$(CLANG_BIN)clang
+else ifeq ($(origin CC),default)
+CC=cc
 endif
 COPT=-O
 ANTLR=${BIN_DIR}/antlr
 DLG=${BIN_DIR}/dlg
-BUILD_CFLAGS= $(COPT) -I. -I$(SET) -I$(PCCTS_H) -DUSER_ZZSYN -DZZLEXBUFSIZE=65536
-BUILD_CPPFLAGS=
+CFLAGS= $(COPT) -I. -I$(SET) -I$(PCCTS_H) -DUSER_ZZSYN -DZZLEXBUFSIZE=65536
+CPPFLAGS=
 OBJ_EXT=o
 OUT_OBJ = -o
 OBJ = dlg_p.o dlg_a.o main.o err.o set.o support.o output.o \
         relabel.o automata.o
 
 $(BIN_DIR)/dlg : $(OBJ) $(SRC)
-		$(BUILD_CC) $(BUILD_CFLAGS) -o $(BIN_DIR)/dlg $(OBJ)
+		$(CC) $(CFLAGS) -o $(BIN_DIR)/dlg $(OBJ)
 
 SRC = dlg_p.c dlg_a.c main.c err.c $(SET)/set.c support.c output.c \
         relabel.c automata.c
@@ -142,19 +142,19 @@ SRC = dlg_p.c dlg_a.c main.c err.c $(SET)/set.c support.c output.c \
 #	$(DLG) -C2 parser.dlg dlg_a.c
 
 dlg_p.$(OBJ_EXT) : dlg_p.c dlg.h tokens.h mode.h
-	$(BUILD_CC) $(BUILD_CFLAGS) -c dlg_p.c
+	$(CC) $(CFLAGS) -c dlg_p.c
 
 dlg_a.$(OBJ_EXT) : dlg_a.c dlg.h tokens.h mode.h
-	$(BUILD_CC) $(BUILD_CFLAGS) -c dlg_a.c
+	$(CC) $(CFLAGS) -c dlg_a.c
 
 main.$(OBJ_EXT) : main.c dlg.h
-	$(BUILD_CC) $(BUILD_CFLAGS) -c main.c
+	$(CC) $(CFLAGS) -c main.c
 
 set.$(OBJ_EXT) : $(SET)/set.c
-	$(BUILD_CC) -c $(BUILD_CFLAGS) $(SET)/set.c
+	$(CC) -c $(CFLAGS) $(SET)/set.c
 
 %.o : %.c 
-	$(BUILD_CC) -c $(BUILD_CFLAGS) $(BUILD_CPPFLAGS) $< -o $@
+	$(CC) -c $(CFLAGS) $(CPPFLAGS) $< -o $@
 
 lint:
 	lint *.c
diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py b/BaseTools/Source/Python/Workspace/DscBuildData.py
index 73c6ee40061b..8fd949dc50b6 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -89,7 +89,7 @@ PcdMakefileHeader = '''
 '''
 
 WindowsCFLAGS = 'CFLAGS = $(CFLAGS) /wd4200 /wd4034 /wd4101 '
-LinuxCFLAGS = 'BUILD_CFLAGS += -Wno-pointer-to-int-cast -Wno-unused-variable '
+LinuxCFLAGS = 'CFLAGS += -Wno-pointer-to-int-cast -Wno-unused-variable '
 PcdMakefileEnd = '''
 !INCLUDE $(BASE_TOOLS_PATH)\Source\C\Makefiles\ms.common
 !INCLUDE $(BASE_TOOLS_PATH)\Source\C\Makefiles\ms.app
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/3] BaseTools: Improve detection of users wanting to build using clang
  2023-02-17  3:50 [PATCH 0/3] BaseTools: allow users to override CC and CXX on the make command line Rebecca Cran
  2023-02-17  3:50 ` [PATCH 1/3] BaseTools: Allow users to specify compiler to use with make CC= CXX= Rebecca Cran
@ 2023-02-17  3:51 ` Rebecca Cran
  2023-03-15 10:11   ` [edk2-devel] " Gerd Hoffmann
  2023-02-17  3:51 ` [PATCH 3/3] BaseTools: Build against C++14 when building with clang Rebecca Cran
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Rebecca Cran @ 2023-02-17  3:51 UTC (permalink / raw)
  To: devel, Andrew Fish, Leif Lindholm, Michael D Kinney, Bob Feng,
	Liming Gao, Yuwei Chen
  Cc: Rebecca Cran

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>
---
 BaseTools/Source/C/DevicePath/GNUmakefile          | 3 ++-
 BaseTools/Source/C/Makefiles/header.makefile       | 7 ++++---
 BaseTools/Source/C/VfrCompile/GNUmakefile          | 3 ++-
 BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 2 +-
 BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile   | 3 ++-
 5 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/BaseTools/Source/C/DevicePath/GNUmakefile b/BaseTools/Source/C/DevicePath/GNUmakefile
index 3afc7fc0504e..f61b1b2f171d 100644
--- a/BaseTools/Source/C/DevicePath/GNUmakefile
+++ b/BaseTools/Source/C/DevicePath/GNUmakefile
@@ -14,8 +14,9 @@ OBJECTS = DevicePath.o UefiDevicePathLib.o DevicePathFromText.o  DevicePathUtili
 include $(MAKEROOT)/Makefiles/app.makefile
 
 GCCVERSION = $(shell $(CC) -dumpversion | awk -F'.' '{print $$1}')
+CLANG := $(shell $(CC) --version | grep clang)
 ifneq ("$(GCCVERSION)", "5")
-ifneq ($(CXX), llvm)
+ifeq ($(CLANG),)
 ifneq ($(DARWIN),Darwin)
 # gcc 12 trips over device path handling
 CFLAGS += -Wno-error=stringop-overflow
diff --git a/BaseTools/Source/C/Makefiles/header.makefile b/BaseTools/Source/C/Makefiles/header.makefile
index 347918c7d4fa..bcc2791998b0 100644
--- a/BaseTools/Source/C/Makefiles/header.makefile
+++ b/BaseTools/Source/C/Makefiles/header.makefile
@@ -44,7 +44,8 @@ endif
 CYGWIN:=$(findstring CYGWIN, $(shell uname -s))
 LINUX:=$(findstring Linux, $(shell uname -s))
 DARWIN:=$(findstring Darwin, $(shell uname -s))
-ifeq ($(CXX), llvm)
+CLANG:=$(shell $(CC) --version | grep clang)
+ifneq ($(CLANG),)
 CC ?= $(CLANG_BIN)clang
 CXX ?= $(CLANG_BIN)clang++
 AS ?= $(CLANG_BIN)clang
@@ -91,7 +92,7 @@ ifeq ($(DARWIN),Darwin)
 CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror \
 -Wno-deprecated-declarations -Wno-self-assign -Wno-unused-result -nostdlib -g
 else
-ifeq ($(CXX), llvm)
+ifneq ($(CLANG),)
 CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -fwrapv \
 -fno-delete-null-pointer-checks -Wall -Werror \
 -Wno-deprecated-declarations -Wno-self-assign \
@@ -103,7 +104,7 @@ CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -fwrapv \
 -Wno-unused-result -nostdlib -g
 endif
 endif
-ifeq ($(CXX), llvm)
+ifneq ($(CLANG),)
 LDFLAGS =
 CXXFLAGS = -Wno-deprecated-register -Wno-unused-result
 else
diff --git a/BaseTools/Source/C/VfrCompile/GNUmakefile b/BaseTools/Source/C/VfrCompile/GNUmakefile
index 9fbaaaba21d7..fdd19f55f77e 100644
--- a/BaseTools/Source/C/VfrCompile/GNUmakefile
+++ b/BaseTools/Source/C/VfrCompile/GNUmakefile
@@ -16,7 +16,8 @@ TOOL_INCLUDE = -I Pccts/h
 #OBJECTS = VfrSyntax.o VfrServices.o DLGLexer.o EfiVfrParser.o ATokenBuffer.o DLexerBase.o AParser.o
 OBJECTS = AParser.o DLexerBase.o ATokenBuffer.o EfiVfrParser.o VfrLexer.o VfrSyntax.o \
           VfrFormPkg.o VfrError.o VfrUtilityLib.o VfrCompiler.o
-ifeq ($(CXX), llvm)
+CLANG:=$(shell $(CC) --version | grep clang)
+ifneq ($(CLANG),)
 VFR_CPPFLAGS = -Wno-deprecated-register -DPCCTS_USE_NAMESPACE_STD $(CPPFLAGS)
 else
 VFR_CPPFLAGS = -DPCCTS_USE_NAMESPACE_STD $(CPPFLAGS)
diff --git a/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile b/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
index 558d2f7b0111..42b603571fab 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
+++ b/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
@@ -164,7 +164,7 @@ PCCTS_H=../h
 #
 #   UNIX  (default)
 #
-ifeq ($(CXX), llvm)
+ifneq ($(CLANG),)
 CC?=$(CLANG_BIN)clang
 else ifeq ($(origin CC),default)
 CC=gcc
diff --git a/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile b/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
index e214b35ab5e1..69dac6a59789 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
+++ b/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
@@ -114,7 +114,8 @@ PCCTS_H=../h
 #
 #   UNIX
 #
-ifeq ($(CXX), llvm)
+CLANG:=$(shell $(CC) --version | grep clang)
+ifneq ($(CLANG),)
 CC?=$(CLANG_BIN)clang
 else ifeq ($(origin CC),default)
 CC=cc
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/3] BaseTools: Build against C++14 when building with clang
  2023-02-17  3:50 [PATCH 0/3] BaseTools: allow users to override CC and CXX on the make command line Rebecca Cran
  2023-02-17  3:50 ` [PATCH 1/3] BaseTools: Allow users to specify compiler to use with make CC= CXX= Rebecca Cran
  2023-02-17  3:51 ` [PATCH 2/3] BaseTools: Improve detection of users wanting to build using clang Rebecca Cran
@ 2023-02-17  3:51 ` Rebecca Cran
  2023-03-15 10:11   ` [edk2-devel] " Gerd Hoffmann
  2023-03-09 15:47 ` [edk2-devel] [PATCH 0/3] BaseTools: allow users to override CC and CXX on the make command line Rebecca Cran
       [not found] ` <174ACADCDC6439C2.24021@groups.io>
  4 siblings, 1 reply; 15+ messages in thread
From: Rebecca Cran @ 2023-02-17  3:51 UTC (permalink / raw)
  To: devel, Andrew Fish, Leif Lindholm, Michael D Kinney, Bob Feng,
	Liming Gao, Yuwei Chen
  Cc: Rebecca Cran

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>
---
 BaseTools/Source/C/Makefiles/header.makefile | 2 +-
 BaseTools/Source/C/VfrCompile/GNUmakefile    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/Makefiles/header.makefile b/BaseTools/Source/C/Makefiles/header.makefile
index bcc2791998b0..1bf003523baf 100644
--- a/BaseTools/Source/C/Makefiles/header.makefile
+++ b/BaseTools/Source/C/Makefiles/header.makefile
@@ -106,7 +106,7 @@ endif
 endif
 ifneq ($(CLANG),)
 LDFLAGS =
-CXXFLAGS = -Wno-deprecated-register -Wno-unused-result
+CXXFLAGS = -Wno-deprecated-register -Wno-unused-result -std=c++14
 else
 LDFLAGS =
 CXXFLAGS = -Wno-unused-result
diff --git a/BaseTools/Source/C/VfrCompile/GNUmakefile b/BaseTools/Source/C/VfrCompile/GNUmakefile
index fdd19f55f77e..7d851c38887c 100644
--- a/BaseTools/Source/C/VfrCompile/GNUmakefile
+++ b/BaseTools/Source/C/VfrCompile/GNUmakefile
@@ -23,7 +23,7 @@ else
 VFR_CPPFLAGS = -DPCCTS_USE_NAMESPACE_STD $(CPPFLAGS)
 endif
 # keep BUILD_OPTFLAGS last
-VFR_CXXFLAGS = $(BUILD_OPTFLAGS)
+VFR_CXXFLAGS = $(BUILD_OPTFLAGS) -std=c++14
 
 # keep EXTRA_LDFLAGS last
 VFR_LFLAGS = $(EXTRA_LDFLAGS)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH 0/3] BaseTools: allow users to override CC and CXX on the make command line
  2023-02-17  3:50 [PATCH 0/3] BaseTools: allow users to override CC and CXX on the make command line Rebecca Cran
                   ` (2 preceding siblings ...)
  2023-02-17  3:51 ` [PATCH 3/3] BaseTools: Build against C++14 when building with clang Rebecca Cran
@ 2023-03-09 15:47 ` Rebecca Cran
       [not found] ` <174ACADCDC6439C2.24021@groups.io>
  4 siblings, 0 replies; 15+ messages in thread
From: Rebecca Cran @ 2023-03-09 15:47 UTC (permalink / raw)
  To: devel, Andrew Fish, Leif Lindholm, Michael D Kinney, Bob Feng,
	Liming Gao, Yuwei Chen

Could I get some reviews on this please?


Thanks.

Rebecca Cran


On 2/16/23 8:50 PM, Rebecca Cran wrote:
> 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".
>
> Rebecca Cran (3):
>    BaseTools: Allow users to specify compiler to use with make CC= CXX=
>    BaseTools: Improve detection of users wanting to build using clang
>    BaseTools: Build against C++14 when building with clang
>
>   BaseTools/Source/C/DevicePath/GNUmakefile          |  7 ++-
>   BaseTools/Source/C/LzmaCompress/GNUmakefile        |  2 +-
>   BaseTools/Source/C/Makefiles/app.makefile          |  2 +-
>   BaseTools/Source/C/Makefiles/footer.makefile       |  6 +-
>   BaseTools/Source/C/Makefiles/header.makefile       | 59 ++++++++++----------
>   BaseTools/Source/C/VfrCompile/GNUmakefile          | 19 ++++---
>   BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 20 +++----
>   BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile   | 31 +++++-----
>   BaseTools/Source/Python/Workspace/DscBuildData.py  |  2 +-
>   9 files changed, 76 insertions(+), 72 deletions(-)
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH 0/3] BaseTools: allow users to override CC and CXX on the make command line
       [not found] ` <174ACADCDC6439C2.24021@groups.io>
@ 2023-03-14 16:16   ` Rebecca Cran
  2023-03-23  1:09     ` 回复: " gaoliming
       [not found]     ` <174EE719E87C3CB3.19937@groups.io>
  0 siblings, 2 replies; 15+ messages in thread
From: Rebecca Cran @ 2023-03-14 16:16 UTC (permalink / raw)
  To: devel, Andrew Fish, Leif Lindholm, Michael D Kinney, Bob Feng,
	Liming Gao, Yuwei Chen

This is a patch series that I'd really like to get committed.

It'll allow us to drop the dependency on gcc for building edk2 on FreeBSD.


The Github branch is https://github.com/bcran/edk2/tree/mdepkg-c11 and 
there's a PR at https://github.com/tianocore/edk2/pull/4142.


-- 

Rebecca Cran


On 3/9/23 8:47 AM, Rebecca Cran wrote:
> Could I get some reviews on this please?
>
>
> Thanks.
>
> Rebecca Cran
>
>
> On 2/16/23 8:50 PM, Rebecca Cran wrote:
>> 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".
>>
>> Rebecca Cran (3):
>>    BaseTools: Allow users to specify compiler to use with make CC= CXX=
>>    BaseTools: Improve detection of users wanting to build using clang
>>    BaseTools: Build against C++14 when building with clang
>>
>>   BaseTools/Source/C/DevicePath/GNUmakefile          |  7 ++-
>>   BaseTools/Source/C/LzmaCompress/GNUmakefile        |  2 +-
>>   BaseTools/Source/C/Makefiles/app.makefile          |  2 +-
>>   BaseTools/Source/C/Makefiles/footer.makefile       |  6 +-
>>   BaseTools/Source/C/Makefiles/header.makefile       | 59 
>> ++++++++++----------
>>   BaseTools/Source/C/VfrCompile/GNUmakefile          | 19 ++++---
>>   BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 20 +++----
>>   BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile   | 31 +++++-----
>>   BaseTools/Source/Python/Workspace/DscBuildData.py  |  2 +-
>>   9 files changed, 76 insertions(+), 72 deletions(-)
>>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH 1/3] BaseTools: Allow users to specify compiler to use with make CC= CXX=
  2023-02-17  3:50 ` [PATCH 1/3] BaseTools: Allow users to specify compiler to use with make CC= CXX= Rebecca Cran
@ 2023-03-15 10:07   ` Gerd Hoffmann
  2023-03-17 10:43     ` Rebecca Cran
  0 siblings, 1 reply; 15+ messages in thread
From: Gerd Hoffmann @ 2023-03-15 10:07 UTC (permalink / raw)
  To: devel, quic_rcran
  Cc: Andrew Fish, Leif Lindholm, Michael D Kinney, Bob Feng,
	Liming Gao, Yuwei Chen, Rebecca Cran

  Hi,

> 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++'.

Hmm, not sure this is a good idea.  I suspect there was some reason to
use BUILD_CC instead of CC in the first place ...

take care,
  Gerd


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH 2/3] BaseTools: Improve detection of users wanting to build using clang
  2023-02-17  3:51 ` [PATCH 2/3] BaseTools: Improve detection of users wanting to build using clang Rebecca Cran
@ 2023-03-15 10:11   ` Gerd Hoffmann
  0 siblings, 0 replies; 15+ messages in thread
From: Gerd Hoffmann @ 2023-03-15 10:11 UTC (permalink / raw)
  To: devel, quic_rcran
  Cc: Andrew Fish, Leif Lindholm, Michael D Kinney, Bob Feng,
	Liming Gao, Yuwei Chen, Rebecca Cran

On Thu, Feb 16, 2023 at 08:51:00PM -0700, Rebecca Cran wrote:
> 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.

That totally makes sense.  And maybe simply use 'cc' by default then?

take care,
  Gerd


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH 3/3] BaseTools: Build against C++14 when building with clang
  2023-02-17  3:51 ` [PATCH 3/3] BaseTools: Build against C++14 when building with clang Rebecca Cran
@ 2023-03-15 10:11   ` Gerd Hoffmann
  0 siblings, 0 replies; 15+ messages in thread
From: Gerd Hoffmann @ 2023-03-15 10:11 UTC (permalink / raw)
  To: devel, quic_rcran
  Cc: Andrew Fish, Leif Lindholm, Michael D Kinney, Bob Feng,
	Liming Gao, Yuwei Chen, Rebecca Cran

On Thu, Feb 16, 2023 at 08:51:01PM -0700, Rebecca Cran wrote:
> 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.

Acked-by: Gerd Hoffmann <kraxel@redhat.com>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH 1/3] BaseTools: Allow users to specify compiler to use with make CC= CXX=
  2023-03-15 10:07   ` [edk2-devel] " Gerd Hoffmann
@ 2023-03-17 10:43     ` Rebecca Cran
  2023-03-20  9:35       ` Gerd Hoffmann
  0 siblings, 1 reply; 15+ messages in thread
From: Rebecca Cran @ 2023-03-17 10:43 UTC (permalink / raw)
  To: devel, kraxel, quic_rcran
  Cc: Andrew Fish, Leif Lindholm, Michael D Kinney, Bob Feng,
	Liming Gao, Yuwei Chen, Rebecca Cran

On 3/15/23 4:07 AM, Gerd Hoffmann wrote:
>> 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++'.
> Hmm, not sure this is a good idea.  I suspect there was some reason to
> use BUILD_CC instead of CC in the first place ...

It looks like the change to use BUILD_CC was introduced by Liming in:


     BaseTools GnuMakefile: Update GCC Flags to the specific one with 
BUILD_ prefix

     To avoid the conflict with the default GCC flag name, BUILD_ prefix 
is added.

     Cc: Giri P Mudusuru <giri.p.mudusuru@intel.com>
     Cc: Yonghong Zhu <yonghong.zhu@intel.com>
     Contributed-under: TianoCore Contribution Agreement 1.0
     Signed-off-by: Liming Gao <liming.gao@intel.com>
     Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>
     Reviewed-by: Giri P Mudusuru <giri.p.mudusuru@intel.com>


I don't understand how that would help, and I'm wondering if it was 
perhaps the result of a misunderstanding.


-- 
Rebecca Cran


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH 1/3] BaseTools: Allow users to specify compiler to use with make CC= CXX=
  2023-03-17 10:43     ` Rebecca Cran
@ 2023-03-20  9:35       ` Gerd Hoffmann
  2023-03-20 13:13         ` Rebecca Cran
  0 siblings, 1 reply; 15+ messages in thread
From: Gerd Hoffmann @ 2023-03-20  9:35 UTC (permalink / raw)
  To: Rebecca Cran
  Cc: devel, quic_rcran, Andrew Fish, Leif Lindholm, Michael D Kinney,
	Bob Feng, Liming Gao, Yuwei Chen, Rebecca Cran

On Fri, Mar 17, 2023 at 04:43:24AM -0600, Rebecca Cran wrote:
> On 3/15/23 4:07 AM, Gerd Hoffmann wrote:
> > > 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++'.
> > Hmm, not sure this is a good idea.  I suspect there was some reason to
> > use BUILD_CC instead of CC in the first place ...
> 
> It looks like the change to use BUILD_CC was introduced by Liming in:
> 
> 
>     BaseTools GnuMakefile: Update GCC Flags to the specific one with BUILD_
> prefix
> 
>     To avoid the conflict with the default GCC flag name, BUILD_ prefix is
> added.
> 
>     Cc: Giri P Mudusuru <giri.p.mudusuru@intel.com>
>     Cc: Yonghong Zhu <yonghong.zhu@intel.com>
>     Contributed-under: TianoCore Contribution Agreement 1.0
>     Signed-off-by: Liming Gao <liming.gao@intel.com>
>     Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>
>     Reviewed-by: Giri P Mudusuru <giri.p.mudusuru@intel.com>
> 
> 
> I don't understand how that would help, and I'm wondering if it was perhaps
> the result of a misunderstanding.

Hmm, neither the commit message nor the patch itself have any hints of
actual problems fixed.  The "conflict" notion indeed hints this might
have been a misunderstanding.

So, going back to just use CC + CFLAGS + friends looks fine to me.

The suggestion to simply not set CC stands stands.

take care,
  Gerd


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH 1/3] BaseTools: Allow users to specify compiler to use with make CC= CXX=
  2023-03-20  9:35       ` Gerd Hoffmann
@ 2023-03-20 13:13         ` Rebecca Cran
  0 siblings, 0 replies; 15+ messages in thread
From: Rebecca Cran @ 2023-03-20 13:13 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: devel, Andrew Fish, Leif Lindholm, Michael D Kinney, Bob Feng,
	Liming Gao, Yuwei Chen

Sorry, I don't think I saw a suggestion from you to not set CC - though 
I may have missed emails since this patch series was sent from the email 
of my former employer.

Marvin did suggest that though, so I can certainly add another patch to 
the series to do that.


-- 

Rebecca Cran


On 3/20/23 3:35 AM, Gerd Hoffmann wrote:
> On Fri, Mar 17, 2023 at 04:43:24AM -0600, Rebecca Cran wrote:
>> On 3/15/23 4:07 AM, Gerd Hoffmann wrote:
>>>> 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++'.
>>> Hmm, not sure this is a good idea.  I suspect there was some reason to
>>> use BUILD_CC instead of CC in the first place ...
>> It looks like the change to use BUILD_CC was introduced by Liming in:
>>
>>
>>      BaseTools GnuMakefile: Update GCC Flags to the specific one with BUILD_
>> prefix
>>
>>      To avoid the conflict with the default GCC flag name, BUILD_ prefix is
>> added.
>>
>>      Cc: Giri P Mudusuru <giri.p.mudusuru@intel.com>
>>      Cc: Yonghong Zhu <yonghong.zhu@intel.com>
>>      Contributed-under: TianoCore Contribution Agreement 1.0
>>      Signed-off-by: Liming Gao <liming.gao@intel.com>
>>      Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>
>>      Reviewed-by: Giri P Mudusuru <giri.p.mudusuru@intel.com>
>>
>>
>> I don't understand how that would help, and I'm wondering if it was perhaps
>> the result of a misunderstanding.
> Hmm, neither the commit message nor the patch itself have any hints of
> actual problems fixed.  The "conflict" notion indeed hints this might
> have been a misunderstanding.
>
> So, going back to just use CC + CFLAGS + friends looks fine to me.
>
> The suggestion to simply not set CC stands stands.
>
> take care,
>    Gerd
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* 回复: [edk2-devel] [PATCH 0/3] BaseTools: allow users to override CC and CXX on the make command line
  2023-03-14 16:16   ` Rebecca Cran
@ 2023-03-23  1:09     ` gaoliming
       [not found]     ` <174EE719E87C3CB3.19937@groups.io>
  1 sibling, 0 replies; 15+ messages in thread
From: gaoliming @ 2023-03-23  1:09 UTC (permalink / raw)
  To: devel, rebecca, 'Andrew Fish', 'Leif Lindholm',
	'Michael D Kinney', 'Bob Feng',
	'Yuwei Chen'

Rebecca:
  This patch looks good to me. Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn> 

  I will help to merge this patch set. 

Thanks
Liming
> -----邮件原件-----
> 发件人: Rebecca Cran <rebecca@bsdio.com>
> 发送时间: 2023年3月15日 0:17
> 收件人: devel@edk2.groups.io; Andrew Fish <afish@apple.com>; Leif
> Lindholm <quic_llindhol@quicinc.com>; Michael D Kinney
> <michael.d.kinney@intel.com>; Bob Feng <bob.c.feng@intel.com>; Liming
> Gao <gaoliming@byosoft.com.cn>; Yuwei Chen <yuwei.chen@intel.com>
> 主题: Re: [edk2-devel] [PATCH 0/3] BaseTools: allow users to override CC and
> CXX on the make command line
> 
> This is a patch series that I'd really like to get committed.
> 
> It'll allow us to drop the dependency on gcc for building edk2 on FreeBSD.
> 
> 
> The Github branch is https://github.com/bcran/edk2/tree/mdepkg-c11 and
> there's a PR at https://github.com/tianocore/edk2/pull/4142.
> 
> 
> --
> 
> Rebecca Cran
> 
> 
> On 3/9/23 8:47 AM, Rebecca Cran wrote:
> > Could I get some reviews on this please?
> >
> >
> > Thanks.
> >
> > Rebecca Cran
> >
> >
> > On 2/16/23 8:50 PM, Rebecca Cran wrote:
> >> 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".
> >>
> >> Rebecca Cran (3):
> >>    BaseTools: Allow users to specify compiler to use with make CC=
> CXX=
> >>    BaseTools: Improve detection of users wanting to build using clang
> >>    BaseTools: Build against C++14 when building with clang
> >>
> >>   BaseTools/Source/C/DevicePath/GNUmakefile          |  7 ++-
> >>   BaseTools/Source/C/LzmaCompress/GNUmakefile        |  2 +-
> >>   BaseTools/Source/C/Makefiles/app.makefile          |  2 +-
> >>   BaseTools/Source/C/Makefiles/footer.makefile       |  6 +-
> >>   BaseTools/Source/C/Makefiles/header.makefile       | 59
> >> ++++++++++----------
> >>   BaseTools/Source/C/VfrCompile/GNUmakefile          | 19
> ++++---
> >>   BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 20 +++----
> >>   BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile   | 31 +++++-----
> >>   BaseTools/Source/Python/Workspace/DscBuildData.py  |  2 +-
> >>   9 files changed, 76 insertions(+), 72 deletions(-)
> >>



^ permalink raw reply	[flat|nested] 15+ messages in thread

* 回复: [edk2-devel] [PATCH 0/3] BaseTools: allow users to override CC and CXX on the make command line
       [not found]     ` <174EE719E87C3CB3.19937@groups.io>
@ 2023-03-24  4:57       ` gaoliming
  2023-03-24 21:39         ` Rebecca Cran
  0 siblings, 1 reply; 15+ messages in thread
From: gaoliming @ 2023-03-24  4:57 UTC (permalink / raw)
  To: devel, gaoliming, rebecca, 'Andrew Fish',
	'Leif Lindholm', 'Michael D Kinney',
	'Bob Feng', 'Yuwei Chen'

Rebecca:
  I create PR https://github.com/tianocore/edk2/pull/4162 . But PR shows some failure. Can you help fix them?

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 gaoliming via
> groups.io
> 发送时间: 2023年3月23日 9:10
> 收件人: devel@edk2.groups.io; rebecca@bsdio.com; 'Andrew Fish'
> <afish@apple.com>; 'Leif Lindholm' <quic_llindhol@quicinc.com>; 'Michael D
> Kinney' <michael.d.kinney@intel.com>; 'Bob Feng' <bob.c.feng@intel.com>;
> 'Yuwei Chen' <yuwei.chen@intel.com>
> 主题: 回复: [edk2-devel] [PATCH 0/3] BaseTools: allow users to override CC
> and CXX on the make command line
> 
> Rebecca:
>   This patch looks good to me. Reviewed-by: Liming Gao
> <gaoliming@byosoft.com.cn>
> 
>   I will help to merge this patch set.
> 
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: Rebecca Cran <rebecca@bsdio.com>
> > 发送时间: 2023年3月15日 0:17
> > 收件人: devel@edk2.groups.io; Andrew Fish <afish@apple.com>; Leif
> > Lindholm <quic_llindhol@quicinc.com>; Michael D Kinney
> > <michael.d.kinney@intel.com>; Bob Feng <bob.c.feng@intel.com>; Liming
> > Gao <gaoliming@byosoft.com.cn>; Yuwei Chen <yuwei.chen@intel.com>
> > 主题: Re: [edk2-devel] [PATCH 0/3] BaseTools: allow users to override CC
> and
> > CXX on the make command line
> >
> > This is a patch series that I'd really like to get committed.
> >
> > It'll allow us to drop the dependency on gcc for building edk2 on FreeBSD.
> >
> >
> > The Github branch is https://github.com/bcran/edk2/tree/mdepkg-c11 and
> > there's a PR at https://github.com/tianocore/edk2/pull/4142.
> >
> >
> > --
> >
> > Rebecca Cran
> >
> >
> > On 3/9/23 8:47 AM, Rebecca Cran wrote:
> > > Could I get some reviews on this please?
> > >
> > >
> > > Thanks.
> > >
> > > Rebecca Cran
> > >
> > >
> > > On 2/16/23 8:50 PM, Rebecca Cran wrote:
> > >> 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".
> > >>
> > >> Rebecca Cran (3):
> > >>    BaseTools: Allow users to specify compiler to use with make CC=
> > CXX=
> > >>    BaseTools: Improve detection of users wanting to build using clang
> > >>    BaseTools: Build against C++14 when building with clang
> > >>
> > >>   BaseTools/Source/C/DevicePath/GNUmakefile          |  7 ++-
> > >>   BaseTools/Source/C/LzmaCompress/GNUmakefile        |  2 +-
> > >>   BaseTools/Source/C/Makefiles/app.makefile          |  2 +-
> > >>   BaseTools/Source/C/Makefiles/footer.makefile       |  6 +-
> > >>   BaseTools/Source/C/Makefiles/header.makefile       | 59
> > >> ++++++++++----------
> > >>   BaseTools/Source/C/VfrCompile/GNUmakefile          | 19
> > ++++---
> > >>   BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 20 +++----
> > >>   BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile   | 31
> +++++-----
> > >>   BaseTools/Source/Python/Workspace/DscBuildData.py  |  2 +-
> > >>   9 files changed, 76 insertions(+), 72 deletions(-)
> > >>
> 
> 
> 
> 
> 
> 




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: 回复: [edk2-devel] [PATCH 0/3] BaseTools: allow users to override CC and CXX on the make command line
  2023-03-24  4:57       ` gaoliming
@ 2023-03-24 21:39         ` Rebecca Cran
  0 siblings, 0 replies; 15+ messages in thread
From: Rebecca Cran @ 2023-03-24 21:39 UTC (permalink / raw)
  To: gaoliming, devel, 'Andrew Fish', 'Leif Lindholm',
	'Michael D Kinney', 'Bob Feng',
	'Yuwei Chen'
  Cc: Bret Barkelew, Sean Brogan, Michael Kubacki

I see what's going wrong.

First, PatchCheck.py isn't ignoring tabs in files named "*.makefile" or 
"makefile".

Michael Kubacki has a patch to fix that (I also submitted a patch, but 
I'll let him commit his changes).


Secondly, edk2basetools has a copy of Workspace/DscBuildData.py which is 
still setting BUILD_CFLAGS instead of CFLAGS., and it's that copy that 
the stuart tools use.

I'm not sure how we should coordinate changes between the two repos, 
since it's a breaking change.


-- 

Rebecca Cran


On 3/23/23 10:57 PM, gaoliming wrote:
> Rebecca:
>    I create PR https://github.com/tianocore/edk2/pull/4162 . But PR shows some failure. Can you help fix them?
>
> Thanks
> Liming
>> -----邮件原件-----
>> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 gaoliming via
>> groups.io
>> 发送时间: 2023年3月23日 9:10
>> 收件人: devel@edk2.groups.io; rebecca@bsdio.com; 'Andrew Fish'
>> <afish@apple.com>; 'Leif Lindholm' <quic_llindhol@quicinc.com>; 'Michael D
>> Kinney' <michael.d.kinney@intel.com>; 'Bob Feng' <bob.c.feng@intel.com>;
>> 'Yuwei Chen' <yuwei.chen@intel.com>
>> 主题: 回复: [edk2-devel] [PATCH 0/3] BaseTools: allow users to override CC
>> and CXX on the make command line
>>
>> Rebecca:
>>    This patch looks good to me. Reviewed-by: Liming Gao
>> <gaoliming@byosoft.com.cn>
>>
>>    I will help to merge this patch set.
>>
>> Thanks
>> Liming
>>> -----邮件原件-----
>>> 发件人: Rebecca Cran <rebecca@bsdio.com>
>>> 发送时间: 2023年3月15日 0:17
>>> 收件人: devel@edk2.groups.io; Andrew Fish <afish@apple.com>; Leif
>>> Lindholm <quic_llindhol@quicinc.com>; Michael D Kinney
>>> <michael.d.kinney@intel.com>; Bob Feng <bob.c.feng@intel.com>; Liming
>>> Gao <gaoliming@byosoft.com.cn>; Yuwei Chen <yuwei.chen@intel.com>
>>> 主题: Re: [edk2-devel] [PATCH 0/3] BaseTools: allow users to override CC
>> and
>>> CXX on the make command line
>>>
>>> This is a patch series that I'd really like to get committed.
>>>
>>> It'll allow us to drop the dependency on gcc for building edk2 on FreeBSD.
>>>
>>>
>>> The Github branch is https://github.com/bcran/edk2/tree/mdepkg-c11 and
>>> there's a PR at https://github.com/tianocore/edk2/pull/4142.
>>>
>>>
>>> --
>>>
>>> Rebecca Cran
>>>
>>>
>>> On 3/9/23 8:47 AM, Rebecca Cran wrote:
>>>> Could I get some reviews on this please?
>>>>
>>>>
>>>> Thanks.
>>>>
>>>> Rebecca Cran
>>>>
>>>>
>>>> On 2/16/23 8:50 PM, Rebecca Cran wrote:
>>>>> 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".
>>>>>
>>>>> Rebecca Cran (3):
>>>>>     BaseTools: Allow users to specify compiler to use with make CC=
>>> CXX=
>>>>>     BaseTools: Improve detection of users wanting to build using clang
>>>>>     BaseTools: Build against C++14 when building with clang
>>>>>
>>>>>    BaseTools/Source/C/DevicePath/GNUmakefile          |  7 ++-
>>>>>    BaseTools/Source/C/LzmaCompress/GNUmakefile        |  2 +-
>>>>>    BaseTools/Source/C/Makefiles/app.makefile          |  2 +-
>>>>>    BaseTools/Source/C/Makefiles/footer.makefile       |  6 +-
>>>>>    BaseTools/Source/C/Makefiles/header.makefile       | 59
>>>>> ++++++++++----------
>>>>>    BaseTools/Source/C/VfrCompile/GNUmakefile          | 19
>>> ++++---
>>>>>    BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 20 +++----
>>>>>    BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile   | 31
>> +++++-----
>>>>>    BaseTools/Source/Python/Workspace/DscBuildData.py  |  2 +-
>>>>>    9 files changed, 76 insertions(+), 72 deletions(-)
>>>>>
>>
>>
>>
>> 
>>
>
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-03-24 21:39 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-17  3:50 [PATCH 0/3] BaseTools: allow users to override CC and CXX on the make command line Rebecca Cran
2023-02-17  3:50 ` [PATCH 1/3] BaseTools: Allow users to specify compiler to use with make CC= CXX= Rebecca Cran
2023-03-15 10:07   ` [edk2-devel] " Gerd Hoffmann
2023-03-17 10:43     ` Rebecca Cran
2023-03-20  9:35       ` Gerd Hoffmann
2023-03-20 13:13         ` Rebecca Cran
2023-02-17  3:51 ` [PATCH 2/3] BaseTools: Improve detection of users wanting to build using clang Rebecca Cran
2023-03-15 10:11   ` [edk2-devel] " Gerd Hoffmann
2023-02-17  3:51 ` [PATCH 3/3] BaseTools: Build against C++14 when building with clang Rebecca Cran
2023-03-15 10:11   ` [edk2-devel] " Gerd Hoffmann
2023-03-09 15:47 ` [edk2-devel] [PATCH 0/3] BaseTools: allow users to override CC and CXX on the make command line Rebecca Cran
     [not found] ` <174ACADCDC6439C2.24021@groups.io>
2023-03-14 16:16   ` Rebecca Cran
2023-03-23  1:09     ` 回复: " gaoliming
     [not found]     ` <174EE719E87C3CB3.19937@groups.io>
2023-03-24  4:57       ` gaoliming
2023-03-24 21:39         ` Rebecca Cran

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox