Skip to content

Change default install path to $XDG_DATA_HOME/zigup on non-Windows systems #178

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Mar 15, 2025
Merged

Change default install path to $XDG_DATA_HOME/zigup on non-Windows systems #178

merged 2 commits into from
Mar 15, 2025

Conversation

AloisMahdal
Copy link
Contributor

Fixes #177. Also related to discussion in #146.

Windows path is not changed because currently on Windows the path is derived from the location of the binary, so assuming Windows does not allow two identcal paths, the most logical path name, zigup, is not available.

I don't know about Windows conventions and don't have a Windows machine available to be able to test my assumptions.

@AloisMahdal AloisMahdal force-pushed the default_install_path branch from 2cd9ec5 to 258aed2 Compare March 12, 2025 20:22
@AloisMahdal
Copy link
Contributor Author

258aed2 built with zig 0.14 and tested on my Debian 12 machine.

If there's interest it would be probably less of a hassle to change it on Windows as well, but I don't know Windows conventions, and especially I don't know a good-enough way how to get a "good" default programmaticaclly (presumably without extra dependencies such as known_paths).

So if anyone wants to provide a snippet and test it, I'm willing to include it, but it might just be easier to do it in another PR.

@Grazfather
Copy link
Contributor

Instead of hardcoding it, could you check $XDG_DATA_HOME, and if it's not set, default to ~/.local/share?

@AloisMahdal
Copy link
Contributor Author

Instead of hardcoding it, could you check $XDG_DATA_HOME, and if it's not set, default to ~/.local/share?

I have prepared implementation which prefers and handles XDG_DATA_HOME properly, but in the meantime I've found a bug, which actually affects even the current implementation.

zigup will not create parent directories when creating the installdir, so ~/.local/share/zigup will fail if ~/.local/share does not already exist. I'm not sure if stdlib has an easy option on auto-creating parent dirs.

@AloisMahdal
Copy link
Contributor Author

OK, turns out there's not a function for it but it's relatively straightforward using std.fs.path.componentIterator().

So, updating the PR:

I've replaced loggyMakeDirAbsolute() with loggyMakeDirAbsoluteParents(), which will use the mentioned iterator to get all parents of the directory starting with the top-most, and consecutively check if they exist; if not it will create them, producing the same mkdir logging as the original implementation.

Again, I can't test this on Windows, but

  • From what I understood from componentIterator() doc (see std.fs.path.ComponentIterator), paths like C:\, \\some\share\ or / are never returned so this should work the same on both platforms.

    Corner cases might include when \\some\share is used as base of the XDG path, yet does not exist, but that seems pretty far-fetched already...

  • For Windows, now we just always use zig ion the directory of the binary, so unless someone changes that, the above will not be relevant.

@AloisMahdal AloisMahdal force-pushed the default_install_path branch from 258aed2 to 5fffe8c Compare March 13, 2025 18:26
@AloisMahdal
Copy link
Contributor Author

5fffe8c built with zig 0.14 and tested on my Debian 12 machine:

@nauron:~$ env | grep ^XDG_DATA_HOME

@nauron:~!1!$ vcs.upstream/zigup/zig-out/bin/zigup fetch 0.11.0
install directory '/home/netvor/.local/share/zigup'
mkdir '/home/netvor/.local/share/zigup'
rm -rf '/home/netvor/.local/share/zigup/0.11.0.installing'
mkdir '/home/netvor/.local/share/zigup/0.11.0.installing'
downloading 'https://ziglang.org/download/0.11.0/zig-linux-x86_64-0.11.0.tar.xz' to '/home/netvor/.local/share/zigup/0.11.0.installing/zig-linux-x86_64-0.11.0.tar.xz'
[RUN] tar xf /home/netvor/.local/share/zigup/0.11.0.installing/zig-linux-x86_64-0.11.0.tar.xz -C /home/netvor/.local/share/zigup/0.11.0.installing
rm -rf '/home/netvor/.local/share/zigup/0.11.0.installing/zig-linux-x86_64-0.11.0.tar.xz'
mv '/home/netvor/.local/share/zigup/0.11.0.installing/zig-linux-x86_64-0.11.0' '/home/netvor/.local/share/zigup/0.11.0.installing/files'
mv '/home/netvor/.local/share/zigup/0.11.0.installing' '/home/netvor/.local/share/zigup/0.11.0'

@nauron:~$ XDG_DATA_HOME="" vcs.upstream/zigup/zig-out/bin/zigup fetch 0.12.0
install directory '/home/netvor/.local/share/zigup'
rm -rf '/home/netvor/.local/share/zigup/0.12.0.installing'
mkdir '/home/netvor/.local/share/zigup/0.12.0.installing'
downloading 'https://ziglang.org/download/0.12.0/zig-linux-x86_64-0.12.0.tar.xz' to '/home/netvor/.local/share/zigup/0.12.0.installing/zig-linux-x86_64-0.12.0.tar.xz'
[RUN] tar xf /home/netvor/.local/share/zigup/0.12.0.installing/zig-linux-x86_64-0.12.0.tar.xz -C /home/netvor/.local/share/zigup/0.12.0.installing
rm -rf '/home/netvor/.local/share/zigup/0.12.0.installing/zig-linux-x86_64-0.12.0.tar.xz'
mv '/home/netvor/.local/share/zigup/0.12.0.installing/zig-linux-x86_64-0.12.0' '/home/netvor/.local/share/zigup/0.12.0.installing/files'
mv '/home/netvor/.local/share/zigup/0.12.0.installing' '/home/netvor/.local/share/zigup/0.12.0'

@nauron:~$ XDG_DATA_HOME=foo vcs.upstream/zigup/zig-out/bin/zigup fetch 0.12.0
error: $XDG_DATA_HOME environment variable 'foo' is not an absolute path
error: BadXdgDataHomeEnvironmentVariable

@nauron:~!1!$ HOME="foo" vcs.upstream/zigup/zig-out/bin/zigup fetch 0.12.0
error: $HOME environment variable 'foo' is not an absolute path
error: BadHomeEnvironmentVariable

@nauron:~!1!$ XDG_DATA_HOME="$HOME/.local/share2" vcs.upstream/zigup/zig-out/bin/zigup fetch 0.12.0
install directory '/home/netvor/.local/share2/zigup'
mkdir '/home/netvor/.local/share2'
mkdir '/home/netvor/.local/share2/zigup'
rm -rf '/home/netvor/.local/share2/zigup/0.12.0.installing'
mkdir '/home/netvor/.local/share2/zigup/0.12.0.installing'
downloading 'https://ziglang.org/download/0.12.0/zig-linux-x86_64-0.12.0.tar.xz' to '/home/netvor/.local/share2/zigup/0.12.0.installing/zig-linux-x86_64-0.12.0.tar.xz'
[RUN] tar xf /home/netvor/.local/share2/zigup/0.12.0.installing/zig-linux-x86_64-0.12.0.tar.xz -C /home/netvor/.local/share2/zigup/0.12.0.installing
rm -rf '/home/netvor/.local/share2/zigup/0.12.0.installing/zig-linux-x86_64-0.12.0.tar.xz'
mv '/home/netvor/.local/share2/zigup/0.12.0.installing/zig-linux-x86_64-0.12.0' '/home/netvor/.local/share2/zigup/0.12.0.installing/files'
mv '/home/netvor/.local/share2/zigup/0.12.0.installing' '/home/netvor/.local/share2/zigup/0.12.0'

@nauron:~$

@AloisMahdal AloisMahdal changed the title Change default install path to .local/share/zigup on non-Windows systems Change default install path to $XDG_DATA_HOME/zigup on non-Windows systems Mar 13, 2025
marler8997
marler8997 reviewed Mar 14, 2025
View reviewed changes
@AloisMahdal AloisMahdal force-pushed the default_install_path branch from 5fffe8c to 01c9a8d Compare March 14, 2025 15:54
@AloisMahdal
Copy link
Contributor Author

AloisMahdal commented Mar 14, 2025 •
edited
Loading

So I've replaced my implementation with std.fs.Dir.makePath() and updated the log to say mkdir -p (on Windows this is irrelevant since parent dir creation seems to be the default).

Re-tested:

@nauron:~$ XDG_DATA_HOME="$HOME/.local/share2" vcs.upstream/zigup/zig-out/bin/zigup fetch 0.12.0
install directory '/home/netvor/.local/share2/zigup'
mkdir -p '/home/netvor/.local/share2/zigup'
rm -rf '/home/netvor/.local/share2/zigup/0.12.0.installing'
mkdir -p '/home/netvor/.local/share2/zigup/0.12.0.installing'
downloading 'https://ziglang.org/download/0.12.0/zig-linux-x86_64-0.12.0.tar.xz' to '/home/netvor/.local/share2/zigup/0.12.0.installing/zig-linux-x86_64-0.12.0.tar.xz'
[RUN] tar xf /home/netvor/.local/share2/zigup/0.12.0.installing/zig-linux-x86_64-0.12.0.tar.xz -C /home/netvor/.local/share2/zigup/0.12.0.installing
rm -rf '/home/netvor/.local/share2/zigup/0.12.0.installing/zig-linux-x86_64-0.12.0.tar.xz'
mv '/home/netvor/.local/share2/zigup/0.12.0.installing/zig-linux-x86_64-0.12.0' '/home/netvor/.local/share2/zigup/0.12.0.installing/files'
mv '/home/netvor/.local/share2/zigup/0.12.0.installing' '/home/netvor/.local/share2/zigup/0.12.0'
@nauron:~$ XDG_DATA_HOME=foo vcs.upstream/zigup/zig-out/bin/zigup fetch 0.12.0
error: $XDG_DATA_HOME environment variable 'foo' is not an absolute path
error: BadXdgDataHomeEnvironmentVariable
@nauron:~!1!$

@marler8997
Copy link
Owner

Couple notes:

  • rename allocInstallDirStringXdg to just allocInstallDirString (since it may not actually use XDG if the env var doesn't exist)
  • I'll let you decide but I think I'd be find removing the isAbsolute checks in allocInstallDirString. If you decide to keep them then I don't think we need to add new error codes, you can just return error.AlreadyReported.
  • to be consistent with std, let's rename loggyMakeDirAbsoluteParents to just loggyMakePath

@AloisMahdal
Change default install path to $XDG_DATA_HOME/zigup on non-Windows sy…
beeecd4 
…stems

Fixes marler8997#177.  Also related to discussion in marler8997#146.

Windows path is not changed because currently on Windows the path is
derived from the location of the binary, so assuming Windows does not
allow two identcal paths, the most logical path name, `zigup`, is not
available.

I don't know about Windows conventions and don't have a Windows machine
available to be able to test my assumptions.
@AloisMahdal
Just use errror.AlreadyReported instead of overly specific codes
5012a6f 
error.AlreadyReported just means exit with 1 withot printing any
more errors.
@AloisMahdal AloisMahdal force-pushed the default_install_path branch from 01c9a8d to 5012a6f Compare March 14, 2025 18:46
@AloisMahdal
Copy link
Contributor Author

  • rename allocInstallDirStringXdg to just allocInstallDirString (since it may not actually use XDG if the env var doesn't exist)

allocInstallDirString is already taken by the fn that switches between Windows and the rest. i can merge them if that's what you'd prefer.

I considered few names but ended up adding Xdg to signify that the function works according to the XDG spec. IIUC, not using the envvar if it's not defined is being XDG compliant -- XDG also specfies the fallback to $HOME/.local/share.

  • I'll let you decide but I think I'd be find removing the isAbsolute checks in allocInstallDirString. If you decide to keep them then I don't think we need to add new error codes, you can just return error.AlreadyReported.

Don't have a strong opinion but ti me they seem worth keeping as an extra safeguard... So I kept them and replaced errors with AlreadyReported. (I did that as a separate commit but feel free to squash...)

  • to be consistent with std, let's rename loggyMakeDirAbsoluteParents to just loggyMakePath

done!

@marler8997 marler8997 merged commit 1a6f17c into marler8997:master Mar 15, 2025
2 of 3 checks passed
@AloisMahdal AloisMahdal deleted the default_install_path branch March 15, 2025 15:45
@AloisMahdal AloisMahdal mentioned this pull request Mar 15, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marler8997 marler8997 marler8997 left review comments

+1 more reviewer

@Grazfather Grazfather Grazfather left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Proposal: Don't use ~/.zig, use eg. ~/.zigup instead
3 participants
@AloisMahdal @Grazfather @marler8997