Pkg xen devel - Jan 2021 - [PATCH 04/16] debian/rules: Correct shim install step for current Xen

When originally implemented, the separated shim install step relied on
the shim install being a NOP on shimless architectures.  Either this is
no longer the case, or else cross-building confuses the architecture
detection.

Take out a typo while at it.

Signed-off-by: Elliott Mitchell <ehem+debian at m5p.com>
---
 debian/rules | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/debian/rules b/debian/rules
index 60d99e6dc2..0a7fcc9553 100755
--- a/debian/rules
+++ b/debian/rules
@@ -251,11 +251,13 @@ override_dh_auto_install: $(TEMPLATED_FILES)
 	:
 	@# shim install target needs to be run separately because we
 	@# need to pass it the make_args_xen settings, in particular
-	@# on i386 bwe need to pass x86_64 here to actually build it.
-	@# Luckily this target, unlike the build, is a noop on
-	@# shimless arches, so it does not need to be conditional.
-	$(MAKE) $(make_args_xen) DESTDIR=$t $(make_args_xen) \
-		-C tools/firmware install-shim
+	@# on i386 we need to pass x86_64 here to actually build it.
+	case $(flavour) in \
+	amd64|i386) \
+		$(MAKE) $(make_args_xen) DESTDIR=$t $(make_args_xen) \
+			-C tools/firmware install-shim \
+	;; \
+	esac
 	:
 	@# Inexplicably, upstream puts the efi binares in usr/lib64
 	find "$t"/usr/lib*/efi -mindepth 1 -maxdepth 1 -print0 | xargs -r -0
mv -t "$t/boot"
-- 


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg at m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445

Ian,

Can you take a look at this? There are too many intricacies involved in
the shim story for me to figure out what's actually happening.

Because, when trying to get 4.14 in, I ended up doing the following
(from d/changelog):

    "* Revert upstream commit a516bddbd3 ("tools/firmware/Makefile:
CONFIG_PV_SHIM: enable only on x86_64") and cherry-pick our previous
commits 0b898ccc2 ("tools/firmware/Makfile: Respect caller's
CONFIG_PV_SHIM") and a516bddbd3 ("tools/firmware/Makefile:
CONFIG_PV_SHIM: enable only on x86_64") again to work around a FTBFS
where the shim would not be built during the i386 package build."

This was a workaround to at least get it going, but it should be
improved, to have something that DTRT based on the new upstream change
a516bddbd3.

Elliot, did you test/use this based on a package that does or does not
have the above mentioned workaround?

Thanks,

Hans

On 7/17/20 7:16 AM, Elliott Mitchell wrote:
> When originally implemented, the separated shim install step relied on
> the shim install being a NOP on shimless architectures.  Either this is
> no longer the case, or else cross-building confuses the architecture
> detection.
> 
> Take out a typo while at it.
> 
> Signed-off-by: Elliott Mitchell <ehem+debian at m5p.com>
> ---
>  debian/rules | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/debian/rules b/debian/rules
> index 60d99e6dc2..0a7fcc9553 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -251,11 +251,13 @@ override_dh_auto_install: $(TEMPLATED_FILES)
>  	:
>  	@# shim install target needs to be run separately because we
>  	@# need to pass it the make_args_xen settings, in particular
> -	@# on i386 bwe need to pass x86_64 here to actually build it.
> -	@# Luckily this target, unlike the build, is a noop on
> -	@# shimless arches, so it does not need to be conditional.
> -	$(MAKE) $(make_args_xen) DESTDIR=$t $(make_args_xen) \
> -		-C tools/firmware install-shim
> +	@# on i386 we need to pass x86_64 here to actually build it.
> +	case $(flavour) in \
> +	amd64|i386) \
> +		$(MAKE) $(make_args_xen) DESTDIR=$t $(make_args_xen) \
> +			-C tools/firmware install-shim \
> +	;; \
> +	esac
>  	:
>  	@# Inexplicably, upstream puts the efi binares in usr/lib64
>  	find "$t"/usr/lib*/efi -mindepth 1 -maxdepth 1 -print0 | xargs
-r -0 mv -t "$t/boot"
>

On 7/17/20 7:16 AM, Elliott Mitchell wrote:
> When originally implemented, the separated shim install step relied on
> the shim install being a NOP on shimless architectures.  Either this is
> no longer the case, or else cross-building confuses the architecture
> detection.
This does not tell what the problem was you were running into, and how
this this change helps to correct that problem.

For the normal builds, I don't see any difference.

Can you rewrite the message into something that explains why the old
lines do a wrong thing when cross-building, and how the new lines
correct that problem? 'Something getting confused' is not a sufficiently
clear explanation.
> Take out a typo while at it.
> 
> Signed-off-by: Elliott Mitchell <ehem+debian at m5p.com>
> ---
>  debian/rules | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/debian/rules b/debian/rules
> index 60d99e6dc2..0a7fcc9553 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -251,11 +251,13 @@ override_dh_auto_install: $(TEMPLATED_FILES)
>  	:
>  	@# shim install target needs to be run separately because we
>  	@# need to pass it the make_args_xen settings, in particular
> -	@# on i386 bwe need to pass x86_64 here to actually build it.
> -	@# Luckily this target, unlike the build, is a noop on
> -	@# shimless arches, so it does not need to be conditional.
> -	$(MAKE) $(make_args_xen) DESTDIR=$t $(make_args_xen) \
> -		-C tools/firmware install-shim
> +	@# on i386 we need to pass x86_64 here to actually build it.
> +	case $(flavour) in \
> +	amd64|i386) \
> +		$(MAKE) $(make_args_xen) DESTDIR=$t $(make_args_xen) \
> +			-C tools/firmware install-shim \
> +	;; \
> +	esac
>  	:
>  	@# Inexplicably, upstream puts the efi binares in usr/lib64
>  	find "$t"/usr/lib*/efi -mindepth 1 -maxdepth 1 -print0 | xargs
-r -0 mv -t "$t/boot"
>