[Buildroot] [PATCH 1/1] Makefile: fix SDK relocation for per-package-dirs

[Buildroot] [PATCH 1/1] Makefile: fix SDK relocation for per-package-dirs

[Brandon Maier via buildroot]

The relocate-sdk.sh script does not work correctly when
BR2_PER_PACKAGE_DIRECTORIES is enabled. relocate-sdk.sh expects
everything to point at $HOST_DIR, but each package will be pointing at
its $(O)/per-package/*/host.

Before packing up the SDK, scrub the HOST_DIR to replace all matches of
the per-package directory to point at the final HOST_DIR.

Signed-off-by: Brandon Maier <brandon.maier@collins.com>
---
 DEVELOPERS                 |  1 +
 Makefile                   |  1 +
 support/scripts/fix-perpkg | 31 +++++++++++++++++++++++++++++++
 3 files changed, 33 insertions(+)
 create mode 100755 support/scripts/fix-perpkg

diff --git a/DEVELOPERS b/DEVELOPERS
index 81e6cd54ab..238f8754e3 100644
--- a/DEVELOPERS
+++ b/DEVELOPERS
@@ -488,6 +488,7 @@ F:	package/ncdu/
 
 N:	Brandon Maier <brandon.maier@collins.com>
 F:	package/vmtouch/
+F:	support/scripts/fix-perpkg
 
 N:	Brock Williams <brock@cottonwoodcomputer.com>
 F:	package/pdmenu/
diff --git a/Makefile b/Makefile
index 7c1c07a2e4..929042396a 100644
--- a/Makefile
+++ b/Makefile
@@ -595,6 +595,7 @@ prepare-sdk: world
 	@$(call MESSAGE,"Rendering the SDK relocatable")
 	PER_PACKAGE_DIR=$(PER_PACKAGE_DIR) $(TOPDIR)/support/scripts/fix-rpath host
 	PER_PACKAGE_DIR=$(PER_PACKAGE_DIR) $(TOPDIR)/support/scripts/fix-rpath staging
+	PER_PACKAGE_DIR=$(PER_PACKAGE_DIR) $(TOPDIR)/support/scripts/fix-perpkg
 	$(INSTALL) -m 755 $(TOPDIR)/support/misc/relocate-sdk.sh $(HOST_DIR)/relocate-sdk.sh
 	mkdir -p $(HOST_DIR)/share/buildroot
 	echo $(HOST_DIR) > $(HOST_DIR)/share/buildroot/sdk-location
diff --git a/support/scripts/fix-perpkg b/support/scripts/fix-perpkg
new file mode 100755
index 0000000000..56d7d7a45d
--- /dev/null
+++ b/support/scripts/fix-perpkg
@@ -0,0 +1,31 @@
+#!/usr/bin/env bash
+
+usage() {
+	cat <<-EOF >&2
+	Usage: fix-perpkg
+
+	This script rewrites file's PER_PACKAGE_DIR to point at the HOST_DIR
+	before they are bundled into an SDK. So that relocate-sdk.sh can find
+	and relocate them.
+	EOF
+	exit 1
+}
+
+if [ "$#" -gt 1 ]; then
+	usage
+fi
+
+# Check if per-package-directories is disabled
+if ! [ -e "$PER_PACKAGE_DIR" ]; then
+	exit
+fi
+
+# Make sure `file` uses the right language
+export LC_ALL=C
+
+# Replace the old path with the new one in all text files
+grep -lr "${PER_PACKAGE_DIR}/[-_a-zA-Z0-9]\+/host" "${HOST_DIR}" \
+	| file --mime-type -f - \
+	| grep ': \+text/[-_a-zA-Z0-9]*$' \
+	| sed 's|: \+text/[-_a-zA-Z0-9]*$||' \
+	| xargs -r sed -i "s|${PER_PACKAGE_DIR}/[-_a-zA-Z0-9]\+/host|${HOST_DIR}|g"
-- 
2.38.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] Makefile: fix SDK relocation for per-package-dirs

[Arnout Vandecappelle]

  Hi Brandon,

  I was going to apply, but I have a few comments/improvements that I prefer if 
you bring them to completion.

On 10/11/2022 19:47, Brandon Maier via buildroot wrote:
> The relocate-sdk.sh script does not work correctly when
> BR2_PER_PACKAGE_DIRECTORIES is enabled. relocate-sdk.sh expects
> everything to point at $HOST_DIR, but each package will be pointing at
> its $(O)/per-package/*/host.
> 
> Before packing up the SDK, scrub the HOST_DIR to replace all matches of
> the per-package directory to point at the final HOST_DIR.
> 
> Signed-off-by: Brandon Maier <brandon.maier@collins.com>
> ---
>   DEVELOPERS                 |  1 +
>   Makefile                   |  1 +
>   support/scripts/fix-perpkg | 31 +++++++++++++++++++++++++++++++
>   3 files changed, 33 insertions(+)
>   create mode 100755 support/scripts/fix-perpkg
> 
> diff --git a/DEVELOPERS b/DEVELOPERS
> index 81e6cd54ab..238f8754e3 100644
> --- a/DEVELOPERS
> +++ b/DEVELOPERS
> @@ -488,6 +488,7 @@ F:	package/ncdu/
>   
>   N:	Brandon Maier <brandon.maier@collins.com>
>   F:	package/vmtouch/
> +F:	support/scripts/fix-perpkg
>   
>   N:	Brock Williams <brock@cottonwoodcomputer.com>
>   F:	package/pdmenu/
> diff --git a/Makefile b/Makefile
> index 7c1c07a2e4..929042396a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -595,6 +595,7 @@ prepare-sdk: world
>   	@$(call MESSAGE,"Rendering the SDK relocatable")
>   	PER_PACKAGE_DIR=$(PER_PACKAGE_DIR) $(TOPDIR)/support/scripts/fix-rpath host
>   	PER_PACKAGE_DIR=$(PER_PACKAGE_DIR) $(TOPDIR)/support/scripts/fix-rpath staging
> +	PER_PACKAGE_DIR=$(PER_PACKAGE_DIR) $(TOPDIR)/support/scripts/fix-perpkg

  I'm not sure if it's really useful to make a separate script out of this - 
it's just a simple pipeline after all.

>   	$(INSTALL) -m 755 $(TOPDIR)/support/misc/relocate-sdk.sh $(HOST_DIR)/relocate-sdk.sh
>   	mkdir -p $(HOST_DIR)/share/buildroot
>   	echo $(HOST_DIR) > $(HOST_DIR)/share/buildroot/sdk-location
> diff --git a/support/scripts/fix-perpkg b/support/scripts/fix-perpkg
> new file mode 100755
> index 0000000000..56d7d7a45d
> --- /dev/null
> +++ b/support/scripts/fix-perpkg
> @@ -0,0 +1,31 @@
> +#!/usr/bin/env bash
> +
> +usage() {
> +	cat <<-EOF >&2
> +	Usage: fix-perpkg
> +
> +	This script rewrites file's PER_PACKAGE_DIR to point at the HOST_DIR
> +	before they are bundled into an SDK. So that relocate-sdk.sh can find
> +	and relocate them.
> +	EOF
> +	exit 1
> +}
> +
> +if [ "$#" -gt 1 ]; then
> +	usage
> +fi
> +
> +# Check if per-package-directories is disabled
> +if ! [ -e "$PER_PACKAGE_DIR" ]; then

  PER_PACKAGE_DIR is always set, even if per-package-directories is disabled, so 
the comment is wrong. I anyway don't think it's necessary to check for this - if 
PPD is disabled, then there will never be a match.

> +	exit
> +fi
> +
> +# Make sure `file` uses the right language
> +export LC_ALL=C
> +
> +# Replace the old path with the new one in all text files
> +grep -lr "${PER_PACKAGE_DIR}/[-_a-zA-Z0-9]\+/host" "${HOST_DIR}" \

  Check what is used in PPD_FIXUP_PATHS:

grep --binary-files=without-match -lr '$(PER_PACKAGE_DIR)/[^/]\+/' $(HOST_DIR)

  The --binary-files=without-match speeds things up a lot because it will 
immediately skip large binary files.

  PPD_FIXUP_PATHS also uses a shell loop instead of xargs; not sure which one is 
better.

> +	| file --mime-type -f - \
> +	| grep ': \+text/[-_a-zA-Z0-9]*$' \
> +	| sed 's|: \+text/[-_a-zA-Z0-9]*$||' \

  You can merge those two in a single

	| sed -n '/\(.*\): \+text\/[-_a-zA-Z0-9]*$/s//\1/p' \

> +	| xargs -r sed -i "s|${PER_PACKAGE_DIR}/[-_a-zA-Z0-9]\+/host|${HOST_DIR}|g"

  There is no constraint on package names containing only those symbols, so 
better use [^/]\+ instead.


  However, I think it would be better to refactor fix-perpkg against 
PPD_FIXUP_PATHS. It only differens in the replacement part of the final sed, so 
that can easily be done with a make function.

  Regards,
  Arnout
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [External] Re: [PATCH 1/1] Makefile: fix SDK relocation for per-package-dirs

[Maier, Brandon L Collins via buildroot]

> From: Arnout Vandecappelle <arnout@mind.be>
> On 10/11/2022 19:47, Brandon Maier via buildroot wrote:
> > +# Replace the old path with the new one in all text files
> > +grep -lr "${PER_PACKAGE_DIR}/[-_a-zA-Z0-9]\+/host" "${HOST_DIR}" \
> 
>   Check what is used in PPD_FIXUP_PATHS:
> 
> grep --binary-files=without-match -lr '$(PER_PACKAGE_DIR)/[^/]\+/'
> $(HOST_DIR)
> 
>   The --binary-files=without-match speeds things up a lot because it will
> immediately skip large binary files.
> 
>   PPD_FIXUP_PATHS also uses a shell loop instead of xargs; not sure which
> one is
> better.
 
Before switching to --binary-files, the shell for-loop was much slower closer to
a minute for me. With --binary-files it's about 5 seconds, which is closer to the
xargs which is about 1 second. So I don't mind just using the PPD_FIXUP_PATHS
code.

> 
>   However, I think it would be better to refactor fix-perpkg against
> PPD_FIXUP_PATHS. It only differens in the replacement part of the final sed,
> so
> that can easily be done with a make function.

Agreed with all comments. And reusing PPD_FIXUP_PATHS will address them.
Will send updated patch shortly.

Thanks,
Brandon

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot