diff mbox

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

Message ID 1454946165-8192-1-git-send-email-nicolas.cavallari@green-communications.fr
State Accepted
Commit 670fa0c96fb8fa06c924ddbc3bc28a8e66a5ca33
Headers show

Commit Message

Nicolas Cavallari Feb. 8, 2016, 3:42 p.m. UTC
/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%)

Comments

Thomas Petazzoni Feb. 8, 2016, 4:25 p.m. UTC | #1
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 Claveirole Feb. 8, 2016, 4:45 p.m. UTC | #2
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 Petazzoni Feb. 8, 2016, 4:57 p.m. UTC | #3
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 Claveirole Feb. 8, 2016, 5:14 p.m. UTC | #4
> 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.  :-)
Yann E. MORIN Feb. 8, 2016, 5:19 p.m. UTC | #5
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.
Arnout Vandecappelle Feb. 9, 2016, 9:47 p.m. UTC | #6
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
>
Peter Korsgaard Feb. 10, 2016, 6:49 a.m. UTC | #7
>>>>> "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.
diff mbox

Patch

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