hedronvision / bazel-compile-commands-extractor

Goal: Enable awesome tooling for Bazel users of the C language family.
Other
689 stars 114 forks source link

145963b9787b69aa8c464bd50bff9f1bd701ca32 doesn't play nicely with git worktrees #103

Closed xortive closed 1 year ago

xortive commented 1 year ago

145963b9787b69aa8c464bd50bff9f1bd701ca32 assumes that git rev-parse --git-dir is a subdirectory of the project root. When git worktrees are in use, this is not true, as the gitdir is stored in the parent repository's .git/worktrees folder.

cpsauer commented 1 year ago

Hey, Matt! Yep. Sure does; you're right. Thanks for your careful eye and for teaching me something new that I hadn't considered.

Before I make the change (or you do if you want to get on the contributor board for your good observation): I'd propose we solve this by getting the relative paths to use in the gitignore with git rev-parse --show-prefix--{pattern_prefix} in the code--but still rely on git rev-parse --git-dir to get the .git folder for the /info/exclude path. Could I ask you to check that that'd work for your setup?

Cheers, Chris

xortive commented 1 year ago

Hey, thanks for the quick response! I think git rev-parse --show-prefix will work perfectly for determining the value of pattern_prefix.

kjteske commented 1 year ago

Adding a +1, we're hitting this issue too with git worktrees too.

I would actually prefer to be disable _ensure_gitignore_entries_exist() entirely, for our use case we can just commit to .gitignore in the repo instead of the hidden git_dir/info/exclude.

Maybe we can default to calling _ensure_gitignore_entries_exist(), but make this configurable so we could opt out?

cpsauer commented 1 year ago

Hey guys! Just pushed up a commit aiming to fix things for worktrees, switching pattern_prefix to git rev-parse --show-prefix, as above.

Could I ask you to give the latest a go and confirm that it fixes things for you? (And then we'll close.)

Thanks for reporting and sorry for the inconvenience. Chris

cpsauer commented 1 year ago

Guys?

kjteske commented 1 year ago

Thanks - needs one more fix for me:

INFO: Running command line: bazel-bin/refresh_compile_commands '<redacted>'
Traceback (most recent call last):
  File "<redacted>/refresh_compile_commands.py", line 1048, in <module>
    _ensure_gitignore_entries_exist()
  File "<redacted>/refresh_compile_commands.py", line 1013, in _ensure_gitignore_entries_exist
    with open(hidden_gitignore_path, 'a+') as gitignore:
FileNotFoundError: [Errno 2] No such file or directory: '<redacted>/<our_repo>/<our_repo>.git/worktrees/<my_worktree_name>/info/exclude'

info/exclude is actually at <redacted>/<our_repo>/<our_repo>.git/info/exclude, not <redacted>/<our_repo>/<our_repo>.git/<my_worktree_name>/info/exclude.

The git docs at https://git-scm.com/docs/gitignore say this should be $GIT_DIR/info/exclude, but it seems to be at $GIT_COMMON_DIR/info/exclude instead. Maybe a git documentation bug?

This is fixed by --git-common-dir instead of --git-dir:

-    git_dir_process = subprocess.run('git rev-parse --git-dir',
+    git_dir_process = subprocess.run('git rev-parse --git-common-dir',
cpsauer commented 1 year ago

Done. Just pushed up a commit doing exactly that. All good now? (I, too, was looking at those docs and am surprised, but should have tested more.)

Thanks for tracking down and proposing a fix.

rosds commented 1 year ago

can confirm the update works :+1: thanks for the fix!

kjteske commented 1 year ago

Looks good, thanks!

cpsauer commented 1 year ago

Sweet! Thanks all for reporting, helping track things down to a solution, and testing. Sorry for the inconvenience. May the tool serve you well into the future!

Cheers, Chris