[pacman-dev] sourcing /etc/profile and set -e bug in makepkg
So, despite me assuring Dan that nothing could possibly go wrong, the commit to source /etc/profile rather than individual files in /etc/profile.d (http://projects.archlinux.org/?p=pacman.git;a=commitdiff;h=0e0a8461) has caused a problem. See FS#11179 (http://bugs.archlinux.org/task/11179) for more details. Xavier and I are stuck as to what is actually causing this problem. The guilty script being sourced is /etc/bash_completion.d/modules and specifically the "type modules" statement at the top. What we don't understand is that this file is being sourced under both methods and why it is causing problems only with one. Even more confusing, the way it gets sourced in /etc/profile is essentially the way makepkg used to do it (loop over /etc/profile.d/* files). A work-around is to add "set +e" and "set -e" on either side of the "source /etc/profile" call. That is probably a good idea anyway as that sourcing should probably not inherit the "-e" property. But can anyone explain why we now need this? Very confused, Allan
On Mon, Aug 11, 2008 at 5:24 PM, Allan McRae <allan@archlinux.org> wrote:
So, despite me assuring Dan that nothing could possibly go wrong, the commit to source /etc/profile rather than individual files in /etc/profile.d (http://projects.archlinux.org/?p=pacman.git;a=commitdiff;h=0e0a8461) has caused a problem. See FS#11179 (http://bugs.archlinux.org/task/11179) for more details.
Xavier and I are stuck as to what is actually causing this problem. The guilty script being sourced is /etc/bash_completion.d/modules and specifically the "type modules" statement at the top. What we don't understand is that this file is being sourced under both methods and why it is causing problems only with one. Even more confusing, the way it gets sourced in /etc/profile is essentially the way makepkg used to do it (loop over /etc/profile.d/* files).
A work-around is to add "set +e" and "set -e" on either side of the "source /etc/profile" call. That is probably a good idea anyway as that sourcing should probably not inherit the "-e" property. But can anyone explain why we now need this?
Very confused, Allan
Ahah, I was also confused, and desperate of figuring it out, but I finally did :) The reason is the /etc/profile.d/bash_completion.sh script It checks for the existence of $PS1, apparently for guessing if the shell is interactive or not. Isn't this a broken assumption? Because when the new makepkg 3.2.0 sources /etc/profile, then PS1 is defined there. Though it was not an interactive shell. On the other hand, when we have another script, like the old makepkg ( <= 3.1), which sources /etc/profile.d/bash_completion.sh directly, then PS1 is not defined, and /etc/bash_completion is not sourced. So we could not detect the breakage caused by /etc/bash_completion.d/modules before. Here are the information I found about how to detect an interactive shell : http://www.faqs.org/faqs/unix-faq/faq/part5/section-5.html This indeed mention checking $PS1 but also says checking $- is better. And that would work indeed. But this info is very old, and the following one looks much more recent : http://defindit.com/readme_files/bash_examples.html "Using shopt -q login_shell seems to be the modern and reliable way to detect interactive shell sessions." So if we fix the detection of interactive shell in /etc/profile.d/bash_completion.sh , then we don't have to edit anything in makepkg. It will still source /etc/profile.d/bash_completion.sh but will stop here and not source /etc/bash_completion
Xavier wrote:
On Mon, Aug 11, 2008 at 5:24 PM, Allan McRae <allan@archlinux.org> wrote:
So, despite me assuring Dan that nothing could possibly go wrong, the commit to source /etc/profile rather than individual files in /etc/profile.d (http://projects.archlinux.org/?p=pacman.git;a=commitdiff;h=0e0a8461) has caused a problem. See FS#11179 (http://bugs.archlinux.org/task/11179) for more details.
Xavier and I are stuck as to what is actually causing this problem. The guilty script being sourced is /etc/bash_completion.d/modules and specifically the "type modules" statement at the top. What we don't understand is that this file is being sourced under both methods and why it is causing problems only with one. Even more confusing, the way it gets sourced in /etc/profile is essentially the way makepkg used to do it (loop over /etc/profile.d/* files).
A work-around is to add "set +e" and "set -e" on either side of the "source /etc/profile" call. That is probably a good idea anyway as that sourcing should probably not inherit the "-e" property. But can anyone explain why we now need this?
Very confused, Allan
Ahah, I was also confused, and desperate of figuring it out, but I finally did :) The reason is the /etc/profile.d/bash_completion.sh script It checks for the existence of $PS1, apparently for guessing if the shell is interactive or not. Isn't this a broken assumption?
Because when the new makepkg 3.2.0 sources /etc/profile, then PS1 is defined there. Though it was not an interactive shell. On the other hand, when we have another script, like the old makepkg ( <= 3.1), which sources /etc/profile.d/bash_completion.sh directly, then PS1 is not defined, and /etc/bash_completion is not sourced. So we could not detect the breakage caused by /etc/bash_completion.d/modules before.
Here are the information I found about how to detect an interactive shell : http://www.faqs.org/faqs/unix-faq/faq/part5/section-5.html This indeed mention checking $PS1 but also says checking $- is better. And that would work indeed.
But this info is very old, and the following one looks much more recent : http://defindit.com/readme_files/bash_examples.html "Using shopt -q login_shell seems to be the modern and reliable way to detect interactive shell sessions."
So if we fix the detection of interactive shell in /etc/profile.d/bash_completion.sh , then we don't have to edit anything in makepkg. It will still source /etc/profile.d/bash_completion.sh but will stop here and not source /etc/bash_completion
Wow, I am very, very impressed that you found this! It does seem bash-completion is broken but it does not appear to be developed upstream any longer. I wonder if there is a replacement project somewhere? Despite locating the actual cause, I am still in favour of including the work-around in makepkg for robustness. Even though it was not makepkg's fault, it should not fail without any obvious cause like it currently is. Allan
On Mon, Aug 11, 2008 at 6:41 PM, Allan McRae <allan@archlinux.org> wrote:
Wow, I am very, very impressed that you found this! It does seem bash-completion is broken but it does not appear to be developed upstream any longer. I wonder if there is a replacement project somewhere?
Well, it just took me 1 hour of adding debug statements everywhere :D We don't even need to care about upstream, because the /etc/profile.d/bash-completion.sh script in fault was made for Arch, as far as I can tell. So let's just fix that for now. But on that note, I just noticed a very weird thing. The bash-completion package is newer in the old cvs repo than in the new svn repo. http://repos.archlinux.org/viewvc.cgi/extra/system/bash-completion/?root=ext... http://repos.archlinux.org/viewvc.cgi/bash-completion/repos/extra-i686/ I know this is offtopic here, but since this was made by both Dan and Aaron, maybe one of them could take care of it? Otherwise, I am willing to take the time to write a bug tracker if necessary About upstream, there was apparently an attempt of Debian to take it over : http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=472468 But the alioth page seems still empty, the code was not even imported : http://alioth.debian.org/projects/bash-completion/ I also found this, but well, no comment : http://bash-completion.alioth.debian.org/ I can't even find the 20080705 version than Debian is using anywhere else than on the debian package page : http://packages.debian.org/fr/sid/bash-completion In the bug report above, the last comment mentions this new project : http://fvue.nl/wiki/Bash_completion_lib http://code.google.com/p/bash-completion-lib/ That could be a better replacement. But anyway, as I said, this is not related to our problem.
Despite locating the actual cause, I am still in favour of including the work-around in makepkg for robustness. Even though it was not makepkg's fault, it should not fail without any obvious cause like it currently is.
Hm yeah, that looks like a good idea indeed.
participants (2)
-
Allan McRae
-
Xavier