[Buildroot] [PATCH 1/1] skeleton: Rename /etc/profile.d/umask to umask.sh

[Buildroot] [PATCH 1/1] skeleton: Rename /etc/profile.d/umask to umask.sh

[Nicolas Cavallari]

/etc/profile only sources files that matches the /etc/profile.d/*.sh
pattern, so /etc/profile.d/umask was never sourced.

Signed-off-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
---
 system/skeleton/etc/profile.d/{umask => umask.sh} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename system/skeleton/etc/profile.d/{umask => umask.sh} (100%)

diff --git a/system/skeleton/etc/profile.d/umask b/system/skeleton/etc/profile.d/umask.sh
similarity index 100%
rename from system/skeleton/etc/profile.d/umask
rename to system/skeleton/etc/profile.d/umask.sh
-- 
2.7.0

[Buildroot] [PATCH 1/1] skeleton: Rename /etc/profile.d/umask to umask.sh

[Thomas Petazzoni]

Nicolas,

Thanks for noticing this bug!

On Mon,  8 Feb 2016 16:42:45 +0100, Nicolas Cavallari wrote:
> /etc/profile only sources files that matches the /etc/profile.d/*.sh
> pattern, so /etc/profile.d/umask was never sourced.
> 
> Signed-off-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
> ---
>  system/skeleton/etc/profile.d/{umask => umask.sh} | 0
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  rename system/skeleton/etc/profile.d/{umask => umask.sh} (100%)
> 
> diff --git a/system/skeleton/etc/profile.d/umask b/system/skeleton/etc/profile.d/umask.sh
> similarity index 100%
> rename from system/skeleton/etc/profile.d/umask
> rename to system/skeleton/etc/profile.d/umask.sh

I'd rather think that /etc/profile should source all files
in /etc/profile.d/. At least that's what I would expect from a
<something>.d/ directory.

What do others think about this?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] skeleton: Rename /etc/profile.d/umask to umask.sh

[Thomas Claveirole]

Hi all,

> I'd rather think that /etc/profile should source all files
> in /etc/profile.d/. At least that's what I would expect from a
> <something>.d/ directory.

Beware of backup files.  Such directories often end-up with ~-
terminated files (e.g., foobar~ after one edited foobar) and this 
often cause hard-to-detect bugs because of obsolete files being 
sourced.

If not relying on a common suffix, maybe you should use the same rules 
as Debian's run-parts?  These are:

If  neither  the --lsbsysinit option nor the --regex option is given 
then the names must consist entirely of ASCII upper- and lower-case 
letters, ASCII digits,  ASCII  underscores,  and ASCII minus-hyphens.

-- 
Thomas Claveirole <thomas.claveirole@green-communications.fr>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20160208/0675f95b/attachment.asc>

[Buildroot] [PATCH 1/1] skeleton: Rename /etc/profile.d/umask to umask.sh

[Thomas Petazzoni]

Hello,

On Mon, 08 Feb 2016 17:45:10 +0100, Thomas Claveirole wrote:

> Beware of backup files.  Such directories often end-up with ~-
> terminated files (e.g., foobar~ after one edited foobar) and this 
> often cause hard-to-detect bugs because of obsolete files being 
> sourced.
> 
> If not relying on a common suffix, maybe you should use the same rules 
> as Debian's run-parts?  These are:
> 
> If  neither  the --lsbsysinit option nor the --regex option is given 
> then the names must consist entirely of ASCII upper- and lower-case 
> letters, ASCII digits,  ASCII  underscores,  and ASCII minus-hyphens.

Sounds like a good idea. So I guess it should match [A-Za-z0-9_-]*,
correct ? Isn't "." part of the allowed characters ?

In any case, patches are welcome :)

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] skeleton: Rename /etc/profile.d/umask to umask.sh

[Thomas Claveirole]

> Sounds like a good idea. So I guess it should match [A-Za-z0-9_-]*,
> correct ? Isn't "." part of the allowed characters ?

Correct.  Debian's run-parts effectively ignores files that have a dot 
in their names (I would guess the rationale is something like "avoid 
*.bak, *.old, etc.")

I would rather stick to this behavior, but have no strong opinion.  Do 
you, or others, prefer something else?

> In any case, patches are welcome :)

I'll propose something tomorrow then.  :-)

-- 
Thomas Claveirole <thomas.claveirole@green-communications.fr>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20160208/e668b399/attachment.asc>

[Buildroot] [PATCH RESEND 1/1] skeleton: Have /etc/profile source [A-Za-z0-9_-]+ files in profile.d.

[Nicolas Cavallari]

Instead of sourcing all files ending with .sh (which is unexpected
enough that /etc/profile.d/umask was missed), source all files which
matches [A-Za-z0-9_-]+.  This way, backup files from most text editors
(e.g.  umask~, umask.dpkg-old, .umask.swp~) will not be sourced.

Signed-off-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
---

Resend with the correct list address.

diff --git a/system/skeleton/etc/profile b/system/skeleton/etc/profile
index 3a97427..2fb7743 100644
--- a/system/skeleton/etc/profile
+++ b/system/skeleton/etc/profile
@@ -12,9 +12,14 @@ export PAGER='/bin/more '
 export EDITOR='/bin/vi'
 
 # Source configuration files from /etc/profile.d
-for i in /etc/profile.d/*.sh ; do
-	if [ -r "$i" ]; then
-		. $i
-	fi
-	unset i
+for i in /etc/profile.d/* ; do
+	case "$i" in
+	/etc/profile.d/*[^A-Za-z0-9_-]*)
+		continue;;
+	*)
+		if [ -r "$i" ]; then
+			. $i
+		fi
+	esac
 done
+unset i
-- 
2.7.0

[Buildroot] [PATCH RESEND 1/1] skeleton: Have /etc/profile source [A-Za-z0-9_-]+ files in profile.d.

[Arnout Vandecappelle]

On 09-02-16 16:06, Nicolas Cavallari wrote:
> Instead of sourcing all files ending with .sh (which is unexpected
> enough that /etc/profile.d/umask was missed), source all files which
> matches [A-Za-z0-9_-]+.  This way, backup files from most text editors
> (e.g.  umask~, umask.dpkg-old, .umask.swp~) will not be sourced.

 Actually, I'm with Yann, it's better to keep it as *.sh. In addition to the
reasons he quotes, I'd also add that the code with this patch becomes a lot more
complicated than just sourcing *.sh.


 Regards,
 Arnout

> 
> Signed-off-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
> ---
> 
> Resend with the correct list address.
> 
> diff --git a/system/skeleton/etc/profile b/system/skeleton/etc/profile
> index 3a97427..2fb7743 100644
> --- a/system/skeleton/etc/profile
> +++ b/system/skeleton/etc/profile
> @@ -12,9 +12,14 @@ export PAGER='/bin/more '
>  export EDITOR='/bin/vi'
>  
>  # Source configuration files from /etc/profile.d
> -for i in /etc/profile.d/*.sh ; do
> -	if [ -r "$i" ]; then
> -		. $i
> -	fi
> -	unset i
> +for i in /etc/profile.d/* ; do
> +	case "$i" in
> +	/etc/profile.d/*[^A-Za-z0-9_-]*)
> +		continue;;
> +	*)
> +		if [ -r "$i" ]; then
> +			. $i
> +		fi
> +	esac
>  done
> +unset i
> 


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 1/1] skeleton: Rename /etc/profile.d/umask to umask.sh

[Yann E. MORIN]

Thomas, All,

On 2016-02-08 17:25 +0100, Thomas Petazzoni spake thusly:
> On Mon,  8 Feb 2016 16:42:45 +0100, Nicolas Cavallari wrote:
> > /etc/profile only sources files that matches the /etc/profile.d/*.sh
> > pattern, so /etc/profile.d/umask was never sourced.
> > 
> > Signed-off-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
> > ---
> >  system/skeleton/etc/profile.d/{umask => umask.sh} | 0
> >  1 file changed, 0 insertions(+), 0 deletions(-)
> >  rename system/skeleton/etc/profile.d/{umask => umask.sh} (100%)
> > 
> > diff --git a/system/skeleton/etc/profile.d/umask b/system/skeleton/etc/profile.d/umask.sh
> > similarity index 100%
> > rename from system/skeleton/etc/profile.d/umask
> > rename to system/skeleton/etc/profile.d/umask.sh
> 
> I'd rather think that /etc/profile should source all files
> in /etc/profile.d/. At least that's what I would expect from a
> <something>.d/ directory.
> 
> What do others think about this?

The *.sh is customary on standard distros. So I'd keep it named as thus
so users are not surprised that we include things that would not have
been included.

The reasoning behind this is that a Bourne-compliant (aka POSIX) shell
would parse only the *.sh scripts, which ought to Bounre-compliant,
leaving aside other scripts for the other shells (like csh, which is not
Bourne-compliant but may still scan files from /etc/profile.d with a
pattern not-unlike *.csh).

Furthermore, as the (other) Thomas pointed out, we do not want to source
any stray (backup) file (even though I'd argue that the user should be
responsible for cleaning up that location before generating the image,
but that leaves local edits as a possible source of confusion).

So, I prefer that we only source /etc/profile.d/*.sh because of the
aforementioned reasons, i.e.;
  - Bourne/POSIX-compliant scripts
  - principple of least-surprise

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/1] skeleton: Rename /etc/profile.d/umask to umask.sh

[Arnout Vandecappelle]

On 08-02-16 16:42, Nicolas Cavallari wrote:
> /etc/profile only sources files that matches the /etc/profile.d/*.sh
> pattern, so /etc/profile.d/umask was never sourced.
> 
> Signed-off-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>

 Given that I don't like the alternative you posted:

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>


 However, I think it would be even better to throw away umask.sh completely and
embed it in /etc/profile. There is really no reason to make this a separate file.

 Regards,
 Arnout

> ---
>  system/skeleton/etc/profile.d/{umask => umask.sh} | 0
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  rename system/skeleton/etc/profile.d/{umask => umask.sh} (100%)
> 
> diff --git a/system/skeleton/etc/profile.d/umask b/system/skeleton/etc/profile.d/umask.sh
> similarity index 100%
> rename from system/skeleton/etc/profile.d/umask
> rename to system/skeleton/etc/profile.d/umask.sh
> 


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 1/1] skeleton: Rename /etc/profile.d/umask to umask.sh

[Peter Korsgaard]

>>>>> "Nicolas" == Nicolas Cavallari <nicolas.cavallari@green-communications.fr> writes:

 > /etc/profile only sources files that matches the /etc/profile.d/*.sh
 > pattern, so /etc/profile.d/umask was never sourced.

 > Signed-off-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>

Committed, thanks.

-- 
Bye, Peter Korsgaard