1246998 - git mozreview doesn't work from linked git worktrees

Status: RESOLVED FIXED

(MozReview Graveyard :: General, defect)

Description

[Jamie Nicol [:jnicol]]

I have successfully ran "git mozreview configure" and "git mozreview push" from my repository's main worktree. However, from another worktree (created with git 2.5+'s "git worktree add" command) it does not work.

Running git mozreview push results in:

> abort: configuration needed; run `git mozreview configure`

And running git mozreview configure results in:

> installing commit-msg hook
> Traceback (most recent call last):
>  File "/home/jamie/src/version-control-tools/git/commands/git-mozreview", line 763, in <module>
>    sys.exit(main(sys.argv[1:]))
>  File "/home/jamie/src/version-control-tools/git/commands/git-mozreview", line 751, in main
>    return configure_command(args)
>  File "/home/jamie/src/version-control-tools/git/commands/git-mozreview", line 640, in configure_command
>    os.symlink(source_hook, dest_hook)
> OSError: [Errno 2] No such file or directory

Comment 1

[Gregory Szorc [:gps]]

Should "just" be a matter of teaching the custom git command to recognize .git as a file with a link to the gitdir.

Comment 2

[Gregory Szorc [:gps]]

Hmmm. We already run `git rev-parse --git-dir` to resolve the path to the .git directory. And AFAICT we use it everywhere we should. Not sure what's going on.

Anyway, we removed the hook symlinking code yesterday. So please pull version-control-tools and paste the output with the latest code.

Comment 3

[Jamie Nicol [:jnicol]]

> ~/src/gecko/subframe-displayport$ git mozreview configure
> installing commit-msg hook
> Traceback (most recent call last):
>   File "/home/jamie/src/version-control-tools/git/commands/git-mozreview", line 770, in <module>
>     sys.exit(main(sys.argv[1:]))
>   File "/home/jamie/src/version-control-tools/git/commands/git-mozreview", line 758, in main
>     return configure_command(args)
>   File "/home/jamie/src/version-control-tools/git/commands/git-mozreview", line 644, in configure_command
>     with open(dest_hook, 'wb') as fh:
> IOError: [Errno 2] No such file or directory: u'/home/jamie/src/gecko/repo/.git/worktrees/subframe-displayport/hooks/commit-msg'

/home/jamie/src/gecko/repo is my main repo, and "subframe-displayport" is linked worktree name.

Hooks should just go in repo/.git/hooks/, and not anywhere under repo/.git/worktrees/?

Comment 4

[Gregory Szorc [:gps]]

The way git worktrees work is that worktrees have a .git *file* (not a directory) whose content is basically a path to the actual .git directory. We run `git rev-parse --git-dir` to resolve the actual .git dir and use it everywhere. So when we install a hook info <git_dir>/hooks, it should install globally.

From your output above, it looks like `git rev-parse --git-dir` is printing "/home/jamie/src/gecko/repo/.git/worktrees/subframe-displayport". Can you confirm that?

Comment 5

[Jamie Nicol [:jnicol]]

Yes that is exactly what it is printing.

Comment 6

[Gregory Szorc [:gps]]

According to glandium on IRC, we want to call `git rev-parse --git-common-dir`, as it honors work trees. However, this argument is relatively new, so we likely need to code in a fallback if the command fails. Or they may print "--git-common-dir" to stdout. We should definitely test this.

Comment 7

[Mike Hommey [:glandium]]

I just built the commit just before the one that added --git-common-dir, here's how it goes:

$ ./git rev-parse --git-common-dir
--git-common-dir
$ ./git rev-parse --no-flags --git-common-dir

with a version that does support --git-common-dir:
$ git rev-parse --no-flags --git-common-dir
.git

in a worktree:
$ git rev-parse --no-flags --git-common-dir
/home/glandium/gecko/.git

so we could check the output of `git rev-parse --no-flags --git-common-dir` and if it's empty, try again with `git rev-parse --no-flags --git-dir`

FWIW, the first tagged release of git supporting the --no-flags flag is v0.99.

Comment 8

[Mike Hommey [:glandium]]

Attachment: MozReview Request: Bug 1246998 - Take worktrees into account in git mozreview

Review commit: https://reviewboard.mozilla.org/r/34465/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/34465/

Comment 9

[Mike Hommey [:glandium]]

Comment on attachment 8718201 [details]
MozReview Request: Bug 1246998 - Take worktrees into account in git mozreview

Please test this is enough.

Comment 10

[Jamie Nicol [:jnicol]]

Comment on attachment 8718201 [details]
MozReview Request: Bug 1246998 - Take worktrees into account in git mozreview

Works perfectly, thanks!

Comment 11

[Gregory Szorc [:gps]]

Comment on attachment 8718201 [details]
MozReview Request: Bug 1246998 - Take worktrees into account in git mozreview

https://reviewboard.mozilla.org/r/34465/#review31251

Comment 12

[Gregory Szorc [:gps]]

https://hg.mozilla.org/hgcustom/version-control-tools/rev/a1ec4b577c09

Comment 13

[Mike Hommey [:glandium]]

This broke running git mozreview from a subdirectory of the working tree.

Somehow git rev-parse --git-common-dir is broken in that case: it returns ".git" instead of the full path or a relative path (--git-dir returns an absolute path)