I have an feature idea to speed up the function pathmunge for /etc/bashrc and /etc/profile. By using bash regular expression instead of echo and egrep. pathmunge () { #if ! echo $PATH | /bin/egrep -q "(^|:)$1($|:)" ; then if [[ ! "$PATH" =~ "(^|:)$1(:|$)" ]];then if [ "$2" = "after" ];then PATH=$PATH:$1 else PATH=$1:$PATH fi fi } Is there any limitation from using regular expression for this purpose?
Thanks for suggestion, looks ok. Will check some corner cases and probably add this (without commented out line) in next rawhide setup build.
> [[ ! "$PATH" =~ "(^|:)$1(:|$)" ]] This is a bashism but /etc/profile is read at least by ksh and zsh in addition to bash, so it shouldn't be done for that. I suppose doing it in /etc/bashrc would be fine though. While at it, could change the /etc/profile version to use "/bin/grep -qE" instead of "/bin/egrep -q"; egrep is deprecated (see man page and egrep --help).
Ah, ok ... thanks for bashism warning. I'll fix this egrep usage.
I guess fixing in Rawhide is enough... Fixed in setup-2.8.12-1.fc13 , closing RAWHIDE.
setup-2.8.9-2.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/setup-2.8.9-2.fc12
setup-2.8.9-2.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report.
We can avoid Bashism by using the case command like this: pathmunge () { case ":${PATH}:" in *:"$1":*) ;; *) if [ "$2" = "after" ]; then PATH=$PATH:$1 else PATH=$1:$PATH fi esac } This is compatible with most shells.
/etc/profile doesn't contain bashism - it still uses grep ... /etc/bashrc could use bashism - as it is used only by bash. Do you have some speed comparison between grep and your case?
The case command should run much faster than grep because it's the shell's builtin syntax while grep is an external command, which requires starting a new process to execute the command. I'm not sure about speed comparison between the case command and the bashism regular expression, but both should run sufficiently fast since they're the shell's builtin.
Representative times on my F-13 box for the attached script: $ time bash pathmunge.sh 1 real 0m2.809s user 0m0.482s sys 0m1.221s $ time bash pathmunge.sh 2 real 0m0.175s user 0m0.143s sys 0m0.009s $ time bash pathmunge.sh 3 real 0m0.172s user 0m0.145s sys 0m0.006s The version 3 (from comment 7) also appears to work with bash, zsh, ksh, and dash. And there's little if any difference between it and the "bashism" version on bash. So I suppose switching to Yuki's version in /etc/profile would be a good thing to do. And why not also in /etc/bashrc while at it.
Created attachment 415796 [details] pathmunge() performance test script
setup-2.8.20-1.fc13 has been submitted as an update for Fedora 13. http://admin.fedoraproject.org/updates/setup-2.8.20-1.fc13
setup-2.8.9-3.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/setup-2.8.9-3.fc12
setup-2.8.9-3.fc12 has been pushed to the Fedora 12 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update setup'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/setup-2.8.9-3.fc12
setup-2.8.20-1.fc13 has been pushed to the Fedora 13 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update setup'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/setup-2.8.20-1.fc13
setup-2.8.9-3.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report.
setup-2.8.20-1.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report.
A quick grep shows that a similar thing could be done for the packages krb5-devel, krb5-workstation and qt3, which drop scripts into /etc/profile.d/ which also try to add to PATH. (Between them, that's 5 calls to grep.) No doubt there are other packages too, those are just the ones that I happen to have installed here. (I can't find a way through koji to search all packages for any file matching /etc/profile.d/*, which would help identify them.)
Ok, adding their maintainers (or better said comaintainer in the case of qt3 ;) ) to CC to let them know...
What, no love for csh users?
(In reply to comment #20) > What, no love for csh users? There is no pathmunge() function in csh.login nor csh.cshrc ... so I can't speed up it for them ...
It looks like the csh versions of the three package profile.d scripts I mentioned above use built-in csh regex features to modify path, so no change needed there. (Not a csh user, but it looks from the man page like csh can't do functions and aliases aren't powerful enough, so repeating the code each time is the only way.) It also looks like no (bourne) script in my current profile.d (with whatever random selection of packages happen to be installed at the moment) makes use of the pathmunge function at all, rolling it themselves. Hence the 5 greps above. If pathmunge is a supported facility that (bourne) profile.d scripts ought to be making use of, then the correct change would seem to be to modify such scripts to use it rather than roll their own whether or not they use grep directly or the case magic.
There is one way to refactor the csh path munging - place in profile.d the file pathmunge.csh-inc: foreach pathmunge_i (${path:q}) if ( "$pathmunge_i" == "$1" ) then exit endif end unset pathmunge_i if ( "$2" == "after" ) then set path = ( ${path:q} ${1:q} ) else set path = ( ${1:q} ${path:q} ) endif Then call as: source /etc/profile.d/pathmunge.csh-inc /usr/kerberos/bin I would expect this to be somewhat faster than the exec method, but very slightly (but not much) slower than the existing method. Note however that it doesn't suffer from certain bugs in the way the existing krb5/qt3 usage: 1) Because it tests each component of $path separately it does not get confused by directories with embedded spaces. The existing code will not add "bar" if "/tmp/foo\ bar" is already in the path. (Nor will it add "/foo\ /bar" if both "/foo" and "/bar" are already in the path next to each other.) 2) Similarly because each component is tested as a whole, it does not get confused by substrings. The existing code fails to add "/usr/kerberos/bin" if "/usr/kerberos/bin/somethingelse" is already in the path. 3) The quote-modifiers in the path assignment means that any existing path components with embedded spaces are not mangled. The existing code will convert ( "/tmp/foo\ bar" ) into ( "/tmp/foo" "bar" ) if it modified the path.
(In reply to comment #22) > If pathmunge is a supported facility that (bourne) profile.d scripts ought to > be making use of, then the correct change would seem to be to modify such > scripts to use it rather than roll their own whether or not they use grep > directly or the case magic. My experience with that: I tried to do that in ccache for a while, but it was found that zsh didn't provide pathmunge for profile.d scripts. It was then implemented/fixed, but due to zshrc being %config(noreplace) pathmunge might still not be available after upgrades, so I reverted to not using it and chose to use the "case magic" approach (which isn't that bad IMO) and have no plans to go back to pathmunge at least in a while, see bug 548960 for more info. I wouldn't be surprised if there were some other shell packages with similar issues although I don't know of any at the moment.