Bug List: (This bug is not in your last search results)   Show last search results      Search page      Enter new bug
Bug#: 190143
Alias:
Product:
Component:
Status: RESOLVED
Resolution: FIXED
Assigned To: Gentoo Dialup Developers <net-dialup@gentoo.org>
Hardware:
OS:
Version:
Priority:
Severity:
Reporter: Martin Väth <vaeth@mathematik.uni-wuerzburg.de>
Add CC:
CC:
URL:
Summary:
Status Whiteboard:
Keywords:

Filename Description Type Creator Created Size Actions
Create a New Attachment (proposed patch, testcase, etc.) View All

Bug 190143 depends on: Show dependency tree
Bug 190143 blocks:
Votes: 0    Show votes for this bug    Vote for this bug

Additional Comments: (this is where you put emerge --info)


Not eligible to see or edit group visibility for this bug.






View Bug Activity   |   Format For Printing   |   XML   |   Clone This Bug


Description:   Opened: 2007-08-25 07:01 0000
I request to execute all files in /etc/ppp/ip-up.d and /etc/ppp/ip-down.d in
alphabetical order from the scripts /etc/ppp/ip-up and /etc/ppp/ip-down,
respectively (of course, passing all environment variables and arguments).

This would allow third party ebuilds (e.g., I was just writing an ebuild for
SuSE's accounting tool from smpppd) to add required scripts (or even programs)
during opening/closing the net connection without touching existing scripts.

Moreover, if the current three functionalities of the /etc/ppp/ip-{up,down}
scripts would be put into the files
/etc/ppp/ip-{up,down}/40-mangle-resolv.conf
/etc/ppp/ip-{up,down}/50-net.x-init
/etc/ppp/ip-{up,down}/99-run-local
   (the latter might be a symbolic link to /etc/ppp/ip-{up,down}.local)
the user could execute something before/between/after this functionality, while
currently only the latter is possible with /etc/ppp/ip-{up,down}.local

------- Comment #1 From Alin Năstac 2007-08-26 05:46:44 0000 -------
This change is important, so I will ask for comments on dev ml. 

------- Comment #2 From Alin Năstac 2007-08-31 08:47:38 0000 -------
Fixed in the pmasked revision ppp-2.4.4-r12. 
I don't have a test environment right now so I need you to test it for me. Once
I have you confirmation that it works as it should, I'll remove the pmask.

------- Comment #3 From Martin Väth 2007-09-01 04:44:16 0000 -------
Thanks. On my machine it works smoothly.

However, I am wondering whether it wouldn't be better to use
   ./"${SCRIPT}" "$@"
instead of
   source ./"${SCRIPT}" "$@"
(BTW: IIRC there are some shells which only understand the "." keyword instead
of the "source" keyword).
Of course, if you change from "source" to normal execution, you should also
chmod +x 90-local after moving to make sure that the user's files are executed
even if the user had relied on "source" (and also 40-dns 50-initd would have to
be executable, of course).

Reasons against "source"/".": It might happen that variable settings (or even
worse: things like set -e) of the scripts interfere with each other
unintentionally.
Moreover, since you changed from #!/bin/bash to #!/bin/sh, the user might still
want the possibility to change back to #!/bin/bash for certain scripts if he
has to rely on bash features: This is not so easily possible when "source"/"."
is used.

The only reason in favour of "source"/"." which I found is that the
corresponding partition might be mounted with noexec option; however,
ip-up/ip-down cannot be on such a partition anyway.

BTW: If you do not like moving the user's ip-{up,down}.local, another solution
might be to still source it in addition (IIRC this is what SuSE does). However,
the change from #!/bin/bash to #!/bin/sh might cause troubles for the user's
ip-{up,down}.local in any case (although I welcome this change).

------- Comment #4 From Alin Năstac 2007-09-01 05:13:15 0000 -------
(In reply to comment #3)
> (BTW: IIRC there are some shells which only understand the "." keyword instead
> of the "source" keyword).

I tried those scripts with busybox and they worked, so I reckon "source" is not
bash-ism.

> Reasons against "source"/".": It might happen that variable settings (or even
> worse: things like set -e) of the scripts interfere with each other
> unintentionally.

On the other hand, using source will prevent the initial process to fork for
every script you have in ip-*.d directory. 

> Moreover, since you changed from #!/bin/bash to #!/bin/sh, the user might still
> want the possibility to change back to #!/bin/bash for certain scripts if he
> has to rely on bash features: This is not so easily possible when "source"/"."
> is used.

If their /bin/sh is a symlink to /bin/bash, I don't see why they would need to
change it.

> BTW: If you do not like moving the user's ip-{up,down}.local, another solution
> might be to still source it in addition (IIRC this is what SuSE does). However,
> the change from #!/bin/bash to #!/bin/sh might cause troubles for the user's
> ip-{up,down}.local in any case (although I welcome this change).

I'm perfectly okay with that. In the previous version ip-up script "sourced"
the local script, therefore previous scripts are bash ones.
I will ask for other opinions on IRC. You might try to expose your suggestions
on ml, although I didn't saw much interest on the matter.

------- Comment #5 From Matthias Schwarzott 2007-09-01 13:27:34 0000 -------
I really like that idea.

for SCRIPT in * ; do
        source ./"${SCRIPT}" "$@"
done

But some critics: Using source when using /bin/sh is really bad. I use dash on
one system - and it does NOT support source.
$ source /tmp/x
dash: source: not found

And please make that loop ignore backup files like editors create them. (*.bak
*~ maybe).

Most other such loops had fixed file-name endings (hotplug using *.dev) or just
*.sh.

Nevertheless the change to /bin/sh will not cause trouble. Only for people
switching /bin/sh symlink to anything else - and still using bashisms in their
local scriptlets.

------- Comment #6 From Martin Väth 2007-09-02 10:14:35 0000 -------
I consider the bug as fixed.
Again, thank you very much.

Anyway, some final comments:

> I tried those scripts with busybox and they worked,
> so I reckon "source" is not bash-ism.

"source" is not POSIX compatible but dot (".") is, see
http://www.opengroup.org/onlinepubs/009695399/idx/sbi.html
see also e.g. the ChangeLog of keychain 1.3.

> On the other hand, using source will prevent the initial process to fork for
> every script you have in ip-*.d directory.

I had thought that the time difference is not so large, since the shell is in
memory anyway, but some tests showed it is actually enormously.
To prevent scripts from interfering with each other unintentionally, you might
still think about using a subshell
  ( . ./"${SCRIPT}" "$@" )
which is for some reason still much faster than execution, even in bash.
But on the other hand, it is slower than without a subshell anyway, and the
user should know what he is doing if he adds scripts...

> If their /bin/sh is a symlink to /bin/bash, I don't see why they would need
> to change it.

Even if /bin/sh is a symlink to /bin/bash, the bash enters POSIX compatibility
mode which has slightly different behavior as if /bin/bash is run directly.
Moreover, there are some reasons (speed or compatibility testing) to change the
/bin/sh symlink to another shell, so the user might have done it and relied on
the previous #!/bin/bash for ip-{up,down}.local anyway. Perhaps an ewarn is
appropriate.

------- Comment #7 From Alin Năstac 2007-09-03 10:20:02 0000 -------
Fixed in -r13 as follows:
 - add ipv6-{up,down} symlinks to ip-{up,down} scripts. This should make pppd
work seamlessly for IPv6 links
 - use . instead source
 - avoid using echo -e (another portability problem)
 - force .sh suffix on ip-{up,down}.d scripts (/bin/sh scripts are the only one
supported by this scheme)

------- Comment #8 From SpanKY 2007-09-10 07:29:19 0000 -------
bash's "POSIX compatibility" mode when invoked as /bin/sh is pretty lenient ...
it continues to support pretty much all bashisms, so for all intents and
purposes, there would be no functional difference

Bug List: (This bug is not in your last search results)   Show last search results      Search page      Enter new bug