diff mbox

infra: fix autotools .la fixing when no .la files are present

Message ID 297a83abd0584662acb7.1404656584@localhost
State Superseded
Headers show

Commit Message

Thomas De Schampheleire July 6, 2014, 2:23 p.m. UTC
Commit 6f638ea9c9c7bacc646430a74ad3dd2740eae676 ("infra: /usr support:
STAGING_DIR can be outside BASE_DIR") changed a for-construct around 'sed
-i' in a 'find | xargs sed -i'. However, if the find returns no results,
sed complains with:
    '/bin/sed: no input files'

Since a for construct is not very nice to handle this case, this commit
changes the 'find | xargs sed -i' into 'find -exec sed -i'.

Reported-by: Rohit Kumar <rkthebest@gmail.com>
Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

---
 package/pkg-autotools.mk |  8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

Comments

Yann E. MORIN July 6, 2014, 2:41 p.m. UTC | #1
Thomas, All,

On 2014-07-06 16:23 +0200, Thomas De Schampheleire spake thusly:
> Commit 6f638ea9c9c7bacc646430a74ad3dd2740eae676 ("infra: /usr support:
> STAGING_DIR can be outside BASE_DIR") changed a for-construct around 'sed
> -i' in a 'find | xargs sed -i'. However, if the find returns no results,
> sed complains with:
>     '/bin/sed: no input files'
> 
> Since a for construct is not very nice to handle this case, this commit
> changes the 'find | xargs sed -i' into 'find -exec sed -i'.

Why not use: 'xargs -r' instead?

    --no-run-if-empty
    -r     If the standard input does not contain any nonblanks, do
           not run the command.  Normally, the command is run once
           even if there is no input. This option is a GNU extension.

Using 'find -exec' is OK, but it spawns a process for each result. I
doubt we end up in a case with trillions of results, so the overhead
should not be noticeable.

Note that it is possible to have 'find' behave like xargs, with a
construct like:
    find DIR PREDICATES -exec command {} +

    -exec command {} +
           This  variant  of the -exec action runs the specified command on
           the selected files, but the command line is built  by  appending
           each  selected file name at the end; the total number of invoca‐
           tions of the command will  be  much  less  than  the  number  of
           matched  files.   The command line is built in much the same way
           that xargs builds its command lines.  Only one instance of  `{}'
           is  allowed  within the command.  The command is executed in the
           starting directory.

Regards,
Yann E. MORIN.

> Reported-by: Rohit Kumar <rkthebest@gmail.com>
> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
> 
> ---
>  package/pkg-autotools.mk |  8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff -r c8ed6565ea09 -r 297a83abd058 package/pkg-autotools.mk
> --- a/package/pkg-autotools.mk	Thu Jul 03 12:35:57 2014 +0300
> +++ b/package/pkg-autotools.mk	Sun Jul 06 16:19:04 2014 +0200
> @@ -277,16 +277,20 @@
>  # of these directories as @BASE_DIR@ and @STAGING_DIR@. Note that
>  # STAGING_DIR can be outside BASE_DIR when the user sets BR2_HOST_DIR
>  # to a custom value.
> +# We use 'find ... -exec sed' instead of 'find ... | xargs sed' because
> +# find may return no results and 'sed -i' requires at least one input
> +# file.
>  #
>  ifndef $(2)_INSTALL_STAGING_CMDS
>  define $(2)_INSTALL_STAGING_CMDS
>  	$$(TARGET_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_INSTALL_STAGING_OPT) -C $$($$(PKG)_SRCDIR)
> -	find $$(STAGING_DIR)/usr/lib* -name "*.la" | xargs \
> +	find $$(STAGING_DIR)/usr/lib* -name "*.la" -exec \
>  		$$(SED) "s:$$(BASE_DIR):@BASE_DIR@:g" \
>  			-e "s:$$(STAGING_DIR):@STAGING_DIR@:g" \
>  			-e "s:\(['= ]\)/usr:\\1@STAGING_DIR@/usr:g" \
>  			-e "s:@STAGING_DIR@:$$(STAGING_DIR):g" \
> -			-e "s:@BASE_DIR@:$$(BASE_DIR):g"
> +			-e "s:@BASE_DIR@:$$(BASE_DIR):g" \
> +		{} \;
>  endef
>  endif
>  
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Thomas De Schampheleire July 6, 2014, 2:59 p.m. UTC | #2
"Yann E. MORIN" <yann.morin.1998@free.fr> schreef:
>Thomas, All,
>
>On 2014-07-06 16:23 +0200, Thomas De Schampheleire spake thusly:
>> Commit 6f638ea9c9c7bacc646430a74ad3dd2740eae676 ("infra: /usr support:
>> STAGING_DIR can be outside BASE_DIR") changed a for-construct around 'sed
>> -i' in a 'find | xargs sed -i'. However, if the find returns no results,
>> sed complains with:
>>     '/bin/sed: no input files'
>> 
>> Since a for construct is not very nice to handle this case, this commit
>> changes the 'find | xargs sed -i' into 'find -exec sed -i'.
>
>Why not use: 'xargs -r' instead?
>
>    --no-run-if-empty
>    -r     If the standard input does not contain any nonblanks, do
>           not run the command.  Normally, the command is run once
>           even if there is no input. This option is a GNU extension.
>
>Using 'find -exec' is OK, but it spawns a process for each result. I
>doubt we end up in a case with trillions of results, so the overhead
>should not be noticeable.

Aha, I didn't know this option. I did know about the overhead difference, but at least the -exec should be on par with the for loop.

I'll resubmit with this -r instead, thanks!

Best regards,
Thomas
diff mbox

Patch

diff -r c8ed6565ea09 -r 297a83abd058 package/pkg-autotools.mk
--- a/package/pkg-autotools.mk	Thu Jul 03 12:35:57 2014 +0300
+++ b/package/pkg-autotools.mk	Sun Jul 06 16:19:04 2014 +0200
@@ -277,16 +277,20 @@ 
 # of these directories as @BASE_DIR@ and @STAGING_DIR@. Note that
 # STAGING_DIR can be outside BASE_DIR when the user sets BR2_HOST_DIR
 # to a custom value.
+# We use 'find ... -exec sed' instead of 'find ... | xargs sed' because
+# find may return no results and 'sed -i' requires at least one input
+# file.
 #
 ifndef $(2)_INSTALL_STAGING_CMDS
 define $(2)_INSTALL_STAGING_CMDS
 	$$(TARGET_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_INSTALL_STAGING_OPT) -C $$($$(PKG)_SRCDIR)
-	find $$(STAGING_DIR)/usr/lib* -name "*.la" | xargs \
+	find $$(STAGING_DIR)/usr/lib* -name "*.la" -exec \
 		$$(SED) "s:$$(BASE_DIR):@BASE_DIR@:g" \
 			-e "s:$$(STAGING_DIR):@STAGING_DIR@:g" \
 			-e "s:\(['= ]\)/usr:\\1@STAGING_DIR@/usr:g" \
 			-e "s:@STAGING_DIR@:$$(STAGING_DIR):g" \
-			-e "s:@BASE_DIR@:$$(BASE_DIR):g"
+			-e "s:@BASE_DIR@:$$(BASE_DIR):g" \
+		{} \;
 endef
 endif