Subject: bug#30265: Fish shell has wrong path variables

From: [email-masked] (Ludovic Courtès)
Date: Wed, 19 Sep 2018 22:39:19 +0200

Hi Pierre,

Pierre Neidhardt <[email-masked]> 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’.