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

[PATCH v2 0/5] 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_v2

Version 1 of this set was posted at

  http://mid.mail-archive.com/20180726004415.13381-1-lersek@redhat.com
  https://lists.01.org/pipermail/edk2-devel/2018-July/027606.html

In version 2 (i.e., this version), the PCCTS tools (the "dlg" lexer
generator and the "antlr" parser generator) are not modified. Relative
to v1:

- "[PATCH 4/6] BaseTools/Pccts: clean up antlr and dlg makefiles" has
  been dropped,

- the "BaseTools/Source/C/VfrCompile/Pccts" hunks have been removed from
  "[PATCH 5/6] BaseTools/Source/C: take EXTRA_OPTFLAGS from the caller"
  and "[PATCH 6/6] BaseTools/Source/C: take EXTRA_LDFLAGS from the
  caller".

In other words, v2 is a proper subset of v1, so that PCCTS is left
alone.

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

Thanks!
Laszlo

Laszlo Ersek (5):
  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/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 +++++++----
 3 files changed, 21 insertions(+), 8 deletions(-)

-- 
2.14.1.3.gb7cf6e02401b

[PATCH v2 1/5] 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>
Reviewed-by: Liming Gao <liming.gao@intel.com>
---

Notes:
    v2:
    - pick up Liming's R-b

 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 v2 2/5] 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>
Reviewed-by: Liming Gao <liming.gao@intel.com>
---

Notes:
    v2:
    - pick up Liming's R-b

 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 v2 3/5] 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>
Reviewed-by: Liming Gao <liming.gao@intel.com>
---

Notes:
    v2:
    - pick up Liming's R-b

 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 v2 4/5] 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>
---

Notes:
    v2:
    - do not patch "antlr/makefile" and "dlg/makefile" under
      "BaseTools/Source/C/VfrCompile/Pccts" [Liming]

 BaseTools/Source/C/Makefiles/header.makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

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
-- 
2.14.1.3.gb7cf6e02401b

[PATCH v2 5/5] 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>
---

Notes:
    v2:
    - do not patch "antlr/makefile" and "dlg/makefile" under
      "BaseTools/Source/C/VfrCompile/Pccts" [Liming]

 BaseTools/Source/C/Makefiles/header.makefile | 3 +++
 1 file changed, 3 insertions(+)

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
-- 
2.14.1.3.gb7cf6e02401b

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

[Laszlo Ersek]

Hi Liming,

On 08/09/18 15:22, Laszlo Ersek wrote:
> Repo:   https://github.com/lersek/edk2.git
> Branch: extra_flags_rhbz1540244_v2
> 
> Version 1 of this set was posted at
> 
>   http://mid.mail-archive.com/20180726004415.13381-1-lersek@redhat.com
>   https://lists.01.org/pipermail/edk2-devel/2018-July/027606.html
> 
> In version 2 (i.e., this version), the PCCTS tools (the "dlg" lexer
> generator and the "antlr" parser generator) are not modified. Relative
> to v1:
> 
> - "[PATCH 4/6] BaseTools/Pccts: clean up antlr and dlg makefiles" has
>   been dropped,
> 
> - the "BaseTools/Source/C/VfrCompile/Pccts" hunks have been removed from
>   "[PATCH 5/6] BaseTools/Source/C: take EXTRA_OPTFLAGS from the caller"
>   and "[PATCH 6/6] BaseTools/Source/C: take EXTRA_LDFLAGS from the
>   caller".
> 
> In other words, v2 is a proper subset of v1, so that PCCTS is left
> alone.
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Yonghong Zhu <yonghong.zhu@intel.com>

do you have any comments on patches #4 and #5?

Thanks,
Laszlo

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

[Gao, Liming]

Laszlo:
  The patch is good. Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Wednesday, August 15, 2018 10:19 AM
> To: Gao, Liming <liming.gao@intel.com>
> Cc: edk2-devel-01 <edk2-devel@lists.01.org>
> Subject: Re: [edk2] [PATCH v2 0/5] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller
> 
> Hi Liming,
> 
> On 08/09/18 15:22, Laszlo Ersek wrote:
> > Repo:   https://github.com/lersek/edk2.git
> > Branch: extra_flags_rhbz1540244_v2
> >
> > Version 1 of this set was posted at
> >
> >   http://mid.mail-archive.com/20180726004415.13381-1-lersek@redhat.com
> >   https://lists.01.org/pipermail/edk2-devel/2018-July/027606.html
> >
> > In version 2 (i.e., this version), the PCCTS tools (the "dlg" lexer
> > generator and the "antlr" parser generator) are not modified. Relative
> > to v1:
> >
> > - "[PATCH 4/6] BaseTools/Pccts: clean up antlr and dlg makefiles" has
> >   been dropped,
> >
> > - the "BaseTools/Source/C/VfrCompile/Pccts" hunks have been removed from
> >   "[PATCH 5/6] BaseTools/Source/C: take EXTRA_OPTFLAGS from the caller"
> >   and "[PATCH 6/6] BaseTools/Source/C: take EXTRA_LDFLAGS from the
> >   caller".
> >
> > In other words, v2 is a proper subset of v1, so that PCCTS is left
> > alone.
> >
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> 
> do you have any comments on patches #4 and #5?
> 
> Thanks,
> Laszlo

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

[Laszlo Ersek]

On 08/15/18 19:37, Gao, Liming wrote:
> Laszlo:
>   The patch is good. Reviewed-by: Liming Gao <liming.gao@intel.com>

Thanks!

Pushed as commit range 9becf2f0759e..81502cee20ac.

Laszlo

> 
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Wednesday, August 15, 2018 10:19 AM
>> To: Gao, Liming <liming.gao@intel.com>
>> Cc: edk2-devel-01 <edk2-devel@lists.01.org>
>> Subject: Re: [edk2] [PATCH v2 0/5] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller
>>
>> Hi Liming,
>>
>> On 08/09/18 15:22, Laszlo Ersek wrote:
>>> Repo:   https://github.com/lersek/edk2.git
>>> Branch: extra_flags_rhbz1540244_v2
>>>
>>> Version 1 of this set was posted at
>>>
>>>   http://mid.mail-archive.com/20180726004415.13381-1-lersek@redhat.com
>>>   https://lists.01.org/pipermail/edk2-devel/2018-July/027606.html
>>>
>>> In version 2 (i.e., this version), the PCCTS tools (the "dlg" lexer
>>> generator and the "antlr" parser generator) are not modified. Relative
>>> to v1:
>>>
>>> - "[PATCH 4/6] BaseTools/Pccts: clean up antlr and dlg makefiles" has
>>>   been dropped,
>>>
>>> - the "BaseTools/Source/C/VfrCompile/Pccts" hunks have been removed from
>>>   "[PATCH 5/6] BaseTools/Source/C: take EXTRA_OPTFLAGS from the caller"
>>>   and "[PATCH 6/6] BaseTools/Source/C: take EXTRA_LDFLAGS from the
>>>   caller".
>>>
>>> In other words, v2 is a proper subset of v1, so that PCCTS is left
>>> alone.
>>>
>>> Cc: Liming Gao <liming.gao@intel.com>
>>> Cc: Yonghong Zhu <yonghong.zhu@intel.com>
>>
>> do you have any comments on patches #4 and #5?
>>
>> Thanks,
>> Laszlo
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>