Subject: [PATCH v2 0/4] Never report references we not push

From: Jiang Xin <[email-masked]>
Date: Sun, 29 Mar 2020 10:33:25 -0400

From: Jiang Xin <[email-masked]>

Hi,

Patches 2/4, 4/3 and 4/4 can be squashed into one commit.

## Changes since v1:

* Add a new common method `reject_atomic_push()`, which is reused for 
  atomic push rejection for SSH and HTTP protocol.

* Other changes see the attached range diff.

## Range diff v1..v2

1:  e1c7aa3501 ! 1:  d84ebf30b5 t5543: never report what we do not push
    @@ t/t5543-atomic-push.sh: test_refs () {
      	test_cmp expect actual
      }
      
    -+format_git_output () {
    -+	awk '/^(To| !) / {print}' | \
    -+	sed \
    -+		-e "s/  *\$//g" \
    -+		-e "s/'/\"/g"
    ++fmt_status_report () {
    ++	sed -n \
    ++		-e "/^To / { s/   */ /g; p; }" \
    ++		-e "/^ ! / { s/   */ /g; p; }"
     +}
     +
      test_expect_success 'atomic push works for a single branch' '
    @@ t/t5543-atomic-push.sh: test_expect_success 'atomic push is not advertised if co
     +	mk_repo_pair &&
     +	(
     +		cd workbench &&
    -+		# Keep constant output.
    -+		git config core.abbrev 7 &&
     +		test_commit one &&
     +		git branch foo &&
     +		git push up master one foo &&
    @@ t/t5543-atomic-push.sh: test_expect_success 'atomic push is not advertised if co
     +
     +		if test "\$1" = "refs/heads/bar"
     +		then
    -+			echo >2 "Pusing to branch bar is prohibited"
    ++			echo >&2 "Pusing to branch bar is prohibited"
     +			exit 1
     +		fi
     +		EOF
    @@ t/t5543-atomic-push.sh: test_expect_success 'atomic push is not advertised if co
     +	) &&
     +	test_must_fail git -C workbench \
     +		push --atomic up master two bar >out 2>&1 &&
    -+	format_git_output <out >actual &&
    ++	fmt_status_report <out >actual &&
     +	cat >expect <<-EOF &&
     +	To ../upstream
     +	 ! [remote rejected] master -> master (atomic push failure)
    @@ t/t5543-atomic-push.sh: test_expect_success 'atomic push is not advertised if co
     +	) &&
     +	test_must_fail git -C workbench \
     +		push --atomic --mirror up >out 2>&1 &&
    -+	format_git_output <out >actual &&
    ++	fmt_status_report <out >actual &&
     +	cat >expect <<-EOF &&
     +	To ../upstream
     +	 ! [remote rejected] master -> master (atomic push failure)
    @@ t/t5543-atomic-push.sh: test_expect_success 'atomic push is not advertised if co
     +	) &&
     +	test_must_fail git -C workbench \
     +		push --atomic up master foo bar >out 2>&1 &&
    -+	format_git_output <out >actual &&
    ++	fmt_status_report <out >actual &&
     +	cat >expect <<-EOF &&
     +	To ../upstream
    -+	 ! [rejected]        master -> master (non-fast-forward)
    -+	 ! [rejected]        bar -> bar (atomic push failed)
    ++	 ! [rejected] master -> master (non-fast-forward)
    ++	 ! [rejected] bar -> bar (atomic push failed)
     +	EOF
     +	test_cmp expect actual
     +'
2:  50f1e02a2f ! 2:  a76340633b send-pack: mark failure of atomic push properly
    @@ Metadata
      ## Commit message ##
         send-pack: mark failure of atomic push properly
     
    -    When pusing with SSH or other smart protocol, references are validated
    -    by function `check_to_send_update()` beforce they are sent in commands
    +    When pushing with SSH or other smart protocol, references are validated
    +    by function `check_to_send_update()` before they are sent in commands
         to `send_pack()` of "receve-pack".  For atomic push, if a reference is
         rejected after the validation, only references pushed by user should be
         marked as failure, instead of report failure on all remote references.
    @@ Commit message
         push_refs_with_push, 2019-07-11) wanted to fix report issue of HTTP
         protocol, but marked all remote references failure for atomic push.
     
    -    Revert part of that commit and add additional status for function
    -    `atomic_push_failure()`.  The additional status for it except the
    -    "REF_STATUS_EXPECTING_REPORT" status are:
    +    In order to fix the issue of status report for SSH or other built-in
    +    smart protocol, revert part of that commit and add additional status
    +    for function `atomic_push_failure()`.  The additional status for it
    +    except the "REF_STATUS_EXPECTING_REPORT" status are:
     
         - REF_STATUS_NONE : Not marked as "REF_STATUS_EXPECTING_REPORT" yet.
         - REF_STATUS_OK   : Assume OK for dryrun or status_report is disabled.
     
    -    This commit break test case in t5541, and will fix in other commit.
    +    This fix won't resolve the issue of status report in transport-helper
    +    for HTTP or other protocols, and breaks test case in t5541.  Will fix
    +    it in additional commit.
     
         Signed-off-by: Jiang Xin <[email-masked]>
     
3:  50bb31ed3a ! 3:  82a8b19430 transport-helper: enforce atomic in push_refs_with_push
    @@ Metadata
     Author: Jiang Xin <[email-masked]>
     
      ## Commit message ##
    -    transport-helper: enforce atomic in push_refs_with_push
    +    transport-helper: mark failure for atomic push
     
         Commit v2.22.0-1-g3bca1e7f9f (transport-helper: enforce atomic in
         push_refs_with_push, 2019-07-11) noticed the incomplete report of
-:  ---------- > 4:  fc8c1af33c transport-helper: new method reject_atomic_push()

Jiang Xin (4):
  t5543: never report what we do not push
  send-pack: mark failure of atomic push properly
  transport-helper: mark failure for atomic push
  transport-helper: new method reject_atomic_push()

 send-pack.c                | 27 ++----------
 t/t5541-http-push-smart.sh | 12 ++++-
 t/t5543-atomic-push.sh     | 89 ++++++++++++++++++++++++++++++++++++++
 transport-helper.c         | 23 ++++++++++
 transport.c                | 14 ------
 transport.h                |  3 ++
 6 files changed, 128 insertions(+), 40 deletions(-)

-- 
2.26.0.4.g39bcdcb101.dirty