Created attachment 429476 [details, diff] portage-2.2.28-env-update-fish-support.patch Exporting variables works slightly different in app-shells/fish, especially for the PATH variable. Hence /etc/profile.env cannot be sourced by the fish shell. Attached patch makes env-update generate an /etc/profile.fish file that adheres to fish shell's syntax. This one possible solution to bug #545830.
Created attachment 429478 [details, diff] portage-2.2.28-env-update-fish-support.patch
Created attachment 429480 [details, diff] portage-2.2.28-env-update-fish-support.patch
Any chance of getting this merged before 2.3? The patch still applies cleanly to sys-apps/portage-2.3.0_rc1. The generated /etc/profile.fish file still works with app-shells/fish-2.3.0.
(In reply to Dennis Schridde from comment #2) > Created attachment 429480 [details, diff] [details, diff] > portage-2.2.28-env-update-fish-support.patch The patch looks pretty reasonable, except for a few things: It should use " ".join instead of string.join (string.join does not exist anymore in python3), like this: env_path = " ".join("'%s'" % v for v in specials["PATH"])) Note that my version also uses specials["PATH"], which is a list containing the elements of PATH. Also, env_keys should not include LDPATH, since that's a special onE that none of the other environment files export. We can re-use the filtered and sorted env_keys variable defined earlier in the function, as follows: for k in (x for x in env_keys if x != "PATH"): outfile.write("set -gx %s '%s'\n" % (k, env[k]))
Created attachment 436018 [details, diff] portage-2.2.28-env-update-fish-support.patch (In reply to Zac Medico from comment #4) > The patch looks pretty reasonable, except for a few things: Thanks for reviewing this! I implemented your suggestions and also took the liberty of porting the string formatting to str.format, which is afaik the preferred way to do this in Python 3. The patched env-update works with both Python 2.7.11 and 3.4.3.
@polynomial-c: As app-shells/fish maintainer, please review. (In reply to Dennis Schridde from comment #5) > Created attachment 436018 [details, diff] [details, diff] > portage-2.2.28-env-update-fish-support.patch The patch looks pretty reasonable now. However, I wonder about the PATH setting: outfile.write("set -gx PATH {!s} $PATH\n".format(env_path)) I guess the $PATH part preserves an earlier PATH setting? Is that really needed? Will that cause the PATH variable to keep growing endlessly if the file is sourced multiple times?
(In reply to Zac Medico from comment #6) > @polynomial-c: As app-shells/fish maintainer, please review. > > (In reply to Dennis Schridde from comment #5) > > Created attachment 436018 [details, diff] [details, diff] [details, diff] > > portage-2.2.28-env-update-fish-support.patch > > The patch looks pretty reasonable now. However, I wonder about the PATH > setting: > > outfile.write("set -gx PATH {!s} $PATH\n".format(env_path)) > > I guess the $PATH part preserves an earlier PATH setting? Yes > Will that cause the PATH variable to keep growing endlessly if the > file is sourced multiple times? Yes I'll remove it.
Created attachment 437932 [details, diff] portage-2.2.28-env-update-fish-support.patch (In reply to Dennis Schridde from comment #7) > (In reply to Zac Medico from comment #6) > > Will that cause the PATH variable to keep growing endlessly if the > > file is sourced multiple times? > > I'll remove it. done
(In reply to Dennis Schridde from comment #8) > done This version requires that following file be included in sys-apps/baselayout (which is where the bash equivalent, /etc/profile, comes from) or app-shells/fish (where the directory /etc/fish/conf.d comes from): ```name=/etc/fish/conf.d/00-profile.fish #!/usr/bin/fish if test -f /etc/profile.fish source /etc/profile.fish end if [ "$EUID" = "0" ] ; or [ "$USER" = "root" ] set -xg PATH /usr/local/sbin /usr/local/bin /usr/sbin /usr/bin /sbin /bin $ROOTPATH else set -xg PATH /usr/local/bin /usr/bin /bin $PATH end set -eug ROOTPATH ``` Otherwise the user will end up in a shell without /usr/bin, /bin and similar in the path.
P.S. This still messes up $fish_user_paths: It is supposed to prepend elements to $PATH, but it appears that /etc/fish/conf.d/*.fish is being sourced after doing this. Thus the 00-profile.fish file, that I pasted earlier, will override any user paths. I "fixed" this by adding the following to 00-profile.fish: ``` Re-prepend $fish_user_paths __fish_reconstruct_path ``` But I assume that more variables might be affected and that the same problem will affect other variable injection mechanisms like pam_env. Thus someone with more experience in this area should have a look at it.
Created attachment 438830 [details, diff] portage-2.2.28-env-update-fish-support.patch 00-profile.fish also needs a changed patch to env-update to include ROOTPATH in the logic.
Created attachment 438832 [details] 00-profile.fish
I wanted to bump this bug. patch works for me, but I'm not sure it's correct way to solve the problem. I currently maintain a couple of fish related projects, this includes packaging for MacOS and it's brew package manager, and familiar with fish. here are my points 1) according to upstream fish, file 00-profile.fish should go to: /usr/share/fish/vendor_conf.d It's a special directory for distro vendor provided configurations not meant to be edited by regular users or admins. see: https://github.com/fish-shell/fish-shell/issues/1956#issuecomment-140583144 http://fishshell.com/docs/current/index.html#initialization 2) env-update should not explicitly support fish. there is no reason to have this functionality in portage by default and no reason to add new code. no reason to have an additional file in baselayout if a user never installs fish.fish can take care of itself. 3) fish ebuild should install a file to /usr/share/fish/vendor_conf.d that will read existing /etc/profile.env and parse it. here is an example https://github.com/gyakovlev/portage-overlay/tree/master/app-shells/fish
Created attachment 459674 [details] modified fish ebuild
Created attachment 459676 [details] /etc/profile.env parser for fish
(In reply to Georgy Yakovlev from comment #13) > 2) env-update should not explicitly support fish. there is no reason to have > this functionality in portage by default and no reason to add new code. no > reason to have an additional file in baselayout if a user never installs > fish.fish can take care of itself. That seems like a very good solution. (In reply to Georgy Yakovlev from comment #15) > Created attachment 459676 [details] > /etc/profile.env parser for fish Here's a tigher grep expression to ensure that only the desired lines are filtered: egrep -v "^(#|export (PATH|ROOTPATH)=)" /etc/profile.env
bug #545830 now has a solution (implemented in app-shells/fish-2.4.0-r1), which is almost correct and does not require a modification of sys-apps/portage (env-update). When/if bug #545830 comment #14 is addressed, this bug here does not contain any additional value over the solution proposed there. *** This bug has been marked as a duplicate of bug 545830 ***
solution does non provide handling /etc/profile.d/*.sh files. there are about 21 ebuilds in portage which utilize /etc/profile.d/*.sh files to add customization and some other ebuilds that I cannot simply grep. >app-shells/autojump/autojump-22.2.4-r4.ebuild: sed -e "s: \(/etc/profile.d\): \"${EPREFIX}\1\":" \ >app-shells/autojump/autojump-22.2.4-r4.ebuild: insinto /etc/profile.d >dev-vcs/colorcvs/colorcvs-1.4-r1.ebuild: insinto /etc/profile.d >gnustep-base/gnustep-make/gnustep-make-2.6.2.ebuild: exeinto /etc/profile.d >gnustep-base/gnustep-make/gnustep-make-2.6.6.ebuild: exeinto /etc/profile.d >gnustep-base/gnustep-make/gnustep-make-2.6.7-r1.ebuild: exeinto /etc/profile.d >gnustep-base/gnustep-make/gnustep-make-2.6.7.ebuild: exeinto /etc/profile.d >gnustep-base/gnustep-make/gnustep-make-2.6.8.ebuild: exeinto /etc/profile.d >sci-chemistry/cns/cns-1.2.1-r8.ebuild: insinto /etc/profile.d >sci-chemistry/coot/coot-0.8.2-r1.ebuild: source "${EPREFIX}/etc/profile.d/40ccp4.setup.sh" >sys-apps/nix/nix-1.11.6-r3.ebuild: rm "${ED}"/etc/profile.d/nix.sh || die >sys-apps/nix/nix-1.11.6-r3.ebuild: ewarn "${EROOT}etc/profile.d/nix.sh was removed (due to USE=-etc_profile)." >sys-fs/udisks/udisks-1.0.5-r1.ebuild: rm -f "${ED}"/etc/profile.d/udisks-bash-completion.sh >sys-process/prll/prll-0.6.2.ebuild: insinto /etc/profile.d/ >x11-libs/vte/vte-0.44.2.ebuild: mv "${D}"/etc/profile.d/vte{,-${SLOT}}.sh || die >x11-libs/vte/vte-0.44.3.ebuild: mv "${D}"/etc/profile.d/vte{,-${SLOT}}.sh || die >x11-libs/vte/vte-0.46.1.ebuild: mv "${D}"/etc/profile.d/vte{,-${SLOT}}.sh || die >x11-misc/cdm/cdm-0.5.3.ebuild: insinto /etc/profile.d/ >x11-misc/cdm/cdm-0.6.1_pre20130419.ebuild: insinto /etc/profile.d/ >x11-terms/gnome-terminal/gnome-terminal-3.20.3.ebuild: . /etc/profile.d/vte.sh" >x11-terms/gnome-terminal/gnome-terminal-3.22.1.ebuild: . /etc/profile.d/vte.sh" major examples are: sys-apps/baselayout-java provides some variables like MANPATH, JAVA_HOME and gentoo_system/user_vm handling for eselect. x11-libs/vte provides some terminal quirks x11-terms/gnome-terminal uses above vte as backend. So it's not a good idea to ship a semi-broken solution, unless someone wishes to fix ALL ebuilds above and provide fish equivalent profile.d file where it makes sense.
