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

error: the path link '/home/sanxiyn/download/zig' is not in PATH #65

Open
sanxiyn opened this issue Aug 13, 2022 · 6 comments
Open

error: the path link '/home/sanxiyn/download/zig' is not in PATH #65

sanxiyn opened this issue Aug 13, 2022 · 6 comments

Comments

@sanxiyn
Copy link

sanxiyn commented Aug 13, 2022

To reproduce, download zigup to $HOME/download/zigup, and symbolic link it to $HOME/bin/zigup, whereas $HOME/download is not in PATH and $HOME/bin is.

When zigup is executed, /proc/self/exe points to $HOME/download/zigup and not $HOME/bin/zigup, even if $HOME/bin/zigup was what was found in PATH, because kernel already resolved symbolic link.

@marler8997
Copy link
Owner

What's the behavior you'd expect or like in this case?

@sanxiyn
Copy link
Author

sanxiyn commented Aug 14, 2022

The behavior I want is to behave as if --path-link $HOME/bin/zig is passed, not (as it currently does) as if --path-link $HOME/download/zig is passed, since the later fails anyway.

This probably can be implemented by searching PATH for zigup like which, instead of reading /proc/self/exe.

@marler8997
Copy link
Owner

There are actually use cases for not wanting the resulting zig path link in PATH. For example the zigup tests themselves don't want to actually put the resutling zig symlink in PATH. zigup's design has changed a bit so not putting zig in PATH may not be fully supported with every command (fetch seems to work but setting the default compiler doesn't). The fact that setting the default compiler in this case fails is a problem, but I'm not sure if we want to require that zigup be stored in a PATH directory. Since I'm not sure, I'm inclined to fix the error we currently get when setting the default compiler and just not require that it be a PATH directory. Which zigup command or commands did you run that presented the error?

@sanxiyn
Copy link
Author

sanxiyn commented Aug 14, 2022

I ran zigup 0.9.1.

@marler8997
Copy link
Owner

marler8997 commented Aug 14, 2022

You know what, looking at the code a bit more, it looks like zigup actually does make an effort to ensure that when you set the default compiler it is actually in PATH and is also the highest priority. The tests actually modify PATH. So ignore my last comment.

Your idea to detect when zigup does not live in a valid directory PATH but has been symlinked from a valid directory is an interesting one. My hesitation is do we need to add this complexity to zigup? The current solution in this case would be to copy zigup to the bin directory. Adding support for zigup to be symlinked from another directory adds some complexity/overhead. here is the current algorithm to the determine the path link directory:

zigup/zigup.zig

Lines 120 to 127 in 9f8c5f9

fn makeZigPathLinkString(allocator: Allocator) ![]const u8 {
if (global_optional_path_link) |path| return path;
const zigup_dir = try std.fs.selfExeDirPathAlloc(allocator);
defer allocator.free(zigup_dir);
return try std.fs.path.join(allocator, &[_][]const u8{ zigup_dir, "zig" ++ builtin.target.exeFileExt() });
}

With your suggestion, this function would need to always traverse every directory in PATH to find any instance of zigup. Note that we would want to do this every time so that the path-link directory doesn't change depending on whether we are setting the default compiler or not. This also means that multiple instances of zigup in PATH would interfere with each other. Granted these are corner cases that would be rare, but my point is that having to handle these situations is the cost of adding such a feature. Without this feature, the status quo is that zigup must be installed in the directory where the user would like the path-link to be. If the user doesn't do this, then zigup is installed incorrectly and you get an error message. Another option would be to improve the error message:

error: the path link '/foo/bar/zig' is not in PATH, please move zigup into a directory that is in PATH

Is there an environment or a reason why you wouldn't be able to move zigup to a directory in PATH?

@sanxiyn
Copy link
Author

sanxiyn commented Aug 14, 2022

I think improving the error message would be fine. I actually solved this for myself just as you suggested, by copying (instead of symlinking) zigup to bin directory. But it was mystifying at first why it failed, and it was unclear the solution is to copy.

There is no particular reason I symlink, it's just that I always did it that way for like a decade and it worked fine. In download directory, I keep metadata (like where and when I downloaded it), and those don't belong in bin directory, and copying seemed wasteful of space, so I symlinked.

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

No branches or pull requests

2 participants