Fish embeds store file names in UCS-4/UTF-32 literal strings

  • Done
  • quality assurance status badge
Details
9 participants
  • John Soo
  • Ludovic Courtès
  • Pierre Neidhardt
  • Maxim Cournoyer
  • Meiyo Peng
  • Meiyo Peng
  • Mark H Weaver
  • ng0
  • Ricardo Wurmus
Owner
unassigned
Submitted by
Meiyo Peng
Severity
important
M
M
Meiyo Peng wrote on 27 Jan 2018 10:11
Fish shell has wrong path variables
(address . bug-guix@gnu.org)
87inbnpsx9.fsf@gmail.com
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
N
(address . 30265@debbugs.gnu.org)
87shar8u6d.fsf@abyayala.i-did-not-set--mail-host-address--so-tickle-me
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:
Toggle quote (5 lines)
> 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?

Toggle quote (82 lines)
> #+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


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.
--
A88C8ADD129828D7EAC02E52E22F9BBFEE348588 :: https://ea.n0.is/keys/
R
R
Ricardo Wurmus wrote on 27 Jan 2018 13:19
(name . Meiyo Peng)(address . meiyo.peng@gmail.com)(address . 30265@debbugs.gnu.org)
87inbntryj.fsf@elephly.net
Hi Meiyo,

thanks for the report.

Toggle quote (2 lines)
> 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.
[…]
Toggle quote (12 lines)
> 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?

Toggle quote (4 lines)
> 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
M
M
Meiyo Peng wrote on 27 Jan 2018 14:45
(name . Ricardo Wurmus)(address . rekado@elephly.net)(address . 30265@debbugs.gnu.org)
87wp03h0us.fsf@gmail.com
Ricardo Wurmus <rekado@elephly.net> writes:

Toggle quote (1 lines)
> 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.

Toggle quote (10 lines)
> 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
M
M
Meiyo Peng wrote on 27 Jan 2018 15:13
(address . 30265@debbugs.gnu.org)(address . 30265@debbugs.gnu.org)
87lggjgzk3.fsf@gmail.com
ng0 wrote:
Toggle quote (4 lines)
> 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
L
L
Ludovic Courtès wrote on 27 Jan 2018 17:25
(address . 30265@debbugs.gnu.org)
87bmhf1d7k.fsf@gnu.org
Hi ng0 and Meiyo,

ng0@n0.is skribis:

Toggle quote (10 lines)
> 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:

Toggle snippet (8 lines)
$ 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

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

Any ideas, ng0?

Thanks,
Ludo’.
N
(address . 30265@debbugs.gnu.org)
878tcjjham.fsf@abyayala.i-did-not-set--mail-host-address--so-tickle-me
On Sat, 27 Jan 2018, ludo@gnu.org (Ludovic Courtès) wrote:
Toggle quote (38 lines)
> 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.
--
A88C8ADD129828D7EAC02E52E22F9BBFEE348588 :: https://ea.n0.is/keys/
P
P
Pierre Neidhardt wrote on 19 Sep 2018 11:09
(address . 30265@debbugs.gnu.org)
87pnx9yhsx.fsf@ambrevar.xyz
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
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEUPM+LlsMPZAEJKvom9z0l6S7zH8FAluiEl4ACgkQm9z0l6S7
zH/dYQf/RtmcrvyaEQl1MIHNXn1MBT+FTiHWnwcPTV7/70pwhlXwuNj+qX3Y+xqq
RO9mfHGTmX2p66vq7aQxEKVrzQLAr+WH+wEMQsQst2aiU+5nwvjGoWRhW53FFuAN
sK4kDCdpvO6nNMZk75hwbIuCprWoEqcHmIO3yoWr2lurbjvPRYskPVV3m44wMxSF
Jz8f6v4BWYv2uktFXmI1Pom9nJ5MnKWp86/JqequySab88LfOiJ+uVqujfC5Dx9Q
N/HXl/qaMcRMgyG+zHtV6uO0A9vPdUYm5gSU3rPAWb7/wjLrKOhizxRXpLdH9DiF
XnqF6QKi0uGF4EnJ+Cn29GN0wbEnPg==
=d86p
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 19 Sep 2018 22:39
(name . Pierre Neidhardt)(address . mail@ambrevar.xyz)
87o9ctdxxk.fsf@gnu.org
Hi Pierre,

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

Toggle quote (9 lines)
> 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’.
P
P
Pierre Neidhardt wrote on 20 Sep 2018 18:09
(name . Ludovic Courtès)(address . ludo@gnu.org)
87efdoxia5.fsf@ambrevar.xyz
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
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEUPM+LlsMPZAEJKvom9z0l6S7zH8FAlujxjIACgkQm9z0l6S7
zH9fXQf/X4xIgeDOu0uIof0OCQ6Z2kRZGIXBvOF+L/zRvNkmMvMNM7+QKaU4rk31
mmQ1uiosHQYu/8qmhqmB962TIm9I/L6+XY/a5mol22F8e46EjvU/iuDzDk589Sm0
MUHSG/7Jp3xGbraUuQ4hXrsxaMYrVqfsi3+iE8pOzAtn0pb1gk/8sEp2GGwBP4Ke
66Qzv0uOEHUsq0gntqR/up2pFzgOs/Fd16kopVtJSanFM5Sun4mdK/tNZrpzcOHt
dVg4Qhx6QsKSv/7u25CQ7IUgYjmaf407rKk0vE4SO8ta4Uq6HC/j1rh1J4mq6cuW
AmzoYC8nlrju/Xyh7k5rymbXr6a4lw==
=H7uz
-----END PGP SIGNATURE-----

R
R
Ricardo Wurmus wrote on 20 Sep 2018 19:00
(name . Pierre Neidhardt)(address . mail@ambrevar.xyz)
877ejgumrn.fsf@elephly.net
Pierre Neidhardt <mail@ambrevar.xyz> writes:

Toggle quote (4 lines)
> 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.

Toggle quote (3 lines)
> - 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
P
P
Pierre Neidhardt wrote on 20 Sep 2018 19:12
(name . Ricardo Wurmus)(address . rekado@elephly.net)
87bm8sxfd4.fsf@ambrevar.xyz
Toggle quote (3 lines)
> 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
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEUPM+LlsMPZAEJKvom9z0l6S7zH8FAluj1PcACgkQm9z0l6S7
zH/F6wgApMaCgiNO4asSDXxOJxB9De5ktO7MN6T1zRc0kmQWc7Z93LAXk3ZJICmM
OhT1LH1o8tf/jes5dfktA5hB/acyOKd+p2hAidGXIm3OJvQLAUyp2410NM3F8Cor
G2GE9xZkqd1Yjj8rnj8dIOlQ41fq9I6dvydEj5+PbFCWaTrMngIWxnlql8V0VTcp
1fL6toKtP+k3cwREO28a6b1kYgOAeC+5dRroXVpp7eCnB49U2dVKlRZD+QeL5I2B
Fs0xu3SEarcPCbiNlpx34GSYMcHjcmO5ISKSPhr5IsbxUKW8SrdN5zVLS1uKgb+H
3dktJsrcxsqe9Dva8z6Vf8KklnlmEQ==
=YOf0
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 21 Sep 2018 14:03
(name . Pierre Neidhardt)(address . mail@ambrevar.xyz)
87d0t72h2g.fsf@gnu.org
Pierre Neidhardt <mail@ambrevar.xyz> skribis:

Toggle quote (7 lines)
>> 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:

Toggle snippet (7 lines)
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>)

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:

Toggle snippet (4 lines)
$ guix gc -R $(guix build fish) | grep -E '(perl|ghostscript)'
/gnu/store/7ifc22sh86zblnzamqimgmv06idyx69v-perl-5.26.1

HTH!

Ludo’.
L
L
Ludovic Courtès wrote on 21 Sep 2018 14:05
(name . Pierre Neidhardt)(address . mail@ambrevar.xyz)
877ejf2gyp.fsf@gnu.org
Pierre Neidhardt <mail@ambrevar.xyz> skribis:

Toggle quote (3 lines)
> - 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’.
P
P
Pierre Neidhardt wrote on 21 Sep 2018 16:42
(name . Ludovic Courtès)(address . ludo@gnu.org)
877ejeykro.fsf@ambrevar.xyz
Toggle quote (9 lines)
> 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
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEUPM+LlsMPZAEJKvom9z0l6S7zH8FAlulA1sACgkQm9z0l6S7
zH9t4Qf/enxSBr53IaoJsKXXqrtI9XQ0KB+CV5BPZLXjgjIAENHo+qzKU+9UUmz9
PJxcrua7DGcphiA89d0j2xY2utRrHQP4crBfRR2uUb+DArRybf/zSLrelIAzCxqO
bY4QzOQ16Mu4LVQZl2FSg6qHd3fuRstMzVRZ29WU3b+sK4ZjrqaeCGyMf/BEbeEl
th2qnJKLBiUvmAkgytVf8O6fkxp1ZIe7TkRnW0gBQYQxtB0TinN4aOT82U3jjiwb
q07KN+qCtGd+ohvdqICoEmuc4Zt4jXCyfi0cv9lYgCeoE7pZ1UePAg9+ztuTm6cU
HL4HCn1Wn0aIUvqRAEM29OR1SHQ3fA==
=tclm
-----END PGP SIGNATURE-----

R
R
Ricardo Wurmus wrote on 21 Sep 2018 16:46
(name . Pierre Neidhardt)(address . mail@ambrevar.xyz)
87y3buucw2.fsf@elephly.net
Pierre Neidhardt <mail@ambrevar.xyz> writes:

Toggle quote (16 lines)
>> 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
P
P
Pierre Neidhardt wrote on 21 Sep 2018 17:11
(name . Ricardo Wurmus)(address . rekado@elephly.net)
875zyyyjez.fsf@ambrevar.xyz
Makes sense, that explains it. Thanks!

--
Pierre Neidhardt
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEUPM+LlsMPZAEJKvom9z0l6S7zH8FAlulCjQACgkQm9z0l6S7
zH9yVwf/fnG+29yRw8/4Qq24ZxP/aBKBUImj+0xZBDNZY0zGOnIQ+08zxvEjEq5v
LVfdZmIdGPkvBwpSrXcoO2bBTmFucbtbSIiCQrwfeU/oMfGGEFqrptWC6V1BJU2r
4fZVjqhrebciyYg/YHWw+HCouQBiNwnVg2iDLTT/fOny4R7dyLNJOMLhzWNNxw5S
10V0rnJgQ8GZWn5kxccIZ1ypjavt/dDd1NMOELGc+d9W9Y/TJaJDTAIgi1YkKscl
zQyYZADTRwGc2sXwJH+vj1q2cHwFOPs14e1sr4rfi8YiNu3zKrf3g96mE901z+by
6v1GXgHQNEjtYoeDoIfy0PXBmI7SZA==
=ROaI
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 26 Dec 2018 17:08
control message for bug #30265
(address . control@debbugs.gnu.org)
87pntoe0ek.fsf@gnu.org
retitle 30265 Fish embeds store file names in UCS-4/UTF-32 literal strings
L
L
Ludovic Courtès wrote on 26 Dec 2018 17:08
(address . control@debbugs.gnu.org)
87o998e0eg.fsf@gnu.org
severity 30265 important
M
M
Meiyo Peng wrote on 2 Feb 2019 08:20
Re: bug#30265: Fish embeds store file names in UCS-4/UTF-32 literal strings
(address . 30265@debbugs.gnu.org)
871s4q7j23.fsf@disroot.org
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
L
L
Ludovic Courtès wrote on 4 Feb 2019 23:16
(name . Meiyo Peng)(address . meiyo@disroot.org)(address . 30265@debbugs.gnu.org)
87va1znqrn.fsf@gnu.org
Hi Meiyo,

Meiyo Peng <meiyo@disroot.org> skribis:

Toggle quote (3 lines)
> `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’.
J
J
John Soo wrote on 30 Mar 2020 08:29
Fish embeds store file names in UCS-4/UTF-32 literal strings
(address . 30265@debbugs.gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
8B84EF65-4CCC-4CAF-A6E9-BF6F5566D11E@asu.edu
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
M
M
Maxim Cournoyer wrote on 7 Oct 2022 21:42
(name . John Soo)(address . jsoo1@asu.edu)
8735bz5sjo.fsf@gmail.com
Hello,

John Soo <jsoo1@asu.edu> writes:

Toggle quote (15 lines)
> 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
M
M
Maxim Cournoyer wrote on 7 Oct 2022 21:43
(name . John Soo)(address . jsoo1@asu.edu)
871qrj5sj9.fsf@gmail.com
Hello,

John Soo <jsoo1@asu.edu> writes:

Toggle quote (8 lines)
> 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
J
J
John Soo wrote on 7 Oct 2022 21:44
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
f6645b99-829d-400b-aebd-aa389687af00@Canary
I looked into it and I think a patch to fish might be required but I got buried in other work.
Attachment: file
M
M
Mark H Weaver wrote on 7 Oct 2022 22:57
87czb3uza5.fsf@netris.org
John Soo <jsoo1@asu.edu> writes:
Toggle quote (3 lines)
> 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.
M
M
Maxim Cournoyer wrote on 10 Oct 2022 05:38
(name . Mark H Weaver)(address . mhw@netris.org)
87fsfw2vr0.fsf@gmail.com
Hi,

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

Toggle quote (20 lines)
> 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:

Toggle snippet (16 lines)
$ 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

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

Thanks for having shared the history and background.

Closing.

--
Maxim
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 30265@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 30265
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch