receive.denyCurrentBranch=updateInstead won't update the repo

receive.denyCurrentBranch=updateInstead won't update the repo

[Adam "Sinus" Skawiński]

I'm using Git to push-to-checkout a website.

The receive.denyCurrentBranch=updateInstead option (set on the server
repo), according to the docs, should allow the push and do a checkout
on the worktree if it's clean, or just refuse the push if the worktree
isn't clean. Instead, for me it allows the push, but doesn't update
the worktree - I have to `git checkout -f` on the remote shell after
each push, even though the worktree is clean.

Note: The `push-to-checkout` hook doesn't seem to run, either, even
though it's in the .git/hooks/push-to-checkout file, with +x rights as
needed. I made it output some lines and exit with code 1, so it should
abort after producing the output - and yet the pushes come through.
Other hooks, like `update`, work fine and can abort if they exit 1.

Setup:

My client is 2.31.0.windows.1 (TortoiseGit)
Server’s git is 2.39.1.
My remote is set as ssh://user@hostname.com/path.

Please advise what I can do to investigate further.

-- 
Adam "Sinus" Skawiński

Re: receive.denyCurrentBranch=updateInstead won't update the repo

[Junio C Hamano]

"Adam \"Sinus\" Skawiński"  <adam.skawinski@sinpi.net> writes:

> Note: The `push-to-checkout` hook doesn't seem to run, either, even
> though it's in the .git/hooks/push-to-checkout file, with +x rights as
> needed. I made it output some lines and exit with code 1, so it should
> abort after producing the output - and yet the pushes come through.
> Other hooks, like `update`, work fine and can abort if they exit 1.

I have nothing to offer offhand, but have you compared on the server
side your "update" and "push-to-checkout" hooks?  Even a simple typo
of the filename "push-to-checkout" would make it untable to run ;-).
Similarly to the name of the configuration variable, which is rather
long and prone to be mistyped.  After staring at a problem for a
long time getting frustrated, I myself would miss such a trivial
mistake, and would want to go back and double check just to be sure.

In fact, while trying to reproduce the problem with a simple pair of
repositories, I did manage to mistype denyCurrentBranch and saw that
the machinery not to kick in at all ;-).

Re: receive.denyCurrentBranch=updateInstead won't update the repo

[Junio C Hamano]

"Adam \"Sinus\" Skawiński"  <adam.skawinski@sinpi.net> writes:

> needed. I made it output some lines and exit with code 1, so it should
> abort after producing the output - and yet the pushes come through.

Did the output come back to your client side?  "git push -v" should
show.

If there is no need to update (i.e. after a successful push, you try
to push the same thing), the hook would not even run, so you may need
to rewind the "server" side while experimenting.

Re: receive.denyCurrentBranch=updateInstead won't update the repo

[Adam "Sinus" Skawiński]

(I seem to have responded privately to the helpful Junio C Hamano,
instead of to the list, sorry about that.)

Well, there's progress. Indeed I must have borked something with the
hook, as I can now confirm that the hook works and fails as intended.

Still, local-to-server pushes fail to checkout on the server, without
any indication as to why. The server clearly recognizes the
updateInstead option, as it runs the push-to-checkout hook, but then
it silently fails to update the remote worktree.

Any idea what could be causing this?

Pushing to ssh://*hostname*/home/uuuusername/domains/mywebsite.com
Enumerating objects: 5, done.
Counting objects: 100% (5/5), done.
Delta compression using up to 8 threads
Compressing objects: 100% (3/3), done.
Writing objects: 100% (3/3), 284 bytes | 284.00 KiB/s, done.
Total 3 (delta 2), reused 0 (delta 0), pack-reused 0
remote: HOOK UPDATE: 1=refs/heads/mywebsite
2=650585f1c2abdbbd68de71acfb31b5d66ee1d0f3
3=adc4a555cfc23e037ad026a405a13e7a468188c4 GITDIR=
HOOK PUSH-TO-CHECKOUT 1=adc4a555cfc23e037ad026a405a13e7a468188c4 2= 3= GITDIR=
To ssh://*hostname*/home/uuuusername/domains/mywebsite.com
650585f..adc4a55  mywebsite -> mywebsite
updating local tracking ref 'refs/remotes/myserver-mywebsite/mywebsite'

-- 
Adam "Sinus" Skawiński

Re: receive.denyCurrentBranch=updateInstead won't update the repo

[Adam "Sinus" Skawiński]

Correct me if I'm wrong, but I'm looking at receive-pack.c and can't
grasp one thing.

In receive-pack.c:1452-1453,
> if (!invoked_hook)
>   retval = push_to_deploy(sha1, &env, worktree->path);
... push_to_deploy is reached only if... hook didn't get invoked?

I have now re-tested and, indeed, once I got _rid_ of the hook
entirely, deployment proceeded!
So, shouldn't that be a check for retval instead, perhaps..?

On Tue, 18 Jul 2023 at 00:26, Adam "Sinus" Skawiński
<adam.skawinski@sinpi.net> wrote:
>
> (I seem to have responded privately to the helpful Junio C Hamano,
> instead of to the list, sorry about that.)
>
> Well, there's progress. Indeed I must have borked something with the
> hook, as I can now confirm that the hook works and fails as intended.
>
> Still, local-to-server pushes fail to checkout on the server, without
> any indication as to why. The server clearly recognizes the
> updateInstead option, as it runs the push-to-checkout hook, but then
> it silently fails to update the remote worktree.
>
> Any idea what could be causing this?
>
> Pushing to ssh://*hostname*/home/uuuusername/domains/mywebsite.com
> Enumerating objects: 5, done.
> Counting objects: 100% (5/5), done.
> Delta compression using up to 8 threads
> Compressing objects: 100% (3/3), done.
> Writing objects: 100% (3/3), 284 bytes | 284.00 KiB/s, done.
> Total 3 (delta 2), reused 0 (delta 0), pack-reused 0
> remote: HOOK UPDATE: 1=refs/heads/mywebsite
> 2=650585f1c2abdbbd68de71acfb31b5d66ee1d0f3
> 3=adc4a555cfc23e037ad026a405a13e7a468188c4 GITDIR=
> HOOK PUSH-TO-CHECKOUT 1=adc4a555cfc23e037ad026a405a13e7a468188c4 2= 3= GITDIR=
> To ssh://*hostname*/home/uuuusername/domains/mywebsite.com
> 650585f..adc4a55  mywebsite -> mywebsite
> updating local tracking ref 'refs/remotes/myserver-mywebsite/mywebsite'
>
> --
> Adam "Sinus" Skawiński



-- 
Adam "Sinus" Skawiński

Re: receive.denyCurrentBranch=updateInstead won't update the repo

[Junio C Hamano]

"Adam \"Sinus\" Skawiński"  <adam.skawinski@sinpi.net> writes:

> Correct me if I'm wrong, but I'm looking at receive-pack.c and can't
> grasp one thing.
>
> In receive-pack.c:1452-1453,
>> if (!invoked_hook)
>>   retval = push_to_deploy(sha1, &env, worktree->path);
> ... push_to_deploy is reached only if... hook didn't get invoked?

Correct.  The hook is responsible for both DECIDING if it wants to
touch/update the working tree, AND ACTUALLY UPDATING the working
tree itself.

And the entire point of the "hook" is that its update does not have
to be just "checkout the given commit's tree", for which using the
default push-to-deply is sufficient.  It is for those who want to do
more.

Re: receive.denyCurrentBranch=updateInstead won't update the repo

[Adam "Sinus" Skawiński]

Oh. Then it's different from the other hooks, which merely enhance or
abort the default behaviour by returning non-zero! I hadn't thought of
that. The documentation is phrased in a way that made me assume "This
hook is to be used to override the default behaviour" means overriding
the tree cleanliness checks, not replacing the whole deployment
routine, which - if a hook is in place - needs to be fully
reimplemented in the hook.

All in all, I'm glad it works now. Thank you for the explanation, I
shall bother you no more. :)

On Wed, 19 Jul 2023 at 00:29, Junio C Hamano <gitster@pobox.com> wrote:
>
> "Adam \"Sinus\" Skawiński"  <adam.skawinski@sinpi.net> writes:
>
> > Correct me if I'm wrong, but I'm looking at receive-pack.c and can't
> > grasp one thing.
> >
> > In receive-pack.c:1452-1453,
> >> if (!invoked_hook)
> >>   retval = push_to_deploy(sha1, &env, worktree->path);
> > ... push_to_deploy is reached only if... hook didn't get invoked?
>
> Correct.  The hook is responsible for both DECIDING if it wants to
> touch/update the working tree, AND ACTUALLY UPDATING the working
> tree itself.
>
> And the entire point of the "hook" is that its update does not have
> to be just "checkout the given commit's tree", for which using the
> default push-to-deply is sufficient.  It is for those who want to do
> more.



-- 
Adam "Sinus" Skawiński

Re: receive.denyCurrentBranch=updateInstead won't update the repo

[Junio C Hamano]

"Adam \"Sinus\" Skawiński"  <adam.skawinski@sinpi.net> writes:

> Oh. Then it's different from the other hooks, which merely enhance or
> abort the default behaviour by returning non-zero! I hadn't thought of
> that.

I do not know if it is all that different from others, but in any
case it is a clear sign that the documentation needs a bit more love
to make sure that it will not lead new readers and users into the
same confusion.

Thanks.

Re: receive.denyCurrentBranch=updateInstead won't update the repo

[Adam "Sinus" Skawiński]

Glad to have pointed out a possible improvement. :)

By "different from others" I primarily meant that all the other hooks
(save for fsmonitor-watchman, I guess) are safe to simply be "echo
Hello world; exit 0" as they don't REPLACE any functionality. This one
does.

Perhaps there's a need for "pre-deploy" / "post-deploy" hooks,
ensuring that it's a good time to deploy, maybe bringing a live
website into and out of maintenance mode after the hopefully brief
deployment..?

On Wed, 19 Jul 2023 at 01:03, Junio C Hamano <gitster@pobox.com> wrote:
>
> "Adam \"Sinus\" Skawiński"  <adam.skawinski@sinpi.net> writes:
>
> > Oh. Then it's different from the other hooks, which merely enhance or
> > abort the default behaviour by returning non-zero! I hadn't thought of
> > that.
>
> I do not know if it is all that different from others, but in any
> case it is a clear sign that the documentation needs a bit more love
> to make sure that it will not lead new readers and users into the
> same confusion.
>
> Thanks.



-- 
Adam "Sinus" Skawiński