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

[fish shell] /bedrock/run/fprofile overwrites fish inherited path and userpath #202

Open fezboy opened 3 years ago

fezboy commented 3 years ago

The expected behavior when starting fish is that it inherits the path from the environment that invoked it, which is then prepended with the contents of $fish_user_paths, a user variable used to add to one's path, instead of setting it in a .profile. This is a paste of expected behavior from my vanilla Arch machine: https://pastebin.com/fv0eMKU3

Under Bedrock, this seems to be wholly replaced by line 24 of /bedrock/run/fprofile

This is the behavior I observe on my bedrock 0.7.18beta4 machine, the error being the missing perl, jvm, and user entries:

[fez@archpad ~]$ echo $PATH
/bedrock/cross/pin/bin:/bedrock/bin:/usr/local/bin:/usr/local/sbin:/opt/bin:/opt/sbin:/usr/bin:/usr/sbin:/bin:/sbin:/snap/bin:/bedrock/cross/bin:/usr/lib/jvm/default/bin:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl
[fez@archpad ~]$ fish
Welcome to fish, the friendly interactive shell
Type `help` for instructions on how to use fish
[I] ⋊> ~ echo $fish_user_paths                                                           22:36:55
/usr/local/bin /home/fez/bin
[I] ⋊> ~ echo $PATH                                                                      22:36:58
/bedrock/cross/pin/bin /bedrock/bin /usr/local/bin /usr/local/sbin /opt/bin /opt/sbin /usr/bin /usr/sbin /bin /sbin /snap/bin /bedrock/cross/bin
paradigm commented 3 years ago

I see the issue; it's a really silly mistake.

The br_dedup_envvar function in /bedrock/run/fprofile in 0.7.18beta4 was explicitly written with the goal of not wholly replacing the envvar, but rather only prepending and appending the specified items to it. However, I completely failed to actually feed the incoming envvar into it. Here's the bash equivalent:

export PATH="$(br_dedup_envvar "/bedrock/cross/pin/bin:/bedrock/bin::/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin:/bin:/sbin:${PATH}" "/bedrock/cross/bin")"

Note ${PATH} there at the end of the first argument to br_dedup_envvar. When I translated this to fish:

set -x PATH (br_dedup_envvar "/bedrock/cross/pin/bin:/bedrock/bin::/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin:/bin:/sbin" "/bedrock/cross/bin")

I just completely left it out.

Should be an easy fix. I'll try to include it into 0.7.18.beta5.

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.