Since fish is a bit alternative shell which has specific startup files, it does not honor /etc/profile which loads /etc/profile.env (Gentoo-specific file). Maybe we should have a distro-level way to set those variables on interactive shell startup. It could be optional. Best way is to patch /etc/fish/config.fish. I do it in my own config like this: === FRAGMENT OF ~/.config/fish/config.fish === function export set variable (echo $argv|sed -E 's/(.+)=(.+)/\1/') set value (echo $argv|sed -E 's/(.+)=(.+)/\2/') set -gx $variable $value end . /etc/profile.env === END === There is also more complex question of setting correct PATH which has different syntax in fish... Reproducible: Always
See-Also: bug #578826
The function you propose seems to be part of app-shells/fish-2.2.0: # cat /usr/share/fish/functions/export.fish function export --description 'Set global variable. Alias for set -g, made for bash compatibility' if test -z "$argv" set return 0 end for arg in $argv set -l v (echo $arg|tr '=' \n) set -l c (count $v) switch $c case 1 set -gx $v $$v case 2 set -gx $v[1] $v[2] end end end # q file -v /usr/share/fish/functions/export.fish app-shells/fish-2.2.0 (/usr/share/fish/functions/export.fish)
Created attachment 459678 [details] app-shells/fish-2.4.0-r1 ebuild
Created attachment 459680 [details] fish /etc/profile.env parser
Hey, it seems I managed to make fish respect gentoo env. I've attached an updated ebuild and parser. the code can be viewed here for readability. https://github.com/gyakovlev/portage-overlay/tree/master/app-shells/fish
commit 867eb172854e2965a1992c802d10fd3993769170 Author: Lars Wendler <polynomial-c@gentoo.org> Date: Fri Jan 13 06:02:25 2017 app-shells/fish: Attempt to load system-wide variables (bug #545830). Use system pcre2 lib instead of bundled one. Package-Manager: Portage-2.3.3, Repoman-2.3.1 Please report back if that works for you.
thanks for code and ebuild fixes. profile-env part works. not sure if it's worth opening another bug for that: About the USE=X. It's unused in 2.4.0-r1 ebuild. previous ebuild used to pull xsel if USE=X is set. it breaks CTRL+Y to copy CTRL+V to paste https://github.com/fish-shell/fish-shell/blob/master/share/functions/fish_clipboard_paste.fish https://github.com/fish-shell/fish-shell/blob/master/share/functions/fish_clipboard_copy.fish not a big deal on a terminal with copy on highlight, but should be taken into account. maybe get rid of X useflag and reuse existing USE=clipboard ? some ebuilds pull xsel if USE=clipboard is set (neovim and couple others) another option is to add a notice to pkg_postinst(), the same way it mentions compressed man pages support.
commit 2ea867b6bfa846658ce115ef63a99d9121d5183c Author: Lars Wendler <polynomial-c@gentoo.org> Date: Fri Jan 13 11:35:50 2017 app-shells/fish: ebuild cleanup. Added an elog message about cut'n'paste issues that can be compensated by installing x11-misc/xsel. Package-Manager: Portage-2.3.3, Repoman-2.3.1
Due to this change, fish now ignores whatever PATH it gets passed as an environment variable. Is this intended? This behaviour breaks a lot of my scripts. Couldn't you instead append/prepend to the existing PATH?
(In reply to Luis Ressel from comment #9) > Due to this change, fish now ignores whatever PATH it gets passed as an > environment variable. Is this intended? This behaviour breaks a lot of my > scripts. Couldn't you instead append/prepend to the existing PATH? Can you please give an example how you set your PATH?
(In reply to Georgy Yakovlev from comment #10) > (In reply to Luis Ressel from comment #9) > > Due to this change, fish now ignores whatever PATH it gets passed as an > > environment variable. Is this intended? This behaviour breaks a lot of my > > scripts. Couldn't you instead append/prepend to the existing PATH? > > Can you please give an example how you set your PATH? I once had `set -U PATH ...`, which broke with app-shells/fish-2.4.0-r1, as the PATH from the store (I believe `$HOME/.config/fish/fishd.(hostname)` was now overridden by `/usr/share/fish/vendor_conf.d/00-profile-env.fish` and the original value was lost. This was very easy to fix by prepending `$HOME/.config/fish/config.fish` with `set PATH $HOME/bin $HOME/.local/bin $PATH`, just as one would have done in e.g. app-shells/bash by editing `$HOME/.bashrc` instead of using fish's universal variables.
Basically, my local config.fish file contains "status --is-login; and set -gx PATH {$PATH} $HOME/bin". Now, let's say I have a ~/bin/tmp script reading " #!/bin/bash echo $PATH ", this will report the correct (customized) path. If I change the interpreter to /bin/fish, the script will still echo the correct path as long as <=app-shells/fish-2.4.0 is installed. However, after I upgraded to fish-2.4.0-r1, this stopped working; the script will now print the default PATH without $HOME/bin tacked on to the end. In my opinion, only login shells should ever change the PATH variable; everything else should just use the PATH provided by the environment.
(In reply to Dennis Schridde from comment #11) > (In reply to Georgy Yakovlev from comment #10) > > (In reply to Luis Ressel from comment #9) > > > Due to this change, fish now ignores whatever PATH it gets passed as an > > > environment variable. Is this intended? This behaviour breaks a lot of my > > > scripts. Couldn't you instead append/prepend to the existing PATH? > > > > Can you please give an example how you set your PATH? > > I once had `set -U PATH ...`, which broke with app-shells/fish-2.4.0-r1, as > the PATH from the store (I believe `$HOME/.config/fish/fishd.(hostname)` was > now overridden by `/usr/share/fish/vendor_conf.d/00-profile-env.fish` and > the original value was lost. Correction: I was actually using `$fish_user_paths`, which was broken by app-shells/fish-2.4.0-r1.
(In reply to Georgy Yakovlev from comment #5) (In reply to Dennis Schridde from comment #13) Could you please have another look at attachment #438832 [details] from bug #578826? it contains these lines at the end, which I think are very important: ``` # Re-prepend $fish_user_paths __fish_reconstruct_path ```
*** Bug 578826 has been marked as a duplicate of this bug. ***
P.S. The documentation of `$fish_user_paths` is here: http://fishshell.com/docs/current/tutorial.html#tut_path
ok, It seems I adding __fish_reconstruct_path solves non-working fish_user_paths also adding a new condition allows not to overwrite inherited variables if run as non-login shell. I've opened an issue upstream to verify if there are any problems with this approach https://github.com/fish-shell/fish-shell/issues/3736 meanwhile, here is new file attached in the next comment.
Created attachment 460236 [details] improved profile.env parser
Created attachment 460238 [details] improved profile.env parser, fixed forgot to uncomment the lines.
Created attachment 460240 [details] improved profile.env parser, fixed
Created attachment 460242 [details] improved profile.env parser, uncommented I'm sorry, for some reason file uploaded as commented one several times.
Created attachment 460358 [details] profile.env parser using /bin/sh and /bin/env (In reply to Georgy Yakovlev from comment #21) > Created attachment 460242 [details] > improved profile.env parser, uncommented > > I'm sorry, for some reason file uploaded as commented one several times. To be honest I think it is a bad idea to parse sh-syntax using grep/sed. I propose this parser, but honestly the cleanest way would probably be to add fish support to env-update.
(In reply to Wilke Schwiedop from comment #22) > Created attachment 460358 [details] > profile.env parser using /bin/sh and /bin/env > > (In reply to Georgy Yakovlev from comment #21) > > Created attachment 460242 [details] > > improved profile.env parser, uncommented > > > > I'm sorry, for some reason file uploaded as commented one several times. > > To be honest I think it is a bad idea to parse sh-syntax using grep/sed. > I propose this parser, but honestly the cleanest way would probably be to > add fish support to env-update. That would be what I did in bug #578826, because I felt the same. But Portage maintainers do not seem to like adding support for more shells to env-update living in sys-apps/portage (bug #578826 comment #16). Maybe env-update should be pulled into an own package to decouple portage from unrelated tasks?
Created attachment 460452 [details] profile.env parser using /bin/sh and /bin/env (In reply to Dennis Schridde from comment #23) > That would be what I did in bug #578826, because I felt the same. But > Portage maintainers do not seem to like adding support for more shells to > env-update living in sys-apps/portage (bug #578826 comment #16). Maybe > env-update should be pulled into an own package to decouple portage from > unrelated tasks? In general I agree, but I can understand the portage dev's stance. Supporting every possible shell syntax out there is a hassle, considering all you want is set up an initial environment. Personally I'd use an envdir, but sh with exports works just as well since sh is mandatory on every gentoo system.
Created attachment 460480 [details] parser using fish only Here is another parser using fish only as suggested by fish developers. Using csh env file, because it's cleaner and easier to parse. A lot of shell tools in the wild support fish that way (ssh-agent, keychain one of them) I'm not that great at regex, need someone to check it if there any way to make it more robust. It works for me, it re-uses fish_user_paths, it does not overwrite path if run non-interactively.
Created attachment 460482 [details] parser using fish only 2 small fix to set -e scope
(In reply to Georgy Yakovlev from comment #25) > I'm not that great at regex, need someone to check it if there any way to > make it more robust. The most robust way is not to use regex at all.
Hey guys, just FYI, I'm following your discussion and I do _not_ ignore this issue. I am just waiting for you to come to a solution that satifies everyone best and will then add that solution to portage. Unfortunately I cannot take part in finding a satifying solution as I have quite some private life duties that keep me quite distracted. Thanks for helping with this issue :)
(In reply to Wilke Schwiedop from comment #27) > (In reply to Georgy Yakovlev from comment #25) > > I'm not that great at regex, need someone to check it if there any way to > > make it more robust. > The most robust way is not to use regex at all. Why regex much worse that parsing that file in python with env-update? What is the difference from technical point of view? Honest question, don't take it as offence. I'm just curious. My view is that env-update parses the path for us and we just consume it's output. With csh syntax we only need to strip colons and single '. That's relatively simple task. IF you want to see how env-update handles it, try adding SOMETHING=${VARNAME} into /etc/env.d file It does not handle it at all, and you'll have literal SOMETHING=${VARNAME}, it will not expand VARNAME # cat /etc/env.d/99test TESTVAR="/tmp" BREAKTEST="${TESTVAR}" # grep TEST /etc/profile.env export BREAKTEST='${TESTVAR}' export TESTVAR='/tmp'
and fish script with fish-only parser completely ignores this BREAKTEST='${TESTVAR}'. It does not get exported into environment. bash on the other hand exports it literally just as seen in the file.
(In reply to Georgy Yakovlev from comment #29) > Why regex much worse that parsing that file in python with env-update? > What is the difference from technical point of view? > […] > My view is that env-update parses the path for us and we just consume it's > output. With csh syntax we only need to strip colons and single '. > That's relatively simple task. I think you misunderstand something. env-update does not parse profile.env and csh.env. It parses /etc/env.d/* and writes profile.env and csh.env. Is using regex to parse csh.env much worse than what what env-update does? Well first of all I don't think that what env-update does is a good idea for the reasons you pointed out: It looks like sh, but isn't and then users are suprised when things don't work as they might expect. Using regex to parse profile.env (or csh.env) is worse because it's just another point where things can go wrong or not work as expected. But if you use sh (or csh) to parse the files and things go wrong then it is most likely a problem with env-update.
(In reply to Wilke Schwiedop from comment #31) > (In reply to Georgy Yakovlev from comment #29) > > Why regex much worse that parsing that file in python with env-update? > > What is the difference from technical point of view? > > […] > > My view is that env-update parses the path for us and we just consume it's > > output. With csh syntax we only need to strip colons and single '. > > That's relatively simple task. > I think you misunderstand something. > env-update does not parse profile.env and csh.env. > It parses /etc/env.d/* and writes profile.env and csh.env. > Is using regex to parse csh.env much worse than what what env-update does? > Well first of all I don't think that what env-update does is a good idea for > the reasons you pointed out: It looks like sh, but isn't and then users are > suprised when things don't work as they might expect. Using regex to parse > profile.env (or csh.env) is worse because it's just another point where > things can go wrong or not work as expected. But if you use sh (or csh) to > parse the files and things go wrong then it is most likely a problem with > env-update. I understand how env-update works, sometimes I have problems expressing myself in written English clearly enough =) /etc/env.d/* -> env-update -> /etc/profile.env|csh.env -> fish Well, Gentoo is for power users, no exclusions here. It's not novice friendly. We should not try protect a power user from himself. After 1 or 2 times new user learns that env.d and profile.d are different and learns how to do things Gentoo way. With my (limited) testing fish behaves even safer than bash with my parser, as it ignores uncertain input. I'm sure there are many interesting ways to break that kind of parser, but honestly, I use gentoo for a really long time and I have never seen anything breaking because of env.d files, except for then I put something bad myself in there. I tried to put some silly things into env.d, and env-update does have some protection and skips that input. It'd be really scary to parse executable code, something like /etc/profile.d, but it's not the case. This bug title reads "provide a way to load system-wide variables (profile.env) on start", which we did. Adding code to portage/baselayout is cleaner, but significantly harder to push into tree. Let the maintainer decide which parser to use. my vote for fish-only parser, because it does not rely on external binaries. sh/env trick is also interesting. Someone can still try push a change to portage for direct fish support (not me, sorry), but meanwhile new users will have working env with fish using this reasonable workaround.
(In reply to Georgy Yakovlev from comment #32) > With my (limited) testing fish behaves even safer than bash with my parser, > as it ignores uncertain input. I hate do disagree, but I don't think there is anything safe about throwing a file with csh syntax into the fish parser. However if we just assume that csh.env only consists of setenv-calls we should be able to source it without any preprocessing. The only issue then is that fish handles PATH differently. However in that case I think fish's setenv function should be patched to do that handling of the PATH variable, since as far as I can tell it is supposed to be a csh-compatibility-function. So setenv PATH /foo:/bar should be translated to set -gx PATH /foo /bar
Created attachment 460670 [details, diff] setenv with PATH compatibility
Created attachment 460672 [details] csh.env parser using the patched setenv To clarify our Gentoo-provided fish initialization would then look like this.
(In reply to Georgy Yakovlev from comment #32) > Someone can still try push a change to portage for direct fish support (not > me, sorry), but meanwhile new users will have working env with fish using > this reasonable workaround. That code has already been written and it works: bug #578826. It has been reviewed by the portage maintainer (Zac Medico) and all of his concerns have been addressed. Please correct me, if I am wrong.
Any news here? I'd like to fix this bug with fish-2.5.0 (which has already been released).
(In reply to Lars Wendler (Polynomial-C) from comment #37) > Any news here? I'd like to fix this bug with fish-2.5.0 (which has already > been released). I have nothing to add. my latest proposed parser is https://545830.bugs.gentoo.org/attachment.cgi?id=460482 Pure fish solution which reuses /etc/csh.env. I've been living with it for more some time and it shows no regressions in my limited testing. Wilke Schwiedop also offered some ways of loading env. It either involves using /bin/sh and /bin/env, or with patching fish provided functions, both of which I don't really like. Code does look cleaner, but I'd rather skip patching upstream provided functions unless changes are merged upstream. portage support would be cleaner, fish can use plugin meanwhile if someone is willing to take the lead on integrating existing solution into portage https://bugs.gentoo.org/show_bug.cgi?id=578826 Seems portage devs liked that parser solution and closed the bug.
(In reply to Georgy Yakovlev from comment #38) > https://bugs.gentoo.org/show_bug.cgi?id=578826 > Seems portage devs liked that parser solution and closed the bug. Actually it was me, the author of the portage patches, who closed it in bug #578826 comment #17, assuming that another solution was already merged into the fish ebuild in this bug, comment #8. I reopened bug #578826 later, when I understood that was not the case and there was still discussion as to what would be the correct solution.
commit e99a31824bb89ed9ea45649d6b4a602fc922d8a3 Author: Lars Wendler <polynomial-c@gentoo.org> Date: Mon Feb 6 01:48:53 2017 app-shells/fish: Bump to version 2.5.0. Removed old. Package-Manager: Portage-2.3.3, Repoman-2.3.1 I've changes the env file to what Georgy suggested in comment #38. Let's see if people are happy now.
issues found so far: 1) If a dir in path in either empty or inaccessible fish will print a warning. /opt/bin and /usr/games/bin (if user is not in games group) are examples. It's pretty harmless. 2) Gentoo Java handling can be broken, as /etc/profile.d/java-config* is not read/executed by fish. 3) as a part of 2) virtually anything in /etc/profile.d will be broken in fish. (vte, gnome-terminal, udisks:1 are major ones, at least 20 more ebuilds are affected) also 2) and 3) greatly depends on how a user logs into session. for me it's broken on the console but works in X, as login manager picks up the env. but it's not consistent and not every login manager handles it the same way. the cleanest way of loading fish and respecting all profile startup dirs is adding # keep this line at the very bottom of ~/.bashrc exec /bin/fish at the bottom of user's .bashrc and keeping bash as user shell. https://wiki.archlinux.org/index.php/Fish#Not_setting_fish_as_default_shell maybe get rid of the parser and just add a postinst message of that trick? benefits are: 1) all env is read and handled by bash and just propagates to fish. no need to handle anything. 2) .bashrc startup still can be used for ssh-agent or any other conditional stuff, without the need to handle fish config files separately. just keeping exec fish at the bottom of the bashrc is enough. 3) user still can drop to bash by running bash --norc (whis can be mentioned in postinst) 4) it works if you ssh into the machine. 5) it should still work with kde/other de autostart scripts, but I have no kde installed and cannot test. I'm still searching for downsides.
small addition. the last line in ~/.bashrc should be SHELL=/bin/fish exec /bin/fish (or subshell breaks in some apps, mc for example)
some news. as of this commit https://github.com/fish-shell/fish-shell/commit/f38646593ca2add583b83465f31c51b234f5ce28 fish's export wrapper will support semicolon delimeted PATH-type variables, making possible to parse profile.env almost directly without any hacks. but it still does not adress /etc/profile.d and I thinks it's better to ditch the parser and launch fish using bash Everything is documented on wiki: https://wiki.gentoo.org/wiki/Fish#Caveats I'll remove the parser and send a pull request a bit later after I address another long-standing bug with locales.
From e2fd0ed7efae55eadfe15572cb6ff0883ae42fcf Mon Sep 17 00:00:00 2001 From: Georgy Yakovlev <ya@sysdump.net> Date: Thu, 23 Feb 2017 22:27:53 -0800 Subject: [PATCH] app-shells/fish: remove profile.env parser This removes profile.env parser and modifies postinst message advising to use bash to launch fish. Fixes bug 545830 Closes: https://github.com/gentoo/gentoo/pull/4072 --- Closing as CANTFIX. While profile.env parser works, where is no reasonable solution exists for fish profile.d support. Shipping partially working solution is not good. Anyone feel free to use parsers attached to this bug if direct profile.env support is required without via launching bash. Workaround is documented on wiki: https://wiki.gentoo.org/wiki/Fish#Caveats
