git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 0/3] pre-push hook support

From
Junio C Hamano <gitster@pobox.com>
Date
Jan 15, 2013, 00:24 UTC
Message-ID
<7vpq1770pr.fsf@alter.siamese.dyndns.org>
In-Reply-To
<7va9sb8jg7.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
Show 20 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Aaron Schrab <aaron@schrab.com> writes:
>>
>>> Main changes since the initial version:
>>>
>>>  * The first patch converts the existing hook callers to use the new
>>>    find_hook() function.
>>>  * Information about what is to be pushed is now sent over a pipe rather
>>>    than passed as command-line parameters.
>>>
>>> Aaron Schrab (3):
>>>   hooks: Add function to check if a hook exists
>>>   push: Add support for pre-push hooks
>>>   Add sample pre-push hook script
>>
>> Getting much nicer.  Thanks.
>
> Hmph, t5571 seems to be flaky in that it sometimes fails but passes
> when run again.  Something timing dependent is going on???
With this patch applied, repeatedly try to
 - make sure "foreign" ref does not exist; and
 - attempt pushing the HEAD:foreign to create the "foreign" ref

until it fails, I can get it stop before the output scrolls off of my 114 line terminal. Then when I revert the changes to transport.[ch] and builtin/push.c in this series, the test will keep going.

Wait. The sample hook used in the test _is_ fed some input but it exits without reading any. What happens when we fork it, and it completes execution before we even have a chance to feed a single byte? Wont' we get a sigpipe and die?

Yup, I think that is what is missing from run_pre_push_hook() implementation.

diff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh
index d68fed7..050318b 100755
--- a/t/t5571-pre-push-hook.sh
+++ b/t/t5571-pre-push-hook.sh
@@ -16,8 +16,15 @@ test_expect_success 'setup' '
 	git init --bare repo1 &&
 	git remote add parent1 repo1 &&
 	test_commit one &&
-	git push parent1 HEAD:foreign
+	while :
+	do
+		git push parent1 :refs/heads/foreign &&
+		git push parent1 HEAD:foreign || break
+	done
 '
+
+exit
+
 write_script "$HOOK" <<EOF
 exit 1
 EOF
Previous: Junio C Hamano
Message 22 of 22 in “pre-push hook support”
  1. 0/4 pre-push hook supportAaron Schrab, Dec 28, 2012
  2. 1/4 hooks: Add function to check if a hook existsAaron Schrab, Dec 28, 2012
  3. Junio C HamanoDec 29, 2012
  4. Aaron SchrabDec 29, 2012
  5. Junio C HamanoDec 29, 2012
  6. 2/4 hooks: support variable number of parametersAaron Schrab, Dec 28, 2012
  7. 3/4 push: Add support for pre-push hooksAaron Schrab, Dec 28, 2012
  8. 4/4 Add sample pre-push hook scriptAaron Schrab, Dec 28, 2012
  9. Junio C HamanoDec 29, 2012
  10. Aaron SchrabDec 29, 2012
  11. Junio C HamanoDec 29, 2012
  12. 0/3 pre-push hook supportAaron Schrab, Jan 13, 2013
  13. 1/3 hooks: Add function to check if a hook existsAaron Schrab, Jan 13, 2013
  14. 2/3 push: Add support for pre-push hooksAaron Schrab, Jan 13, 2013
  15. Junio C HamanoJan 14, 2013
  16. Junio C HamanoJan 15, 2013
  17. Junio C HamanoJan 15, 2013
  18. 3/3 Add sample pre-push hook scriptAaron Schrab, Jan 13, 2013
  19. Junio C HamanoJan 14, 2013
  20. Junio C HamanoJan 14, 2013
  21. Junio C HamanoJan 14, 2013
  22. Junio C HamanoJan 15, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.