NetBSD / pkgsrc

Automatic conversion of the NetBSD pkgsrc CVS module, use with care
https://www.pkgsrc.org
308 stars 164 forks source link

macOS: Some environment variables are not working for alternatives #66

Closed bartoszkosiorek closed 4 years ago

bartoszkosiorek commented 4 years ago

Due to System Integrity Protection (SIP) on macOS systems, the binaries from the /bin (eg. /bin/bash) are not allowed to handle this environment variables. More information: https://support.apple.com/en-us/HT204899

Example of SIP:

#!/bin/bash
echo $DYLD_LIBRARY_PATH

Result:

The current generated alternative script files on macOS from the pkg_alternatives package are having as interpreter /bin/bash, eg. pip3:

$ head /usr/local/pkg/bin/pip3
#!/bin/bash
#
# $NetBSD: wrapper.sh,v 1.2 2012/06/13 15:35:32 jperkin Exp $
#
# pkg_alternatives - Generic wrappers for programs with similar interfaces
# Copyright (c) 2005 Julio M. Merino Vidal <jmmv@NetBSD.org>
...

This is a bit problematic in case of macOS as due to SIP protection for example declared environment variable like DYLD_LIBRARY_PATH is rejected.

This will lead eventually to problems for trying to do things like DYLD_LIBRARY_PATH=/path_with_dylibs /usr/local/pkg//bin/ctest which is pretty common.

jperkin commented 4 years ago

I think I've seen this worked around by using a shell from pkgsrc, we may want to investigate going back to bootstrapping our own and using it for everything.

bartoszkosiorek commented 4 years ago

For my own project I would like to change the default shebang for wrappers. It seems that it could be modified in: https://github.com/NetBSD/pkgsrc/blob/trunk/pkgtools/pkg_alternatives/files/wrapper.sh#L1

Do you know by which option the __SH__ or @SH@ could be set?

bsiegert commented 4 years ago

I think it is set to the value of the SHELL environment variable during bootstrap?

Bartosz notifications@github.com schrieb am Fr., 12. Juni 2020, 16:13:

For my own project I would like to change the default shebang for wrappers. It seems that it could be modified in:

https://github.com/NetBSD/pkgsrc/blob/trunk/pkgtools/pkg_alternatives/files/wrapper.sh#L1

Do you know by which option the SH or @SH@ could be set?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/NetBSD/pkgsrc/issues/66#issuecomment-643291989, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGTQGVY6SWRFL4KFGVY6R3RWIZYVANCNFSM4N4F3IVA .

jperkin commented 4 years ago

Simplest way is:

$ cd pkgsrc/pkgtools/pkg_alternatives
$ bmake TOOLS_PLATFORM.sh=/opt/pkg/bin/bash install

or whatever shell you want to use outside of /bin.

bartoszkosiorek commented 4 years ago

@jperkin I have found also WRAPPER_SH variable in settings: https://github.com/NetBSD/pkgsrc/blob/pkgsrc-2019Q1/mk/wrapper/wrapper-defs.mk#L41

jperkin commented 4 years ago

No, that's something completely different (and something you're unlikely to be using).

bartoszkosiorek commented 4 years ago

What do you think about idea, of use bash from pkgsrc/shells/bash package by pkg_alternatives for wrappers?

It will resolve such kind of issues, but it will be additional dependency to shells/bash.

jperkin commented 4 years ago

As a local change it would be fine, nothing depends on pkg_alternatives so you aren't running any risks during upgrades etc.

For a wider pkgsrc fix we'll need to look into what the current status of shells/pdksh is on Darwin by running some bulk builds, and then if it looks ok consider switching bootstrap over to using it by default.

bartoszkosiorek commented 4 years ago

After set TOOLS_PLATFORM.sh=/usr/local/pkg/bin/bash, the wrappers have changed shebang correctly.

I will try to overwrite TOOLS_PLATFORM.sh variable in pkg_alternatives/Makefile and add dependency, to use the default shells/bash shell.

I have build the shells/pdksh without problems.

Logs from shells/pdksh build on macOS Catalina 10.15.5

Click to expand! #### pdksh build logs ``` $ sudo /usr/local/pkg/bin/bmake deinstall ===> Deinstalling for pdksh-5.2.14nb7 Running /usr/local/pkg/sbin/pkg_delete -K /usr/local/pkg/pkgdb pdksh-5.2.14nb7 $ sudo /usr/local/pkg/bin/bmake clean ===> Cleaning for pdksh-5.2.14nb7 $ sudo /usr/local/pkg/bin/bmake install ===> Installing dependencies for pdksh-5.2.14nb7 => Build dependency cwrappers>=20150314: found cwrappers-20180325 ===> Overriding tools for pdksh-5.2.14nb7 ===> Extracting for pdksh-5.2.14nb7 /bin/cp -R /Users/NavKit/dev/pkgsrc/shells/pdksh/files /private/var/tmp/pkgsrc-obj/shells/pdksh/work/pdksh-5.2.14 ===> Patching for pdksh-5.2.14nb7 ===> Creating toolchain wrappers for pdksh-5.2.14nb7 ===> Configuring for pdksh-5.2.14nb7 => Modifying GNU configure scripts to avoid --recheck => Replacing config-guess with pkgsrc versions => Replacing config-sub with pkgsrc versions => Replacing install-sh with pkgsrc version creating cache ./config.cache checking for gcc... clang checking whether we are using GNU C... yes checking how to run the C preprocessor... clang -E checking whether clang needs -traditional... no checking if this is a problematic os... checking for minix/config.h... no no checking for dirent.h that defines DIR... yes checking for opendir in -ldir... no checking for opendir in -lndir... no checking for sane unistd.h... yes checking terminal interface... termios checking for stddef.h... yes checking for stdlib.h... yes checking for string.h... yes checking for memory.h... yes checking for fcntl.h... yes checking for limits.h... yes checking for paths.h... yes checking for sys/param.h... yes checking for sys/resource.h... yes checking for values.h... no checking for ulimit.h... yes checking for sys/time.h... yes checking whether time.h and sys/time.h may both be included... yes checking for sys/wait.h that is POSIX.1 compatible... yes checking for off_t in sys/types.h... yes checking for mode_t in sys/types.h... yes checking for pid_t in sys/types.h... yes checking for uid_t in sys/types.h... yes checking return type of signal handlers... void checking size of int... 4 checking size of long... 8 checking for clock_t in any of , and ... yes checking for sigset_t in and ... yes checking for rlim_t in and ... yes checking for working memmove... yes checking for memset... yes checking for confstr... yes checking for dup2... yes checking for flock... yes checking for getcwd... yes checking for getwd... yes checking for killpg... yes checking for nice... yes checking for setrlimit... yes checking for strerror... yes checking for strcasecmp... yes checking for strstr... yes checking for sysconf... yes checking for tcsetpgrp... yes checking for ulimit... yes checking for waitpid... yes checking for wait3... yes checking for strlcpy... yes checking for strlcat... yes checking for sigsetjmp... yes checking for valloc... yes checking for getpagesize... yes checking for working mmap... yes checking for lstat... yes checking for sys_errlist declaration in errno.h... no checking for sys_errlist in library... yes checking for sys_siglist declaration in signal.h or unistd.h... yes checking for sys_siglist in library... yes checking time() declaration in time.h... yes checking if times() is present/working... yes checking whether stat file-mode macros are broken... no checking for st_rdev in struct stat... yes checking for working const... yes checking if compiler understands void... yes checking if compiler understands volatile... yes checking if compiler understands prototypes... yes checking if C compiler groks __attribute__(( .. ))... yes checking whether #! works in shell scripts... yes checking for a BSD compatible install... /usr/bin/install -c -o root -g wheel checking if dup2() works (ie, resets the close-on-exec flag)... yes checking flavour of signal routines... posix checking flavour of pgrp routines... posix checking if process group synchronization is required... no checking if opendir() fails to open non-directories... yes checking if you have /dev/fd/n... yes updating cache ./config.cache creating ./config.status creating Makefile creating config.h ===> Building for pdksh-5.2.14nb7 --- emacs.out --- --- ksh.1 --- --- stamp-h --- --- emacs.out --- ./emacs-gen.sh ./emacs.c > tmpemacs.out --- ksh.1 --- ./mkman ksh ./ksh.Man > tmpksh.1 --- stamp-h --- CONFIG_FILES="" CONFIG_HEADERS=config.h ./config.status --- emacs.out --- mv tmpemacs.out emacs.out --- ksh.1 --- mv tmpksh.1 ksh.1 --- stamp-h --- creating config.h config.h is unchanged date > stamp-h --- siglist.out --- --- alloc.o --- --- c_ksh.o --- --- c_sh.o --- --- c_test.o --- --- c_ulimit.o --- --- edit.o --- --- emacs.o --- --- siglist.out --- ./siglist.sh "clang -E -P -DHAVE_CONFIG_H -I. -I." < ./siglist.in > tmpsiglist.out --- alloc.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 alloc.c --- c_ksh.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 c_ksh.c --- c_sh.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 c_sh.c --- c_test.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 c_test.c --- c_ulimit.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 c_ulimit.c --- edit.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 edit.c --- emacs.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 emacs.c --- c_ksh.o --- c_ksh.c:571:13: warning: format string is not a string literal (potentially insecure) [-Wformat-security] shprintf(newline); ^~~~~~~ c_ksh.c:571:13: note: treat the string as an argument to avoid this shprintf(newline); ^ "%s", c_ksh.c:871:17: warning: format string is not a string literal (potentially insecure) [-Wformat-security] shprintf(newline); ^~~~~~~ c_ksh.c:871:17: note: treat the string as an argument to avoid this shprintf(newline); ^ "%s", c_ksh.c:962:14: warning: format string is not a string literal (potentially insecure) [-Wformat-security] shprintf(newline); ^~~~~~~ c_ksh.c:962:14: note: treat the string as an argument to avoid this shprintf(newline); ^ "%s", c_ksh.c:986:14: warning: format string is not a string literal (potentially insecure) [-Wformat-security] shprintf(newline); ^~~~~~~ c_ksh.c:986:14: note: treat the string as an argument to avoid this shprintf(newline); ^ "%s", c_ksh.c:1234:13: warning: format string is not a string literal (potentially insecure) [-Wformat-security] bi_errorf(null); ^~~~ c_ksh.c:1234:13: note: treat the string as an argument to avoid this bi_errorf(null); ^ "%s", c_ksh.c:1255:13: warning: format string is not a string literal (potentially insecure) [-Wformat-security] shprintf(newline); ^~~~~~~ c_ksh.c:1255:13: note: treat the string as an argument to avoid this shprintf(newline); ^ "%s", --- eval.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 eval.c --- exec.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 exec.c --- siglist.out --- mv tmpsiglist.out siglist.out --- expr.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 expr.c --- eval.o --- eval.c:340:19: warning: implicit conversion from 'int' to 'char' changes value from 192 to -64 [-Wconstant-conversion] *dp++ = '@' + 0x80; ~ ~~~~^~~~~~ --- history.o --- --- exec.o --- exec.c:148:13: warning: format string is not a string literal (potentially insecure) [-Wformat-security] errorf(null); ^~~~ exec.c:148:13: note: treat the string as an argument to avoid this errorf(null); ^ "%s", --- history.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 history.c --- expr.o --- expr.c:190:11: warning: format string is not a string literal (potentially insecure) [-Wformat-security] errorf(null); ^~~~ expr.c:190:11: note: treat the string as an argument to avoid this errorf(null); ^ "%s", --- io.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 io.c --- expr.o --- 1 warning generated. --- jobs.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 jobs.c --- lex.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 lex.c --- c_ksh.o --- 6 warnings generated. --- mail.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 mail.c --- jobs.o --- jobs.c:897:11: warning: format string is not a string literal (potentially insecure) [-Wformat-security] shprintf(newline); ^~~~~~~ jobs.c:897:11: note: treat the string as an argument to avoid this shprintf(newline); ^ "%s", --- main.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 main.c --- misc.o --- --- lex.o --- lex.c:858:9: warning: format string is not a string literal (potentially insecure) [-Wformat-security] errorf(null); ^~~~ lex.c:858:9: note: treat the string as an argument to avoid this errorf(null); ^ "%s", --- misc.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 misc.c --- main.o --- main.c:562:13: warning: format string is not a string literal (potentially insecure) [-Wformat-security] shellf(newline); ^~~~~~~ main.c:562:13: note: treat the string as an argument to avoid this shellf(newline); ^ "%s", main.c:861:9: warning: format string is not a string literal (potentially insecure) [-Wformat-security] errorf(null); /* this is never executed - keeps gcc quiet */ ^~~~ main.c:861:9: note: treat the string as an argument to avoid this errorf(null); /* this is never executed - keeps gcc quiet */ ^ "%s", --- exec.o --- 1 warning generated. --- missing.o --- --- path.o --- --- missing.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 missing.c --- path.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 path.c --- misc.o --- misc.c:262:12: warning: format string is not a string literal (potentially insecure) [-Wformat-security] shprintf(newline); ^~~~~~~ misc.c:262:12: note: treat the string as an argument to avoid this shprintf(newline); ^ "%s", --- eval.o --- 1 warning generated. --- misc.o --- misc.c:1014:15: warning: format string is not a string literal (potentially insecure) [-Wformat-security] bi_errorf(null); ^~~~ misc.c:1014:15: note: treat the string as an argument to avoid this bi_errorf(null); ^ "%s", misc.c:1040:15: warning: format string is not a string literal (potentially insecure) [-Wformat-security] bi_errorf(null); ^~~~ misc.c:1040:15: note: treat the string as an argument to avoid this bi_errorf(null); ^ "%s", misc.c:1091:20: warning: adding 'int' to a string does not append to the string [-Wstring-plus-int] shprintf("'\\'" + 1 - inquote); ~~~~~~~^~~ misc.c:1091:20: note: use array indexing to silence this warning shprintf("'\\'" + 1 - inquote); ^ & [ ] misc.c:1091:13: warning: format string is not a string literal (potentially insecure) [-Wformat-security] shprintf("'\\'" + 1 - inquote); ^~~~~~~~~~~~~~~~~~~~ misc.c:1091:13: note: treat the string as an argument to avoid this shprintf("'\\'" + 1 - inquote); ^ "%s", --- shf.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 shf.c --- sigact.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 sigact.c --- main.o --- 2 warnings generated. --- syn.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 syn.c --- table.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 table.c --- tree.o --- --- jobs.o --- 1 warning generated. --- tree.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 tree.c --- tty.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 tty.c --- var.o --- --- version.o --- --- var.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 var.c --- version.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 version.c --- vi.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 vi.c --- misc.o --- 5 warnings generated. --- trap.o --- clang -c -DHAVE_CONFIG_H -I. -I. -O2 trap.c --- var.o --- var.c:373:11: warning: format string is not a string literal (potentially insecure) [-Wformat-security] errorf(null); ^~~~ var.c:373:11: note: treat the string as an argument to avoid this errorf(null); ^ "%s", var.c:727:14: warning: format string is not a string literal (potentially insecure) [-Wformat-security] errorf(null); ^~~~ var.c:727:14: note: treat the string as an argument to avoid this errorf(null); ^ "%s", --- vi.o --- vi.c:1761:6: warning: address of array 'newline' will always evaluate to 'true' [-Wpointer-bool-conversion] if (newline) { ~~ ^~~~~~~ --- lex.o --- 1 warning generated. --- var.o --- 2 warnings generated. --- vi.o --- 1 warning generated. --- ksh --- clang -L/usr/local/pkg/lib -o ksh alloc.o c_ksh.o c_sh.o c_test.o c_ulimit.o edit.o emacs.o eval.o exec.o expr.o history.o io.o jobs.o lex.o mail.o main.o misc.o missing.o path.o shf.o sigact.o syn.o table.o trap.o tree.o tty.o var.o version.o vi.o ===> Installing for pdksh-5.2.14nb7 => Generating pre-install file lists => Creating installation directories /usr/bin/install -c -o root -g wheel -m 755 /private/var/tmp/pkgsrc-obj/shells/pdksh/work/pdksh-5.2.14/ksh /private/var/tmp/pkgsrc-obj/shells/pdksh/work/.destdir/usr/local/pkg/bin/pdksh /usr/bin/install -c -o root -g wheel -m 644 /private/var/tmp/pkgsrc-obj/shells/pdksh/work/pdksh-5.2.14/ksh.1 /private/var/tmp/pkgsrc-obj/shells/pdksh/work/.destdir/usr/local/pkg/man/man1/pdksh.1 => Automatic stripping binaries => Automatic manual page handling => Generating post-install file lists => Checking file-check results for pdksh-5.2.14nb7 => Creating binary package /private/var/tmp/pkgsrc-obj/shells/pdksh/work/.packages/pdksh-5.2.14nb7.tgz ===> Building binary package for pdksh-5.2.14nb7 => Creating binary package /Users/NavKit/dev/pkgsrc/packages/All/pdksh-5.2.14nb7.tgz ===> Installing binary package of pdksh-5.2.14nb7 =========================================================================== The following lines can be added to /etc/shells: /usr/local/pkg/bin/pdksh =========================================================================== $ /usr/local/pkg/bin/pdksh $ exit $ uname -a Darwin pl1mcl-5287921 19.5.0 Darwin Kernel Version 19.5.0: Tue May 26 20:41:44 PDT 2020; root:xnu-6153.121.2~2/RELEASE_X86_64 x86_64 ```
bartoszkosiorek commented 4 years ago

@jperkin Do you know how to setup shell from pkgsrc globally in bootstrap? The bootstrap tools are defined here: https://github.com/NetBSD/pkgsrc/blob/trunk/mk/tools/tools.Darwin.mk#L100

How these paths needs to be set properly to use pkgsrc shell and how add dependency to boostrap?

jperkin commented 4 years ago

Yes, I'm currently testing this in a bulk build.

bartoszkosiorek commented 4 years ago

Could you please share the code with updated bootstrap? I would like to start my own testing.

jperkin commented 4 years ago

For testing I'm just using bootstrap with the --full argument, this ensures that pdksh is bootstrapped and set as the default shell. If it looks ok then the permanent fix would be adding need_ksh=true to the Darwin setup section, probably being limited to OS releases that ship with SIP.

bartoszkosiorek commented 4 years ago

@jperkin Do you know if the zsh could be used by default for macOS/Darwin, as it is default shell: https://www.theverge.com/2019/6/4/18651872/apple-macos-catalina-zsh-bash-shell-replacement-features ?

jperkin commented 4 years ago

One of the build hosts crashed over the weekend (Catalina is highly unreliable), I'll need to restart it.

As for zsh, I wouldn't recommend it, it's not designed to be a fast POSIX-only shell, but more as a feature-full interactive shell.

bartoszkosiorek commented 4 years ago

I think I found the root cause of my problems: During run bootstrap, I was using SH=/bin/bash environment variable: SH=/bin/bash ./bootstrap --prefix "$PKGSRC_PREFIX" --prefer-pkgsrc yes --workdir "$WORK_DIR" $__UNPRIVILEGED

After removing SH=/bin/bash, and use default shell, most/all of alternatives are using #!/usr/local/pkg/bin/bash shebang (/usr/local/pkg/ is a directory where pkgsrc is installed).

Do you know why it choose the bash instead of sh, as defined here? https://github.com/NetBSD/pkgsrc/blob/trunk/mk/tools/tools.Darwin.mk#L100

The /usr/local/pkg/etc/mk.conf content:

# Example /usr/local/pkg/etc/mk.conf file produced by bootstrap-pkgsrc
# Tue Jun 16 11:12:26 CEST 2020
.ifdef BSD_PKG_MK       # begin pkgsrc settings
ABI=                    64
PKGSRC_COMPILER=        clang
CC=                     clang
CXX=                    clang++
CPP=                    ${CC} -E
CLANGBASE=              /usr
UNPRIVILEGED=           yes
PKG_DBDIR=              /usr/local/pkg/pkgdb
LOCALBASE=              /usr/local/pkg
VARBASE=                /usr/local/pkg/var
PKG_TOOLS_BIN=          /usr/local/pkg/sbin
PKGINFODIR=             info
PKGMANDIR=              man
PREFER_PKGSRC=          yes
MAKE_JOBS=8
SKIP_LICENSE_CHECK=yes
WRKOBJDIR=/private/var/tmp/pkgsrc-obj
X11_TYPE=modular
RUBY_VERSION_DEFAULT=25
ALLOW_VULNERABLE_PACKAGES=yes
PREFER_PKGSRC=  yes
PKG_OPTIONS.libfetch+= openssl
INSTALL_UNSTRIPPED=no
STRIP_BINARIES=yes
PKGSRC_USE_RELRO=full
BUILDLINK_USE_HARDLINKS=no
.endif                  # end pkgsrc settings
bartoszkosiorek commented 4 years ago

@jperkin Do you find any issue with building ksh by default in bootstrap?

jperkin commented 4 years ago

Bootstrap will default to /bin/sh (i.e. bash) unless you override SH in the bootstrap environment. If it's using /usr/local/pkg/bin/bash then that will be from a setting you have somewhere (you mentioned earlier about patching things so it's probably from that).

The bulk build isn't finished yet, they usually take around 3 days to complete on macOS from scratch.

jperkin commented 4 years ago

FWIW it's currently at 17380/22794, producing 13451 packages, so looks reasonable so far.

bartoszkosiorek commented 4 years ago

Thanks. I found an issue in my internal scripts.

After enabling need_ksh on Darwin, the shell used by pkgsrc (pdksh) will be completely independent from what is installed on system (etc. /bin/bash or /bin/zsh).

So it will resolve issue with: https://support.apple.com/en-us/HT208050

Apple could remove /bin/bash and /bin/zsh and the pkgsrc will be still working with shells/pdksh.

Am I right?

jperkin commented 4 years ago

I've posted the results of the bulk build along with a patch for bootstrap here for review: http://mail-index.netbsd.org/tech-pkg/2020/06/18/msg023387.html

bartoszkosiorek commented 4 years ago

@jperkin Thanks for your patch!

Could you please enable need_ksh=yes as default, and disable it only on old systems? Otherwise the versions needs to be updated once the new macOS version will be released.

Something similar to: https://github.com/NetBSD/pkgsrc/pull/67/files

bartoszkosiorek commented 4 years ago

@jperkin I think every package, which use pkg_alternatives, should have running dependency on shells/pdksh shell. Otherwise, it could happen that the application wrapper couldn't be run due to missing pdksh.

jperkin commented 4 years ago

That would already be handled on account of it being a BOOTSTRAP_PKG.

bartoszkosiorek commented 4 years ago

After setting need_ksh, almost all +INSTALL scripts are using default shell (for macOS it will be pdksh). For example pkg_alternatives: https://github.com/NetBSD/pkgsrc/blob/trunk/pkgtools/pkg_alternatives/INSTALL

But for some packages, the shell is hardcoded. For example for pkg_install: https://github.com/NetBSD/pkgsrc/blob/trunk/pkgtools/pkg_install/INSTALL#L1

Do you know the reason for that? Why the default shell is not used for pkg_install?

jperkin commented 4 years ago

I think just an oversight, though it's probably a good idea to be as safe as possible for critical packages and there's nothing in the pkg_install INSTALL script that would cause problems for any Bourne shell.

bartoszkosiorek commented 4 years ago

What else needs to be done to enable ksh by default for macOS bootstrap?

jperkin commented 4 years ago

After running into a separate issue in pdksh I went an alternative route and implemented mksh as a bootstrap shell. I'm just waiting on review of http://mail-index.netbsd.org/tech-pkg/2020/06/29/msg023478.html and will, once that's imported, switch macOS over to it.

Mitsos101 commented 4 years ago

The issue is with pkg_alternatives, not SIP.

Environment variables passed to the wrapper script are not intended for the shell, but the process it will spawn. This is the reason SIP does not allow passing DYLD_* environment variables directly to the shell. Replacing /bin/bash with mksh does not solve the issue where environment variables intended for the child are also passed to the parent. Fixing this issue is the responsibility of pkg_alternatives, not pkgsrc. Commit https://github.com/NetBSD/pkgsrc/commit/8fc2bb66534de42d08fb9f93333af6904f6c6782, currently the 7th largest commit by number of insertions in the pkgsrc repository, is merely part of a temporary fix that does nothing to address the potentially harmful use of DYLD_* variables.

For the reasons outlined above, all commits related to this issue should be reverted.

jperkin commented 4 years ago

Regardless of whether there are additional issues with pkg_alternatives that need resolving, there are still legitimate reasons for switching to a non-system shell on SIP-enabled systems, and this is not going to be reverted.

jperkin commented 4 years ago

Closing this one out for now after mksh was imported and set as the bootstrap shell, if there are additional issues in pkg_alternatives that need looking at then they can be logged in separate tickets. Thanks.

Mitsos101 commented 4 years ago

I have commented on https://github.com/NetBSD/pkgsrc/commit/9545f5f3f93a0d2594c8db1fee928504e271dbc7 as not to clutter the discussion on this issue.

bartoszkosiorek commented 4 years ago

@jperkin Thank you very much Jonathan for fixing this issue. It is fixing all our SIP related issues for all projects in my company.