[bug#36021] [PATCH] search-paths: 'environment-variable-definition' output for fish

[Dan Frumin <dfrumin@cs.ru.nl>]

Hi,

On 04-06-19 05:53, Meiyo Peng wrote:
> Hi Dan,
> 
> Dan Frumin writes:
> 
>>>> Some background on this patch:
>>>> Right now whenever I do any Guix operation that requires me to modify
>>>> environment variables (e.g. installing a Guile library requires me to update
>>>> $GUILE_LOAD_PATH afterwards), Guix helpful tells me what commands I have to run
>>>> to update the variables.
>>>>
>>>> However, those commands are currently in bash/POSIX(?) format `export
>>>> VAR=VALUE`. I've modified the `environment-variable-definition` function to
>>>> support the syntax for Fish shell as well. I don't know if this method of
>>>> looking at the $SHELL variable is sound, but it works  on my machine.
>>>>
>>>> Documentation for the `set' function in Fish: https://fishshell.com/docs/current/commands.html#set
>>>>
>>>> PS: this is my first non-package patch for Guix so I apologize if there is something wrong with the patch
>>>
>>> I think this patch will cause more trouble than good.  Does this patch
>>> replaces etc/profile with fish syntax script?  etc/profile is supposed
>>> to be in POSIX shell syntax.  Fish shell has it's own configuration
>>> files in /etc/fish/config.fish and /etc/fish/conf.d/*.fish.  If you
>>> really want to generate fish syntax scripts, please put them in
>>> /etc/fish/config.fish or /etc/fish/conf.d/*.fish.
>>>
>>
>> No, this patch doesn't touch etc/profile in any way, it is only concerned with the output that you get from `guix package` on your terminal.
>> So I guess it's kind of orthogonal to the upgraded fish package that you made?
> 
> Alright.  Then that's fine.  But there is an on going discussion about
> change content of the message in bug #35942.  I suggest we hold this
> patch until that bug is closed.
> 


Sure, makes sense to me.

>>> See:
>>> 1. https://lists.gnu.org/archive/html/guix-devel/2019-01/msg00071.html
>>> 2. https://lists.gnu.org/archive/html/guix-devel/2019-01/msg00333.html
>>> 3. https://issues.guix.gnu.org/issue/34153
>>>
>>> Have you tried the fish-foreign-env package?  You can source bash
>>> scripts in fish via `fenv source ~/.guix-profile/etc/profile`.  That's how
>>> the fish shell in Guix set up environment variables upon login.
>>>
>>
>> Thanks! This is quite nice, I should upgrade my Fish installation.
> 
> Your fish shell is quite old.  Older than Guix 1.0.0!
> 
>>> Perhaps we can handle this better.  Do you have any suggestions?  My
>>> idea:
>>>
>>> 1. The fish-foreign-env package is very useful but it's not installed by
>>> default in order to avoid polluting user's profiles according to the
>>> Guix convention.  In this case I think it's a useful utility rather than
>>> pollution.  Do you want it to be installed together with the fish
>>> package by default?
>>
>> Well, you added it as part of the fish dependency, right? I think it makes sense, especially on Guix SD.
> 
> Yes, it's a dependency of fish.  But it's not a propagated input.  We
> can change it into a propagated input if most users agree.
> 
>>> 2. etc/profile is supposed to be sourced by a login shell.  And we
>>> should avoid sourcing it from a non-login shell.  That's why I made the
>>> fish package in Guix only sources it upon login.  That means starting a
>>> new fish shell won't source it automatically.  So please source it
>>> manually if you need the new environment variables (fenv source
>>> ~/.guix-profile/etc/profile).
>>
>> I am not knowledgeable enough to comment on this, but it seems to me that it is
>> done similarly if you are using bash: e.g. you have to source the profile
>> yourself if you want to get it in a non-login shell.
>>
>>>
>>> 3. We can write better documentations explaining how to deal with fish
>>> shell in Guix.  There's an old discussion about having a Guix wiki or
>>> a Guix book.
>>>
>>
>> Yeah, more documentation is always better :)
> 
> 
> --
> Meiyo Peng
> https://www.pengmeiyu.com/
>