search

denolehov / obsidian-git

Backup your Obsidian.md vault with git
MIT License
6.15k stars 252 forks source link

Adjust git.cwd to use a relative path to git root #587

Closed H3mul closed 4 weeks ago

H3mul commented 10 months ago

This is a simple adjustment for the git cwd command to use a relative path to the git root instead of an absolute path. It should keep the behavior identical, but it solves an issue I ran into in my setup (described below).

More details on --show-cdup vs --show-toplevel here: https://git-scm.com/docs/git-rev-parse#Documentation/git-rev-parse.txt---show-cdup

Motivation: I ran into this issue during my esoteric obsidian-git setup using WSL2 (following the solution in #141). Basically, I want to use WSL's git instead of native windows git.exe, since I set it up with git-crypt and it has my ssh credentials. I ran into the issue that in this case, git.revparse("--show-toplevel") will return the root as an absolute WSL path (eg, /mnt/c/Users/...). git.cwd() checks path existence prior to actually switching the working directory, but does so using fs - which will be using the windows filesystem instead, and, of course, not find the root directory, and throw an error.

A quick solution is to pass a relative path to git.cwd() instead of an absolute path, as those will work the same way in WSL and Windows filesystems for a given repo. This is exactly what the --show-cdup flag returns. Note that it will be an empty string if we are already at the repo root, hence the additional existence check.

Some sources for my deep dive:

https://github.com/steveukx/git-js/blob/d64b31ca8670edd7af5a7fe5658516f5717c79a8/simple-git/src/lib/tasks/change-working-directory.ts#L7 https://github.com/steveukx/git-js/blob/d64b31ca8670edd7af5a7fe5658516f5717c79a8/simple-git/src/lib/utils/util.ts#L74 https://github.com/kwsites/file-exists/blob/13f0789cbd8df52588acdd185cd33de3b77fb342/src/index.ts#L10C13-L10C18

Vinzent03 commented 10 months ago

Looks really promising! Thanks for that fix. Will test later or start next week. Please ignore all tests other than the format test. I've messed them up in my latest commits. But the format seems actually to be wrong.

Vinzent03 commented 9 months ago

Have you tested this on normal windows without wsl? I'm getting the following error:

not a git repository (or any of the parent directores): .git

So I don't think the relative cwd is working properly.

H3mul commented 9 months ago

Interesting, I havent, let me do some testing as well. I would expect that in this case, git.checkIsRepo() to also fail.

H3mul commented 9 months ago

Looks like you were right, the relative path cwd seems to break. Adjusted this feature to resolve the relative path to an absolute one in the nodeJS filesystem, which works both in my WSL case and the windows git.exe case based on my testing

H3mul commented 4 months ago

Apologies for the force push, rebased my fork's master to be up-to-date with this repo and forgot this issue tracks that branch. No new changes have been introduced since my last comment.