bug#30265: Fish shell has wrong path variables

bug#30265: Fish shell has wrong path variables

[Meiyo Peng]

Hi,

I am using GuixSD 0.14. After upgrading fish shell to latest version(v2.7.1) and
running `guix gc`, fish shell does not work well.
#+BEGIN_EXAMPLE
  meiyo@guix ~$ fish
  fish:
  echo $_ " "; __fish_pwd
               ^
  in command substitution
          called on standard input

  fish:
  __fish_pwd
  ^
  in command substitution
          called on standard input

  in command substitution
          called on standard input

  fish:
  echo $_ " "; __fish_pwd
               ^
  in command substitution
          called on standard input
#+END_EXAMPLE

__fish_pwd is a fish function. It's defined in
`share/fish/functions/__fish_pwd.fish`.  The error message shows that fish
cannot load __fish_pwd's function definition from disk. After doing some
research, I found out that the error was caused by wrong environment variables.

Fish shell is installed in:
#+BEGIN_EXAMPLE
  /gnu/store/ajbbi9cgj9j0my7v5habp0lcysaf2a51-fish-2.7.1/
#+END_EXAMPLE

But the environment variable $fish_function_path does not exist. And these
environment variables point to non-existent paths:
#+BEGIN_EXAMPLE
  __fish_bin_dir /gnu/store/4jkxcz8kpy621ycmqn3rvs0fv6c98h6p-fish-2.7.1/bin
  __fish_datadir /gnu/store/4jkxcz8kpy621ycmqn3rvs0fv6c98h6p-fish-2.7.1/share/fish
#+END_EXAMPLE

Setting $fish_function_path to the correct path reduces the error message.
#+BEGIN_EXAMPLE
  set fish_function_path /gnu/store/ajbbi9cgj9j0my7v5habp0lcysaf2a51-fish-2.7.1/share/fish/functions
#+END_EXAMPLE

`share/fish/config.fish` states $__fish_datadir is set by fish.cpp,
and $fish_function_path is derived from $__fish_datadir.
#+BEGIN_SRC fish
  # __fish_datadir, __fish_sysconfdir, __fish_help_dir, __fish_bin_dir
  # are expected to have been set up by read_init from fish.cpp


  # Set up function and completion paths. Make sure that the fish
  # default functions/completions are included in the respective path.

  if not set -q fish_function_path
      set fish_function_path $configdir/fish/functions $__fish_sysconfdir/functions $__extra_functionsdir $__fish_datadir/functions
  end

  if not contains -- $__fish_datadir/functions $fish_function_path
      set fish_function_path $fish_function_path $__fish_datadir/functions
  end
#+END_SRC

In conclusion, I think some path related variables are not set correctly when
fish is compiled from source code and that caused the bug I met. But since I'm
not good at C++ programming, I will not dive deeper.

I hope that the information provided above is helpful.


Meiyo Peng

bug#30265: Fish shell has wrong path variables

[ng0]

Hi Meiyo,

thanks for your report. Indeed, Fish has some problems in Guix.

On Sat, 27 Jan 2018, Meiyo Peng <meiyo.peng@gmail.com> wrote:
> Hi,
>
> I am using GuixSD 0.14. After upgrading fish shell to latest version(v2.7.1) and
> running `guix gc`, fish shell does not work well.

Can you explain a bit more about your setup? I assume you use
fish as you user shell and not just as a shell you switch into
from a Bash enabled user, correct?

> #+BEGIN_EXAMPLE
>   meiyo@guix ~$ fish
>   fish:
>   echo $_ " "; __fish_pwd
>                ^
>   in command substitution
>           called on standard input
>
>   fish:
>   __fish_pwd
>   ^
>   in command substitution
>           called on standard input
>
>   in command substitution
>           called on standard input
>
>   fish:
>   echo $_ " "; __fish_pwd
>                ^
>   in command substitution
>           called on standard input
> #+END_EXAMPLE
>
>
> __fish_pwd is a fish function. It's defined in
> `share/fish/functions/__fish_pwd.fish`.  The error message shows that fish
> cannot load __fish_pwd's function definition from disk. After doing some
> research, I found out that the error was caused by wrong environment variables.
>
> Fish shell is installed in:
>
> #+BEGIN_EXAMPLE
>   /gnu/store/ajbbi9cgj9j0my7v5habp0lcysaf2a51-fish-2.7.1/
> #+END_EXAMPLE
>
>
> But the environment variable $fish_function_path does not exist. And these
> environment variables point to non-existent paths:
>
> #+BEGIN_EXAMPLE
>   __fish_bin_dir /gnu/store/4jkxcz8kpy621ycmqn3rvs0fv6c98h6p-fish-2.7.1/bin
>   __fish_datadir /gnu/store/4jkxcz8kpy621ycmqn3rvs0fv6c98h6p-fish-2.7.1/share/fish
> #+END_EXAMPLE
>
>
> Setting $fish_function_path to the correct path reduces the error message.
>
> #+BEGIN_EXAMPLE
>   set fish_function_path /gnu/store/ajbbi9cgj9j0my7v5habp0lcysaf2a51-fish-2.7.1/share/fish/functions
> #+END_EXAMPLE
>
>
> `share/fish/config.fish` states $__fish_datadir is set by fish.cpp,
> and $fish_function_path is derived from $__fish_datadir.
>
> #+BEGIN_SRC fish
>   # __fish_datadir, __fish_sysconfdir, __fish_help_dir, __fish_bin_dir
>   # are expected to have been set up by read_init from fish.cpp
>
>
>   # Set up function and completion paths. Make sure that the fish
>   # default functions/completions are included in the respective path.
>
>   if not set -q fish_function_path
>       set fish_function_path $configdir/fish/functions $__fish_sysconfdir/functions $__extra_functionsdir $__fish_datadir/functions
>   end
>
>   if not contains -- $__fish_datadir/functions $fish_function_path
>       set fish_function_path $fish_function_path $__fish_datadir/functions
>   end
> #+END_SRC
>
> In conclusion, I think some path related variables are not set correctly when
> fish is compiled from source code and that caused the bug I met. But since I'm
> not good at C++ programming, I will not dive deeper.
>
> I hope that the information provided above is helpful.
>
>
> Meiyo Peng

It's more or less known, I assume this is related to bug#27206

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27206

Which to my knowledge and sources I've read doesn't require C
knowledge but more knowledge of how Fish interacts on
system/vendor level and some testing with the resources I've
provided in the other thread/bug.
-- 
ng0 :: https://ea.n0.is
A88C8ADD129828D7EAC02E52E22F9BBFEE348588 :: https://ea.n0.is/keys/

bug#30265: Fish shell has wrong path variables

[Ludovic Courtès]

Hi ng0 and Meiyo,

ng0@n0.is skribis:

> On Sat, 27 Jan 2018, Meiyo Peng <meiyo.peng@gmail.com> wrote:
>> Hi,
>>
>> I am using GuixSD 0.14. After upgrading fish shell to latest version(v2.7.1) and
>> running `guix gc`, fish shell does not work well.
>
> Can you explain a bit more about your setup? I assume you use
> fish as you user shell and not just as a shell you switch into
> from a Bash enabled user, correct?

Note that the current ‘guix’ package, 0.14.0-7.33988f9, includes Fish
completion support, which was not the case before:

--8<---------------cut here---------------start------------->8---
$ ls -R $(guix build guix)/share/fish
/gnu/store/apji9j8cwf9xpd5jk5mk8s6r1a391yvq-guix-0.14.0-7.33988f9/share/fish:
vendor_completions.d

/gnu/store/apji9j8cwf9xpd5jk5mk8s6r1a391yvq-guix-0.14.0-7.33988f9/share/fish/vendor_completions.d:
guix.fish
--8<---------------cut here---------------end--------------->8---

Could it be the reason why you’re having problems now that you didn’t
experience earlier?

Any ideas, ng0?

Thanks,
Ludo’.

bug#30265: Fish shell has wrong path variables

[ng0]

On Sat, 27 Jan 2018, ludo@gnu.org (Ludovic Courtès) wrote:
> Hi ng0 and Meiyo,
>
> ng0@n0.is skribis:
>
>> On Sat, 27 Jan 2018, Meiyo Peng <meiyo.peng@gmail.com> wrote:
>>> Hi,
>>>
>>> I am using GuixSD 0.14. After upgrading fish shell to latest version(v2.7.1) and
>>> running `guix gc`, fish shell does not work well.
>>
>> Can you explain a bit more about your setup? I assume you use
>> fish as you user shell and not just as a shell you switch into
>> from a Bash enabled user, correct?
>
> Note that the current ‘guix’ package, 0.14.0-7.33988f9, includes Fish
> completion support, which was not the case before:
>
> --8<---------------cut here---------------start------------->8---
> $ ls -R $(guix build guix)/share/fish
> /gnu/store/apji9j8cwf9xpd5jk5mk8s6r1a391yvq-guix-0.14.0-7.33988f9/share/fish:
> vendor_completions.d
>
> /gnu/store/apji9j8cwf9xpd5jk5mk8s6r1a391yvq-guix-0.14.0-7.33988f9/share/fish/vendor_completions.d:
> guix.fish
> --8<---------------cut here---------------end--------------->8---
>
> Could it be the reason why you’re having problems now that you didn’t
> experience earlier?
>
> Any ideas, ng0?
>
> Thanks,
> Ludo’.
>
>
>
>

No, I'm pretty sure this has nothing to do with adding fish
support to Guix.
-- 
ng0 :: https://ea.n0.is
A88C8ADD129828D7EAC02E52E22F9BBFEE348588 :: https://ea.n0.is/keys/

bug#30265: Fish shell has wrong path variables

[Ricardo Wurmus]

Hi Meiyo,

thanks for the report.

> I am using GuixSD 0.14. After upgrading fish shell to latest version(v2.7.1) and
> running `guix gc`, fish shell does not work well.
[…]
> Fish shell is installed in:
> #+BEGIN_EXAMPLE
>   /gnu/store/ajbbi9cgj9j0my7v5habp0lcysaf2a51-fish-2.7.1/
> #+END_EXAMPLE
>
> But the environment variable $fish_function_path does not exist. And these
> environment variables point to non-existent paths:
> #+BEGIN_EXAMPLE
>   __fish_bin_dir /gnu/store/4jkxcz8kpy621ycmqn3rvs0fv6c98h6p-fish-2.7.1/bin
>   __fish_datadir /gnu/store/4jkxcz8kpy621ycmqn3rvs0fv6c98h6p-fish-2.7.1/share/fish
> #+END_EXAMPLE

Where are these environment variables defined?

> In conclusion, I think some path related variables are not set
> correctly when fish is compiled from source code and that caused the
> bug I met.

Some other packages record exact store references in cache files in the
user’s home directory.  As packages get upgraded and removed after
garbage collection, these references become invalid.

The question is whether these old store references are kept in files
outside of the store (and thus are not directly under our control), or
if the packages that Guix builds include incorrect references.  I guess
that it’s the former.

As is the case for these other programmes that record paths in cache
files, we would need to patch fish to record only the stable names of
links to those files (e.g. ~/.guix-profile/foo/bar) rather than the
actual file names in the store, which may disappear upon “guix gc”.

--
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net

bug#30265: Fish shell has wrong path variables

[Meiyo Peng]

Ricardo Wurmus <rekado@elephly.net> writes:

> Where are these environment variables defined?
According to `share/fish/config.fish`, they are defined in fish.cpp.
#+BEGIN_SRC fish
  # __fish_datadir, __fish_sysconfdir, __fish_help_dir, __fish_bin_dir
  # are expected to have been set up by read_init from fish.cpp
#+END_SRC

You can get fish shell's environment variables using fish's `set` command.
A single `set` without argument will print all environment variables.

> The question is whether these old store references are kept in files
> outside of the store (and thus are not directly under our control), or
> if the packages that Guix builds include incorrect references.  I guess
> that it’s the former.
>
> As is the case for these other programmes that record paths in cache
> files, we would need to patch fish to record only the stable names of
> links to those files (e.g. ~/.guix-profile/foo/bar) rather than the
> actual file names in the store, which may disappear upon “guix gc”.
>
Maybe you are right. Guix is still new to me, so I do not know details
about guix's mechanism.


Meiyo Peng

bug#30265: Fish shell has wrong path variables

[Meiyo Peng]

ng0 wrote:
> Can you explain a bit more about your setup? I assume you use
> fish as you user shell and not just as a shell you switch into
> from a Bash enabled user, correct?

My GuixSD is a fresh setup. I did no customization to fish and did not
set fish as login shell.

The bug I reported is reproducible. I tried it in a virtual machine
several times.
1. Install GuixSD 0.14 with fish
2. guix pull
3. Upgrade all packages
4. guix gc
5. Start a fish shell, and the bug I reported occur

bug#30265: Fish shell has wrong path variables

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 1541 bytes --]

I haven't investigated too deep into this, but I've figured out a few
things.

First of all, to reproduce the issue:

- Install fish.

- Run `guix build --check fish`, it should output something like

  #+BEGIN_SRC sh
  > guix build --check fish
  Building /gnu/store/g9ra27952ay2a7j3mn7yp13b7m18kl1b-fish-2.7.1.drv - x86_64-linux
  grafting '/gnu/store/vgrav12zra9zky21ahm4x1qg8g4v58fj-fish-2.7.1' -> '/gnu/store/avk637800w1n7z1z0hnzx80r0fpd6729-fish-2.7.1'...
  /gnu/store/avk637800w1n7z1z0hnzx80r0fpd6729-fish-2.7.1
  #+END_SRC

- The graft source is a dead GC item:
	#+BEGIN_SRC sh
	> guix gc --list-dead | grep vgrav12zra9zky21ahm4x1qg8g4v58fj
	/gnu/store/vgrav12zra9zky21ahm4x1qg8g4v58fj-fish-2.7.1
	#+END_SRC

  Collect it:
	#+BEGIN_SRC sh
  guix gc --delete /gnu/store/vgrav12zra9zky21ahm4x1qg8g4v58fj-fish-2.7.1
	#+END_SRC

- Start fish:
  #+BEGIN_SRC sh
  > fish
  fish:
  echo $_ " "; __fish_pwd
                 ^
  in command substitution
          called on standard input
  ...
  #+END_SRC

I think Ricardo got a hunch of what's happening, except that it's not
about cache files in the user home's directory.  Instead, Fish seems to
record the path to the graft source.  A grep in the fish folder does not
seem to reveal anything, nor does `strings
/gnu/store/...-fish.../bin/fish`.

Could this be a grafting issue?  Like what was recently mentioned about
Racket?

Otherwise, as Meiyo pointed out, the solution might lie in patching
fish.cpp if grafting is not responsible for this.

--
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#30265: Fish shell has wrong path variables

[Ludovic Courtès]

Hi Pierre,

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

> I think Ricardo got a hunch of what's happening, except that it's not
> about cache files in the user home's directory.  Instead, Fish seems to
> record the path to the graft source.  A grep in the fish folder does not
> seem to reveal anything, nor does `strings
> /gnu/store/...-fish.../bin/fish`.
>
> Could this be a grafting issue?  Like what was recently mentioned about
> Racket?

It may well be a reference that doesn’t get properly grafted.  We can
see it when running the grafted fish under ‘strace’.

The culprit is this bit from fish.cpp:

        // Fall back to what got compiled in.
        debug(2, L"Using compiled in paths:");
        paths.data = L"" DATADIR "/fish";
        paths.sysconf = L"" SYSCONFDIR "/fish";
        paths.doc = L"" DOCDIR;
        paths.bin = L"" BINDIR;

The “L” here means these are “wide string” literals, and indeed, the
“Using…” string above looks like this in the ELF file:

001140d0: 5500 0000 7300 0000 6900 0000 6e00 0000  U...s...i...n...
001140e0: 6700 0000 2000 0000 6300 0000 6f00 0000  g... ...c...o...
001140f0: 6d00 0000 7000 0000 6900 0000 6c00 0000  m...p...i...l...
00114100: 6500 0000 6400 0000 2000 0000 6900 0000  e...d... ...i...
00114110: 6e00 0000 2000 0000 7000 0000 6100 0000  n... ...p...a...
00114120: 7400 0000 6800 0000 7300 0000 3a00 0000  t...h...s...:...

The DATADIR literal is similarly “hidden”, and thus the grafting code
doesn’t see it.

Possible fixes include:

  1. Finding a way to make the run-time detection in
     ‘determine_config_directory_paths’ to always work without going to
     the fallback case where it relies on string literals.  This could
     be done by attempting to read /proc/self/exe (on GNU/Linux) instead
     of relying on argv0.

  2. Using “regular” strings, or at least arranging to store DATADIR &
     co. in regular “const char” arrays.  Maybe something like:

       static const char datadir[] = DATADIR;

       …

       paths.data = L"" + datadir + L"/fish";

     It probably takes some more casts from char[] to std::string to
     wcstring, but you get the idea.  ;-)

Thoughts?

Ludo’.

bug#30265: Fish shell has wrong path variables

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 349 bytes --]

Thanks for this deep investigation, Ludo!

I'm not so well versed in grafting, so let me ask a few questions:

- Why is fish grafted in the first place?

- Is the issue here that grafting does not support wide string literals?
  Shouldn't we fix the Guix code to support wide strings as well?

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#30265: Fish shell has wrong path variables

[Ricardo Wurmus]

Pierre Neidhardt <mail@ambrevar.xyz> writes:

> I'm not so well versed in grafting, so let me ask a few questions:
>
> - Why is fish grafted in the first place?

Almost any package can be grafted.  A package will be grafted when any
of its inputs (direct or transitive) have been replaced.  The goal of
grafting is to build a replacement package *quickly* and then only
rewrite references to the replaced package in all dependent packages.

The advantage is that dependent packages do not need to be rebuilt; they
just need to be copied, scanned for references, and have their
references updated.  This is usually *much* faster than rebuilding
all dependent packages, which may be an important consideration in
distributing security fixes.

> - Is the issue here that grafting does not support wide string literals?
>   Shouldn't we fix the Guix code to support wide strings as well?

Grafting succeeds when we can find all references to items that should
be replaced – in plain text files but also in binaries.  Past problems
with grafting were triggered by compiler behaviour that chopped up these
reference strings, or by build systems that split these reference
strings.

A grafting problem is usually also a garbage collection problem, because
both depend on successful scanning for store references.

--
Ricardo

bug#30265: Fish shell has wrong path variables

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 328 bytes --]


> A package will be grafted when any
> of its inputs (direct or transitive) have been replaced.

I understand why that would happen when _updating_ fish, but why does it happen
when (re-)building it from scratch, for instance when the graft source is gone
from the store?

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#30265: Fish shell has wrong path variables

[Ludovic Courtès]

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

>> A package will be grafted when any
>> of its inputs (direct or transitive) have been replaced.
>
> I understand why that would happen when _updating_ fish, but why does it happen
> when (re-)building it from scratch, for instance when the graft source is gone
> from the store?

Whether a package is grafted depends on its dependencies and the set of
applicable grafts.  For instance, these are the grafts that can
potentially be applied to Fish as of commit
1df40d3dbff82c2990271b406b32633fe216d143:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> ,use(guix)
scheme@(guile-user)> ,use(gnu packages shells)
scheme@(guile-user)> (define s (open-connection))
scheme@(guile-user)> (package-grafts s fish)
$8 = (#<graft /gnu/store/f4d2v9y191zvj66c4rr65s8bix67wnma-libtiff-4.0.9 ==> /gnu/store/p2531jppdwwgn312bzwmm6q2cbmcdyc5-libtiff-4.0.9 2ff7840> #<graft /gnu/store/74iniafz6s76vbshzkq7zvx6nicd91y6-jbig2dec-0.14 ==> /gnu/store/vjailgb48w3jcf7brb2cgf61j9an3blm-jbig2dec-0.15 33b2630> #<graft /gnu/store/dysmb6hz7rr5rvcb05p23dazc5hz26qm-libgcrypt-1.8.2 ==> /gnu/store/hc5cak3fj0dijbm86kpz2asl7ld4gf8y-libgcrypt-1.8.3 32688d0> #<graft /gnu/store/r8ppi963s5xlf78lxd74y31263m1fxl2-libx11-1.6.5 ==> /gnu/store/bid7hvpnm8nq04vm4dszywxsw9g2kmf2-libx11-1.6.6 3268630> #<graft /gnu/store/4n6v2zp5mslq2784j878dmfzzj4vvmza-openssl-1.0.2o ==> /gnu/store/x8nacy2qpqlwi0gm7r6slcynv1cwmicb-openssl-1.0.2o 305de40> #<graft /gnu/store/jxa597l2zp6ydi345djxwabg5gp9h4di-ghostscript-9.23 ==> /gnu/store/fhbiaq9bnp4m79bd6wdfi9px41mwmdib-ghostscript-9.24 294aa80> #<graft /gnu/store/6zz27h4l21b8f2mifrk9sidvib9cns2i-perl-5.26.1 ==> /gnu/store/7ifc22sh86zblnzamqimgmv06idyx69v-perl-5.26.1 3a7be40> #<graft /gnu/store/wvd2bm9zqgy2v6yw8cp9id6hw4zlwa4i-curl-7.59.0 ==> /gnu/store/ia117b5q4pzcm81xj1hkv2qgg898v7x5-curl-7.61.1 20b2c90> #<graft /gnu/store/k177ng58xf43g5v22n60g0w75pqdv339-perl-5.26.1 ==> /gnu/store/v6c0fksl6q8bkshwb0rb74l9n4lyjfnn-perl-5.26.1 3518360>)
--8<---------------cut here---------------end--------------->8---

In practice only a subset of these grafts are applied because, for
instance, Fish doesn’t depend (directly or indirectly) on Ghostscript at
run time whereas it does depend on Perl:

--8<---------------cut here---------------start------------->8---
$ guix gc -R $(guix build fish) | grep -E '(perl|ghostscript)'
/gnu/store/7ifc22sh86zblnzamqimgmv06idyx69v-perl-5.26.1
--8<---------------cut here---------------end--------------->8---

HTH!

Ludo’.

bug#30265: Fish shell has wrong path variables

[Ludovic Courtès]

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

> - Is the issue here that grafting does not support wide string literals?
>   Shouldn't we fix the Guix code to support wide strings as well?

I’m not too keen on doing that: the scanner in (guix build grafts) would
have to be quite different if it were to catch /gnu/store references
burried in wide strings.  That’d be a real challenge.

The proposed changes (using Fish’ relocatability mechanism or ensuring
that the /gnu/store references are in char[] arrays) would be far
easier.

Let me know if you need clarifications regarding these.

Thanks,
Ludo’.

bug#30265: Fish shell has wrong path variables

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 849 bytes --]

> In practice only a subset of these grafts are applied because, for
> instance, Fish doesn’t depend (directly or indirectly) on Ghostscript at
> run time whereas it does depend on Perl:
> 
> --8<---------------cut here---------------start------------->8---
> $ guix gc -R $(guix build fish) | grep -E '(perl|ghostscript)'
> /gnu/store/7ifc22sh86zblnzamqimgmv06idyx69v-perl-5.26.1
> --8<---------------cut here---------------end--------------->8---

Thanks for pointing out this nuance, I wasn't aware of it.

But I still wonder: why is fish first built to
vgrav12zra9zky21ahm4x1qg8g4v58fj... and then immediately grafted to
avk637800w1n7z1z0hnzx80r0fpd6729... Why not building directly to
avk637800w1n7z1z0hnzx80r0fpd6729...?

I might be misunderstanding the basics of grafting :p

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#30265: Fish shell has wrong path variables

[Ricardo Wurmus]

Pierre Neidhardt <mail@ambrevar.xyz> writes:

>> In practice only a subset of these grafts are applied because, for
>> instance, Fish doesn’t depend (directly or indirectly) on Ghostscript at
>> run time whereas it does depend on Perl:
>>
>> --8<---------------cut here---------------start------------->8---
>> $ guix gc -R $(guix build fish) | grep -E '(perl|ghostscript)'
>> /gnu/store/7ifc22sh86zblnzamqimgmv06idyx69v-perl-5.26.1
>> --8<---------------cut here---------------end--------------->8---
>
> Thanks for pointing out this nuance, I wasn't aware of it.
>
> But I still wonder: why is fish first built to
> vgrav12zra9zky21ahm4x1qg8g4v58fj... and then immediately grafted to
> avk637800w1n7z1z0hnzx80r0fpd6729... Why not building directly to
> avk637800w1n7z1z0hnzx80r0fpd6729...?

Grafting is independent of building a package.  These are separate
derivations.  The first derivation is merely about building the package
— it is unaware of the need for grafting.

The second derivation only performs the graft.  All it knows about is
that it takes “vgrav12zra9zky21ahm4x1qg8g4v58fj” as an input and should
produce “avk637800w1n7z1z0hnzx80r0fpd6729”.

--
Ricardo

bug#30265: Fish shell has wrong path variables

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 89 bytes --]

Makes sense, that explains it.  Thanks!

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#30265: Fish embeds store file names in UCS-4/UTF-32 literal strings

[Meiyo Peng]

Hi,

`guix gc` does not break fish shell any more.  I am not sure if this is
related to changes in fish shell v3.0.0.

If no one is interested in doing more investigation, maybe we should
close this bug.

--
Meiyo Peng
https://www.pengmeiyu.com/

bug#30265: Fish embeds store file names in UCS-4/UTF-32 literal strings

[Ludovic Courtès]

Hi Meiyo,

Meiyo Peng <meiyo@disroot.org> skribis:

> `guix gc` does not break fish shell any more.  I am not sure if this is
> related to changes in fish shell v3.0.0.

It’s not really ‘guix gc’ that’s problematic but rather grafting: those
UCS-4 strings do not get grafted, and eventually become “dangling
references.”

Could you check whether fish.cpp still looks like
<https://issues.guix.info/issue/30265#8>, with the ‘L’ prefix for
literal strings?

Thanks,
Ludo’.

bug#30265: Fish embeds store file names in UCS-4/UTF-32 literal strings

[John Soo]

Hi Ludo,

I am willing to work on this. I use fish as my login shell and noticed some more funky behavior when I tried to use fish-foreign-env.

It looks like fish.cpp still uses widestrings. So the references to old store paths I think become stale, if I understand the rest of the conversation. 

I will do some more research on fish semantics and see what might be done.

Thanks, as always,

John

bug#30265: Fish embeds store file names in UCS-4/UTF-32 literal strings

[Maxim Cournoyer]

Hello,

John Soo <jsoo1@asu.edu> writes:

> Hi Ludo,
>
> I am willing to work on this. I use fish as my login shell and noticed some more funky behavior when I tried to use fish-foreign-env.
>
> It looks like fish.cpp still uses widestrings. So the references to old store paths I think become stale, if I understand the rest of the conversation. 
>
> I will do some more research on fish semantics and see what might be done.
>
> Thanks, as always,
>
> John
>
>
>

-- 
Thanks,
Maxim

bug#30265: Fish embeds store file names in UCS-4/UTF-32 literal strings

[Maxim Cournoyer]

Hello,

John Soo <jsoo1@asu.edu> writes:

> Hi Ludo,
>
> I am willing to work on this. I use fish as my login shell and noticed some more funky behavior when I tried to use fish-foreign-env.
>
> It looks like fish.cpp still uses widestrings. So the references to old store paths I think become stale, if I understand the rest of the conversation. 
>
> I will do some more research on fish semantics and see what might be done.

Were you able to make progress on this?

-- 
Thanks,
Maxim

bug#30265: Fish embeds store file names in UCS-4/UTF-32 literal strings

[John Soo]

[-- Attachment #1: Type: text/plain, Size: 96 bytes --]

I looked into it and I think a patch to fish might be required but I got buried in other work.


[-- Attachment #2: Type: text/html, Size: 464 bytes --]

bug#30265: Fish embeds store file names in UCS-4/UTF-32 literal strings

[Mark H Weaver]

John Soo <jsoo1@asu.edu> writes:
> I looked into it and I think a patch to fish might be required but I
> got buried in other work.

Note that commit 1bab9b9f17256a9e4f45f5b0cceb8b52e0a1b1ed (April 2021)
added support in our grafting code to find and rewrite UTF-16 and UTF-32
store references.  That might have mitigated or even eliminated the
adverse effects of this bug.

However, the Guix daemon's reference scanner still does not detect
UTF-16/32 references.  This could be a problem if some store item is
reachable *only* via UTF-16/32 store references, because "guix gc" might
delete it even though it is still needed.

However, if it is the case that every referenced store item is
represented in ASCII or UTF-8 at least once, everything should work.
Therefore, an easy workaround would be to add another phase that simply
creates a file in the output(s) that contains ASCII or UTF-8 references
to any needed store items.

      Mark

-- 
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>.

bug#30265: Fish embeds store file names in UCS-4/UTF-32 literal strings

[Maxim Cournoyer]

Hi,

Mark H Weaver <mhw@netris.org> writes:

> John Soo <jsoo1@asu.edu> writes:
>> I looked into it and I think a patch to fish might be required but I
>> got buried in other work.
>
> Note that commit 1bab9b9f17256a9e4f45f5b0cceb8b52e0a1b1ed (April 2021)
> added support in our grafting code to find and rewrite UTF-16 and UTF-32
> store references.  That might have mitigated or even eliminated the
> adverse effects of this bug.
>
> However, the Guix daemon's reference scanner still does not detect
> UTF-16/32 references.  This could be a problem if some store item is
> reachable *only* via UTF-16/32 store references, because "guix gc" might
> delete it even though it is still needed.
>
> However, if it is the case that every referenced store item is
> represented in ASCII or UTF-8 at least once, everything should work.
> Therefore, an easy workaround would be to add another phase that simply
> creates a file in the output(s) that contains ASCII or UTF-8 references
> to any needed store items.

Working with what I see (the fish build outputs results), the only UCS-4
references (either big or small endian) it registered to the store via
multi-byte encoded strings are:

--8<---------------cut here---------------start------------->8---
$ strings -e L /gnu/store/qfy1rxm1vzd68y9jvcvq4zzz0cnbla8i-fish-3.5.1/bin/fish* | grep /gnu
/gnu/store/qfy1rxm1vzd68y9jvcvq4zzz0cnbla8i-fish-3.5.1/share/doc/fish
/gnu/store/qfy1rxm1vzd68y9jvcvq4zzz0cnbla8i-fish-3.5.1/share/fish
/gnu/store/qfy1rxm1vzd68y9jvcvq4zzz0cnbla8i-fish-3.5.1/etc/fish
/gnu/store/qfy1rxm1vzd68y9jvcvq4zzz0cnbla8i-fish-3.5.1/bin
/gnu/store/qfy1rxm1vzd68y9jvcvq4zzz0cnbla8i-fish-3.5.1/bin
/gnu/store/qfy1rxm1vzd68y9jvcvq4zzz0cnbla8i-fish-3.5.1/bin
strings -e B /gnu/store/qfy1rxm1vzd68y9jvcvq4zzz0cnbla8i-fish-3.5.1/bin/fish* | grep /gnu
/gnu/store/qfy1rxm1vzd68y9jvcvq4zzz0cnbla8i-fish-3.5.1/share/doc/fish
/gnu/store/qfy1rxm1vzd68y9jvcvq4zzz0cnbla8i-fish-3.5.1/share/fish
/gnu/store/qfy1rxm1vzd68y9jvcvq4zzz0cnbla8i-fish-3.5.1/etc/fish
/gnu/store/qfy1rxm1vzd68y9jvcvq4zzz0cnbla8i-fish-3.5.1/bin
/gnu/store/qfy1rxm1vzd68y9jvcvq4zzz0cnbla8i-fish-3.5.1/bin
/gnu/store/qfy1rxm1vzd68y9jvcvq4zzz0cnbla8i-fish-3.5.1/bin
--8<---------------cut here---------------end--------------->8---

No UCS-2 references are detected via 'strings'.

Thanks for having shared the history and background.

Closing.

-- 
Maxim