All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 0/3] core: fixup PPD paths (branch yem/ppd-fixup-paths)
@ 2022-01-08 17:35 Yann E. MORIN
  2022-01-08 17:35 ` [Buildroot] [PATCH 1/3] core/pkg-utils: properly indent per-package-rsync Yann E. MORIN
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Yann E. MORIN @ 2022-01-08 17:35 UTC (permalink / raw)
  To: buildroot
  Cc: Herve Codina, Andreas Naumann, Thomas Petazzoni,
	Louis-Paul CORDIER, Yann E . MORIN, Adam Duskett

Hello All!

This series builds upon previous good work from Andreas [0], Louis-Paul
[1], and Adam [2].

The second patch is the core of the change: rather than have multiple
hooks that each cater for different types of files (.py, .pc, .pri,
.cmake and so on), we jut do what we already do in relocate-sdk: we just
look for all text files that have a PPD prefix in them, and replace tht
with the cirrent package's PPD.

The third patch ensures that we can build reliable filrs lists, so that
files we tweak in the infra are not believed to be installed/modified by
the current package.

The first patch is just a little formatting cleanup.

[0] https://lore.kernel.org/buildroot/20200217212350.29750-21-anaumann@ultratronik.de/
[1] https://lore.kernel.org/buildroot/a339f273-33f3-f232-eac4-6e50427abf6d@cordier.org/
[2] https://lore.kernel.org/buildroot/20220106171720.12857-1-aduskett@gmail.com/

Regards,
Yann E. MORIN.


----------------------------------------------------------------
Yann E. MORIN (3):
      core/pkg-utils: properly indent per-package-rsync
      core/pkg-generic: fixup all PPD paths in a generic fashion
      core/pkg-generic: apply post-prepare hooks before monitoring directories

 package/pkg-generic.mk | 45 ++++++++++++++++++++++-----------------------
 package/pkg-utils.mk   |  4 ++--
 2 files changed, 24 insertions(+), 25 deletions(-)

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 1/3] core/pkg-utils: properly indent per-package-rsync
  2022-01-08 17:35 [Buildroot] [PATCH 0/3] core: fixup PPD paths (branch yem/ppd-fixup-paths) Yann E. MORIN
@ 2022-01-08 17:35 ` Yann E. MORIN
  2022-01-08 17:52   ` Peter Korsgaard
  2022-01-08 17:35 ` [Buildroot] [PATCH 2/3] core/pkg-generic: fixup all PPD paths in a generic fashion Yann E. MORIN
  2022-01-08 17:35 ` [Buildroot] [PATCH 3/3] core/pkg-generic: apply post-prepare hooks before monitoring directories Yann E. MORIN
  2 siblings, 1 reply; 10+ messages in thread
From: Yann E. MORIN @ 2022-01-08 17:35 UTC (permalink / raw)
  To: buildroot; +Cc: Yann E. MORIN

Continuation lines should be indented one level more than the line the
are the continuation of.

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
---
 package/pkg-utils.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
index 38ba5bca93..6aa64a2096 100644
--- a/package/pkg-utils.mk
+++ b/package/pkg-utils.mk
@@ -213,8 +213,8 @@ define per-package-rsync
 	mkdir -p $(3)
 	$(foreach pkg,$(1),\
 		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/$(2)/ \
-		$(PER_PACKAGE_DIR)/$(pkg)/$(2)/ \
-		$(3)$(sep))
+			$(PER_PACKAGE_DIR)/$(pkg)/$(2)/ \
+			$(3)
 endef
 
 # prepares the per-package HOST_DIR and TARGET_DIR of the current
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 2/3] core/pkg-generic: fixup all PPD paths in a generic fashion
  2022-01-08 17:35 [Buildroot] [PATCH 0/3] core: fixup PPD paths (branch yem/ppd-fixup-paths) Yann E. MORIN
  2022-01-08 17:35 ` [Buildroot] [PATCH 1/3] core/pkg-utils: properly indent per-package-rsync Yann E. MORIN
@ 2022-01-08 17:35 ` Yann E. MORIN
  2022-01-09 12:14   ` Herve Codina
  2022-01-12 20:42   ` Arnout Vandecappelle
  2022-01-08 17:35 ` [Buildroot] [PATCH 3/3] core/pkg-generic: apply post-prepare hooks before monitoring directories Yann E. MORIN
  2 siblings, 2 replies; 10+ messages in thread
From: Yann E. MORIN @ 2022-01-08 17:35 UTC (permalink / raw)
  To: buildroot
  Cc: Andreas Naumann, Thomas Petazzoni, Louis-Paul CORDIER,
	Yann E. MORIN, Adam Duskett

Some files contain hard-coded absolute paths that point to the host
and/or staging directories.

With per-package directories (aka. PPD), these paths point to the PPD
of the package that created the files, when we want them to point to the
PPD of the package that uses them.

Up until now, we had two hooks that attempted to fix those files:

  - a libtool-specific hook that searches for all .la files and seds
    them with the proper PPD,

  - a python-specific hook that tweaks just the sysconfigdata and
    removes the byte-compiled version of the sysconfigdata.

But now, we also have a few other kinds of files for which we need to
fix the PPD: .cmake, .pc, or .pri files, and probably a bunch of others
as well.

We solve this issue by just replacing any PPD in text files, with the
current package's PPD.

This is very similar to, and inspired from what is done when relocating
the SDK. However, we can't use the existing relocate-sdk script, because
that needs to know the original location, which we do not have when we
aggregate the PPD (we could store it, but we can easily do without it).

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Cc: Adam Duskett <aduskett@gmail.com>
Cc: Louis-Paul CORDIER <lpdev@cordier.org>
Cc: Andreas Naumann <anaumann@ultratronik.de>
---
 package/pkg-generic.mk | 43 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 6a5fe5507b..1022062bcf 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -90,21 +90,24 @@ endif
 #######################################
 # Helper functions
 
-# Make sure .la files only reference the current per-package
-# directory.
-
-# $1: package name (lower case)
-# $2: staging directory of the package
 ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
-define fixup-libtool-files
-	$(Q)find $(2) \( -path '$(2)/lib*' -o -path '$(2)/usr/lib*' \) \
-		-name "*.la" -print0 | xargs -0 --no-run-if-empty \
-		$(SED) "s:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$(1)/:g"
+
+# Ensure files like .la, .pc, .pri, .cmake, and so on, point to the
+# proper staging and host directories for the current package: find
+# all text files that contain the PPD root, and replace it with the
+# current package's PPD.
+define PPD_FIXUP_PATHS
+	$(Q)grep -lrZ '$(PER_PACKAGE_DIR)/[^/]\+/' $(HOST_DIR) \
+	|while read -d '' f; do \
+		file -b --mime-type "$${f}" | grep -q '^text/' || continue; \
+		printf '%s\0' "$${f}"; \
+	done \
+	|xargs -0 --no-run-if-empty \
+		$(SED) 's:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/:g'
 endef
-endif
 
-# Make sure python _sysconfigdata*.py files only reference the current
-# per-package directory.
+# Remove python's pre-compiled "sysconfigdata", as it may contain paths to
+# the original staging or host dirs.
 #
 # Can't use $(foreach d, $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*, ...)
 # because those directories may be created in the same recipe this macro will
@@ -113,19 +116,15 @@ endif
 # fail.
 # So we just use HOST_DIR as a starting point, and filter on the two directories
 # of interest.
-ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
 define FIXUP_PYTHON_SYSCONFIGDATA
 	$(Q)find $(HOST_DIR) \
 		\(    -path '$(HOST_DIR)/lib/python*' \
 		   -o -path '$(STAGING_DIR)/usr/lib/python*' \
 		\) \
-		\(    \( -name "_sysconfigdata*.pyc" -delete \) \
-		   -o \( -name "_sysconfigdata*.py" -print0 \) \
-		\) \
-	| xargs -0 --no-run-if-empty \
-		$(SED) 's:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/:g'
+		\( -name "_sysconfigdata*.pyc" -delete \)
 endef
-endif
+
+endif  # PPD
 
 # Functions to collect statistics about installed files
 
@@ -278,8 +277,6 @@ $(BUILD_DIR)/%/.stamp_configured:
 	@$(call pkg_size_before,$(STAGING_DIR),-staging)
 	@$(call pkg_size_before,$(BINARIES_DIR),-images)
 	@$(call pkg_size_before,$(HOST_DIR),-host)
-	$(call fixup-libtool-files,$(NAME),$(HOST_DIR))
-	$(call fixup-libtool-files,$(NAME),$(STAGING_DIR))
 	$(foreach hook,$($(PKG)_POST_PREPARE_HOOKS),$(call $(hook))$(sep))
 	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
 	$($(PKG)_CONFIGURE_CMDS)
@@ -836,7 +833,9 @@ $(2)_EXTRACT_CMDS ?= \
 		$$(TAR_OPTIONS) -)
 
 # pre/post-steps hooks
-$(2)_POST_PREPARE_HOOKS += FIXUP_PYTHON_SYSCONFIGDATA
+$(2)_POST_PREPARE_HOOKS += \
+	PPD_FIXUP_PATHS \
+	FIXUP_PYTHON_SYSCONFIGDATA
 
 ifeq ($$($(2)_TYPE),target)
 ifneq ($$(HOST_$(2)_KCONFIG_VAR),)
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 3/3] core/pkg-generic: apply post-prepare hooks before monitoring directories
  2022-01-08 17:35 [Buildroot] [PATCH 0/3] core: fixup PPD paths (branch yem/ppd-fixup-paths) Yann E. MORIN
  2022-01-08 17:35 ` [Buildroot] [PATCH 1/3] core/pkg-utils: properly indent per-package-rsync Yann E. MORIN
  2022-01-08 17:35 ` [Buildroot] [PATCH 2/3] core/pkg-generic: fixup all PPD paths in a generic fashion Yann E. MORIN
@ 2022-01-08 17:35 ` Yann E. MORIN
  2022-01-09 12:11   ` Herve Codina
  2 siblings, 1 reply; 10+ messages in thread
From: Yann E. MORIN @ 2022-01-08 17:35 UTC (permalink / raw)
  To: buildroot; +Cc: Herve Codina, Yann E. MORIN, Thomas Petazzoni

Monitoring the target/ and host/ directories and so on, will serve to
generate lists of files installed by the packages. Those lists are then
used to generate graphs of the size those package take on the target
for example.

With PPD, we will also want to use those lists to only copy those files
actually installed by each dependencies of a package, recursively.

Currently, those lists are not entirely reliable, as the starting points
are established before we apply PPD fixup hooks. As such, at the end of
a package installation, fixed up files will be found to belong to the
current package, while they were in fact provided by one of its
dependency.

While this does no big harm, if at all, for the size graphs, it will
trigger overwrite detection when we eventually gather packages together
to aggregate a PPD or te final host and target. So, we better have the
lists of files be reliable.

So, we only start monitoring the directories after we apply the PPD
fixups (or seen the other way around for a smaller diff: we apply the
PPD fixups before we start monitoring the directories).

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Herve Codina <herve.codina@bootlin.com>
---
 package/pkg-generic.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 1022062bcf..b1f4d219bb 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -273,11 +273,11 @@ $(BUILD_DIR)/%/.stamp_configured:
 	@$(call MESSAGE,"Configuring")
 	$(Q)mkdir -p $(HOST_DIR) $(TARGET_DIR) $(STAGING_DIR) $(BINARIES_DIR)
 	$(call prepare-per-package-directory,$($(PKG)_FINAL_DEPENDENCIES))
+	$(foreach hook,$($(PKG)_POST_PREPARE_HOOKS),$(call $(hook))$(sep))
 	@$(call pkg_size_before,$(TARGET_DIR))
 	@$(call pkg_size_before,$(STAGING_DIR),-staging)
 	@$(call pkg_size_before,$(BINARIES_DIR),-images)
 	@$(call pkg_size_before,$(HOST_DIR),-host)
-	$(foreach hook,$($(PKG)_POST_PREPARE_HOOKS),$(call $(hook))$(sep))
 	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
 	$($(PKG)_CONFIGURE_CMDS)
 	$(foreach hook,$($(PKG)_POST_CONFIGURE_HOOKS),$(call $(hook))$(sep))
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [Buildroot] [PATCH 1/3] core/pkg-utils: properly indent per-package-rsync
  2022-01-08 17:35 ` [Buildroot] [PATCH 1/3] core/pkg-utils: properly indent per-package-rsync Yann E. MORIN
@ 2022-01-08 17:52   ` Peter Korsgaard
  0 siblings, 0 replies; 10+ messages in thread
From: Peter Korsgaard @ 2022-01-08 17:52 UTC (permalink / raw)
  To: Yann E. MORIN; +Cc: buildroot

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > Continuation lines should be indented one level more than the line the
 > are the continuation of.

 > Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

Committed, thanks.

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Buildroot] [PATCH 3/3] core/pkg-generic: apply post-prepare hooks before monitoring directories
  2022-01-08 17:35 ` [Buildroot] [PATCH 3/3] core/pkg-generic: apply post-prepare hooks before monitoring directories Yann E. MORIN
@ 2022-01-09 12:11   ` Herve Codina
  0 siblings, 0 replies; 10+ messages in thread
From: Herve Codina @ 2022-01-09 12:11 UTC (permalink / raw)
  To: Yann E. MORIN; +Cc: Thomas Petazzoni, buildroot

On Sat,  8 Jan 2022 18:35:42 +0100
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> Monitoring the target/ and host/ directories and so on, will serve to
> generate lists of files installed by the packages. Those lists are then
> used to generate graphs of the size those package take on the target
> for example.
> 
> With PPD, we will also want to use those lists to only copy those files
> actually installed by each dependencies of a package, recursively.
> 
> Currently, those lists are not entirely reliable, as the starting points
> are established before we apply PPD fixup hooks. As such, at the end of
> a package installation, fixed up files will be found to belong to the
> current package, while they were in fact provided by one of its
> dependency.
> 
> While this does no big harm, if at all, for the size graphs, it will
> trigger overwrite detection when we eventually gather packages together
> to aggregate a PPD or te final host and target. So, we better have the
> lists of files be reliable.
> 
> So, we only start monitoring the directories after we apply the PPD
> fixups (or seen the other way around for a smaller diff: we apply the
> PPD fixups before we start monitoring the directories).
> 
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Herve Codina <herve.codina@bootlin.com>
> ---
>  package/pkg-generic.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 1022062bcf..b1f4d219bb 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -273,11 +273,11 @@ $(BUILD_DIR)/%/.stamp_configured:
>  	@$(call MESSAGE,"Configuring")
>  	$(Q)mkdir -p $(HOST_DIR) $(TARGET_DIR) $(STAGING_DIR) $(BINARIES_DIR)
>  	$(call prepare-per-package-directory,$($(PKG)_FINAL_DEPENDENCIES))
> +	$(foreach hook,$($(PKG)_POST_PREPARE_HOOKS),$(call $(hook))$(sep))
>  	@$(call pkg_size_before,$(TARGET_DIR))
>  	@$(call pkg_size_before,$(STAGING_DIR),-staging)
>  	@$(call pkg_size_before,$(BINARIES_DIR),-images)
>  	@$(call pkg_size_before,$(HOST_DIR),-host)
> -	$(foreach hook,$($(PKG)_POST_PREPARE_HOOKS),$(call $(hook))$(sep))
>  	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
>  	$($(PKG)_CONFIGURE_CMDS)
>  	$(foreach hook,$($(PKG)_POST_CONFIGURE_HOOKS),$(call $(hook))$(sep))

Acked-by: Herve Codina <herve.codina@bootlin.com>

Best regards,
Hervé

-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Buildroot] [PATCH 2/3] core/pkg-generic: fixup all PPD paths in a generic fashion
  2022-01-08 17:35 ` [Buildroot] [PATCH 2/3] core/pkg-generic: fixup all PPD paths in a generic fashion Yann E. MORIN
@ 2022-01-09 12:14   ` Herve Codina
  2022-01-12 20:42   ` Arnout Vandecappelle
  1 sibling, 0 replies; 10+ messages in thread
From: Herve Codina @ 2022-01-09 12:14 UTC (permalink / raw)
  To: Yann E. MORIN
  Cc: Louis-Paul CORDIER, Adam Duskett, Thomas Petazzoni,
	Andreas Naumann, buildroot

On Sat,  8 Jan 2022 18:35:41 +0100
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> Some files contain hard-coded absolute paths that point to the host
> and/or staging directories.
> 
> With per-package directories (aka. PPD), these paths point to the PPD
> of the package that created the files, when we want them to point to the
> PPD of the package that uses them.
> 
> Up until now, we had two hooks that attempted to fix those files:
> 
>   - a libtool-specific hook that searches for all .la files and seds
>     them with the proper PPD,
> 
>   - a python-specific hook that tweaks just the sysconfigdata and
>     removes the byte-compiled version of the sysconfigdata.
> 
> But now, we also have a few other kinds of files for which we need to
> fix the PPD: .cmake, .pc, or .pri files, and probably a bunch of others
> as well.
> 
> We solve this issue by just replacing any PPD in text files, with the
> current package's PPD.
> 
> This is very similar to, and inspired from what is done when relocating
> the SDK. However, we can't use the existing relocate-sdk script, because
> that needs to know the original location, which we do not have when we
> aggregate the PPD (we could store it, but we can easily do without it).
> 
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Cc: Adam Duskett <aduskett@gmail.com>
> Cc: Louis-Paul CORDIER <lpdev@cordier.org>
> Cc: Andreas Naumann <anaumann@ultratronik.de>
> ---
>  package/pkg-generic.mk | 43 +++++++++++++++++++++---------------------
>  1 file changed, 21 insertions(+), 22 deletions(-)
> 

[...]

Acked-by: Herve Codina <herve.codina@bootlin.com>

Best regards,
Hervé

-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Buildroot] [PATCH 2/3] core/pkg-generic: fixup all PPD paths in a generic fashion
  2022-01-08 17:35 ` [Buildroot] [PATCH 2/3] core/pkg-generic: fixup all PPD paths in a generic fashion Yann E. MORIN
  2022-01-09 12:14   ` Herve Codina
@ 2022-01-12 20:42   ` Arnout Vandecappelle
  2022-01-13 21:58     ` Yann E. MORIN
  1 sibling, 1 reply; 10+ messages in thread
From: Arnout Vandecappelle @ 2022-01-12 20:42 UTC (permalink / raw)
  To: Yann E. MORIN, buildroot
  Cc: Louis-Paul CORDIER, Andreas Naumann, Adam Duskett, Thomas Petazzoni



On 08/01/2022 18:35, Yann E. MORIN wrote:
> Some files contain hard-coded absolute paths that point to the host
> and/or staging directories.
> 
> With per-package directories (aka. PPD), these paths point to the PPD
> of the package that created the files, when we want them to point to the
> PPD of the package that uses them.
> 
> Up until now, we had two hooks that attempted to fix those files:
> 
>    - a libtool-specific hook that searches for all .la files and seds
>      them with the proper PPD,
> 
>    - a python-specific hook that tweaks just the sysconfigdata and
>      removes the byte-compiled version of the sysconfigdata.
> 
> But now, we also have a few other kinds of files for which we need to
> fix the PPD: .cmake, .pc, or .pri files, and probably a bunch of others
> as well.
> 
> We solve this issue by just replacing any PPD in text files, with the
> current package's PPD.
> 
> This is very similar to, and inspired from what is done when relocating
> the SDK. However, we can't use the existing relocate-sdk script, because
> that needs to know the original location, which we do not have when we
> aggregate the PPD (we could store it, but we can easily do without it).

  Looking at the resulting code, it's so small that it's not worth factoring out.

> 
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Cc: Adam Duskett <aduskett@gmail.com>
> Cc: Louis-Paul CORDIER <lpdev@cordier.org>
> Cc: Andreas Naumann <anaumann@ultratronik.de>
> ---
>   package/pkg-generic.mk | 43 +++++++++++++++++++++---------------------
>   1 file changed, 21 insertions(+), 22 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 6a5fe5507b..1022062bcf 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -90,21 +90,24 @@ endif
>   #######################################
>   # Helper functions
>   
> -# Make sure .la files only reference the current per-package
> -# directory.
> -
> -# $1: package name (lower case)
> -# $2: staging directory of the package
>   ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
> -define fixup-libtool-files
> -	$(Q)find $(2) \( -path '$(2)/lib*' -o -path '$(2)/usr/lib*' \) \
> -		-name "*.la" -print0 | xargs -0 --no-run-if-empty \
> -		$(SED) "s:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$(1)/:g"
> +
> +# Ensure files like .la, .pc, .pri, .cmake, and so on, point to the
> +# proper staging and host directories for the current package: find
> +# all text files that contain the PPD root, and replace it with the
> +# current package's PPD.
> +define PPD_FIXUP_PATHS
> +	$(Q)grep -lrZ '$(PER_PACKAGE_DIR)/[^/]\+/' $(HOST_DIR) \

  This will trawl to large binary files and potentially take a long time...

> +	|while read -d '' f; do \
> +		file -b --mime-type "$${f}" | grep -q '^text/' || continue; \

... just to be ignored here.

  More importantly though: if a file is a symlink, it's going to be hit twice. 
Worse, if it's a symlink to an absolute path (which most likely points *outside* 
of STAGING_DIR), we may end up sed'ing something on the host...

  I notice now that the same (theoretical) issue exists in relocate-sdk.sh. 
Obviously that script doesn't get thoroughly tested so it may very well be the 
wrong thing to do...

  Do you remember perhaps why we don't simply do

find $(HOST_DIR) -type f -print0 \

? I was going to say that we can skip the grep, but then we're back at the large 
(text) file thing.

> +		printf '%s\0' "$${f}"; \

  Why not do the sed right here, like is done in relocate-sdk.sh? In fact, I'd 
keep the code as close as possible to relocate-sdk.sh to make later refactoring 
easier.

> +	done \
> +	|xargs -0 --no-run-if-empty \
> +		$(SED) 's:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/:g'
>   endef
> -endif
>   
> -# Make sure python _sysconfigdata*.py files only reference the current
> -# per-package directory.
> +# Remove python's pre-compiled "sysconfigdata", as it may contain paths to
> +# the original staging or host dirs.
>   #
>   # Can't use $(foreach d, $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*, ...)
>   # because those directories may be created in the same recipe this macro will
> @@ -113,19 +116,15 @@ endif
>   # fail.
>   # So we just use HOST_DIR as a starting point, and filter on the two directories
>   # of interest.
> -ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
>   define FIXUP_PYTHON_SYSCONFIGDATA

  Maybe rename to REMOVE_PYTHON_SYSCONFIGATA_PYC

  Regards,
  Arnout

>   	$(Q)find $(HOST_DIR) \
>   		\(    -path '$(HOST_DIR)/lib/python*' \
>   		   -o -path '$(STAGING_DIR)/usr/lib/python*' \
>   		\) \
> -		\(    \( -name "_sysconfigdata*.pyc" -delete \) \
> -		   -o \( -name "_sysconfigdata*.py" -print0 \) \
> -		\) \
> -	| xargs -0 --no-run-if-empty \
> -		$(SED) 's:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/:g'
> +		\( -name "_sysconfigdata*.pyc" -delete \)
>   endef
> -endif
> +
> +endif  # PPD
>   
>   # Functions to collect statistics about installed files
>   
> @@ -278,8 +277,6 @@ $(BUILD_DIR)/%/.stamp_configured:
>   	@$(call pkg_size_before,$(STAGING_DIR),-staging)
>   	@$(call pkg_size_before,$(BINARIES_DIR),-images)
>   	@$(call pkg_size_before,$(HOST_DIR),-host)
> -	$(call fixup-libtool-files,$(NAME),$(HOST_DIR))
> -	$(call fixup-libtool-files,$(NAME),$(STAGING_DIR))
>   	$(foreach hook,$($(PKG)_POST_PREPARE_HOOKS),$(call $(hook))$(sep))
>   	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
>   	$($(PKG)_CONFIGURE_CMDS)
> @@ -836,7 +833,9 @@ $(2)_EXTRACT_CMDS ?= \
>   		$$(TAR_OPTIONS) -)
>   
>   # pre/post-steps hooks
> -$(2)_POST_PREPARE_HOOKS += FIXUP_PYTHON_SYSCONFIGDATA
> +$(2)_POST_PREPARE_HOOKS += \
> +	PPD_FIXUP_PATHS \
> +	FIXUP_PYTHON_SYSCONFIGDATA
>   
>   ifeq ($$($(2)_TYPE),target)
>   ifneq ($$(HOST_$(2)_KCONFIG_VAR),)
> 
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Buildroot] [PATCH 2/3] core/pkg-generic: fixup all PPD paths in a generic fashion
  2022-01-12 20:42   ` Arnout Vandecappelle
@ 2022-01-13 21:58     ` Yann E. MORIN
  2022-01-22 10:50       ` Yann E. MORIN
  0 siblings, 1 reply; 10+ messages in thread
From: Yann E. MORIN @ 2022-01-13 21:58 UTC (permalink / raw)
  To: Arnout Vandecappelle
  Cc: Andreas Naumann, Thomas Petazzoni, buildroot, Louis-Paul CORDIER,
	Adam Duskett

Arnout, All,

On 2022-01-12 21:42 +0100, Arnout Vandecappelle spake thusly:
> On 08/01/2022 18:35, Yann E. MORIN wrote:
> >Some files contain hard-coded absolute paths that point to the host
> >and/or staging directories.
> >
> >With per-package directories (aka. PPD), these paths point to the PPD
> >of the package that created the files, when we want them to point to the
> >PPD of the package that uses them.
[--SNIP--]
> >+define PPD_FIXUP_PATHS
> >+	$(Q)grep -lrZ '$(PER_PACKAGE_DIR)/[^/]\+/' $(HOST_DIR) \
>  This will trawl to large binary files and potentially take a long time...

I too thought of that, and I even discussed it slightly with Peter K.
here about what would be most efficient: grep-then-file, or file-then-grep?

But if we add --binary-files=without-match to grep, then it will
bail-out early on binary files, which would be more efficient. And then
we still pass that through file to really get only text files.

But a text-file with utf8 is really a binary file, so would grep ignore
it?

> >+	|while read -d '' f; do \
> >+		file -b --mime-type "$${f}" | grep -q '^text/' || continue; \
> ... just to be ignored here.

Again, not sure, because text/plain but utf8-encoded?

>  More importantly though: if a file is a symlink, it's going to be hit
> twice.

No, I don't think so:

    $ man grep
    [..]
    -r, --recursive
        Read all files under each directory, recursively, following
        symbolic links only if they are on the command line. [...]

And that seems to be the case:

    $ echo Pouet >foo
    $ ln -s foo bar
    $ grep -r Pouet .
    foo:Pouet

So, seems OK to me?

> Worse, if it's a symlink to an absolute path (which most likely
> points *outside* of STAGING_DIR), we may end up sed'ing something on the
> host...
>  I notice now that the same (theoretical) issue exists in relocate-sdk.sh.
> Obviously that script doesn't get thoroughly tested so it may very well be
> the wrong thing to do...
> 
>  Do you remember perhaps why we don't simply do
> 
> find $(HOST_DIR) -type f -print0 \

Maybe we should. But grep -r was an elegant way to combine the recursive
feature of find, and limit to the matching files at the same time...

I can try to time things to see what's the fastest / less-slow... But
given that the symlinks are not an issue in practice, so we care?

> ? I was going to say that we can skip the grep, but then we're back at the
> large (text) file thing.
> 
> >+		printf '%s\0' "$${f}"; \
> 
>  Why not do the sed right here, like is done in relocate-sdk.sh? In fact,
> I'd keep the code as close as possible to relocate-sdk.sh to make later
> refactoring easier.

Using xarg allows to spawn only a few sed. printf is a shell built-in so
it's basically free.

[--SNIP--]
> >  define FIXUP_PYTHON_SYSCONFIGDATA
>  Maybe rename to REMOVE_PYTHON_SYSCONFIGATA_PYC

OK

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Buildroot] [PATCH 2/3] core/pkg-generic: fixup all PPD paths in a generic fashion
  2022-01-13 21:58     ` Yann E. MORIN
@ 2022-01-22 10:50       ` Yann E. MORIN
  0 siblings, 0 replies; 10+ messages in thread
From: Yann E. MORIN @ 2022-01-22 10:50 UTC (permalink / raw)
  To: Arnout Vandecappelle
  Cc: Louis-Paul CORDIER, Adam Duskett, Thomas Petazzoni,
	Andreas Naumann, buildroot

Arnout, All,

On 2022-01-13 22:58 +0100, Yann E. MORIN spake thusly:
> On 2022-01-12 21:42 +0100, Arnout Vandecappelle spake thusly:
> > On 08/01/2022 18:35, Yann E. MORIN wrote:
> > >Some files contain hard-coded absolute paths that point to the host
> > >and/or staging directories.
> > >
> > >With per-package directories (aka. PPD), these paths point to the PPD
> > >of the package that created the files, when we want them to point to the
> > >PPD of the package that uses them.
> [--SNIP--]
> > >+define PPD_FIXUP_PATHS
> > >+	$(Q)grep -lrZ '$(PER_PACKAGE_DIR)/[^/]\+/' $(HOST_DIR) \
> >  This will trawl to large binary files and potentially take a long time...
> 
> I too thought of that, and I even discussed it slightly with Peter K.
> here about what would be most efficient: grep-then-file, or file-then-grep?
> 
> But if we add --binary-files=without-match to grep, then it will
> bail-out early on binary files, which would be more efficient. And then
> we still pass that through file to really get only text files.
> 
> But a text-file with utf8 is really a binary file, so would grep ignore
> it?

So I tried and created a simple file with arbitrary non-ASCII UTF-8
symbols (I just copy-pasted the chinese Wikipedia home page, arbitarily
wrapped for "readability"):

    $ cat foo
    英格兰足球超级联赛金手套奖是英格兰足球超级联赛的一个奖项,
    每年颁发一次,颁给在当赛季英超联赛中完成最多次零封的守门员。
    在足球中,零封指的是守门员在一整场比赛中保持不失球。2005年,
    英超联赛第一次颁发金手套奖,切尔西的彼得·切赫获奖,切赫在
    2004-05赛季完成了24场零封,至今仍是英超联赛的记录。切赫和乔·
    哈特各赢得过四次金手套奖,并列获该奖次数最多的球员,切赫还是
    至今唯一一位为两家俱乐部(切尔西、阿森纳)效力时都获得金手套
    奖的球员。佩佩·雷纳则是第一个能够连续获得该奖的球员,他在2006
    至2008年连续三次获得英超金手套。乔·哈特则在2011-2013年追平了
    这一成就。

    $ file -b --mime-type foo
    text/plain

    $ grep --binary-files=without-match 是英超 foo
    2004-05赛季完成了24场零封,至今仍是英超联赛的记录。切赫和乔·

So, a text file with utf-8 chars is still recognied as a text file,
and grep will also not recognise it as a binary file.

So we can tell grep to bail out early on binary files, to speed up the
stuff. I'd still keep the 'file' test as a further safety net, though.

So, grep does not follow symlinks, and we can can speed grep by
instructing it to skip binary files. As such, I think all your issues
have been addressed now.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-01-22 10:50 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-08 17:35 [Buildroot] [PATCH 0/3] core: fixup PPD paths (branch yem/ppd-fixup-paths) Yann E. MORIN
2022-01-08 17:35 ` [Buildroot] [PATCH 1/3] core/pkg-utils: properly indent per-package-rsync Yann E. MORIN
2022-01-08 17:52   ` Peter Korsgaard
2022-01-08 17:35 ` [Buildroot] [PATCH 2/3] core/pkg-generic: fixup all PPD paths in a generic fashion Yann E. MORIN
2022-01-09 12:14   ` Herve Codina
2022-01-12 20:42   ` Arnout Vandecappelle
2022-01-13 21:58     ` Yann E. MORIN
2022-01-22 10:50       ` Yann E. MORIN
2022-01-08 17:35 ` [Buildroot] [PATCH 3/3] core/pkg-generic: apply post-prepare hooks before monitoring directories Yann E. MORIN
2022-01-09 12:11   ` Herve Codina

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.