All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] core/pkg-infra: tweak only .la files that need it
@ 2019-02-10  8:48 Yann E. MORIN
  2019-02-10 19:23 ` Thomas De Schampheleire
  0 siblings, 1 reply; 6+ messages in thread
From: Yann E. MORIN @ 2019-02-10  8:48 UTC (permalink / raw)
  To: buildroot

Currently, when we tweak the .la files, we do so unconditionally on all
.la files, even those we already fixed in a previous run.

This has the nasty side effect that each .la file will be reported as
being touched by all packages that are installed after the package that
actually installed said .la file.

Since we can't easily know what fiels were isntalled by a package (that
is an instrumetnation hook, and comes after the mangling), we use a
trick (like is done in libtool?): we do mangle all files, each in into a
temporary location; if the result is identical to the input, we remove
the temporary, while if the resiult differes from the output, we move
the temporary to the input.

Reported-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
 package/pkg-generic.mk | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 6168b40e89..afd0d41e2a 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -294,7 +294,8 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
 				$(addprefix $(STAGING_DIR)/usr/bin/,$($(PKG)_CONFIG_SCRIPTS)) ;\
 	fi
 	@$(call MESSAGE,"Fixing libtool files")
-	$(Q)find $(STAGING_DIR)/usr/lib* -name "*.la" | xargs --no-run-if-empty \
+	$(Q)for la in $(find $(STAGING_DIR)/usr/lib* -name "*.la"); do \
+		cp -a "$${la}" "$${la}.fixed" && \
 		$(SED) "s:$(BASE_DIR):@BASE_DIR@:g" \
 			-e "s:$(STAGING_DIR):@STAGING_DIR@:g" \
 			$(if $(TOOLCHAIN_EXTERNAL_INSTALL_DIR),\
@@ -303,7 +304,14 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
 			$(if $(TOOLCHAIN_EXTERNAL_INSTALL_DIR),\
 				-e "s:@TOOLCHAIN_EXTERNAL_INSTALL_DIR@:$(TOOLCHAIN_EXTERNAL_INSTALL_DIR):g") \
 			-e "s:@STAGING_DIR@:$(STAGING_DIR):g" \
-			-e "s:@BASE_DIR@:$(BASE_DIR):g"
+			-e "s:@BASE_DIR@:$(BASE_DIR):g"; \
+			"$${la}.fixed" && \
+		if cmp "$${la}" "$${la}.fixed"; then \
+			rm -f "$${la}.fixed"; \
+		else \
+			mv "$${la}.fixed" "$${la}"; \
+		fi || exit 1; \
+	done
 	@$(call step_end,install-staging)
 	$(Q)touch $@
 
-- 
2.14.1

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

* [Buildroot] [PATCH] core/pkg-infra: tweak only .la files that need it
  2019-02-10  8:48 Yann E. MORIN
@ 2019-02-10 19:23 ` Thomas De Schampheleire
  2019-02-10 19:57   ` Yann E. MORIN
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas De Schampheleire @ 2019-02-10 19:23 UTC (permalink / raw)
  To: buildroot

Hi Yann,

El dom., 10 feb. 2019 a las 9:48, Yann E. MORIN
(<yann.morin.1998@free.fr>) escribi?:
>
> Currently, when we tweak the .la files, we do so unconditionally on all
> .la files, even those we already fixed in a previous run.
>
> This has the nasty side effect that each .la file will be reported as
> being touched by all packages that are installed after the package that
> actually installed said .la file.
>
> Since we can't easily know what fiels were isntalled by a package (that

files
installed

> is an instrumetnation hook, and comes after the mangling), we use a

instrumentation

> trick (like is done in libtool?): we do mangle all files, each in into a

/in/d

> temporary location; if the result is identical to the input, we remove
> the temporary, while if the resiult differes from the output, we move

result differs

> the temporary to the input.
>
> Reported-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
> Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> ---
>  package/pkg-generic.mk | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 6168b40e89..afd0d41e2a 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -294,7 +294,8 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
>                                 $(addprefix $(STAGING_DIR)/usr/bin/,$($(PKG)_CONFIG_SCRIPTS)) ;\
>         fi
>         @$(call MESSAGE,"Fixing libtool files")
> -       $(Q)find $(STAGING_DIR)/usr/lib* -name "*.la" | xargs --no-run-if-empty \
> +       $(Q)for la in $(find $(STAGING_DIR)/usr/lib* -name "*.la"); do \
> +               cp -a "$${la}" "$${la}.fixed" && \
>                 $(SED) "s:$(BASE_DIR):@BASE_DIR@:g" \
>                         -e "s:$(STAGING_DIR):@STAGING_DIR@:g" \
>                         $(if $(TOOLCHAIN_EXTERNAL_INSTALL_DIR),\
> @@ -303,7 +304,14 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
>                         $(if $(TOOLCHAIN_EXTERNAL_INSTALL_DIR),\
>                                 -e "s:@TOOLCHAIN_EXTERNAL_INSTALL_DIR@:$(TOOLCHAIN_EXTERNAL_INSTALL_DIR):g") \
>                         -e "s:@STAGING_DIR@:$(STAGING_DIR):g" \
> -                       -e "s:@BASE_DIR@:$(BASE_DIR):g"
> +                       -e "s:@BASE_DIR@:$(BASE_DIR):g"; \

The semicolon shouldn't be here, right? I.e. the "$${la}.fixed" below
is the argument to the sed command, no?

> +                       "$${la}.fixed" && \
> +               if cmp "$${la}" "$${la}.fixed"; then \
> +                       rm -f "$${la}.fixed"; \
> +               else \
> +                       mv "$${la}.fixed" "$${la}"; \
> +               fi || exit 1; \
> +       done
>         @$(call step_end,install-staging)
>         $(Q)touch $@
>

I did not test it but it looks a valid approach.

Best regards,
Thomas

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

* [Buildroot] [PATCH] core/pkg-infra: tweak only .la files that need it
  2019-02-10 19:23 ` Thomas De Schampheleire
@ 2019-02-10 19:57   ` Yann E. MORIN
  0 siblings, 0 replies; 6+ messages in thread
From: Yann E. MORIN @ 2019-02-10 19:57 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2019-02-10 20:23 +0100, Thomas De Schampheleire spake thusly:
> El dom., 10 feb. 2019 a las 9:48, Yann E. MORIN
> (<yann.morin.1998@free.fr>) escribi?:
> >
> > Currently, when we tweak the .la files, we do so unconditionally on all
> > .la files, even those we already fixed in a previous run.
> >
> > This has the nasty side effect that each .la file will be reported as
> > being touched by all packages that are installed after the package that
> > actually installed said .la file.
> >
> > Since we can't easily know what fiels were isntalled by a package (that
> 
> files
> installed
> 
> > is an instrumetnation hook, and comes after the mangling), we use a
> 
> instrumentation
> 
> > trick (like is done in libtool?): we do mangle all files, each in into a
> 
> /in/d
> 
> > temporary location; if the result is identical to the input, we remove
> > the temporary, while if the resiult differes from the output, we move
> 
> result differs

OK, so it looks like my new glasses are not up to the task of fixing
my eyesight, since I could not spot any of those mistakes when
proof-reading myself...

Or maybe it is not my eyes that are deffective, in fact?

> > the temporary to the input.
> >
> > Reported-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
> > Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> > Cc: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
> > Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> > Cc: Arnout Vandecappelle <arnout@mind.be>
> > ---
> >  package/pkg-generic.mk | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> > index 6168b40e89..afd0d41e2a 100644
> > --- a/package/pkg-generic.mk
> > +++ b/package/pkg-generic.mk
> > @@ -294,7 +294,8 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
> >                                 $(addprefix $(STAGING_DIR)/usr/bin/,$($(PKG)_CONFIG_SCRIPTS)) ;\
> >         fi
> >         @$(call MESSAGE,"Fixing libtool files")
> > -       $(Q)find $(STAGING_DIR)/usr/lib* -name "*.la" | xargs --no-run-if-empty \
> > +       $(Q)for la in $(find $(STAGING_DIR)/usr/lib* -name "*.la"); do \

Meh... The real culp[rit for the borked patch is that $(find ...) is not
the same as $$(find ...) here... So the loop is never, ever entered, and
the issue below...

> > +               cp -a "$${la}" "$${la}.fixed" && \
> >                 $(SED) "s:$(BASE_DIR):@BASE_DIR@:g" \
> >                         -e "s:$(STAGING_DIR):@STAGING_DIR@:g" \
> >                         $(if $(TOOLCHAIN_EXTERNAL_INSTALL_DIR),\
> > @@ -303,7 +304,14 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
> >                         $(if $(TOOLCHAIN_EXTERNAL_INSTALL_DIR),\
> >                                 -e "s:@TOOLCHAIN_EXTERNAL_INSTALL_DIR@:$(TOOLCHAIN_EXTERNAL_INSTALL_DIR):g") \
> >                         -e "s:@STAGING_DIR@:$(STAGING_DIR):g" \
> > -                       -e "s:@BASE_DIR@:$(BASE_DIR):g"
> > +                       -e "s:@BASE_DIR@:$(BASE_DIR):g"; \
> 
> The semicolon shouldn't be here, right? I.e. the "$${la}.fixed" below
> is the argument to the sed command, no?

... is not noticed.

Regards,
Yann E. MORIN.

> > +                       "$${la}.fixed" && \
> > +               if cmp "$${la}" "$${la}.fixed"; then \
> > +                       rm -f "$${la}.fixed"; \
> > +               else \
> > +                       mv "$${la}.fixed" "$${la}"; \
> > +               fi || exit 1; \
> > +       done
> >         @$(call step_end,install-staging)
> >         $(Q)touch $@
> >
> 
> I did not test it but it looks a valid approach.
> 
> Best regards,
> Thomas

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH] core/pkg-infra: tweak only .la files that need it
@ 2019-02-10 21:38 Yann E. MORIN
  2019-03-17 15:47 ` Thomas Petazzoni
  2019-03-27 14:28 ` Peter Korsgaard
  0 siblings, 2 replies; 6+ messages in thread
From: Yann E. MORIN @ 2019-02-10 21:38 UTC (permalink / raw)
  To: buildroot

Currently, when we tweak the .la files, we do so unconditionally on all
.la files, even those we already fixed in a previous run.

This has the nasty side effect that each .la file will be reported as
being touched by all packages that are installed after the package that
actually installed said .la file.

Since we can't easily know what files were installed by a package (that
is an instrumentation hook, and comes after the mangling), we use a
trick (like is done in libtool?): we do mangle all files, each into a
temporary location; if the result is identical to the input, we remove
the temporary, while if the result differs from the output, we move
the temporary to replace the input.

Reported-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>

---
Changes v1 -> v2:
  - drop spurious trailing semicolon on sed command line  (Thomas DS.)
  - use $$(find ...) instead of $(find ...), as we don't want it to be
    evaluated by the Makefile, but by the shell
  - silence the output of cmp
  - typoes in commit log  (Thomas DS.)
---
 package/pkg-generic.mk | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 6168b40e89..c97ce36b9d 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -294,7 +294,8 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
 				$(addprefix $(STAGING_DIR)/usr/bin/,$($(PKG)_CONFIG_SCRIPTS)) ;\
 	fi
 	@$(call MESSAGE,"Fixing libtool files")
-	$(Q)find $(STAGING_DIR)/usr/lib* -name "*.la" | xargs --no-run-if-empty \
+	for la in $$(find $(STAGING_DIR)/usr/lib* -name "*.la"); do \
+		cp -a "$${la}" "$${la}.fixed" && \
 		$(SED) "s:$(BASE_DIR):@BASE_DIR@:g" \
 			-e "s:$(STAGING_DIR):@STAGING_DIR@:g" \
 			$(if $(TOOLCHAIN_EXTERNAL_INSTALL_DIR),\
@@ -303,7 +304,14 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
 			$(if $(TOOLCHAIN_EXTERNAL_INSTALL_DIR),\
 				-e "s:@TOOLCHAIN_EXTERNAL_INSTALL_DIR@:$(TOOLCHAIN_EXTERNAL_INSTALL_DIR):g") \
 			-e "s:@STAGING_DIR@:$(STAGING_DIR):g" \
-			-e "s:@BASE_DIR@:$(BASE_DIR):g"
+			-e "s:@BASE_DIR@:$(BASE_DIR):g" \
+			"$${la}.fixed" && \
+		if cmp -s "$${la}" "$${la}.fixed"; then \
+			rm -f "$${la}.fixed"; \
+		else \
+			mv "$${la}.fixed" "$${la}"; \
+		fi || exit 1; \
+	done
 	@$(call step_end,install-staging)
 	$(Q)touch $@
 
-- 
2.14.1

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

* [Buildroot] [PATCH] core/pkg-infra: tweak only .la files that need it
  2019-02-10 21:38 [Buildroot] [PATCH] core/pkg-infra: tweak only .la files that need it Yann E. MORIN
@ 2019-03-17 15:47 ` Thomas Petazzoni
  2019-03-27 14:28 ` Peter Korsgaard
  1 sibling, 0 replies; 6+ messages in thread
From: Thomas Petazzoni @ 2019-03-17 15:47 UTC (permalink / raw)
  To: buildroot

Hello,

On Sun, 10 Feb 2019 22:38:18 +0100
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> Currently, when we tweak the .la files, we do so unconditionally on all
> .la files, even those we already fixed in a previous run.
> 
> This has the nasty side effect that each .la file will be reported as
> being touched by all packages that are installed after the package that
> actually installed said .la file.
> 
> Since we can't easily know what files were installed by a package (that
> is an instrumentation hook, and comes after the mangling), we use a
> trick (like is done in libtool?): we do mangle all files, each into a
> temporary location; if the result is identical to the input, we remove
> the temporary, while if the result differs from the output, we move
> the temporary to replace the input.
> 
> Reported-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
> Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> 
> ---
> Changes v1 -> v2:
>   - drop spurious trailing semicolon on sed command line  (Thomas DS.)
>   - use $$(find ...) instead of $(find ...), as we don't want it to be
>     evaluated by the Makefile, but by the shell
>   - silence the output of cmp
>   - typoes in commit log  (Thomas DS.)
> ---
>  package/pkg-generic.mk | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)

Applied to master, thanks. Long term, it would be nicer if we could
leverage the list of files installed by the package, in order to only
tweak the files that really need to be tweaked. But that requires more
work in the common infrastructure.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

* [Buildroot] [PATCH] core/pkg-infra: tweak only .la files that need it
  2019-02-10 21:38 [Buildroot] [PATCH] core/pkg-infra: tweak only .la files that need it Yann E. MORIN
  2019-03-17 15:47 ` Thomas Petazzoni
@ 2019-03-27 14:28 ` Peter Korsgaard
  1 sibling, 0 replies; 6+ messages in thread
From: Peter Korsgaard @ 2019-03-27 14:28 UTC (permalink / raw)
  To: buildroot

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

 > Currently, when we tweak the .la files, we do so unconditionally on all
 > .la files, even those we already fixed in a previous run.

 > This has the nasty side effect that each .la file will be reported as
 > being touched by all packages that are installed after the package that
 > actually installed said .la file.

 > Since we can't easily know what files were installed by a package (that
 > is an instrumentation hook, and comes after the mangling), we use a
 > trick (like is done in libtool?): we do mangle all files, each into a
 > temporary location; if the result is identical to the input, we remove
 > the temporary, while if the result differs from the output, we move
 > the temporary to replace the input.

 > Reported-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
 > Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
 > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
 > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
 > Cc: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
 > Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
 > Cc: Arnout Vandecappelle <arnout@mind.be>

 > ---
 > Changes v1 -> v2:
 >   - drop spurious trailing semicolon on sed command line  (Thomas DS.)
 >   - use $$(find ...) instead of $(find ...), as we don't want it to be
 >     evaluated by the Makefile, but by the shell
 >   - silence the output of cmp
 >   - typoes in commit log  (Thomas DS.)

Committed to 2019.02.x, thanks.

-- 
Bye, Peter Korsgaard

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

end of thread, other threads:[~2019-03-27 14:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-10 21:38 [Buildroot] [PATCH] core/pkg-infra: tweak only .la files that need it Yann E. MORIN
2019-03-17 15:47 ` Thomas Petazzoni
2019-03-27 14:28 ` Peter Korsgaard
  -- strict thread matches above, loose matches on Subject: below --
2019-02-10  8:48 Yann E. MORIN
2019-02-10 19:23 ` Thomas De Schampheleire
2019-02-10 19:57   ` Yann E. MORIN

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.