[PATCH v2 0/2] edk2 build scripts: eliminate python 2 dependency

[PATCH v2 0/2] edk2 build scripts: eliminate python 2 dependency

[Laszlo Ersek]

v1 URL:
http://mid.mail-archive.com/20190918171141.15957-1-lersek@redhat.com

Updates are noted on each patch.

Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: John Snow <jsnow@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks
Laszlo

Laszlo Ersek (2):
  edk2 build scripts: honor external BaseTools flags with
    uefi-test-tools
  edk2 build scripts: work around TianoCore#1607 without forcing Python
    2

 roms/Makefile                  |  1 +
 roms/edk2-build.sh             |  4 ++--
 roms/edk2-funcs.sh             | 17 +++++++++++++++++
 tests/uefi-test-tools/Makefile |  5 ++++-
 tests/uefi-test-tools/build.sh |  6 ++++--
 5 files changed, 28 insertions(+), 5 deletions(-)

-- 
2.19.1.3.g30247aa5d201

[PATCH v2 1/2] edk2 build scripts: honor external BaseTools flags with uefi-test-tools

[Laszlo Ersek]

Unify the recipe for "build-edk2-tools" in
"tests/uefi-test-tools/Makefile" with the recipe for "edk2-basetools" in
"roms/Makefile".

Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    v2:
    - new patch

 tests/uefi-test-tools/Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/uefi-test-tools/Makefile b/tests/uefi-test-tools/Makefile
index 1d78bc14d51a..7e0177d7337e 100644
--- a/tests/uefi-test-tools/Makefile
+++ b/tests/uefi-test-tools/Makefile
@@ -99,7 +99,9 @@ Build/bios-tables-test.%.efi: build-edk2-tools
 	+./build.sh $(edk2_dir) BiosTablesTest $* $@
 
 build-edk2-tools:
-	$(MAKE) -C $(edk2_dir)/BaseTools
+	$(MAKE) -C $(edk2_dir)/BaseTools \
+		EXTRA_OPTFLAGS='$(EDK2_BASETOOLS_OPTFLAGS)' \
+		EXTRA_LDFLAGS='$(EDK2_BASETOOLS_LDFLAGS)'
 
 clean:
 	rm -rf Build Conf log
-- 
2.19.1.3.g30247aa5d201

[PATCH v2 2/2] edk2 build scripts: work around TianoCore#1607 without forcing Python 2

[Laszlo Ersek]

It turns out that forcing python2 for running the edk2 "build" utility is
neither necessary nor sufficient.

Forcing python2 is not sufficient for two reasons:

- QEMU is moving away from python2, with python2 nearing EOL,

- according to my most recent testing, the lacking dependency information
  in the makefiles that are generated by edk2's "build" utility can cause
  parallel build failures even when "build" is executed by python2.

And forcing python2 is not necessary because we can still return to the
original idea of filtering out jobserver-related options from MAKEFLAGS.
So do that.

While at it, cut short edk2's auto-detection of the python3.* minor
version, by setting PYTHON_COMMAND to "python3" (which we expect to be
available wherever we intend to build edk2).

With this patch, the guest UEFI binaries that are used as part of the BIOS
tables test, and the OVMF and ArmVirtQemu platform firmwares, will be
built strictly in a single job, regardless of an outermost "-jN" make
option. Alas, there appears to be no reliable way to build edk2 in an
(outer make, inner make) environment, with a jobserver enabled.

Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: John Snow <jsnow@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Reported-by: John Snow <jsnow@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    v2:
    
    - set PYTHON_COMMAND to python3; update commit message [Phil]
    
    - do not pick up any feedback tags from v1 (v2 is not a trivial update)
    
    v1:
    
    - Tested on RHEL7 (where the outer "make" sets the old-style
      "--jobserver-fds" flag) and on Fedora 29 (where the outer "make" sets
      the new-style "--jobserver-auth" flag).
    
    - I've rebuilt all the edk2 binaries with this patch applied. Everything
      works fine. However, if you test this patch, you might notice that git
      reports all the build products as modified. That's because when using
      the python3 code in edk2 BaseTools, the generated makefiles differ
      greatly from the ones generated when running in python2 mode (e.g. due
      to different random seeds in python hashes / dictionaries). As a
      result, parts of the firmware volumes / firmware filesystems could
      appear in a different order than before. This is harmless, and doesn't
      necessitate checking in the rebuilt binaries.

 roms/Makefile                  |  1 +
 tests/uefi-test-tools/Makefile |  1 +
 roms/edk2-build.sh             |  4 ++--
 roms/edk2-funcs.sh             | 17 +++++++++++++++++
 tests/uefi-test-tools/build.sh |  6 ++++--
 5 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/roms/Makefile b/roms/Makefile
index 6cf07d3b44ee..06339efa6fc4 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -141,6 +141,7 @@ build-efi-roms: build-pxe-roms
 #
 edk2-basetools:
 	$(MAKE) -C edk2/BaseTools \
+		PYTHON_COMMAND=$${EDK2_PYTHON_COMMAND:-python3} \
 		EXTRA_OPTFLAGS='$(EDK2_BASETOOLS_OPTFLAGS)' \
 		EXTRA_LDFLAGS='$(EDK2_BASETOOLS_LDFLAGS)'
 
diff --git a/tests/uefi-test-tools/Makefile b/tests/uefi-test-tools/Makefile
index 7e0177d7337e..1dcddcdbbabf 100644
--- a/tests/uefi-test-tools/Makefile
+++ b/tests/uefi-test-tools/Makefile
@@ -100,6 +100,7 @@ Build/bios-tables-test.%.efi: build-edk2-tools
 
 build-edk2-tools:
 	$(MAKE) -C $(edk2_dir)/BaseTools \
+		PYTHON_COMMAND=$${EDK2_PYTHON_COMMAND:-python3} \
 		EXTRA_OPTFLAGS='$(EDK2_BASETOOLS_OPTFLAGS)' \
 		EXTRA_LDFLAGS='$(EDK2_BASETOOLS_LDFLAGS)'
 
diff --git a/roms/edk2-build.sh b/roms/edk2-build.sh
index 4f46f8a6a217..d5391c763728 100755
--- a/roms/edk2-build.sh
+++ b/roms/edk2-build.sh
@@ -27,8 +27,7 @@ shift $num_args
 
 cd edk2
 
-# Work around <https://bugzilla.tianocore.org/show_bug.cgi?id=1607>.
-export PYTHON_COMMAND=python2
+export PYTHON_COMMAND=${EDK2_PYTHON_COMMAND:-python3}
 
 # Source "edksetup.sh" carefully.
 set +e +u +C
@@ -43,6 +42,7 @@ fi
 # any), for the edk2 "build" utility.
 source ../edk2-funcs.sh
 edk2_toolchain=$(qemu_edk2_get_toolchain "$emulation_target")
+MAKEFLAGS=$(qemu_edk2_quirk_tianocore_1607 "$MAKEFLAGS")
 edk2_thread_count=$(qemu_edk2_get_thread_count "$MAKEFLAGS")
 qemu_edk2_set_cross_env "$emulation_target"
 
diff --git a/roms/edk2-funcs.sh b/roms/edk2-funcs.sh
index a9fae7ee891b..3f4485b201f1 100644
--- a/roms/edk2-funcs.sh
+++ b/roms/edk2-funcs.sh
@@ -251,3 +251,20 @@ qemu_edk2_get_thread_count()
     printf '1\n'
   fi
 }
+
+
+# Work around <https://bugzilla.tianocore.org/show_bug.cgi?id=1607> by
+# filtering jobserver-related flags out of MAKEFLAGS. Print the result to the
+# standard output.
+#
+# Parameters:
+#   $1: the value of the MAKEFLAGS variable
+qemu_edk2_quirk_tianocore_1607()
+{
+  local makeflags="$1"
+
+  printf %s "$makeflags" \
+  | LC_ALL=C sed --regexp-extended \
+      --expression='s/--jobserver-(auth|fds)=[0-9]+,[0-9]+//' \
+      --expression='s/-j([0-9]+)?//'
+}
diff --git a/tests/uefi-test-tools/build.sh b/tests/uefi-test-tools/build.sh
index 8aa7935c43bb..3b78f3084069 100755
--- a/tests/uefi-test-tools/build.sh
+++ b/tests/uefi-test-tools/build.sh
@@ -29,8 +29,7 @@ export PACKAGES_PATH=$(realpath -- "$edk2_dir")
 export WORKSPACE=$PWD
 mkdir -p Conf
 
-# Work around <https://bugzilla.tianocore.org/show_bug.cgi?id=1607>.
-export PYTHON_COMMAND=python2
+export PYTHON_COMMAND=${EDK2_PYTHON_COMMAND:-python3}
 
 # Source "edksetup.sh" carefully.
 set +e +u +C
@@ -46,12 +45,15 @@ fi
 source "$edk2_dir/../edk2-funcs.sh"
 edk2_arch=$(qemu_edk2_get_arch "$emulation_target")
 edk2_toolchain=$(qemu_edk2_get_toolchain "$emulation_target")
+MAKEFLAGS=$(qemu_edk2_quirk_tianocore_1607 "$MAKEFLAGS")
+edk2_thread_count=$(qemu_edk2_get_thread_count "$MAKEFLAGS")
 qemu_edk2_set_cross_env "$emulation_target"
 
 # Build the UEFI binary
 mkdir -p log
 build \
   --arch="$edk2_arch" \
+  -n "$edk2_thread_count" \
   --buildtarget=DEBUG \
   --platform=UefiTestToolsPkg/UefiTestToolsPkg.dsc \
   --tagname="$edk2_toolchain" \
-- 
2.19.1.3.g30247aa5d201

Re: [PATCH v2 2/2] edk2 build scripts: work around TianoCore#1607 without forcing Python 2

[Philippe Mathieu-Daudé]

On 9/20/19 10:38 AM, Laszlo Ersek wrote:
> It turns out that forcing python2 for running the edk2 "build" utility is
> neither necessary nor sufficient.
> 
> Forcing python2 is not sufficient for two reasons:
> 
> - QEMU is moving away from python2, with python2 nearing EOL,
> 
> - according to my most recent testing, the lacking dependency information
>   in the makefiles that are generated by edk2's "build" utility can cause
>   parallel build failures even when "build" is executed by python2.
> 
> And forcing python2 is not necessary because we can still return to the
> original idea of filtering out jobserver-related options from MAKEFLAGS.
> So do that.
> 
> While at it, cut short edk2's auto-detection of the python3.* minor
> version, by setting PYTHON_COMMAND to "python3" (which we expect to be
> available wherever we intend to build edk2).

Yes! This fixes it :)

> With this patch, the guest UEFI binaries that are used as part of the BIOS
> tables test, and the OVMF and ArmVirtQemu platform firmwares, will be
> built strictly in a single job, regardless of an outermost "-jN" make
> option. Alas, there appears to be no reliable way to build edk2 in an
> (outer make, inner make) environment, with a jobserver enabled.
> 
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: John Snow <jsnow@redhat.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Reported-by: John Snow <jsnow@redhat.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     v2:
>     
>     - set PYTHON_COMMAND to python3; update commit message [Phil]
>     
>     - do not pick up any feedback tags from v1 (v2 is not a trivial update)
>     
>     v1:
>     
>     - Tested on RHEL7 (where the outer "make" sets the old-style
>       "--jobserver-fds" flag) and on Fedora 29 (where the outer "make" sets
>       the new-style "--jobserver-auth" flag).
>     
>     - I've rebuilt all the edk2 binaries with this patch applied. Everything
>       works fine. However, if you test this patch, you might notice that git
>       reports all the build products as modified. That's because when using
>       the python3 code in edk2 BaseTools, the generated makefiles differ
>       greatly from the ones generated when running in python2 mode (e.g. due
>       to different random seeds in python hashes / dictionaries). As a
>       result, parts of the firmware volumes / firmware filesystems could
>       appear in a different order than before. This is harmless, and doesn't
>       necessitate checking in the rebuilt binaries.
> 
>  roms/Makefile                  |  1 +
>  tests/uefi-test-tools/Makefile |  1 +
>  roms/edk2-build.sh             |  4 ++--
>  roms/edk2-funcs.sh             | 17 +++++++++++++++++
>  tests/uefi-test-tools/build.sh |  6 ++++--
>  5 files changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/roms/Makefile b/roms/Makefile
> index 6cf07d3b44ee..06339efa6fc4 100644
> --- a/roms/Makefile
> +++ b/roms/Makefile
> @@ -141,6 +141,7 @@ build-efi-roms: build-pxe-roms
>  #
>  edk2-basetools:
>  	$(MAKE) -C edk2/BaseTools \
> +		PYTHON_COMMAND=$${EDK2_PYTHON_COMMAND:-python3} \
>  		EXTRA_OPTFLAGS='$(EDK2_BASETOOLS_OPTFLAGS)' \
>  		EXTRA_LDFLAGS='$(EDK2_BASETOOLS_LDFLAGS)'
>  
> diff --git a/tests/uefi-test-tools/Makefile b/tests/uefi-test-tools/Makefile
> index 7e0177d7337e..1dcddcdbbabf 100644
> --- a/tests/uefi-test-tools/Makefile
> +++ b/tests/uefi-test-tools/Makefile
> @@ -100,6 +100,7 @@ Build/bios-tables-test.%.efi: build-edk2-tools
>  
>  build-edk2-tools:
>  	$(MAKE) -C $(edk2_dir)/BaseTools \
> +		PYTHON_COMMAND=$${EDK2_PYTHON_COMMAND:-python3} \
>  		EXTRA_OPTFLAGS='$(EDK2_BASETOOLS_OPTFLAGS)' \
>  		EXTRA_LDFLAGS='$(EDK2_BASETOOLS_LDFLAGS)'
>  
> diff --git a/roms/edk2-build.sh b/roms/edk2-build.sh
> index 4f46f8a6a217..d5391c763728 100755
> --- a/roms/edk2-build.sh
> +++ b/roms/edk2-build.sh
> @@ -27,8 +27,7 @@ shift $num_args
>  
>  cd edk2
>  
> -# Work around <https://bugzilla.tianocore.org/show_bug.cgi?id=1607>.
> -export PYTHON_COMMAND=python2
> +export PYTHON_COMMAND=${EDK2_PYTHON_COMMAND:-python3}
>  
>  # Source "edksetup.sh" carefully.
>  set +e +u +C
> @@ -43,6 +42,7 @@ fi
>  # any), for the edk2 "build" utility.
>  source ../edk2-funcs.sh
>  edk2_toolchain=$(qemu_edk2_get_toolchain "$emulation_target")
> +MAKEFLAGS=$(qemu_edk2_quirk_tianocore_1607 "$MAKEFLAGS")
>  edk2_thread_count=$(qemu_edk2_get_thread_count "$MAKEFLAGS")
>  qemu_edk2_set_cross_env "$emulation_target"
>  
> diff --git a/roms/edk2-funcs.sh b/roms/edk2-funcs.sh
> index a9fae7ee891b..3f4485b201f1 100644
> --- a/roms/edk2-funcs.sh
> +++ b/roms/edk2-funcs.sh
> @@ -251,3 +251,20 @@ qemu_edk2_get_thread_count()
>      printf '1\n'
>    fi
>  }
> +
> +
> +# Work around <https://bugzilla.tianocore.org/show_bug.cgi?id=1607> by
> +# filtering jobserver-related flags out of MAKEFLAGS. Print the result to the
> +# standard output.
> +#
> +# Parameters:
> +#   $1: the value of the MAKEFLAGS variable
> +qemu_edk2_quirk_tianocore_1607()
> +{
> +  local makeflags="$1"
> +
> +  printf %s "$makeflags" \
> +  | LC_ALL=C sed --regexp-extended \
> +      --expression='s/--jobserver-(auth|fds)=[0-9]+,[0-9]+//' \
> +      --expression='s/-j([0-9]+)?//'
> +}
> diff --git a/tests/uefi-test-tools/build.sh b/tests/uefi-test-tools/build.sh
> index 8aa7935c43bb..3b78f3084069 100755
> --- a/tests/uefi-test-tools/build.sh
> +++ b/tests/uefi-test-tools/build.sh
> @@ -29,8 +29,7 @@ export PACKAGES_PATH=$(realpath -- "$edk2_dir")
>  export WORKSPACE=$PWD
>  mkdir -p Conf
>  
> -# Work around <https://bugzilla.tianocore.org/show_bug.cgi?id=1607>.
> -export PYTHON_COMMAND=python2
> +export PYTHON_COMMAND=${EDK2_PYTHON_COMMAND:-python3}
>  
>  # Source "edksetup.sh" carefully.
>  set +e +u +C
> @@ -46,12 +45,15 @@ fi
>  source "$edk2_dir/../edk2-funcs.sh"
>  edk2_arch=$(qemu_edk2_get_arch "$emulation_target")
>  edk2_toolchain=$(qemu_edk2_get_toolchain "$emulation_target")
> +MAKEFLAGS=$(qemu_edk2_quirk_tianocore_1607 "$MAKEFLAGS")
> +edk2_thread_count=$(qemu_edk2_get_thread_count "$MAKEFLAGS")
>  qemu_edk2_set_cross_env "$emulation_target"
>  
>  # Build the UEFI binary
>  mkdir -p log
>  build \
>    --arch="$edk2_arch" \
> +  -n "$edk2_thread_count" \
>    --buildtarget=DEBUG \
>    --platform=UefiTestToolsPkg/UefiTestToolsPkg.dsc \
>    --tagname="$edk2_toolchain" \
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH v2 1/2] edk2 build scripts: honor external BaseTools flags with uefi-test-tools

[Philippe Mathieu-Daudé]

On 9/20/19 10:38 AM, Laszlo Ersek wrote:
> Unify the recipe for "build-edk2-tools" in
> "tests/uefi-test-tools/Makefile" with the recipe for "edk2-basetools" in
> "roms/Makefile".
> 
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     v2:
>     - new patch
> 
>  tests/uefi-test-tools/Makefile | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/uefi-test-tools/Makefile b/tests/uefi-test-tools/Makefile
> index 1d78bc14d51a..7e0177d7337e 100644
> --- a/tests/uefi-test-tools/Makefile
> +++ b/tests/uefi-test-tools/Makefile
> @@ -99,7 +99,9 @@ Build/bios-tables-test.%.efi: build-edk2-tools
>  	+./build.sh $(edk2_dir) BiosTablesTest $* $@
>  
>  build-edk2-tools:
> -	$(MAKE) -C $(edk2_dir)/BaseTools
> +	$(MAKE) -C $(edk2_dir)/BaseTools \
> +		EXTRA_OPTFLAGS='$(EDK2_BASETOOLS_OPTFLAGS)' \
> +		EXTRA_LDFLAGS='$(EDK2_BASETOOLS_LDFLAGS)'
>  
>  clean:
>  	rm -rf Build Conf log
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH v2 0/2] edk2 build scripts: eliminate python 2 dependency

[John Snow]

On 9/20/19 4:38 AM, Laszlo Ersek wrote:
> v1 URL:
> http://mid.mail-archive.com/20190918171141.15957-1-lersek@redhat.com
> 
> Updates are noted on each patch.
> 
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: John Snow <jsnow@redhat.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Thanks
> Laszlo
> 
> Laszlo Ersek (2):
>   edk2 build scripts: honor external BaseTools flags with
>     uefi-test-tools
>   edk2 build scripts: work around TianoCore#1607 without forcing Python
>     2
> 
>  roms/Makefile                  |  1 +
>  roms/edk2-build.sh             |  4 ++--
>  roms/edk2-funcs.sh             | 17 +++++++++++++++++
>  tests/uefi-test-tools/Makefile |  5 ++++-
>  tests/uefi-test-tools/build.sh |  6 ++++--
>  5 files changed, 28 insertions(+), 5 deletions(-)
> 

Thank you again!

I'm sure we'll find all of the rough spots during the rc process, so I
am quite content for now.

Reviewed-by: John Snow <jsnow@redhat.com>

Re: [PATCH v2 0/2] edk2 build scripts: eliminate python 2 dependency

[Philippe Mathieu-Daudé]

On 9/20/19 6:06 PM, John Snow wrote:
> On 9/20/19 4:38 AM, Laszlo Ersek wrote:
>> v1 URL:
>> http://mid.mail-archive.com/20190918171141.15957-1-lersek@redhat.com
>>
>> Updates are noted on each patch.
>>
>> Cc: Eduardo Habkost <ehabkost@redhat.com>
>> Cc: John Snow <jsnow@redhat.com>
>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
>>
>> Thanks
>> Laszlo
>>
>> Laszlo Ersek (2):
>>   edk2 build scripts: honor external BaseTools flags with
>>     uefi-test-tools
>>   edk2 build scripts: work around TianoCore#1607 without forcing Python
>>     2
>>
>>  roms/Makefile                  |  1 +
>>  roms/edk2-build.sh             |  4 ++--
>>  roms/edk2-funcs.sh             | 17 +++++++++++++++++
>>  tests/uefi-test-tools/Makefile |  5 ++++-
>>  tests/uefi-test-tools/build.sh |  6 ++++--
>>  5 files changed, 28 insertions(+), 5 deletions(-)
>>
> 
> Thank you again!
> 
> I'm sure we'll find all of the rough spots during the rc process, so I
> am quite content for now.
> 
> Reviewed-by: John Snow <jsnow@redhat.com>
> 

Thanks, queued on edk2-next.