[pacman-dev] [PATCH] Revert "popen does NOT require /bin/sh in a subchroot"
This reverts commit 9558639d8009483fbf422b138d020745986f82f1. This change was wrong, popen does require /bin/sh in a subchroot. 1) pacman -S lilo -r root Notice no error 2) rm root/bin/sh ; pacman -S lilo -r root Notice an error : error: scriptlet failed to execute correctly Actually, we already get an explicit error here, when popen is run, so there is no need to check for bin/sh explicitely. Besides this check was problematic in some cases. For example, bash itself has a scriptlet, but only post_install and post_upgrade, no pre_install and pre_upgrade. However, since bash has a scriptlet, runscriptlet will also be called before bash is installed. It won't do anything since the scriptlet has no pre_install function. But if we keep the check, we will still get "error : no /bin/sh". Conflicts: lib/libalpm/trans.c Signed-off-by: Xavier Chantry <shiningxc@gmail.com> --- lib/libalpm/trans.c | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-) diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c index 4b83119..32c6d90 100644 --- a/lib/libalpm/trans.c +++ b/lib/libalpm/trans.c @@ -443,13 +443,6 @@ int _alpm_runscriptlet(const char *root, const char *installfn, return(0); } - /* NOTE: popen will use the PARENT's /bin/sh, not the chroot's */ - if(access("/bin/sh", X_OK)) { - /* not found */ - _alpm_log(PM_LOG_ERROR, _("No /bin/sh in parent environment, aborting scriptlet\n")); - return(0); - } - /* creates a directory in $root/tmp/ for copying/extracting the scriptlet */ snprintf(tmpdir, PATH_MAX, "%stmp/", root); if(access(tmpdir, F_OK) != 0) { -- 1.6.2
This reverts commit 9558639d8009483fbf422b138d020745986f82f1.
This change was wrong, popen does require /bin/sh in a subchroot. Since you've removed the opposing comment in the code, can we stick
On Sun, Mar 15, 2009 at 9:10 AM, Xavier Chantry <shiningxc@gmail.com> wrote: the correct comment in the code? We've gone back and forth on this one like 1800 times.
1) pacman -S lilo -r root
Notice no error
2) rm root/bin/sh ; pacman -S lilo -r root
Notice an error : error: scriptlet failed to execute correctly
Actually, we already get an explicit error here, when popen is run, so there is no need to check for bin/sh explicitely.
Besides this check was problematic in some cases. For example, bash itself has a scriptlet, but only post_install and post_upgrade, no pre_install and pre_upgrade. However, since bash has a scriptlet, runscriptlet will also be called before bash is installed. It won't do anything since the scriptlet has no pre_install function. But if we keep the check, we will still get "error : no /bin/sh".
Conflicts:
lib/libalpm/trans.c
Signed-off-by: Xavier Chantry <shiningxc@gmail.com> --- lib/libalpm/trans.c | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-)
diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c index 4b83119..32c6d90 100644 --- a/lib/libalpm/trans.c +++ b/lib/libalpm/trans.c @@ -443,13 +443,6 @@ int _alpm_runscriptlet(const char *root, const char *installfn, return(0); }
- /* NOTE: popen will use the PARENT's /bin/sh, not the chroot's */ - if(access("/bin/sh", X_OK)) { - /* not found */ - _alpm_log(PM_LOG_ERROR, _("No /bin/sh in parent environment, aborting scriptlet\n")); - return(0); - } - /* creates a directory in $root/tmp/ for copying/extracting the scriptlet */ snprintf(tmpdir, PATH_MAX, "%stmp/", root); if(access(tmpdir, F_OK) != 0) { -- 1.6.2
_______________________________________________ pacman-dev mailing list pacman-dev@archlinux.org http://www.archlinux.org/mailman/listinfo/pacman-dev
On Sun, Mar 15, 2009 at 3:52 PM, Dan McGee <dpmcgee@gmail.com> wrote:
This reverts commit 9558639d8009483fbf422b138d020745986f82f1.
This change was wrong, popen does require /bin/sh in a subchroot. Since you've removed the opposing comment in the code, can we stick
On Sun, Mar 15, 2009 at 9:10 AM, Xavier Chantry <shiningxc@gmail.com> wrote: the correct comment in the code? We've gone back and forth on this one like 1800 times.
Well, that's also what I thought, we had the right comment before and it didn't help. I realized it's better anyway to totally skip that check, and to not even worry what is required or what isn't. We just delay this to popen, since the return status is already checked. This is better, transparent and more accurate. We don't care about which sh is needed, the code does not care either, that way everyone is happy and we do not need any comments, wrong or false :)
participants (3)
-
Dan McGee
-
Xavier
-
Xavier Chantry