threads / discuss / 59996

receive.denyCurrentBranch=updateInstead won't update the repo

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

## tl;dr

9 messages between Jul 17, 2023 and Jul 18, 2023.

replies: 8people: 2as markdown or json

Adam Sinus Skawiński· Jul 17, 2023, 20:26 UTC · lore
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
Junio C Hamano· Jul 17, 2023, 20:52 UTC · re: Adam Sinus Skawiński · lore

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

"Adam \"Sinus\" Skawiński"  <adam.skawinski@sinpi.net> writes:
Show 5 quoted lines
> 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 ;-).

Junio C Hamano· Jul 17, 2023, 21:09 UTC · re: Adam Sinus Skawiński · lore

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

"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.

Adam Sinus Skawiński· Jul 17, 2023, 22:26 UTC · lore

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

(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· Jul 18, 2023, 22:20 UTC · re: Adam Sinus Skawiński · lore

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

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:

Show 31 quoted lines
>
> (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
Junio C Hamano· Jul 18, 2023, 22:28 UTC · re: Adam Sinus Skawiński · lore

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

"Adam \"Sinus\" Skawiński"  <adam.skawinski@sinpi.net> writes:
Show 7 quoted lines
> 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· Jul 18, 2023, 22:49 UTC · re: Junio C Hamano · lore

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

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:
Show 19 quoted lines
>
> "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
Junio C Hamano· Jul 18, 2023, 23:03 UTC · re: Adam Sinus Skawiński · lore

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

"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· Jul 18, 2023, 23:29 UTC · re: Junio C Hamano · lore

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

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:
Show 13 quoted lines
>
> "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

← back to recent threads