[PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller

[PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller

[Laszlo Ersek]

Repo:   https://github.com/lersek/edk2.git
Branch: extra_flags_rhbz1540244

In the Fedora distribution, we'd like to pass system-wide flags related
to optimization and linking when the C and C++ language base tools are
built. This series lets the outermost "make" command push the
EXTRA_OPTFLAGS and EXTRA_LDFLAGS macros into the BaseTools build.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>

Thanks
Laszlo

Laszlo Ersek (6):
  BaseTools/footer.makefile: expand BUILD_CFLAGS last for C files too
  BaseTools/header.makefile: remove "-c" from BUILD_CFLAGS
  BaseTools/Source/C: split "-O2" to BUILD_OPTFLAGS
  BaseTools/Pccts: clean up antlr and dlg makefiles
  BaseTools/Source/C: take EXTRA_OPTFLAGS from the caller
  BaseTools/Source/C: take EXTRA_LDFLAGS from the caller

 BaseTools/Source/C/Makefiles/footer.makefile       |  2 +-
 BaseTools/Source/C/Makefiles/header.makefile       | 16 ++++++++---
 BaseTools/Source/C/VfrCompile/GNUmakefile          | 11 +++++---
 BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 22 ++++++++++-----
 BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile   | 28 +++++++++++++-------
 5 files changed, 56 insertions(+), 23 deletions(-)

-- 
2.14.1.3.gb7cf6e02401b

[PATCH 1/6] BaseTools/footer.makefile: expand BUILD_CFLAGS last for C files too

[Laszlo Ersek]

BUILD_CPPFLAGS should be expanded before BUILD_CFLAGS. (The rule for C++
source files already does this, with BUILD_CPPFLAGS and BUILD_CXXFLAGS.)

This patch doesn't change behavior.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1540244
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 BaseTools/Source/C/Makefiles/footer.makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/Makefiles/footer.makefile b/BaseTools/Source/C/Makefiles/footer.makefile
index 0926aa964547..5bda9e4e36d5 100644
--- a/BaseTools/Source/C/Makefiles/footer.makefile
+++ b/BaseTools/Source/C/Makefiles/footer.makefile
@@ -24,7 +24,7 @@ $(LIBRARY): $(OBJECTS)
 	$(BUILD_AR) crs $@ $^
 
 %.o : %.c 
-	$(BUILD_CC)  -c $(BUILD_CFLAGS) $(BUILD_CPPFLAGS) $< -o $@
+	$(BUILD_CC)  -c $(BUILD_CPPFLAGS) $(BUILD_CFLAGS) $< -o $@
 
 %.o : %.cpp
 	$(BUILD_CXX) -c $(BUILD_CPPFLAGS) $(BUILD_CXXFLAGS) $< -o $@
-- 
2.14.1.3.gb7cf6e02401b

[PATCH 2/6] BaseTools/header.makefile: remove "-c" from BUILD_CFLAGS

[Laszlo Ersek]

Option "-c" is a mode selection flag (choosing between compiling and
linking); it should not be in BUILD_CFLAGS, which applies only to
compiling anyway. The compilation rule for C source files, in
"footer.makefile", already includes "-c" -- currently we have double "-c"
options.

This patch doesn't change behavior.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1540244
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 BaseTools/Source/C/Makefiles/header.makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/Makefiles/header.makefile b/BaseTools/Source/C/Makefiles/header.makefile
index db436773cf40..08421ba24cd9 100644
--- a/BaseTools/Source/C/Makefiles/header.makefile
+++ b/BaseTools/Source/C/Makefiles/header.makefile
@@ -71,9 +71,9 @@ INCLUDE = $(TOOL_INCLUDE) -I $(MAKEROOT) -I $(MAKEROOT)/Include/Common -I $(MAKE
 BUILD_CPPFLAGS = $(INCLUDE) -O2
 ifeq ($(DARWIN),Darwin)
 # assume clang or clang compatible flags on OS X
-BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-self-assign -Wno-unused-result -nostdlib -c -g
+BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-self-assign -Wno-unused-result -nostdlib -g
 else
-BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-stringop-truncation -Wno-restrict -Wno-unused-result -nostdlib -c -g
+BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-stringop-truncation -Wno-restrict -Wno-unused-result -nostdlib -g
 endif
 BUILD_LFLAGS =
 BUILD_CXXFLAGS = -Wno-unused-result
-- 
2.14.1.3.gb7cf6e02401b

[PATCH 3/6] BaseTools/Source/C: split "-O2" to BUILD_OPTFLAGS

[Laszlo Ersek]

The option "-O2" is not a preprocessor flag, but a code generation
(compilation) flag. Move it from BUILD_CPPFLAGS to BUILD_CFLAGS and
BUILD_CXXFLAGS.

Because "VfrCompile/GNUmakefile" uses "-O2" through BUILD_CPPFLAGS, and
because it doesn't use BUILD_CXXFLAGS, we have to introduce BUILD_OPTFLAGS
separately, so that "VfrCompile/GNUmakefile" can continue using just this
flag.

This patch doesn't change behavior.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1540244
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 BaseTools/Source/C/Makefiles/header.makefile |  6 +++++-
 BaseTools/Source/C/VfrCompile/GNUmakefile    | 11 +++++++----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/BaseTools/Source/C/Makefiles/header.makefile b/BaseTools/Source/C/Makefiles/header.makefile
index 08421ba24cd9..498c6cf48b4a 100644
--- a/BaseTools/Source/C/Makefiles/header.makefile
+++ b/BaseTools/Source/C/Makefiles/header.makefile
@@ -68,7 +68,8 @@ $(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) -O2
+BUILD_CPPFLAGS = $(INCLUDE)
+BUILD_OPTFLAGS = -O2
 ifeq ($(DARWIN),Darwin)
 # assume clang or clang compatible flags on OS X
 BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-self-assign -Wno-unused-result -nostdlib -g
@@ -91,6 +92,9 @@ ifeq ($(DARWIN),Darwin)
 endif
 endif
 
+# keep BUILD_OPTFLAGS last
+BUILD_CFLAGS   += $(BUILD_OPTFLAGS)
+BUILD_CXXFLAGS += $(BUILD_OPTFLAGS)
   
 .PHONY: all
 .PHONY: install
diff --git a/BaseTools/Source/C/VfrCompile/GNUmakefile b/BaseTools/Source/C/VfrCompile/GNUmakefile
index c4ec61aa6c86..bbe562cbc54f 100644
--- a/BaseTools/Source/C/VfrCompile/GNUmakefile
+++ b/BaseTools/Source/C/VfrCompile/GNUmakefile
@@ -25,6 +25,9 @@ OBJECTS = AParser.o DLexerBase.o ATokenBuffer.o EfiVfrParser.o VfrLexer.o VfrSyn
 
 VFR_CPPFLAGS = -DPCCTS_USE_NAMESPACE_STD $(BUILD_CPPFLAGS)
 
+# keep BUILD_OPTFLAGS last
+VFR_CXXFLAGS = $(BUILD_OPTFLAGS)
+
 LINKER = $(BUILD_CXX)
 
 EXTRA_CLEAN_OBJECTS = EfiVfrParser.cpp EfiVfrParser.h VfrParser.dlg VfrTokens.h VfrLexer.cpp VfrLexer.h VfrSyntax.cpp tokens.h
@@ -58,16 +61,16 @@ Pccts/dlg/dlg:
 	BIN_DIR='.' $(MAKE) -C Pccts/dlg
 
 ATokenBuffer.o: Pccts/h/ATokenBuffer.cpp
-	$(BUILD_CXX) -c $(VFR_CPPFLAGS) $(INC) $? -o $@
+	$(BUILD_CXX) -c $(VFR_CPPFLAGS) $(INC) $(VFR_CXXFLAGS) $? -o $@
 
 DLexerBase.o: Pccts/h/DLexerBase.cpp
-	$(BUILD_CXX) -c $(VFR_CPPFLAGS) $(INC) $? -o $@
+	$(BUILD_CXX) -c $(VFR_CPPFLAGS) $(INC) $(VFR_CXXFLAGS) $? -o $@
 
 AParser.o: Pccts/h/AParser.cpp
-	$(BUILD_CXX) -c $(VFR_CPPFLAGS) $(INC) $? -o $@
+	$(BUILD_CXX) -c $(VFR_CPPFLAGS) $(INC) $(VFR_CXXFLAGS) $? -o $@
 
 VfrSyntax.o: VfrSyntax.cpp
-	$(BUILD_CXX) -c $(VFR_CPPFLAGS) $(INC) $? -o $@
+	$(BUILD_CXX) -c $(VFR_CPPFLAGS) $(INC) $(VFR_CXXFLAGS) $? -o $@
 	
 clean: localClean
 
-- 
2.14.1.3.gb7cf6e02401b

[PATCH 4/6] BaseTools/Pccts: clean up antlr and dlg makefiles

[Laszlo Ersek]

(1) "-I" and "-D" options are for the preprocessor; move them to
    BUILD_CPPFLAGS. (This unifies BUILD_CPPFLAGS between both makefiles.)

(2) COTHER is never set, drop it from "antlr". (This unifies BUILD_CFLAGS
    between both makefiles, as COPT.)

(3) For linking "antlr" and "dlg", both BUILD_CFLAGS and BUILD_CPPFLAGS
    are useless, so drop BUILD_CFLAGS, and don't add BUILD_CPPFLAGS.

(4) For compiling C source files:

(4a) Move the "-c" mode selector to the front.

(4b) Expand both BUILD_CPPFLAGS and BUILD_CFLAGS, in this order. (This
     results in COPT being expanded last.)

(4c) Turn the source file operand into the last argument on the command
     line.

The only change in behavior from this patch is that the following options
disappear from the link-editing steps (due to (3)):

  -O -I. -I../support/set -I../h -DUSER_ZZSYN -DZZLEXBUFSIZE=65536

However these options made no difference for linking in the first place.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1540244
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 13 ++++++++-----
 BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile   | 19 +++++++++++--------
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile b/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
index 8f2cc78c5947..94e6e7292309 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
+++ b/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
@@ -170,8 +170,11 @@ 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=
+
+# keep COPT last
+BUILD_CFLAGS=$(COPT)
+
+BUILD_CPPFLAGS=-I. -I$(SET) -I$(PCCTS_H) -DUSER_ZZSYN -DZZLEXBUFSIZE=65536
 #
 # SGI Users, use this CFLAGS
 #
@@ -180,7 +183,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)
+		$(BUILD_CC) -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
@@ -203,10 +206,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
+	$(BUILD_CC) -c $(BUILD_CPPFLAGS) $(BUILD_CFLAGS) -o set.o $(SET)/set.c
 
 %.o : %.c 
-	$(BUILD_CC) -c $(BUILD_CFLAGS) $(BUILD_CPPFLAGS) $< -o $@
+	$(BUILD_CC) -c $(BUILD_CPPFLAGS) $(BUILD_CFLAGS) -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 b3a34d3b4613..f4babb4b0790 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
+++ b/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
@@ -118,15 +118,18 @@ BUILD_CC?=cc
 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=
+
+# keep COPT last
+BUILD_CFLAGS=$(COPT)
+
+BUILD_CPPFLAGS=-I. -I$(SET) -I$(PCCTS_H) -DUSER_ZZSYN -DZZLEXBUFSIZE=65536
 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)
+		$(BUILD_CC) -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
@@ -138,19 +141,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
+	$(BUILD_CC) -c $(BUILD_CPPFLAGS) $(BUILD_CFLAGS) dlg_p.c
 
 dlg_a.$(OBJ_EXT) : dlg_a.c dlg.h tokens.h mode.h
-	$(BUILD_CC) $(BUILD_CFLAGS) -c dlg_a.c
+	$(BUILD_CC) -c $(BUILD_CPPFLAGS) $(BUILD_CFLAGS) dlg_a.c
 
 main.$(OBJ_EXT) : main.c dlg.h
-	$(BUILD_CC) $(BUILD_CFLAGS) -c main.c
+	$(BUILD_CC) -c $(BUILD_CPPFLAGS) $(BUILD_CFLAGS) main.c
 
 set.$(OBJ_EXT) : $(SET)/set.c
-	$(BUILD_CC) -c $(BUILD_CFLAGS) $(SET)/set.c
+	$(BUILD_CC) -c $(BUILD_CPPFLAGS) $(BUILD_CFLAGS) $(SET)/set.c
 
 %.o : %.c 
-	$(BUILD_CC) -c $(BUILD_CFLAGS) $(BUILD_CPPFLAGS) $< -o $@
+	$(BUILD_CC) -c $(BUILD_CPPFLAGS) $(BUILD_CFLAGS) -o $@ $<
 
 lint:
 	lint *.c
-- 
2.14.1.3.gb7cf6e02401b

[PATCH 5/6] BaseTools/Source/C: take EXTRA_OPTFLAGS from the caller

[Laszlo Ersek]

Allow the caller of the top-level makefile either to set EXTRA_OPTFLAGS in
the environment or to pass EXTRA_OPTFLAGS as a macro definition on the
command line. EXTRA_OPTFLAGS extends (and potentially overrides) default C
compilation flags set in the makefiles.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1540244
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 BaseTools/Source/C/Makefiles/header.makefile       | 5 ++++-
 BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 5 ++++-
 BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile   | 5 ++++-
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/C/Makefiles/header.makefile b/BaseTools/Source/C/Makefiles/header.makefile
index 498c6cf48b4a..1b4cad5497ec 100644
--- a/BaseTools/Source/C/Makefiles/header.makefile
+++ b/BaseTools/Source/C/Makefiles/header.makefile
@@ -69,7 +69,10 @@ 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)
-BUILD_OPTFLAGS = -O2
+
+# 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 -Wno-deprecated-declarations -Wno-self-assign -Wno-unused-result -nostdlib -g
diff --git a/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile b/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
index 94e6e7292309..cefe6a078b39 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
+++ b/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
@@ -165,7 +165,10 @@ PCCTS_H=../h
 #   UNIX  (default)
 #
 BUILD_CC?=gcc
-COPT=-O
+
+# keep EXTRA_OPTFLAGS last
+COPT=-O $(EXTRA_OPTFLAGS)
+
 ANTLR=${BIN_DIR}/antlr
 DLG=${BIN_DIR}/dlg
 OBJ_EXT=o
diff --git a/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile b/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
index f4babb4b0790..a9d1771d4c5b 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
+++ b/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
@@ -115,7 +115,10 @@ PCCTS_H=../h
 #   UNIX
 #
 BUILD_CC?=cc
-COPT=-O
+
+# keep EXTRA_OPTFLAGS last
+COPT=-O $(EXTRA_OPTFLAGS)
+
 ANTLR=${BIN_DIR}/antlr
 DLG=${BIN_DIR}/dlg
 
-- 
2.14.1.3.gb7cf6e02401b

[PATCH 6/6] BaseTools/Source/C: take EXTRA_LDFLAGS from the caller

[Laszlo Ersek]

Allow the caller of the top-level makefile either to set EXTRA_LDFLAGS in
the environment or to pass EXTRA_LDFLAGS as a macro definition on the
command line. EXTRA_LDFLAGS extends (and potentially overrides) default
link-editing flags set in the makefiles.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1540244
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 BaseTools/Source/C/Makefiles/header.makefile       | 3 +++
 BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 6 +++++-
 BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile   | 6 +++++-
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/Makefiles/header.makefile b/BaseTools/Source/C/Makefiles/header.makefile
index 1b4cad5497ec..7f283d6464a8 100644
--- a/BaseTools/Source/C/Makefiles/header.makefile
+++ b/BaseTools/Source/C/Makefiles/header.makefile
@@ -99,6 +99,9 @@ endif
 BUILD_CFLAGS   += $(BUILD_OPTFLAGS)
 BUILD_CXXFLAGS += $(BUILD_OPTFLAGS)
   
+# keep EXTRA_LDFLAGS last
+BUILD_LFLAGS += $(EXTRA_LDFLAGS)
+
 .PHONY: all
 .PHONY: install
 .PHONY: clean
diff --git a/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile b/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
index cefe6a078b39..68897cb81e20 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
+++ b/BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile
@@ -178,6 +178,10 @@ OUT_OBJ = -o
 BUILD_CFLAGS=$(COPT)
 
 BUILD_CPPFLAGS=-I. -I$(SET) -I$(PCCTS_H) -DUSER_ZZSYN -DZZLEXBUFSIZE=65536
+
+# keep EXTRA_LDFLAGS last
+BUILD_LFLAGS = $(EXTRA_LDFLAGS)
+
 #
 # SGI Users, use this CFLAGS
 #
@@ -186,7 +190,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) -o $(BIN_DIR)/antlr $(OBJ)
+		$(BUILD_CC) -o $(BIN_DIR)/antlr $(BUILD_LFLAGS) $(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
diff --git a/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile b/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
index a9d1771d4c5b..9a185b4b9c7a 100644
--- a/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
+++ b/BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile
@@ -126,13 +126,17 @@ DLG=${BIN_DIR}/dlg
 BUILD_CFLAGS=$(COPT)
 
 BUILD_CPPFLAGS=-I. -I$(SET) -I$(PCCTS_H) -DUSER_ZZSYN -DZZLEXBUFSIZE=65536
+
+# keep EXTRA_LDFLAGS last
+BUILD_LFLAGS = $(EXTRA_LDFLAGS)
+
 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) -o $(BIN_DIR)/dlg $(OBJ)
+		$(BUILD_CC) -o $(BIN_DIR)/dlg $(BUILD_LFLAGS) $(OBJ)
 
 SRC = dlg_p.c dlg_a.c main.c err.c $(SET)/set.c support.c output.c \
         relabel.c automata.c
-- 
2.14.1.3.gb7cf6e02401b

Re: [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller

[Gao, Liming]

Laszlo:
  I understand this patch set is to provide the way to append compile and link option for BaseTools source build. If so, the extend flag name may be EXTRA_CCFLAGS and EXTRA_LDFLAGS. And, the extend flags are appended in the tail. 

  Besides, Pccts is the internal tool to generate VfrCompiler syntax source file. It is not used in build process. I am not sure why they also require the additional CC and LD flags. 

Thanks
Liming
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Thursday, July 26, 2018 8:44 AM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
> Subject: [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller
> 
> Repo:   https://github.com/lersek/edk2.git
> Branch: extra_flags_rhbz1540244
> 
> In the Fedora distribution, we'd like to pass system-wide flags related
> to optimization and linking when the C and C++ language base tools are
> built. This series lets the outermost "make" command push the
> EXTRA_OPTFLAGS and EXTRA_LDFLAGS macros into the BaseTools build.
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> 
> Thanks
> Laszlo
> 
> Laszlo Ersek (6):
>   BaseTools/footer.makefile: expand BUILD_CFLAGS last for C files too
>   BaseTools/header.makefile: remove "-c" from BUILD_CFLAGS
>   BaseTools/Source/C: split "-O2" to BUILD_OPTFLAGS
>   BaseTools/Pccts: clean up antlr and dlg makefiles
>   BaseTools/Source/C: take EXTRA_OPTFLAGS from the caller
>   BaseTools/Source/C: take EXTRA_LDFLAGS from the caller
> 
>  BaseTools/Source/C/Makefiles/footer.makefile       |  2 +-
>  BaseTools/Source/C/Makefiles/header.makefile       | 16 ++++++++---
>  BaseTools/Source/C/VfrCompile/GNUmakefile          | 11 +++++---
>  BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 22 ++++++++++-----
>  BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile   | 28 +++++++++++++-------
>  5 files changed, 56 insertions(+), 23 deletions(-)
> 
> --
> 2.14.1.3.gb7cf6e02401b

Re: [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller

[Laszlo Ersek]

On 08/02/18 17:40, Gao, Liming wrote:
> Laszlo:
>   I understand this patch set is to provide the way to append compile and link option for BaseTools source build.

Yes.

> If so, the extend flag name may be EXTRA_CCFLAGS

I can rename EXTRA_OPTFLAGS to EXTRA_CCFLAGS, but in that case,
internally we will have:

  BUILD_OPTFLAGS = -O2 $(EXTRA_CCFLAGS)

in "header.makefile". In that case, I expect to receive a comment that
we shouldn't append a generic "CCFLAGS" variable to a more specialized
"OPTFLAGS" variable.

Obviously, I can rename "BUILD_OPTFLAGS" to "BUILD_CCFLAGS" as well --
but, in that case, I expect to receive a comment that we already have
"BUILD_CFLAGS".

The variable (more precisely, "RPM macro") that the Fedora distribution
will put into EXTRA_OPTFLAGS is also called %{optflags}. So I think
EXTRA_OPTFLAGS is an appropriate name.


If you still disagree, then can you please suggest a new name not just
for EXTRA_OPTFLAGS (-->EXTRA_CCFLAGS), but also for BUILD_OPTFLAGS?
Patch #3 explains why we need a separate BUILD_OPTFLAGS Makefile macro.


> and EXTRA_LDFLAGS.

Right, that's the currently proposed name.

> And, the extend flags are appended in the tail. 

Correct.

>   Besides, Pccts is the internal tool to generate VfrCompiler syntax source file. It is not used in build process. I am not sure why they also require the additional CC and LD flags.

It's a general policy thing; all native binaries should be built with
the system-wide flags. Some of those flags will let the binaries detect
some buffer overflows automatically, for example, which is helpful even
if the utility is never installed / packaged, just used as a one-off
build tool.

Thanks!
Laszlo

> 
> Thanks
> Liming
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Thursday, July 26, 2018 8:44 AM
>> To: edk2-devel-01 <edk2-devel@lists.01.org>
>> Cc: Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
>> Subject: [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller
>>
>> Repo:   https://github.com/lersek/edk2.git
>> Branch: extra_flags_rhbz1540244
>>
>> In the Fedora distribution, we'd like to pass system-wide flags related
>> to optimization and linking when the C and C++ language base tools are
>> built. This series lets the outermost "make" command push the
>> EXTRA_OPTFLAGS and EXTRA_LDFLAGS macros into the BaseTools build.
>>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Cc: Yonghong Zhu <yonghong.zhu@intel.com>
>>
>> Thanks
>> Laszlo
>>
>> Laszlo Ersek (6):
>>   BaseTools/footer.makefile: expand BUILD_CFLAGS last for C files too
>>   BaseTools/header.makefile: remove "-c" from BUILD_CFLAGS
>>   BaseTools/Source/C: split "-O2" to BUILD_OPTFLAGS
>>   BaseTools/Pccts: clean up antlr and dlg makefiles
>>   BaseTools/Source/C: take EXTRA_OPTFLAGS from the caller
>>   BaseTools/Source/C: take EXTRA_LDFLAGS from the caller
>>
>>  BaseTools/Source/C/Makefiles/footer.makefile       |  2 +-
>>  BaseTools/Source/C/Makefiles/header.makefile       | 16 ++++++++---
>>  BaseTools/Source/C/VfrCompile/GNUmakefile          | 11 +++++---
>>  BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 22 ++++++++++-----
>>  BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile   | 28 +++++++++++++-------
>>  5 files changed, 56 insertions(+), 23 deletions(-)
>>
>> --
>> 2.14.1.3.gb7cf6e02401b
> 

Re: [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller

[Gao, Liming]

Laszlo:
  Thanks for your detail information. I understand EXTRA_OPTFLAGS. So, its name is OK to me. 

  On Pccts, it is the third party code. I would like to make the minimal change. So, I ask whether we not touch it. 

Thanks
Liming
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, August 3, 2018 1:41 AM
> To: Gao, Liming <liming.gao@intel.com>; edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Zhu, Yonghong <yonghong.zhu@intel.com>
> Subject: Re: [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller
> 
> On 08/02/18 17:40, Gao, Liming wrote:
> > Laszlo:
> >   I understand this patch set is to provide the way to append compile and link option for BaseTools source build.
> 
> Yes.
> 
> > If so, the extend flag name may be EXTRA_CCFLAGS
> 
> I can rename EXTRA_OPTFLAGS to EXTRA_CCFLAGS, but in that case,
> internally we will have:
> 
>   BUILD_OPTFLAGS = -O2 $(EXTRA_CCFLAGS)
> 
> in "header.makefile". In that case, I expect to receive a comment that
> we shouldn't append a generic "CCFLAGS" variable to a more specialized
> "OPTFLAGS" variable.
> 
> Obviously, I can rename "BUILD_OPTFLAGS" to "BUILD_CCFLAGS" as well --
> but, in that case, I expect to receive a comment that we already have
> "BUILD_CFLAGS".
> 
> The variable (more precisely, "RPM macro") that the Fedora distribution
> will put into EXTRA_OPTFLAGS is also called %{optflags}. So I think
> EXTRA_OPTFLAGS is an appropriate name.
> 
> 
> If you still disagree, then can you please suggest a new name not just
> for EXTRA_OPTFLAGS (-->EXTRA_CCFLAGS), but also for BUILD_OPTFLAGS?
> Patch #3 explains why we need a separate BUILD_OPTFLAGS Makefile macro.
> 
> 
> > and EXTRA_LDFLAGS.
> 
> Right, that's the currently proposed name.
> 
> > And, the extend flags are appended in the tail.
> 
> Correct.
> 
> >   Besides, Pccts is the internal tool to generate VfrCompiler syntax source file. It is not used in build process. I am not sure why they
> also require the additional CC and LD flags.
> 
> It's a general policy thing; all native binaries should be built with
> the system-wide flags. Some of those flags will let the binaries detect
> some buffer overflows automatically, for example, which is helpful even
> if the utility is never installed / packaged, just used as a one-off
> build tool.
> 
> Thanks!
> Laszlo
> 
> >
> > Thanks
> > Liming
> >> -----Original Message-----
> >> From: Laszlo Ersek [mailto:lersek@redhat.com]
> >> Sent: Thursday, July 26, 2018 8:44 AM
> >> To: edk2-devel-01 <edk2-devel@lists.01.org>
> >> Cc: Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
> >> Subject: [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller
> >>
> >> Repo:   https://github.com/lersek/edk2.git
> >> Branch: extra_flags_rhbz1540244
> >>
> >> In the Fedora distribution, we'd like to pass system-wide flags related
> >> to optimization and linking when the C and C++ language base tools are
> >> built. This series lets the outermost "make" command push the
> >> EXTRA_OPTFLAGS and EXTRA_LDFLAGS macros into the BaseTools build.
> >>
> >> Cc: Liming Gao <liming.gao@intel.com>
> >> Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> >>
> >> Thanks
> >> Laszlo
> >>
> >> Laszlo Ersek (6):
> >>   BaseTools/footer.makefile: expand BUILD_CFLAGS last for C files too
> >>   BaseTools/header.makefile: remove "-c" from BUILD_CFLAGS
> >>   BaseTools/Source/C: split "-O2" to BUILD_OPTFLAGS
> >>   BaseTools/Pccts: clean up antlr and dlg makefiles
> >>   BaseTools/Source/C: take EXTRA_OPTFLAGS from the caller
> >>   BaseTools/Source/C: take EXTRA_LDFLAGS from the caller
> >>
> >>  BaseTools/Source/C/Makefiles/footer.makefile       |  2 +-
> >>  BaseTools/Source/C/Makefiles/header.makefile       | 16 ++++++++---
> >>  BaseTools/Source/C/VfrCompile/GNUmakefile          | 11 +++++---
> >>  BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 22 ++++++++++-----
> >>  BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile   | 28 +++++++++++++-------
> >>  5 files changed, 56 insertions(+), 23 deletions(-)
> >>
> >> --
> >> 2.14.1.3.gb7cf6e02401b
> >

Re: [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller

[Laszlo Ersek]

On 08/06/18 16:48, Gao, Liming wrote:
> Laszlo:
>   Thanks for your detail information. I understand EXTRA_OPTFLAGS. So, its name is OK to me. 
> 
>   On Pccts, it is the third party code. I would like to make the minimal change. So, I ask whether we not touch it. 

OK, thank you, I'll look into that.

Laszlo

>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Friday, August 3, 2018 1:41 AM
>> To: Gao, Liming <liming.gao@intel.com>; edk2-devel-01 <edk2-devel@lists.01.org>
>> Cc: Zhu, Yonghong <yonghong.zhu@intel.com>
>> Subject: Re: [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller
>>
>> On 08/02/18 17:40, Gao, Liming wrote:
>>> Laszlo:
>>>   I understand this patch set is to provide the way to append compile and link option for BaseTools source build.
>>
>> Yes.
>>
>>> If so, the extend flag name may be EXTRA_CCFLAGS
>>
>> I can rename EXTRA_OPTFLAGS to EXTRA_CCFLAGS, but in that case,
>> internally we will have:
>>
>>   BUILD_OPTFLAGS = -O2 $(EXTRA_CCFLAGS)
>>
>> in "header.makefile". In that case, I expect to receive a comment that
>> we shouldn't append a generic "CCFLAGS" variable to a more specialized
>> "OPTFLAGS" variable.
>>
>> Obviously, I can rename "BUILD_OPTFLAGS" to "BUILD_CCFLAGS" as well --
>> but, in that case, I expect to receive a comment that we already have
>> "BUILD_CFLAGS".
>>
>> The variable (more precisely, "RPM macro") that the Fedora distribution
>> will put into EXTRA_OPTFLAGS is also called %{optflags}. So I think
>> EXTRA_OPTFLAGS is an appropriate name.
>>
>>
>> If you still disagree, then can you please suggest a new name not just
>> for EXTRA_OPTFLAGS (-->EXTRA_CCFLAGS), but also for BUILD_OPTFLAGS?
>> Patch #3 explains why we need a separate BUILD_OPTFLAGS Makefile macro.
>>
>>
>>> and EXTRA_LDFLAGS.
>>
>> Right, that's the currently proposed name.
>>
>>> And, the extend flags are appended in the tail.
>>
>> Correct.
>>
>>>   Besides, Pccts is the internal tool to generate VfrCompiler syntax source file. It is not used in build process. I am not sure why they
>> also require the additional CC and LD flags.
>>
>> It's a general policy thing; all native binaries should be built with
>> the system-wide flags. Some of those flags will let the binaries detect
>> some buffer overflows automatically, for example, which is helpful even
>> if the utility is never installed / packaged, just used as a one-off
>> build tool.
>>
>> Thanks!
>> Laszlo
>>
>>>
>>> Thanks
>>> Liming
>>>> -----Original Message-----
>>>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>>>> Sent: Thursday, July 26, 2018 8:44 AM
>>>> To: edk2-devel-01 <edk2-devel@lists.01.org>
>>>> Cc: Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
>>>> Subject: [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller
>>>>
>>>> Repo:   https://github.com/lersek/edk2.git
>>>> Branch: extra_flags_rhbz1540244
>>>>
>>>> In the Fedora distribution, we'd like to pass system-wide flags related
>>>> to optimization and linking when the C and C++ language base tools are
>>>> built. This series lets the outermost "make" command push the
>>>> EXTRA_OPTFLAGS and EXTRA_LDFLAGS macros into the BaseTools build.
>>>>
>>>> Cc: Liming Gao <liming.gao@intel.com>
>>>> Cc: Yonghong Zhu <yonghong.zhu@intel.com>
>>>>
>>>> Thanks
>>>> Laszlo
>>>>
>>>> Laszlo Ersek (6):
>>>>   BaseTools/footer.makefile: expand BUILD_CFLAGS last for C files too
>>>>   BaseTools/header.makefile: remove "-c" from BUILD_CFLAGS
>>>>   BaseTools/Source/C: split "-O2" to BUILD_OPTFLAGS
>>>>   BaseTools/Pccts: clean up antlr and dlg makefiles
>>>>   BaseTools/Source/C: take EXTRA_OPTFLAGS from the caller
>>>>   BaseTools/Source/C: take EXTRA_LDFLAGS from the caller
>>>>
>>>>  BaseTools/Source/C/Makefiles/footer.makefile       |  2 +-
>>>>  BaseTools/Source/C/Makefiles/header.makefile       | 16 ++++++++---
>>>>  BaseTools/Source/C/VfrCompile/GNUmakefile          | 11 +++++---
>>>>  BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 22 ++++++++++-----
>>>>  BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile   | 28 +++++++++++++-------
>>>>  5 files changed, 56 insertions(+), 23 deletions(-)
>>>>
>>>> --
>>>> 2.14.1.3.gb7cf6e02401b
>>>
> 

Re: [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller

[Laszlo Ersek]

Hi Liming,

On 08/06/18 17:18, Laszlo Ersek wrote:
> On 08/06/18 16:48, Gao, Liming wrote:
>> Laszlo:
>>   Thanks for your detail information. I understand EXTRA_OPTFLAGS.
>>   So, its name is OK to me.
>>
>>   On Pccts, it is the third party code. I would like to make the
>>   minimal change. So, I ask whether we not touch it.
>
> OK, thank you, I'll look into that.

I started writing up a summary for the stake-holders of
<https://bugzilla.redhat.com/show_bug.cgi?id=1540244>, explaining that
some source code that goes into the VfrCompile utility is native to the
edk2 project, while the code that *generates* the lexer and parser
source code for VfrCompile comes from the PCCTS project, and is used
only temporarily. And, that this should be a good enough reason to
ignore PCCTS, because in upstream the maintainers prefer not touching
PCCTS source.

However: our git history for "BaseTools/Source/C/VfrCompile/Pccts" does
not corroborate this preference.

Consider:

(a)  1  30fdf1140b8d [2009-07-17] Check In tool source code based on Build tool project revision r1655.
     2  b69fd59e6f1a [2014-08-25] Fix nmake cleanall bugs.
     3  5ddccf34c4f5 [2015-07-08] BaseTools: Fix build on FreeBSD and allow use of non-gcc system compiler
     4  819a2394f17f [2016-01-11] BaseTools/VfrCompile: honor CC if it is set
     5  4ac14ceae076 [2016-09-08] BaseTools VfrCompile Pccts: Update GCC Flags to the specific one with BUILD_ prefix

    Commits #3 through #5 modify the same set of files as my patches 4-6
    -- the "antlr" and "dlg" makefiles.

(b)  6  99e55970ff07 [2016-10-20] BaseTools: Fix typos in comments and variables

    This is from Gary's series

      [edk2] [PATCH 00/33] Fix typos in comments and variables

    and it modifies "dlg" source code.

(c)  7  bab5ad2fd14b [2016-11-08] BaseTools/VfrCompile: Add checks for array access
     8  77dee0b1859d [2016-11-08] BaseTools/VfrCompile: Avoid freeing freed memory in classes
     9  d55638362727 [2016-11-08] BaseTools/VfrCompile/Pccts: Add virtual destructor for class DLGInputStream
    10  fef15ecd20dd [2016-11-08] BaseTools/VfrCompile/Pccts: Make assignment operator not returning void

    These four commits (#7 through #10) are from Hao's series

      [edk2] [PATCH v2 00/53] Resolve issues for C source codes in BaseTools

    and they modify PCCTS headers.

(d) 11  5b26adf03a0b [2016-12-20] BaseTools: fix format-security build warnings
    12  8230d45bba51 [2016-12-20] BaseTools: fix format type build warnings

    Commits #11 and #12 are from Heyi's series

      [edk2] [PATCH 0/4] Fix GCC build warnings for BaseTools

    and they modify the "antlr" source code.

(e) 13  0a64f49fde09 [2016-12-23] BaseTools/Pccts: Resolve GCC sting format mismatch build warning

    This patch is again from Hao, and it modifies utility code in PCCTS
    that is built into both "dlg" and "antlr" (namely, "set.c").

(f) 14  a5b84d3480b4 [2018-01-02] BaseTools: eliminate unused expression result
    15  4e97974c1e52 [2018-01-02] BaseTools: silence parentheses-equality warning

    These are from a series that Zenith432 posted without a cover
    letter. They modify "antlr" and "dlg" source code.

The above examples imply that we have modified both the makefiles and
the source code under PCCTS, over time.

Do you still prefer that I drop those parts of my series?

I can attempt to do that, but then I cannot tell the RHBZ#1540244
stakeholders that we "generally" avoid patching the bundled PCCTS
instance -- because, we do patch it whenever necessary.

Thanks!
Laszlo

Re: [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller

[Gao, Liming]

Laszlo:
  I mean to keep the minimal change in PCCT. I don't prevent the necessary change in PCCT code. Per your comments, this change in PCCT is necessary. If so, I am OK to this patch. 

  Reviewed-by: Liming Gao <liming.gao@intel.com>

Thanks
Liming
>-----Original Message-----
>From: Laszlo Ersek [mailto:lersek@redhat.com]
>Sent: Tuesday, August 07, 2018 12:41 AM
>To: Gao, Liming <liming.gao@intel.com>
>Cc: edk2-devel-01 <edk2-devel@lists.01.org>
>Subject: Re: [edk2] [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS
>and EXTRA_LDFLAGS from the caller
>
>Hi Liming,
>
>On 08/06/18 17:18, Laszlo Ersek wrote:
>> On 08/06/18 16:48, Gao, Liming wrote:
>>> Laszlo:
>>>   Thanks for your detail information. I understand EXTRA_OPTFLAGS.
>>>   So, its name is OK to me.
>>>
>>>   On Pccts, it is the third party code. I would like to make the
>>>   minimal change. So, I ask whether we not touch it.
>>
>> OK, thank you, I'll look into that.
>
>I started writing up a summary for the stake-holders of
><https://bugzilla.redhat.com/show_bug.cgi?id=1540244>, explaining that
>some source code that goes into the VfrCompile utility is native to the
>edk2 project, while the code that *generates* the lexer and parser
>source code for VfrCompile comes from the PCCTS project, and is used
>only temporarily. And, that this should be a good enough reason to
>ignore PCCTS, because in upstream the maintainers prefer not touching
>PCCTS source.
>
>However: our git history for "BaseTools/Source/C/VfrCompile/Pccts" does
>not corroborate this preference.
>
>Consider:
>
>(a)  1  30fdf1140b8d [2009-07-17] Check In tool source code based on Build tool
>project revision r1655.
>     2  b69fd59e6f1a [2014-08-25] Fix nmake cleanall bugs.
>     3  5ddccf34c4f5 [2015-07-08] BaseTools: Fix build on FreeBSD and allow use
>of non-gcc system compiler
>     4  819a2394f17f [2016-01-11] BaseTools/VfrCompile: honor CC if it is set
>     5  4ac14ceae076 [2016-09-08] BaseTools VfrCompile Pccts: Update GCC Flags
>to the specific one with BUILD_ prefix
>
>    Commits #3 through #5 modify the same set of files as my patches 4-6
>    -- the "antlr" and "dlg" makefiles.
>
>(b)  6  99e55970ff07 [2016-10-20] BaseTools: Fix typos in comments and
>variables
>
>    This is from Gary's series
>
>      [edk2] [PATCH 00/33] Fix typos in comments and variables
>
>    and it modifies "dlg" source code.
>
>(c)  7  bab5ad2fd14b [2016-11-08] BaseTools/VfrCompile: Add checks for array
>access
>     8  77dee0b1859d [2016-11-08] BaseTools/VfrCompile: Avoid freeing freed
>memory in classes
>     9  d55638362727 [2016-11-08] BaseTools/VfrCompile/Pccts: Add virtual
>destructor for class DLGInputStream
>    10  fef15ecd20dd [2016-11-08] BaseTools/VfrCompile/Pccts: Make
>assignment operator not returning void
>
>    These four commits (#7 through #10) are from Hao's series
>
>      [edk2] [PATCH v2 00/53] Resolve issues for C source codes in BaseTools
>
>    and they modify PCCTS headers.
>
>(d) 11  5b26adf03a0b [2016-12-20] BaseTools: fix format-security build
>warnings
>    12  8230d45bba51 [2016-12-20] BaseTools: fix format type build warnings
>
>    Commits #11 and #12 are from Heyi's series
>
>      [edk2] [PATCH 0/4] Fix GCC build warnings for BaseTools
>
>    and they modify the "antlr" source code.
>
>(e) 13  0a64f49fde09 [2016-12-23] BaseTools/Pccts: Resolve GCC sting format
>mismatch build warning
>
>    This patch is again from Hao, and it modifies utility code in PCCTS
>    that is built into both "dlg" and "antlr" (namely, "set.c").
>
>(f) 14  a5b84d3480b4 [2018-01-02] BaseTools: eliminate unused expression
>result
>    15  4e97974c1e52 [2018-01-02] BaseTools: silence parentheses-equality
>warning
>
>    These are from a series that Zenith432 posted without a cover
>    letter. They modify "antlr" and "dlg" source code.
>
>The above examples imply that we have modified both the makefiles and
>the source code under PCCTS, over time.
>
>Do you still prefer that I drop those parts of my series?
>
>I can attempt to do that, but then I cannot tell the RHBZ#1540244
>stakeholders that we "generally" avoid patching the bundled PCCTS
>instance -- because, we do patch it whenever necessary.
>
>Thanks!
>Laszlo

Re: [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller

[Laszlo Ersek]

On 08/07/18 07:27, Gao, Liming wrote:
> Laszlo:
>   I mean to keep the minimal change in PCCT. I don't prevent the necessary change in PCCT code. Per your comments, this change in PCCT is necessary. If so, I am OK to this patch. 
> 
>   Reviewed-by: Liming Gao <liming.gao@intel.com>

Thanks -- I understand it better now. I will return to the RHBZ and
discuss the PCCTS question with the requestors. I will explain your
preference. If they feel it's really necessary to build PCCTS (antlr and
dlg) with the additional flags, even just for generating the lexer and
the parser for VfrCompile, I'll go ahead with your R-b for all six
patches. If they agree the PCCTS build can ignore the flags, I won't
modify PCCTS.

Thank you for taking the time to discuss this! I'll report back.
Laszlo

Re: [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller

[Laszlo Ersek]

On 08/07/18 14:23, Laszlo Ersek wrote:
> On 08/07/18 07:27, Gao, Liming wrote:
>> Laszlo:
>>   I mean to keep the minimal change in PCCT. I don't prevent the necessary change in PCCT code. Per your comments, this change in PCCT is necessary. If so, I am OK to this patch. 
>>
>>   Reviewed-by: Liming Gao <liming.gao@intel.com>
> 
> Thanks -- I understand it better now. I will return to the RHBZ and
> discuss the PCCTS question with the requestors. I will explain your
> preference. If they feel it's really necessary to build PCCTS (antlr and
> dlg) with the additional flags, even just for generating the lexer and
> the parser for VfrCompile, I'll go ahead with your R-b for all six
> patches. If they agree the PCCTS build can ignore the flags, I won't
> modify PCCTS.
> 
> Thank you for taking the time to discuss this! I'll report back.

I will soon submit a v2 that does not touch the PCCTS (antlr & dlg)
build flags.

Thanks!
Laszlo