[OpenWrt-Devel] [PATCH v4] base-files: add /etc/profile.d support

[karlp]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I like it. One query inline....

Bastian Bittorf <bittorf at bluebottle.com> wrote:
> OpenWrt should support an optional /etc/profile.d directory like
> most other Linux distributions. This allows packages to install
> their own scripts into /etc/profile.d/ directory.
> 
> The file suffix should make clear, that these scripts
> are (sourced) shell-snippets. If the user needs e.g. php or lua,
> one must make sure that the interpreter is called.
> The reverse failsafe test makes sure, that the effective returncode is
> 0.
> 
> A typcal usecase is the inclusion of private helpers,
> special variables or aliases, which at the moment needs
> patching the sourcecode and is not well maintainable.
> Now the builder can simply add there files.
> 
> v1 initial work of Hendrik Lüth <hendrik at linux-nerds.de>
> v2 changes regarding RFC (e.g. thomas.langer at lantiq.com)
> v3 changes regarding RFC (e.g. mschiffer at universe-factory.net)
> v4 keep it simple and mimic OpenWrt style
> 
> Signed-off-by: Bastian Bittorf <bittorf at bluebottle.com>
> ---
>  package/base-files/files/etc/profile |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/package/base-files/files/etc/profile
> b/package/base-files/files/etc/profile
> index 3dd58e1..577b63b 100644
> --- a/package/base-files/files/etc/profile
> +++ b/package/base-files/files/etc/profile
> @@ -14,3 +14,10 @@ export PS1='\u@\h:\w\$ '
>  
>  [ -x /usr/bin/arp ] || arp() { cat /proc/net/arp; }
>  [ -x /usr/bin/ldd ] || ldd() { LD_TRACE_LOADED_OBJECTS=1 $*; }
> +
> +[ -n "$FAILSAFE" ] || {
> +	for FILE in /etc/profile.d/*.sh; do
> +		[ -e "$FILE" ] && . "$FILE"

Why the -e?  You only got existing files in the glob above right? If
you're trying to prevent races, what's to prevent the file disappearing
after the test and before the source starts?  That's an endless rabbit
hole

/etc/lib/functions.sh for instance...

include() {                                                  
        local file                                                         
                                                                        
        for file in $(ls $1/*.sh 2>/dev/null); do         
                . $file                          
        done                                     
}         


Maybe "local" could be used to avoid the unset FILE line, but at least
the -e test seems superfluous


> +	done
> +	unset FILE
> +}

Cheers,
Karl P

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)

iQIcBAEBAgAGBQJV6WmJAAoJEBmotQ/U1cr2PwIQAKAnd6/3ida7ZT81ctMzc0YR
mFN8obDPREbRagPENaEoaYGq1XjXNibySeJNLa9iFKYFh5RVSlSGU9b8jtfcfRBV
emlDfPxs0LZkDtbOWS/yoTgO66F4YX++DXw+WD+EvMdA65Mj9ZHhr2e9cVGy6olV
xFGGLRmmHDS+jJ0Y4pdr6FKrtVRRgY7cvn/fySuOShc7AuyOV19KbIV+Muv7MRBG
hystsebwIcWWmvYz04IBYUzdLudtTGmtdpIYEz7dpPbJluX4vLmOorxcInqJ/PT4
o/p0r3DVSy0tBi7f2SHqiWIupSiMuOj2qOlduS3WmfVookh7KaeTeiO9vsevg1bK
+gLDTxDvvUSIneqzWqVyszBTL5UdY8FPAAQ53GXkIrBz/FORtMSN+z1zo00ixudc
LrFkvYXeDpk5HklGgIWUVArFx4LmzhaXtdHqntA59DEH2F4qO3sYf8rEzS5uf12T
rLWivMFX8c8EVcyHg2Apgg0zaMfH0LEewfF4dVZEl1NcMtzSZFq/kTktVkAOriqv
NvJ4QSUnG73VOsIoz4L+NTXW5dL8oD8Ee+Hs6pzMpYht48qMqe/wICs7peDbuRdL
9InoYBsCMJLboQ43othsCwoRkIHr1rHxM2Hqf2qtzg7ToZC3j7l260SEGmSnPX50
8Sfi6R0VMINT4ksje/0G
=NpiH
-----END PGP SIGNATURE-----
-------------- next part --------------
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel