[pacman-dev] [PATCH] Revert "popen does NOT require /bin/sh in a subchroot"

[Dan McGee]

On Sun, Mar 15, 2009 at 9:10 AM, Xavier Chantry <shiningxc at 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
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 at 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 at archlinux.org
> http://www.archlinux.org/mailman/listinfo/pacman-dev
>