New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fish is not relocatable #125

Closed
timbertson opened this Issue Jun 13, 2012 · 10 comments

Comments

Projects
None yet
3 participants
@timbertson
Contributor

timbertson commented Jun 13, 2012

I'm not sure how big an ask this is, but it would be great if fish were relocatable (as in, once built, it could be moved and run from any directory as long as all the files moved in unison).

Personally I'd like to create a zeroinstall feed for fish, so that I can run it on whatever machine I find myself on without requiring admin priviledges or build tools. The only thing preventing that right now is the lack of relocatability of the build result. This would obviously be useful for more that just 0install - e.g you could also take it with you on a USB stick.

I'm not really sure how hard it would be, but in general it should usually be possible to either construct relative paths (relative to the fish executable), or to use environment variables (perhaps $FISH_ROOT).

One more restriction on 0install implementations is that once unpacked, they must remain readonly. I presume that wouldn't be a problem (config goes under ~/.config), but I thought I'd mention it just in case.

@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish Jun 16, 2012

Member

I believe all of fish's configuration is done via environment variables; their default values come from the installed paths at build time, but they can be overridden.

These are the ones that may need to be changed:

__fish_datadir
__fish_help_dir
__fish_sysconfdir
fish_complete_path
fish_function_path

Member

ridiculousfish commented Jun 16, 2012

I believe all of fish's configuration is done via environment variables; their default values come from the installed paths at build time, but they can be overridden.

These are the ones that may need to be changed:

__fish_datadir
__fish_help_dir
__fish_sysconfdir
fish_complete_path
fish_function_path

@timbertson

This comment has been minimized.

Show comment
Hide comment
@timbertson

timbertson Jun 17, 2012

Contributor

Thanks for the details. I tried creating a launcher script that would set these appropriately:

#!/bin/bash
set -ex
here="$(readlink -f "$(dirname "$(dirname "$0")")")"
export PATH="$here/bin:$PATH"
export __fish_datadir="$here/share/fish"
export __fish_help_dir="$here/doc/fish"
export __fish_sysconfdir="$here/etc/fish"
export fish_complete_path="$HOME/fake/.config/fish/completions:$here/etc/fish/completions:$here/share/fish/completions"
export fish_function_path="$HOME/fake/.config/fish/functions:$here/etc/fish/functions:$here/share/fish/functions"
exec "$here/bin/fish"

Unfortunately, it looks like this isn't enough. When I run it (after moving build to build2 to ensure it doesn't accidentally use any default paths), I get:

$ ./build2/bin/launch.sh 
++++ dirname ./build2/bin/launch.sh
+++ dirname ./build2/bin
++ readlink -f ./build2
+ here=/home/tim/dev/oss/fishfish/build2
+ export PATH=< ... uninteresting long $PATH >
+ PATH=< ... uninteresting long $PATH >
+ export __fish_datadir=/home/tim/dev/oss/fishfish/build2/share/fish
+ __fish_datadir=/home/tim/dev/oss/fishfish/build2/share/fish
+ export __fish_help_dir=/home/tim/dev/oss/fishfish/build2/doc/fish
+ __fish_help_dir=/home/tim/dev/oss/fishfish/build2/doc/fish
+ export __fish_sysconfdir=/home/tim/dev/oss/fishfish/build2/etc/fish
+ __fish_sysconfdir=/home/tim/dev/oss/fishfish/build2/etc/fish
+ export fish_complete_path=/home/tim/.config/fish/completions:/home/tim/dev/oss/fishfish/build2/etc/fish/completions:/home/tim/dev/oss/fishfish/build2/share/fish/completions
+ fish_complete_path=/home/tim/.config/fish/completions:/home/tim/dev/oss/fishfish/build2/etc/fish/completions:/home/tim/dev/oss/fishfish/build2/share/fish/completions
+ export fish_function_path=/home/tim/.config/fish/functions:/home/tim/dev/oss/fishfish/build2/etc/fish/functions:/home/tim/dev/oss/fishfish/build2/share/fish/functions
+ fish_function_path=/home/tim/.config/fish/functions:/home/tim/dev/oss/fishfish/build2/etc/fish/functions:/home/tim/dev/oss/fishfish/build2/share/fish/functions
+ exec /home/tim/dev/oss/fishfish/build2/bin/fish
set_color: Unknown color 'yellow
'
printf: missing operand
Try `printf --help' for more information.

...and the terminal is dead/unresponsive

If I take my $HOME/config paths out of it, I no longer get the errors about set_color and printf (I think they are from my custom fish_prompt function, although it works normally), but the terminal is still unresponsive so something is clearly not right.

Contributor

timbertson commented Jun 17, 2012

Thanks for the details. I tried creating a launcher script that would set these appropriately:

#!/bin/bash
set -ex
here="$(readlink -f "$(dirname "$(dirname "$0")")")"
export PATH="$here/bin:$PATH"
export __fish_datadir="$here/share/fish"
export __fish_help_dir="$here/doc/fish"
export __fish_sysconfdir="$here/etc/fish"
export fish_complete_path="$HOME/fake/.config/fish/completions:$here/etc/fish/completions:$here/share/fish/completions"
export fish_function_path="$HOME/fake/.config/fish/functions:$here/etc/fish/functions:$here/share/fish/functions"
exec "$here/bin/fish"

Unfortunately, it looks like this isn't enough. When I run it (after moving build to build2 to ensure it doesn't accidentally use any default paths), I get:

$ ./build2/bin/launch.sh 
++++ dirname ./build2/bin/launch.sh
+++ dirname ./build2/bin
++ readlink -f ./build2
+ here=/home/tim/dev/oss/fishfish/build2
+ export PATH=< ... uninteresting long $PATH >
+ PATH=< ... uninteresting long $PATH >
+ export __fish_datadir=/home/tim/dev/oss/fishfish/build2/share/fish
+ __fish_datadir=/home/tim/dev/oss/fishfish/build2/share/fish
+ export __fish_help_dir=/home/tim/dev/oss/fishfish/build2/doc/fish
+ __fish_help_dir=/home/tim/dev/oss/fishfish/build2/doc/fish
+ export __fish_sysconfdir=/home/tim/dev/oss/fishfish/build2/etc/fish
+ __fish_sysconfdir=/home/tim/dev/oss/fishfish/build2/etc/fish
+ export fish_complete_path=/home/tim/.config/fish/completions:/home/tim/dev/oss/fishfish/build2/etc/fish/completions:/home/tim/dev/oss/fishfish/build2/share/fish/completions
+ fish_complete_path=/home/tim/.config/fish/completions:/home/tim/dev/oss/fishfish/build2/etc/fish/completions:/home/tim/dev/oss/fishfish/build2/share/fish/completions
+ export fish_function_path=/home/tim/.config/fish/functions:/home/tim/dev/oss/fishfish/build2/etc/fish/functions:/home/tim/dev/oss/fishfish/build2/share/fish/functions
+ fish_function_path=/home/tim/.config/fish/functions:/home/tim/dev/oss/fishfish/build2/etc/fish/functions:/home/tim/dev/oss/fishfish/build2/share/fish/functions
+ exec /home/tim/dev/oss/fishfish/build2/bin/fish
set_color: Unknown color 'yellow
'
printf: missing operand
Try `printf --help' for more information.

...and the terminal is dead/unresponsive

If I take my $HOME/config paths out of it, I no longer get the errors about set_color and printf (I think they are from my custom fish_prompt function, although it works normally), but the terminal is still unresponsive so something is clearly not right.

@timbertson

This comment has been minimized.

Show comment
Hide comment
@timbertson

timbertson Jun 17, 2012

Contributor

A bit of strace reveals the following suspicious calls to open() (these are the only files within ~/dev/oss/fishfish that it attempts to open(), and they are all under build - not build2):

open("/home/tim/dev/oss/fishfish/build/share/fish/config.fish", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
open("/home/tim/dev/oss/fishfish/build/share/locale/en_AU/LC_MESSAGES/fish.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/home/tim/dev/oss/fishfish/build/share/locale/en/LC_MESSAGES/fish.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/home/tim/dev/oss/fishfish/build/etc/fish/config.fish", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)

Looking at e.g read_init() in fish.cpp, it looks like it only uses the hard-coded DATADIR (set in the makefile, -DDATADIR) instead of reading envvars :(

Contributor

timbertson commented Jun 17, 2012

A bit of strace reveals the following suspicious calls to open() (these are the only files within ~/dev/oss/fishfish that it attempts to open(), and they are all under build - not build2):

open("/home/tim/dev/oss/fishfish/build/share/fish/config.fish", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
open("/home/tim/dev/oss/fishfish/build/share/locale/en_AU/LC_MESSAGES/fish.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/home/tim/dev/oss/fishfish/build/share/locale/en/LC_MESSAGES/fish.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/home/tim/dev/oss/fishfish/build/etc/fish/config.fish", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)

Looking at e.g read_init() in fish.cpp, it looks like it only uses the hard-coded DATADIR (set in the makefile, -DDATADIR) instead of reading envvars :(

@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish Jun 17, 2012

Member

Great investigation! Absolutely we should replace the uses of DATADIR and SYSCONFDIR in read_init().

Member

ridiculousfish commented Jun 17, 2012

Great investigation! Absolutely we should replace the uses of DATADIR and SYSCONFDIR in read_init().

@timbertson

This comment has been minimized.

Show comment
Hide comment
@timbertson

timbertson Jun 17, 2012

Contributor

Glad to hear it. I had a go at this, attempting to use http://www.gnu.org/software/autoconf-archive/ax_define_sub_path.html, but messing with autotools inevitably makes me want to throw things at my computer in frustration.

As best I can tell, when you use the above macro in configure.ac it spits out invalid shell script, and if you (manually) fix up the invalid shell script the results are completely wrong anyway. So I'd love to help, but I don't think I know enough about autotools to manage much. Here's what I tried, in case it's a useful start:

https://gist.github.com/975901f650b267fa3178

Contributor

timbertson commented Jun 17, 2012

Glad to hear it. I had a go at this, attempting to use http://www.gnu.org/software/autoconf-archive/ax_define_sub_path.html, but messing with autotools inevitably makes me want to throw things at my computer in frustration.

As best I can tell, when you use the above macro in configure.ac it spits out invalid shell script, and if you (manually) fix up the invalid shell script the results are completely wrong anyway. So I'd love to help, but I don't think I know enough about autotools to manage much. Here's what I tried, in case it's a useful start:

https://gist.github.com/975901f650b267fa3178

@zanchey

This comment has been minimized.

Show comment
Hide comment
@zanchey

zanchey Jun 17, 2012

Member

Incidentally this issue means that fish does not work until it is installed, so you can't run it from the build directory unless you have an existing install. This confused me a bit (and led to me believing I had reproduced #101) so might be worth documenting in the INSTALL or README.

Member

zanchey commented Jun 17, 2012

Incidentally this issue means that fish does not work until it is installed, so you can't run it from the build directory unless you have an existing install. This confused me a bit (and led to me believing I had reproduced #101) so might be worth documenting in the INSTALL or README.

@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish Jun 17, 2012

Member

I think config.fish already does the right thing:

if not set -q __fish_datadir
    set -g __fish_datadir /usr/local/share/fish
end

The broken code is in read_init(), which ought to respect the value of __fish_datadir.

In other words, the paths constructed in read_init should try using the values of the __fish_datadir and __fish_sysconfdir variables, and only use the compiled-in values as a fallback. That could be fixed directly in read_init - no messing with autotools required.

Member

ridiculousfish commented Jun 17, 2012

I think config.fish already does the right thing:

if not set -q __fish_datadir
    set -g __fish_datadir /usr/local/share/fish
end

The broken code is in read_init(), which ought to respect the value of __fish_datadir.

In other words, the paths constructed in read_init should try using the values of the __fish_datadir and __fish_sysconfdir variables, and only use the compiled-in values as a fallback. That could be fixed directly in read_init - no messing with autotools required.

@timbertson

This comment has been minimized.

Show comment
Hide comment
@timbertson

timbertson Jun 18, 2012

Contributor

Ok, that's a relief. I had a go fixing it and it's now not reading anything under the install prefix if the envvars are set properly (aside from some locale files, but I think that's minor). I've updated my gist at https://gist.github.com/975901f650b267fa3178 (I know the code is a bit weird, I couldn't quite figure out the conversion between wcstring and the predefined constants so I just did whatever worked)

However, it still won't actually run properly - as before, fish doesn't respond to input, and until I kill it takes up a whole core of CPU. Not really sure why :/

Contributor

timbertson commented Jun 18, 2012

Ok, that's a relief. I had a go fixing it and it's now not reading anything under the install prefix if the envvars are set properly (aside from some locale files, but I think that's minor). I've updated my gist at https://gist.github.com/975901f650b267fa3178 (I know the code is a bit weird, I couldn't quite figure out the conversion between wcstring and the predefined constants so I just did whatever worked)

However, it still won't actually run properly - as before, fish doesn't respond to input, and until I kill it takes up a whole core of CPU. Not really sure why :/

@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish Jun 27, 2012

Member

Current plan is to provide an .app on OS X, so it better be relocatable.

Member

ridiculousfish commented Jun 27, 2012

Current plan is to provide an .app on OS X, so it better be relocatable.

@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish Jul 8, 2012

Member

The changes in this commit:

4912967

make fish relocatable. In particular, if fish lives in a hierarchy like this:

somewhere/bin/fish
somewhere/share/fish/config.fish
somewhere/etc/fish/config.fish

then fish will discover that based on its executable path, on OSes where we can determine the executable path (OS X, Linux, BSD, Solaris at least).

This should enable the 0install case, and it also plays nice with Homebrew and an OS X .app.

Member

ridiculousfish commented Jul 8, 2012

The changes in this commit:

4912967

make fish relocatable. In particular, if fish lives in a hierarchy like this:

somewhere/bin/fish
somewhere/share/fish/config.fish
somewhere/etc/fish/config.fish

then fish will discover that based on its executable path, on OSes where we can determine the executable path (OS X, Linux, BSD, Solaris at least).

This should enable the 0install case, and it also plays nice with Homebrew and an OS X .app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment