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

Re: [PATCH] transport-helper: enforce atomic in push_refs_with_push

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 2, 2019, 21:37 UTC
Message-ID
<xmqqa7dw2k49.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<20190702005340.66615-1-emilyshaffer@google.com>
Emily Shaffer <emilyshaffer@google.com> writes:
Show 47 quoted lines
> Teach transport-helper how to notice if skipping a ref during push would
> violate atomicity on the client side. We notice that a ref would be
> rejected, and choose not to send it, but don't notice that if the client
> has asked for --atomic we are violating atomicity if all the other
> pushes we are sending would succeed. Asking the server end to uphold
> atomicity wouldn't work here as the server doesn't have any idea that we
> tried to update a ref that's broken.
>
> The added test-case is a succinct way to reproduce this issue that fails
> today. The same steps work fine when we aren't using a transport-helper
> to get to the upstream, i.e. when we've added a local repository as a
> remote:
>
>   git remote add ~/upstream upstream
>
> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> ---
>  t/t5541-http-push-smart.sh | 58 ++++++++++++++++++++++++++++++++++++++
>  transport-helper.c         |  6 ++++
>  transport.c                | 15 +++++++++-
>  3 files changed, 78 insertions(+), 1 deletion(-)
>
> diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh
> index 8ef8763e06..b57f6d480f 100755
> --- a/t/t5541-http-push-smart.sh
> +++ b/t/t5541-http-push-smart.sh
> @@ -177,6 +177,64 @@ test_expect_success 'push (chunked)' '
>  	 test $HEAD = $(git rev-parse --verify HEAD))
>  '
>  
> +test_expect_success 'push --atomic also prevents branch creation' '
> +	# Make up/master
> +	d=$HTTPD_DOCUMENT_ROOT_PATH/atomic-branches.git &&
> +	git init --bare "$d" &&
> +	git --git-dir="$d" config http.receivepack true &&
> +	up="$HTTPD_URL"/smart/atomic-branches.git &&
> +	test_commit atomic1 &&
> +	test_commit atomic2 &&
> +	git push "$up" master &&
> +	# Make master incompatible with up/master
> +	git reset --hard HEAD^ &&
> +	# Add a new branch
> +	git branch atomic &&
> +	# --atomic should roll back creation of up/atomic
> +	test_must_fail git push --atomic "$up" master atomic &&
> +	git ls-remote "$up" >up-remotes &&
> +	test_must_fail grep atomic up-remotes

Don't use test_must_fail on non-git things. We are not in the business of catching segfaulting system programs.

Show 19 quoted lines
> +'
> +
> +test_expect_success 'push --atomic shows all failed refs' '
> +	# Make up/master, up/allrefs
> +	d=$HTTPD_DOCUMENT_ROOT_PATH/atomic-failed-refs.git &&
> +	git init --bare "$d" &&
> +	git --git-dir="$d" config http.receivepack true &&
> +	up="$HTTPD_URL"/smart/atomic-failed-refs.git &&
> +	test_commit allrefs1 &&
> +	test_commit allrefs2 &&
> +	git branch allrefs &&
> +	git push "$up" master allrefs &&
> +	# Make master and allrefs incompatible with up/master, up/allrefs
> +	git checkout allrefs &&
> +	git reset --hard HEAD^ &&
> +	git checkout master &&
> +	git reset --hard HEAD^ &&
> +	# --atomic should complain about both master and allrefs
> +	test_must_fail git push --atomic "$up" master allrefs >&output &&

Don't rely on ">&output", which is an unnecessary bash-ism here. It breaks test run under shells like dash.

	>output 2>&1
should be OK.
Show 18 quoted lines
> +	grep master output &&
> +	grep allrefs output
> +'
> +
> +test_expect_success 'push --atomic indicates collateral failures' '
> +	# Make up/master, up/collateral
> +	d=$HTTPD_DOCUMENT_ROOT_PATH/atomic-collateral.git &&
> +	git init --bare "$d" &&
> +	git --git-dir="$d" config http.receivepack true &&
> +	up="$HTTPD_URL"/smart/atomic-collateral.git &&
> +	test_commit collateral1 &&
> +	test_commit collateral2 &&
> +	git branch collateral &&
> +	git push "$up" master collateral &&
> +	# Make master incompatible with up/master
> +	git reset --hard HEAD^ &&
> +	# --atomic should mention collateral was OK but failed anyway
> +	test_must_fail git push --atomic "$up" master collateral >&output &&
Ditto.
> +	grep "master -> master" output &&
> +	grep "collateral -> collateral" output
> +'
> +
Previous: Emily ShafferNext: Emily Shaffer
Message 13 of 45 in “transport-helper: enforce atomic in push_refs_with_push”
  1. transport-helper: enforce atomic in push_refs_with_pushEmily Shaffer, Jul 2, 2019
  2. Johannes SchindelinJul 2, 2019
  3. Junio C HamanoJul 2, 2019
  4. Emily ShafferJul 3, 2019
  5. Emily ShafferJul 3, 2019
  6. Johannes SchindelinJul 3, 2019
  7. Emily ShafferJul 3, 2019
  8. Johannes SchindelinJul 4, 2019
  9. Emily ShafferJul 9, 2019
  10. Junio C HamanoJul 2, 2019
  11. Junio C HamanoJul 2, 2019
  12. Emily ShafferJul 3, 2019
  13. Junio C HamanoJul 2, 2019
  14. Emily ShafferJul 3, 2019
  15. SZEDER GáborJul 3, 2019
  16. Junio C HamanoJul 3, 2019
  17. Emily ShafferJul 3, 2019
  18. transport-helper: enforce atomic in push_refs_with_pushEmily Shaffer, Jul 9, 2019
  19. Junio C HamanoJul 10, 2019
  20. Junio C HamanoJul 10, 2019
  21. Emily ShafferJul 11, 2019
  22. Emily ShafferJul 11, 2019
  23. Junio C HamanoJul 11, 2019
  24. transport-helper: enforce atomic in push_refs_with_pushEmily Shaffer, Jul 11, 2019
  25. Junio C HamanoJul 12, 2019
  26. Carlo ArenasJul 16, 2019
  27. Junio C HamanoJul 16, 2019
  28. CodingGuidelines: spell out post-C89 rulesJunio C Hamano, Jul 16, 2019
  29. Jonathan NiederJul 17, 2019
  30. Junio C HamanoJul 17, 2019
  31. Jonathan NiederJul 19, 2019
  32. Bryan TurnerJul 17, 2019
  33. Junio C HamanoJul 17, 2019
  34. Carlo ArenasJul 16, 2019
  35. transport-helper: avoid var decl in for () loop controlJunio C Hamano, Jul 16, 2019
  36. Jonathan NiederJul 17, 2019
  37. SZEDER GáborJul 18, 2019
  38. Junio C HamanoJul 18, 2019
  39. SZEDER GáborJul 18, 2019
  40. Eric SunshineJul 18, 2019
  41. Jonathan NiederJul 19, 2019
  42. Carlo ArenasJul 19, 2019
  43. Junio C HamanoJul 19, 2019
  44. SZEDER GáborJul 27, 2019
  45. Junio C HamanoJul 27, 2019

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.