Skip to content
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

Engines do not honor system paths on Linux #695

Closed
skatbude opened this issue Apr 15, 2020 · 7 comments · Fixed by #698
Closed

Engines do not honor system paths on Linux #695

skatbude opened this issue Apr 15, 2020 · 7 comments · Fixed by #698

Comments

@skatbude
Copy link

I use ssh to run my engines remotely on a more powerful machine.

For v0.43.3 and v0.50.0 I used "ssh" as my engine, works fine.
For v0.51.1 "ssh" doesn't work anymore, "/usr/bin/ssh" has to be specified.

Basically this is the same problem as pointed out in issue 117.

Sabaki version: v0.51.1
OS: Linux (SparkyLinux based on Debian testing branch, Kernel 5.6.0)

I think the issue has been introduced in this commit.

@yishn
Copy link
Member

yishn commented Apr 15, 2020

Thanks for the investigation! You're right about the commit that caused this, but unfortunately, it is needed to make relative paths work on the portable version. I'm not sure how to resolve this properly...

@skatbude
Copy link
Author

The problem apparently is that

If no path segments are passed, path.resolve() will return the absolute path of the current working directory [1].

Indeed "ssh" works if I execute Sabaki from "/usr/bin".

Maybe it's possible to use the "shell" spawn option (in conjunction with the already apparent "cwd") [2]. This should pass all name resolution duties over to the shell.

[1] https://nodejs.org/api/path.html#path_path_resolve_paths
[2] https://nodejs.org/api/child_process.html#child_process_child_process_spawn_command_args_options

@yishn
Copy link
Member

yishn commented Apr 15, 2020

This won't really solve the problem. In the portable version, if we don't have an explicit filename resolution, relative paths to engines don't work (relative to Sabaki path).

@ebifrier
Copy link
Contributor

ebifrier commented Apr 17, 2020

One idea is to use the absolute path of a given executable file if it exists, and to use the relative path if the absolute path does not exist.

@yishn
Copy link
Member

yishn commented Apr 17, 2020

Sounds sensible; the only problem I see is that the file extension can't be omitted for relative paths, but that is probably just a minor detail and only affects Windows.

@ebifrier
Copy link
Contributor

ebifrier commented Apr 17, 2020

Another idea is to call spawn in the relative path, and if it fails, call spawn again in the absolute path. But we need to write a little dirty code related 'spawn' to determine that an error occurred due to a specification problem. I think a simpler way would be better, and if the path is relative, it's the specification that even the extension has to be matched.

I've written the code. Can I PR it?

@yishn
Copy link
Member

yishn commented Apr 17, 2020

Sounds good, please do 😊

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

Successfully merging a pull request may close this issue.

3 participants