Running 'git worktree add' in 'pre-commit' hook

Running 'git worktree add' in 'pre-commit' hook

[Cosmin Polifronie]

Hello! I am trying to run 'git worktree add <path> HEAD' in the
'pre-commit' hook, more specifically in a Python script that is being
called from the hook. When doing so, I am greeted with the following
error:

On Windows 10:
Preparing worktree (detached HEAD cbfef18)
fatal: Unable to create 'C:/Users/meh/Desktop/abc/.git/index.lock': No
such file or directory

On Arch Linux:
Preparing worktree (detached HEAD cbfef18)
fatal: Unable to create '/home/cosmin/Downloads/abc/.git/index.lock':
Not a directory

Is it forbidden to call this command from a hook? If yes, what kind of
alternatives do I have? I need to make a copy of the repo in its HEAD
state, process it and then decide if I will pass the current commit or
not.

Thanks! :)

Re: Running 'git worktree add' in 'pre-commit' hook

[Bryan Turner]

On Tue, May 14, 2019 at 7:53 AM Cosmin Polifronie <oppturbv@gmail.com> wrote:
>
> Hello! I am trying to run 'git worktree add <path> HEAD' in the
> 'pre-commit' hook, more specifically in a Python script that is being
> called from the hook. When doing so, I am greeted with the following
> error:
>
> On Windows 10:
> Preparing worktree (detached HEAD cbfef18)
> fatal: Unable to create 'C:/Users/meh/Desktop/abc/.git/index.lock': No
> such file or directory
>
> On Arch Linux:
> Preparing worktree (detached HEAD cbfef18)
> fatal: Unable to create '/home/cosmin/Downloads/abc/.git/index.lock':
> Not a directory
>
> Is it forbidden to call this command from a hook? If yes, what kind of
> alternatives do I have? I need to make a copy of the repo in its HEAD
> state, process it and then decide if I will pass the current commit or
> not.

I can't speak to whether `git worktree add` should succeed or fail
inside a `pre-commit` hook, but...

Why do you need a new work tree, versus whatever working copy you're
running `git commit` in? Is there a reason whatever validation needs
to be done can't be done in the existing working copy? `HEAD` is the
_previous, existing commit_, not the new, currently-being-created
commit, so your validation in the new work tree, if you actually
managed to create one, would be applied to the _latest existing
commit_, not the new changes you're trying to commit. Even trying to
copy the changes over wouldn't necessarily result in the same state,
because there may be unstaged changes.

What type of validation are you trying to do? I think the failure
you're running into is an alarm bell indicating what you're trying to
do may not make sense. However, without any insight into what "process
it and then decide if I will pass the current commit or not" actually
looks like, it's hard to offer you much help.

Bryan

>
> Thanks! :)

Re: Running 'git worktree add' in 'pre-commit' hook

[Cosmin Polifronie]

Hello Bryan,

My project contains a Gradle build that is dependent on a
configuration found in a file called build.gradle. What I need to do
is run Gradle with a proper build.gradle file, meaning: if a new
build.gradle is staged, I will use that for my Gradle build, if not I
will use the latest approved commit's one (as in a commit that has
already passed the pre-commit check). The problem is that build.gradle
can be modified and not be staged, and Gradle cannot know that, it
will use whatever is available on disk. I want to avoid that.

This is why I needed a copy of my current repo with the file versions
already approved (already staged at a previous date). This is why I am
using 'git worktree add'.

Now, I have found another solution using stashes, and those might
work, but I am still curious about this situation.

On Wed, May 15, 2019 at 3:46 AM Bryan Turner <bturner@atlassian.com> wrote:
>
> On Tue, May 14, 2019 at 7:53 AM Cosmin Polifronie <oppturbv@gmail.com> wrote:
> >
> > Hello! I am trying to run 'git worktree add <path> HEAD' in the
> > 'pre-commit' hook, more specifically in a Python script that is being
> > called from the hook. When doing so, I am greeted with the following
> > error:
> >
> > On Windows 10:
> > Preparing worktree (detached HEAD cbfef18)
> > fatal: Unable to create 'C:/Users/meh/Desktop/abc/.git/index.lock': No
> > such file or directory
> >
> > On Arch Linux:
> > Preparing worktree (detached HEAD cbfef18)
> > fatal: Unable to create '/home/cosmin/Downloads/abc/.git/index.lock':
> > Not a directory
> >
> > Is it forbidden to call this command from a hook? If yes, what kind of
> > alternatives do I have? I need to make a copy of the repo in its HEAD
> > state, process it and then decide if I will pass the current commit or
> > not.
>
> I can't speak to whether `git worktree add` should succeed or fail
> inside a `pre-commit` hook, but...
>
> Why do you need a new work tree, versus whatever working copy you're
> running `git commit` in? Is there a reason whatever validation needs
> to be done can't be done in the existing working copy? `HEAD` is the
> _previous, existing commit_, not the new, currently-being-created
> commit, so your validation in the new work tree, if you actually
> managed to create one, would be applied to the _latest existing
> commit_, not the new changes you're trying to commit. Even trying to
> copy the changes over wouldn't necessarily result in the same state,
> because there may be unstaged changes.
>
> What type of validation are you trying to do? I think the failure
> you're running into is an alarm bell indicating what you're trying to
> do may not make sense. However, without any insight into what "process
> it and then decide if I will pass the current commit or not" actually
> looks like, it's hard to offer you much help.
>
> Bryan
>
> >
> > Thanks! :)

Re: Running 'git worktree add' in 'pre-commit' hook

[Duy Nguyen]

On Tue, May 14, 2019 at 9:53 PM Cosmin Polifronie <oppturbv@gmail.com> wrote:
>
> Hello! I am trying to run 'git worktree add <path> HEAD' in the
> 'pre-commit' hook, more specifically in a Python script that is being
> called from the hook. When doing so, I am greeted with the following
> error:
>
> On Windows 10:
> Preparing worktree (detached HEAD cbfef18)
> fatal: Unable to create 'C:/Users/meh/Desktop/abc/.git/index.lock': No
> such file or directory
>
> On Arch Linux:
> Preparing worktree (detached HEAD cbfef18)
> fatal: Unable to create '/home/cosmin/Downloads/abc/.git/index.lock':
> Not a directory
>
> Is it forbidden to call this command from a hook?

pre-commit hook sets GIT_INDEX_FILE to this "index.lock" so you have
the latest index content (which is not the same as from
$GIT_DIR/index). This variable will interfere with any commands that
work on a different worktree.

So you probably can still make it work by backing up $GIT_INDEX_FILE
(in case you need it), then unset it before you use "git worktree" (or
cd to it if you keep a permanent separate worktree for pre-commit
activities). To make sure you don't have similar problems, you
probably should do "env | grep GIT" from the hook and see if any other
variables are set.

> If yes, what kind of
> alternatives do I have? I need to make a copy of the repo in its HEAD
> state, process it and then decide if I will pass the current commit or
> not.
>
> Thanks! :)



-- 
Duy

Re: Running 'git worktree add' in 'pre-commit' hook

[Eric Sunshine]

On Thu, May 16, 2019 at 7:25 AM Duy Nguyen <pclouds@gmail.com> wrote:
> pre-commit hook sets GIT_INDEX_FILE to this "index.lock" so you have
> the latest index content (which is not the same as from
> $GIT_DIR/index). This variable will interfere with any commands that
> work on a different worktree.
>
> So you probably can still make it work by backing up $GIT_INDEX_FILE
> (in case you need it), then unset it before you use "git worktree" (or
> cd to it if you keep a permanent separate worktree for pre-commit
> activities). To make sure you don't have similar problems, you
> probably should do "env | grep GIT" from the hook and see if any other
> variables are set.

I researched this also and concluded that it's a bug in git-commit.
You run afoul of it in other situations, as well. For instance, say
you have your index file in a non-standard location:

    $ export GIT_INDEX_FILE=../storage/index
    $ git worktree add --detach other
    Preparing worktree (detached HEAD c9156d2)
    fatal: Unable to create '/.../foo/other/../storage/index.lock': No
such file or directory
    $

I think the correct fix is for git-commit to assign GIT_INDEX_FILE
with the absolute path to the index file, not a relative path.

Re: Running 'git worktree add' in 'pre-commit' hook

[Eric Sunshine]

On Thu, May 16, 2019 at 7:33 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> I researched this also and concluded that it's a bug in git-commit.
> You run afoul of it in other situations, as well. For instance, say
> you have your index file in a non-standard location:
>
>     $ export GIT_INDEX_FILE=../storage/index
>     $ git worktree add --detach other
>     Preparing worktree (detached HEAD c9156d2)
>     fatal: Unable to create '/.../foo/other/../storage/index.lock': No
> such file or directory
>     $
>
> I think the correct fix is for git-commit to assign GIT_INDEX_FILE
> with the absolute path to the index file, not a relative path.

Thinking more clearly on it, a better fix might be for git-worktree to
deal with this itself, converting such a path to absolute before
cd'ing to the new worktree directory (or something -- I don't have the
code in front of me to provide a more concrete suggestion).

Re: Running 'git worktree add' in 'pre-commit' hook

[Duy Nguyen]

On Thu, May 16, 2019 at 6:33 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> I researched this also and concluded that it's a bug in git-commit.
> You run afoul of it in other situations, as well. For instance, say
> you have your index file in a non-standard location:
>
>     $ export GIT_INDEX_FILE=../storage/index
>     $ git worktree add --detach other
>     Preparing worktree (detached HEAD c9156d2)
>     fatal: Unable to create '/.../foo/other/../storage/index.lock': No
> such file or directory
>     $
>
> I think the correct fix is for git-commit to assign GIT_INDEX_FILE
> with the absolute path to the index file, not a relative path.

Oh if it's relative $GIT_INDEX_FILE then I think its our environment.c
code that does not work so well when we chdir() away. I vaguely recall
something about this when discussing Jeff's chdir-notify series. But
it turns out that's about chdir() in run-command.c [1]. But the idea
is still the same, all variables are supposed to be relative to $CWD.
Whenever you move $CWD you should reparent all of them, including
$GIT_INDEX_FILE.

[1] https://public-inbox.org/git/CACsJy8CdqpNOw+zdMyugX-902Z=gLNij5_xcmE4jGLRBTqiO1g@mail.gmail.com/
-- 
Duy

Re: Running 'git worktree add' in 'pre-commit' hook

[Duy Nguyen]

On Thu, May 16, 2019 at 6:39 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Thu, May 16, 2019 at 7:33 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > I researched this also and concluded that it's a bug in git-commit.
> > You run afoul of it in other situations, as well. For instance, say
> > you have your index file in a non-standard location:
> >
> >     $ export GIT_INDEX_FILE=../storage/index
> >     $ git worktree add --detach other
> >     Preparing worktree (detached HEAD c9156d2)
> >     fatal: Unable to create '/.../foo/other/../storage/index.lock': No
> > such file or directory
> >     $
> >
> > I think the correct fix is for git-commit to assign GIT_INDEX_FILE
> > with the absolute path to the index file, not a relative path.
>
> Thinking more clearly on it, a better fix might be for git-worktree to
> deal with this itself, converting such a path to absolute before
> cd'ing to the new worktree directory (or something -- I don't have the
> code in front of me to provide a more concrete suggestion).

Our mails crossed. But yeah it's basically the same thing.

Although I still doubt a good $GIT_INDEX_FILE would fix the problem.
Even if you manage to create a new worktree, when you cd there and
start using it, this $GIT_INDEX_FILE will still mess things up (unless
you do mean to ignore the worktree's index file and update one
provided by pre-commit hook).
-- 
Duy

Re: Running 'git worktree add' in 'pre-commit' hook

[Eric Sunshine]

On Thu, May 16, 2019 at 7:25 AM Duy Nguyen <pclouds@gmail.com> wrote:
> pre-commit hook sets GIT_INDEX_FILE to this "index.lock" so you have
> the latest index content (which is not the same as from
> $GIT_DIR/index). This variable will interfere with any commands that
> work on a different worktree.

I think that this is not quite accurate. The problem isn't that
git-commit is pointing GIT_INDEX_FILE at a temporary index; that works
fine. The problem is when it is using the normal .git/index file, in
which case the value it assigns to GIT_INDEX_FILE is relative.

For instance, with pre-commit hook:

    #!/bin/sh
    echo "GIT_INDEX_FILE=$GIT_INDEX_FILE"
    git worktree add --detach other

"git-worktree add" with a normal git-commit breaks due to the relative
location specified by GIT_INDEX_FILE:

    $ git add file
    $ git commit -m foo
    GIT_INDEX_FILE=.git/index
    Preparing worktree (detached HEAD 0857fef)
    fatal: Unable to create '/.../other/.git/index.lock': Not a directory
    $

whereas, the "git commit <file>" form succeeds just fine because it
assigns an absolute path for the temporary index:

    $ git commit -m foo file
    GIT_INDEX_FILE=/.../.git/next-index-23675.lock
    Preparing worktree (detached HEAD 0857fef)
    HEAD is now at 0857fef first
    [master 2fa5413] foo
    $

Re: Running 'git worktree add' in 'pre-commit' hook

[Eric Sunshine]

On Thu, May 16, 2019 at 7:45 AM Duy Nguyen <pclouds@gmail.com> wrote:
> On Thu, May 16, 2019 at 6:39 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > Thinking more clearly on it, a better fix might be for git-worktree to
> > deal with this itself, converting such a path to absolute before
> > cd'ing to the new worktree directory (or something -- I don't have the
> > code in front of me to provide a more concrete suggestion).
>
> Our mails crossed. But yeah it's basically the same thing.
>
> Although I still doubt a good $GIT_INDEX_FILE would fix the problem.
> Even if you manage to create a new worktree, when you cd there and
> start using it, this $GIT_INDEX_FILE will still mess things up (unless
> you do mean to ignore the worktree's index file and update one
> provided by pre-commit hook).

I was forgetting that each worktree has its own index. Your advice
that his hook should sanitize GIT_INDEX_FILE (and possibly other GIT_*
variables) is probably the only sane approach. I don't think it's
something we can handle in some automated fashion since we don't know
whence GIT_INDEX_FILE arose.

Re: Running 'git worktree add' in 'pre-commit' hook

[Eric Sunshine]

On Thu, May 16, 2019 at 7:42 AM Duy Nguyen <pclouds@gmail.com> wrote:
> On Thu, May 16, 2019 at 6:33 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > You run afoul of it in other situations, as well. For instance, say
> > you have your index file in a non-standard location:
> >
> >     $ export GIT_INDEX_FILE=../storage/index
> >     $ git worktree add --detach other
> >     Preparing worktree (detached HEAD c9156d2)
> >     fatal: Unable to create '/.../foo/other/../storage/index.lock': No
> > such file or directory
> >     $
> >
> Oh if it's relative $GIT_INDEX_FILE then I think its our environment.c
> code that does not work so well when we chdir() away. I vaguely recall
> something about this when discussing Jeff's chdir-notify series. But
> it turns out that's about chdir() in run-command.c [1]. But the idea
> is still the same, all variables are supposed to be relative to $CWD.
> Whenever you move $CWD you should reparent all of them, including
> $GIT_INDEX_FILE.

builtin/worktree.c:add_worktree() is already assigning
new-worktree-specific values for GIT_DIR and GIT_WORK_TREE, so for the
specific example I showed above, the correct fix would be for
add_worktree() to remove GIT_INDEX_FILE from the environment before
invoking other Git commands, correct?

Re: Running 'git worktree add' in 'pre-commit' hook

[Duy Nguyen]

On Thu, May 16, 2019 at 7:15 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Thu, May 16, 2019 at 7:42 AM Duy Nguyen <pclouds@gmail.com> wrote:
> > On Thu, May 16, 2019 at 6:33 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > > You run afoul of it in other situations, as well. For instance, say
> > > you have your index file in a non-standard location:
> > >
> > >     $ export GIT_INDEX_FILE=../storage/index
> > >     $ git worktree add --detach other
> > >     Preparing worktree (detached HEAD c9156d2)
> > >     fatal: Unable to create '/.../foo/other/../storage/index.lock': No
> > > such file or directory
> > >     $
> > >
> > Oh if it's relative $GIT_INDEX_FILE then I think its our environment.c
> > code that does not work so well when we chdir() away. I vaguely recall
> > something about this when discussing Jeff's chdir-notify series. But
> > it turns out that's about chdir() in run-command.c [1]. But the idea
> > is still the same, all variables are supposed to be relative to $CWD.
> > Whenever you move $CWD you should reparent all of them, including
> > $GIT_INDEX_FILE.
>
> builtin/worktree.c:add_worktree() is already assigning
> new-worktree-specific values for GIT_DIR and GIT_WORK_TREE, so for the
> specific example I showed above, the correct fix would be for
> add_worktree() to remove GIT_INDEX_FILE from the environment before
> invoking other Git commands, correct?

I'll work on the assumption that all $GIT_* are for the _current_
worktree, not the one we're creating. If so, yes we need to sanitize
$GIT_*, maybe do it the same way submodules do (I think
prepare_submodule_repo_env() is the right one).
-- 
Duy

Re: Running 'git worktree add' in 'pre-commit' hook

[Jeff King]

On Thu, May 16, 2019 at 06:25:24PM +0700, Duy Nguyen wrote:

> > Is it forbidden to call this command from a hook?
> 
> pre-commit hook sets GIT_INDEX_FILE to this "index.lock" so you have
> the latest index content (which is not the same as from
> $GIT_DIR/index). This variable will interfere with any commands that
> work on a different worktree.
> 
> So you probably can still make it work by backing up $GIT_INDEX_FILE
> (in case you need it), then unset it before you use "git worktree" (or
> cd to it if you keep a permanent separate worktree for pre-commit
> activities). To make sure you don't have similar problems, you
> probably should do "env | grep GIT" from the hook and see if any other
> variables are set.

If you're entering another repo from a hook, you're supposed to use:

  unset $(git rev-parse --local-env-vars)

I wondered if we'd need another similar mechanism for entering the
worktree of another repo, that would maybe clear fewer variables. But I
think it's actually the same: we really want to clear everything and let
our "cd" pick up the new repository path.

The case of actually _adding_ a new work tree (before we enter it) is
weirder, though. We definitely want to stay in the same repository, and
clearing all of that would not make sense. I do wonder if worktree-add
should be handling GIT_INDEX_FILE (ignoring it when we want to be
dealing with the index of the new worktree we added, and handling any
relative fixups if we chdir inside the worktree code).

-Peff

Re: Running 'git worktree add' in 'pre-commit' hook

[Eric Sunshine]

On Thu, May 16, 2019 at 6:17 PM Jeff King <peff@peff.net> wrote:
> On Thu, May 16, 2019 at 06:25:24PM +0700, Duy Nguyen wrote:
> > So you probably can still make it work by backing up $GIT_INDEX_FILE
> > (in case you need it), then unset it before you use "git worktree" (or
> > cd to it if you keep a permanent separate worktree for pre-commit
> > activities). [...]
>
> The case of actually _adding_ a new work tree (before we enter it) is
> weirder, though. We definitely want to stay in the same repository, and
> clearing all of that would not make sense. I do wonder if worktree-add
> should be handling GIT_INDEX_FILE (ignoring it when we want to be
> dealing with the index of the new worktree we added, and handling any
> relative fixups if we chdir inside the worktree code).

Ignoring GIT_INDEX_FILE was indeed the conclusion reached earlier in
this thread. Addressing your other point, "git worktree add" does
chdir() into the new worktree if a post-checkout hook exists since
that hook needs to run in the new worktree, not in the worktree in
which the "git worktree add" command itself was invoked. For the hook
invocation, it already sanitizes the environment of GIT_DIR and
GIT_WORK_TREE, and GIT_INDEX_FILE ought to be cleaned too. Is there
any existing code in Git for doing the relative fixups you mention for
other Git environment variables?

Re: Running 'git worktree add' in 'pre-commit' hook

[Jeff King]

On Thu, May 16, 2019 at 07:16:54PM -0400, Eric Sunshine wrote:

> On Thu, May 16, 2019 at 6:17 PM Jeff King <peff@peff.net> wrote:
> > On Thu, May 16, 2019 at 06:25:24PM +0700, Duy Nguyen wrote:
> > > So you probably can still make it work by backing up $GIT_INDEX_FILE
> > > (in case you need it), then unset it before you use "git worktree" (or
> > > cd to it if you keep a permanent separate worktree for pre-commit
> > > activities). [...]
> >
> > The case of actually _adding_ a new work tree (before we enter it) is
> > weirder, though. We definitely want to stay in the same repository, and
> > clearing all of that would not make sense. I do wonder if worktree-add
> > should be handling GIT_INDEX_FILE (ignoring it when we want to be
> > dealing with the index of the new worktree we added, and handling any
> > relative fixups if we chdir inside the worktree code).
> 
> Ignoring GIT_INDEX_FILE was indeed the conclusion reached earlier in
> this thread. Addressing your other point, "git worktree add" does
> chdir() into the new worktree if a post-checkout hook exists since
> that hook needs to run in the new worktree, not in the worktree in
> which the "git worktree add" command itself was invoked. For the hook
> invocation, it already sanitizes the environment of GIT_DIR and
> GIT_WORK_TREE, and GIT_INDEX_FILE ought to be cleaned too. Is there
> any existing code in Git for doing the relative fixups you mention for
> other Git environment variables?

You can assign local_repo_env to child_process.env (or push it
individually to env_array if you have to mix with other variables). See
git_connect() for an example.

-Peff

Re: Running 'git worktree add' in 'pre-commit' hook

[Eric Sunshine]

On Thu, May 16, 2019 at 8:19 PM Jeff King <peff@peff.net> wrote:
> On Thu, May 16, 2019 at 07:16:54PM -0400, Eric Sunshine wrote:
> > Is there
> > any existing code in Git for doing the relative fixups you mention for
> > other Git environment variables?
>
> You can assign local_repo_env to child_process.env (or push it
> individually to env_array if you have to mix with other variables). See
> git_connect() for an example.

I wasn't aware of 'local_repo_env', but can see how it could be
helpful as a basis for building machinery to adjust relative paths
contained in those environment variables prior to a chdir(). My
original question was if such machinery already exists, but I'm
guessing from your response that it doesn't.

Re: Running 'git worktree add' in 'pre-commit' hook

[Jeff King]

On Thu, May 16, 2019 at 09:02:06PM -0400, Eric Sunshine wrote:

> On Thu, May 16, 2019 at 8:19 PM Jeff King <peff@peff.net> wrote:
> > On Thu, May 16, 2019 at 07:16:54PM -0400, Eric Sunshine wrote:
> > > Is there
> > > any existing code in Git for doing the relative fixups you mention for
> > > other Git environment variables?
> >
> > You can assign local_repo_env to child_process.env (or push it
> > individually to env_array if you have to mix with other variables). See
> > git_connect() for an example.
> 
> I wasn't aware of 'local_repo_env', but can see how it could be
> helpful as a basis for building machinery to adjust relative paths
> contained in those environment variables prior to a chdir(). My
> original question was if such machinery already exists, but I'm
> guessing from your response that it doesn't.

Oh, I see. I totally missed that you said "relative fixups" and not
"clearing". :)

There's the chdir_notify system. Any variables which need updating need
to register themselves, and then the chdir in the worktree code needs to
be done with chdir_notify(). See chdir-notify.h for details.

-Peff