All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 0/1] fs: fix /tmp and /run clean-up in POSIX shells
@ 2022-01-24 10:41 Evgeni Dobrev via buildroot
  2022-01-24 10:41 ` [Buildroot] [PATCH 1/1] " Evgeni Dobrev via buildroot
  0 siblings, 1 reply; 5+ messages in thread
From: Evgeni Dobrev via buildroot @ 2022-01-24 10:41 UTC (permalink / raw)
  To: buildroot; +Cc: Evgeni Dobrev

Hi,

the fakeroot script uses [^c] to request the deletion of all hidden
files within the tmp and run folders. This however does not work under
dash (or other POSIX shells) without additional configuration.

The proposed patch fixes the clean-up actions introduced with
5e78e7e97d772124f0d31653aca33731abe5b4db .

Regards,

Evgeni Dobrev (1):
  fs: fix /tmp and /run clean-up in POSIX shells

 fs/common.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.34.1

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

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

* [Buildroot] [PATCH 1/1] fs: fix /tmp and /run clean-up in POSIX shells
  2022-01-24 10:41 [Buildroot] [PATCH 0/1] fs: fix /tmp and /run clean-up in POSIX shells Evgeni Dobrev via buildroot
@ 2022-01-24 10:41 ` Evgeni Dobrev via buildroot
  2022-01-26 22:17   ` Yann E. MORIN
  2022-07-27 15:46   ` Yann E. MORIN
  0 siblings, 2 replies; 5+ messages in thread
From: Evgeni Dobrev via buildroot @ 2022-01-24 10:41 UTC (permalink / raw)
  To: buildroot; +Cc: Evgeni Dobrev

In POSIX sh, ^ in place of ! in glob bracket expressions
is undefined.

Signed-off-by: Evgeni Dobrev <evgeni@studio-punkt.com>
---
 fs/common.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/common.mk b/fs/common.mk
index 45beb5ae7b..64a94d9ad8 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -186,7 +186,7 @@ $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): $$(ROOTFS_$(2)_DEPENDENCIES)
 
 	$$(foreach hook,$$(ROOTFS_$(2)_PRE_GEN_HOOKS),\
 		$$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT)$$(sep))
-	echo "rm -rf $$(TARGET_DIR)/run/* $$(TARGET_DIR)/run/.[^.]* $$(TARGET_DIR)/tmp/* $$(TARGET_DIR)/tmp/.[^.]*" >> $$(FAKEROOT_SCRIPT)
+	echo "rm -rf $$(TARGET_DIR)/run/* $$(TARGET_DIR)/run/.[!.]* $$(TARGET_DIR)/tmp/* $$(TARGET_DIR)/tmp/.[!.]*" >> $$(FAKEROOT_SCRIPT)
 	$$(call PRINTF,$$(ROOTFS_REPRODUCIBLE)) >> $$(FAKEROOT_SCRIPT)
 	$$(call PRINTF,$$(ROOTFS_SELINUX)) >> $$(FAKEROOT_SCRIPT)
 	$$(call PRINTF,$$(ROOTFS_$(2)_CMD)) >> $$(FAKEROOT_SCRIPT)
-- 
2.34.1

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

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

* Re: [Buildroot] [PATCH 1/1] fs: fix /tmp and /run clean-up in POSIX shells
  2022-01-24 10:41 ` [Buildroot] [PATCH 1/1] " Evgeni Dobrev via buildroot
@ 2022-01-26 22:17   ` Yann E. MORIN
  2022-01-27 10:34     ` Evgeni Dobrev via buildroot
  2022-07-27 15:46   ` Yann E. MORIN
  1 sibling, 1 reply; 5+ messages in thread
From: Yann E. MORIN @ 2022-01-26 22:17 UTC (permalink / raw)
  To: Evgeni Dobrev; +Cc: buildroot

Evgeni, All,

On 2022-01-24 11:41 +0100, Evgeni Dobrev via buildroot spake thusly:
> In POSIX sh, ^ in place of ! in glob bracket expressions
> is undefined.
> 
> Signed-off-by: Evgeni Dobrev <evgeni@studio-punkt.com>
> ---
>  fs/common.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/common.mk b/fs/common.mk
> index 45beb5ae7b..64a94d9ad8 100644
> --- a/fs/common.mk
> +++ b/fs/common.mk
> @@ -186,7 +186,7 @@ $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): $$(ROOTFS_$(2)_DEPENDENCIES)
>  
>  	$$(foreach hook,$$(ROOTFS_$(2)_PRE_GEN_HOOKS),\
>  		$$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT)$$(sep))
> -	echo "rm -rf $$(TARGET_DIR)/run/* $$(TARGET_DIR)/run/.[^.]* $$(TARGET_DIR)/tmp/* $$(TARGET_DIR)/tmp/.[^.]*" >> $$(FAKEROOT_SCRIPT)
> +	echo "rm -rf $$(TARGET_DIR)/run/* $$(TARGET_DIR)/run/.[!.]* $$(TARGET_DIR)/tmp/* $$(TARGET_DIR)/tmp/.[!.]*" >> $$(FAKEROOT_SCRIPT)

TIL that bracket expressions are actually valid and specified by POSIX.

But we do guarantee that we do have bash as a shell when running
Buildroot, so I think it is fair to also run the fs script with bash:


    diff --git a/fs/common.mk b/fs/common.mk
    index 45beb5ae7b..f8a6da6bc4 100644
    --- a/fs/common.mk
    +++ b/fs/common.mk
    @@ -172,7 +172,7 @@ $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): $$(ROOTFS_$(2)_DEPENDENCIES)
     		$$(BASE_TARGET_DIR)/ \
     		$$(TARGET_DIR)
     
    -	echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT)
    +	echo '#!$(SHELL)' > $$(FAKEROOT_SCRIPT)
     	echo "set -e" >> $$(FAKEROOT_SCRIPT)
     
     	echo "chown -h -R 0:0 $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)

However, $(SHELL) is not always correct, so we'd probably need:

    diff --git a/Makefile b/Makefile
    index 5fc7137e5a..bb9955f5b6 100644
    --- a/Makefile
    +++ b/Makefile
    @@ -30,7 +30,7 @@
     # we want bash as shell
     SHELL := $(shell if [ -x "$$BASH" ]; then echo $$BASH; \
     	 else if [ -x /bin/bash ]; then echo /bin/bash; \
    -	 else echo sh; fi; fi)
    +	 else which sh; fi; fi)
     
     # Set O variable if not already done on the command line;
     # or avoid confusing packages that can use the O=<dir> syntax for out-of-tree

But in the end this is quite more changes than what you propose.

Can we also shorten the command?

    echo "rm -rf $$(TARGET_DIR)/{run,tmp}/* $$(TARGET_DIR)/{run,tmp}/.[!.]*" >> $$(FAKEROOT_SCRIPT)

Regards,
Yann E. MORIN.

>  	$$(call PRINTF,$$(ROOTFS_REPRODUCIBLE)) >> $$(FAKEROOT_SCRIPT)
>  	$$(call PRINTF,$$(ROOTFS_SELINUX)) >> $$(FAKEROOT_SCRIPT)
>  	$$(call PRINTF,$$(ROOTFS_$(2)_CMD)) >> $$(FAKEROOT_SCRIPT)
> -- 
> 2.34.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] 5+ messages in thread

* Re: [Buildroot] [PATCH 1/1] fs: fix /tmp and /run clean-up in POSIX shells
  2022-01-26 22:17   ` Yann E. MORIN
@ 2022-01-27 10:34     ` Evgeni Dobrev via buildroot
  0 siblings, 0 replies; 5+ messages in thread
From: Evgeni Dobrev via buildroot @ 2022-01-27 10:34 UTC (permalink / raw)
  To: Yann E. MORIN; +Cc: buildroot

Hi Yann,

indeed the support for [^] vs [!] is more complicated than I thought
initially ( see "Rationale" under https://github.com/koalaman/shellcheck/wiki/SC3026 ).

Would the following be a bad idea:

echo "find $$(TARGET_DIR)/run $$(TARGET_DIR)/tmp -mindepth 1 -delete" >> $$(FAKEROOT_SCRIPT)

- it avoids the glob support complication
- it is shorter

Regards,

Evgeni



On Wed, Jan 26, 2022 at 11:17:08PM +0100, Yann E. MORIN wrote:
> Evgeni, All,
> 
> On 2022-01-24 11:41 +0100, Evgeni Dobrev via buildroot spake thusly:
> > In POSIX sh, ^ in place of ! in glob bracket expressions
> > is undefined.
> > 
> > Signed-off-by: Evgeni Dobrev <evgeni@studio-punkt.com>
> > ---
> >  fs/common.mk | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/common.mk b/fs/common.mk
> > index 45beb5ae7b..64a94d9ad8 100644
> > --- a/fs/common.mk
> > +++ b/fs/common.mk
> > @@ -186,7 +186,7 @@ $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): $$(ROOTFS_$(2)_DEPENDENCIES)
> >  
> >  	$$(foreach hook,$$(ROOTFS_$(2)_PRE_GEN_HOOKS),\
> >  		$$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT)$$(sep))
> > -	echo "rm -rf $$(TARGET_DIR)/run/* $$(TARGET_DIR)/run/.[^.]* $$(TARGET_DIR)/tmp/* $$(TARGET_DIR)/tmp/.[^.]*" >> $$(FAKEROOT_SCRIPT)
> > +	echo "rm -rf $$(TARGET_DIR)/run/* $$(TARGET_DIR)/run/.[!.]* $$(TARGET_DIR)/tmp/* $$(TARGET_DIR)/tmp/.[!.]*" >> $$(FAKEROOT_SCRIPT)
> 
> TIL that bracket expressions are actually valid and specified by POSIX.
> 
> But we do guarantee that we do have bash as a shell when running
> Buildroot, so I think it is fair to also run the fs script with bash:
> 
> 
>     diff --git a/fs/common.mk b/fs/common.mk
>     index 45beb5ae7b..f8a6da6bc4 100644
>     --- a/fs/common.mk
>     +++ b/fs/common.mk
>     @@ -172,7 +172,7 @@ $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): $$(ROOTFS_$(2)_DEPENDENCIES)
>      		$$(BASE_TARGET_DIR)/ \
>      		$$(TARGET_DIR)
>      
>     -	echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT)
>     +	echo '#!$(SHELL)' > $$(FAKEROOT_SCRIPT)
>      	echo "set -e" >> $$(FAKEROOT_SCRIPT)
>      
>      	echo "chown -h -R 0:0 $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)
> 
> However, $(SHELL) is not always correct, so we'd probably need:
> 
>     diff --git a/Makefile b/Makefile
>     index 5fc7137e5a..bb9955f5b6 100644
>     --- a/Makefile
>     +++ b/Makefile
>     @@ -30,7 +30,7 @@
>      # we want bash as shell
>      SHELL := $(shell if [ -x "$$BASH" ]; then echo $$BASH; \
>      	 else if [ -x /bin/bash ]; then echo /bin/bash; \
>     -	 else echo sh; fi; fi)
>     +	 else which sh; fi; fi)
>      
>      # Set O variable if not already done on the command line;
>      # or avoid confusing packages that can use the O=<dir> syntax for out-of-tree
> 
> But in the end this is quite more changes than what you propose.
> 
> Can we also shorten the command?
> 
>     echo "rm -rf $$(TARGET_DIR)/{run,tmp}/* $$(TARGET_DIR)/{run,tmp}/.[!.]*" >> $$(FAKEROOT_SCRIPT)
> 
> Regards,
> Yann E. MORIN.
> 
> >  	$$(call PRINTF,$$(ROOTFS_REPRODUCIBLE)) >> $$(FAKEROOT_SCRIPT)
> >  	$$(call PRINTF,$$(ROOTFS_SELINUX)) >> $$(FAKEROOT_SCRIPT)
> >  	$$(call PRINTF,$$(ROOTFS_$(2)_CMD)) >> $$(FAKEROOT_SCRIPT)
> > -- 
> > 2.34.1
> > 
> > _______________________________________________
> > buildroot mailing list
> > buildroot@buildroot.org
> > https://lists.buildroot.org/mailman/listinfo/buildroot
> 
> -- 
> .-----------------.--------------------.------------------.--------------------.
> |  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
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

* Re: [Buildroot] [PATCH 1/1] fs: fix /tmp and /run clean-up in POSIX shells
  2022-01-24 10:41 ` [Buildroot] [PATCH 1/1] " Evgeni Dobrev via buildroot
  2022-01-26 22:17   ` Yann E. MORIN
@ 2022-07-27 15:46   ` Yann E. MORIN
  1 sibling, 0 replies; 5+ messages in thread
From: Yann E. MORIN @ 2022-07-27 15:46 UTC (permalink / raw)
  To: Evgeni Dobrev; +Cc: buildroot

Evgeni, All,

Sorry for the late feedback...

On 2022-01-24 11:41 +0100, Evgeni Dobrev via buildroot spake thusly:
> In POSIX sh, ^ in place of ! in glob bracket expressions
> is undefined.
> 
> Signed-off-by: Evgeni Dobrev <evgeni@studio-punkt.com>
> ---
>  fs/common.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/common.mk b/fs/common.mk
> index 45beb5ae7b..64a94d9ad8 100644
> --- a/fs/common.mk
> +++ b/fs/common.mk
> @@ -186,7 +186,7 @@ $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): $$(ROOTFS_$(2)_DEPENDENCIES)
>  
>  	$$(foreach hook,$$(ROOTFS_$(2)_PRE_GEN_HOOKS),\
>  		$$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT)$$(sep))
> -	echo "rm -rf $$(TARGET_DIR)/run/* $$(TARGET_DIR)/run/.[^.]* $$(TARGET_DIR)/tmp/* $$(TARGET_DIR)/tmp/.[^.]*" >> $$(FAKEROOT_SCRIPT)
> +	echo "rm -rf $$(TARGET_DIR)/run/* $$(TARGET_DIR)/run/.[!.]* $$(TARGET_DIR)/tmp/* $$(TARGET_DIR)/tmp/.[!.]*" >> $$(FAKEROOT_SCRIPT)

Turns out we already applied a similar change, that uses find instead,
as you suggested later in the thread:
    38c4f6b807e9 (fs/common.mk: use find instead of shell glob patterns)

Thanks!

Regards,
Yann E. MORIN.

>  	$$(call PRINTF,$$(ROOTFS_REPRODUCIBLE)) >> $$(FAKEROOT_SCRIPT)
>  	$$(call PRINTF,$$(ROOTFS_SELINUX)) >> $$(FAKEROOT_SCRIPT)
>  	$$(call PRINTF,$$(ROOTFS_$(2)_CMD)) >> $$(FAKEROOT_SCRIPT)
> -- 
> 2.34.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] 5+ messages in thread

end of thread, other threads:[~2022-07-27 15:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-24 10:41 [Buildroot] [PATCH 0/1] fs: fix /tmp and /run clean-up in POSIX shells Evgeni Dobrev via buildroot
2022-01-24 10:41 ` [Buildroot] [PATCH 1/1] " Evgeni Dobrev via buildroot
2022-01-26 22:17   ` Yann E. MORIN
2022-01-27 10:34     ` Evgeni Dobrev via buildroot
2022-07-27 15:46   ` 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.