bedrocklinux / bedrocklinux-userland

This tracks development for the things such as scripts and (defaults for) config files for Bedrock Linux
https://bedrocklinux.org
GNU General Public License v2.0
603 stars 64 forks source link

XDG_DATA_DIRS issues when fish is the default shell #199

Open GorrillaRibs opened 4 years ago

GorrillaRibs commented 4 years ago

The function br_dedup_envvar in the fish (seems to?) set environment path variables separated by spaces instead of a colon - for most programs this seems to works fine, but gnome stuff (nautilus, gnome-web/epiphany) wouldn't launch with an error about no gsettings schemas being installed on the system (presumable because it couldn't find /usr/share in XDG_DATA_DIRS, as manually setting it makes them work). Bash and zsh work fine when set as the default shell (or run as a login shell), and fish works fine as well when not a default shell.

For now I've replaced the string replace line in the fprofile template with echo $out | sed -e 's/::*/:/g' -e 's/^://' -e 's/:$//' | sed 's/ /:/g', adapted from the zprofile in 'brl-apply', and it seems to be working fine. This, however seems kinda hacky, and could probably be fixed in pure fish - I'm also not sure what the intended behaviour is, and it's possible the sed line has its own problems I haven't seen yet.

What would be the right way to go about fixing fix this? I'm happy to open a pull request, but that seemed like overkill for a one line change :)

Thanks!

paradigm commented 3 years ago

My fish knowledge is relatively weak; not super surprising there's an issue there.

When I first wrote the code you're examining, I attempted to read how fish handles environment variables like $PATH and found that, at least for $PATH, it uses what appear to be space separated lists: https://fishshell.com/docs/current/tutorial.html#lists

I found it does some magic to translate these to the traditional colon-separated strings in some circumstances:

paradigm@euclid ~> echo "$PATH"
/bedrock/cross/pin/bin:/bedrock/bin:/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin:/bin:/sbin:/bedrock/cross/bin
paradigm@euclid ~> echo $PATH
/bedrock/cross/pin/bin /bedrock/bin /usr/local/bin /usr/local/sbin /usr/bin /usr/sbin /bin /sbin /bedrock/cross/bin

What I did not test until I read your issue here was how consistently it does this magic. It seems that works for $PATH as shown above, but not $XDG_DATA_DIRS:

paradigm@euclid ~> echo $XDG_DATA_DIRS
/bedrock/cross/pin /usr/local/share /usr/share /bedrock/cross
paradigm@euclid ~> echo "$XDG_DATA_DIRS"
/bedrock/cross/pin /usr/local/share /usr/share /bedrock/cross

Experimenting a bit, I suspect it looks for PATH in the envvar name. I appended PATH to $XDG_DATA_DIRS and found it works:

paradigm@euclid ~> echo $XDG_DATA_DIRSPATH
/bedrock/cross/pin /usr/local/share /usr/share /home/paradigm/.local/share/flatpak/exports/share /var/lib/flatpak/exports/share /bedrock/cross
paradigm@euclid ~> echo "$XDG_DATA_DIRSPATH"
/bedrock/cross/pin:/usr/local/share:/usr/share:/home/paradigm/.local/share/flatpak/exports/share:/var/lib/flatpak/exports/share:/bedrock/cross

I was briefly worried we'd have to start special casing things, but it seems fish handles colon-separated stuff fine and, from Bedrock's point of view, we can ignore the fish List-ism:

paradigm@euclid ~> echo $PATH
/bedrock/cross/pin/bin /bedrock/bin /usr/local/bin /usr/local/sbin /usr/bin /usr/sbin /bin /sbin /bedrock/cross/bin
paradigm@euclid ~> echo "$PATH"
/bedrock/cross/pin/bin:/bedrock/bin:/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin:/bin:/sbin:/bedrock/cross/bin
paradigm@euclid ~> set PATH "$PATH"
paradigm@euclid ~> echo $PATH
/bedrock/cross/pin/bin /bedrock/bin /usr/local/bin /usr/local/sbin /usr/bin /usr/sbin /bin /sbin /bedrock/cross/bin
paradigm@euclid ~> echo "$PATH"
/bedrock/cross/pin/bin:/bedrock/bin:/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin:/bin:/sbin:/bedrock/cross/bin

Given this, I'm in agreement we can just make this more like zprofile and build :-separated output.


Coincidentally, the work for the upcoming release is focused on envvar handling already. I'll see if I can include this fix while working on that.

We're currently on 0.7.18beta4; I'll see if I can get this into 0.7.18beta5, probably sometime next week. Once that's out, you can try it early via the beta channel to confirm it resolves things for you, or just wait until 0.7.18 lands in stable.

fezboy commented 3 years ago

Expanding on the use of spaces instead of colons in $PATH, where BASH and other shells store $PATH as a string of colon separated paths, fish stores $PATH as a list of strings, where each string is a single path. Fish inherits it's path from whichever environment invoked it, and magicks the previous environment's $PATH into a list, and vice versa back to a colon separated string when passed to subcommands.

I'd recommend reading the fish tutorial's section on lists for more info: https://fishshell.com/docs/current/tutorial.html#lists

paradigm commented 3 years ago

Should be fixed in 0.7.18beta5 that I just pushed. If you have the time, please try it out and confirm for me that things now act as you expect.

GorrillaRibs commented 3 years ago

I'll give it a try now, thanks a ton!

GorrillaRibs commented 3 years ago

...and I can confirm it works! For reference my XDG_DATA_DIRS is now XDG_DATA_DIRS=/bedrock/cross/pin:/usr/local/share:/usr/share:/home/loto/.local/share/flatpak/exports/share:/var/lib/flatpak/exports/share:/usr/share/gdm:/bedrock/cross:/home/loto/.local/share/flatpak/exports/share which is the same as with bash or zsh as my default shell, so I've swapped back to fish.

paradigm commented 3 years ago

Excellent :)