Merge branch 'bc/smart-http-atomic-push'

The atomic push over smart HTTP transport did not work, which has
been corrected.

* bc/smart-http-atomic-push:
  remote-curl: pass on atomic capability to remote side

diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt
index a5c3c04371205e0a3694d403b934ab43eacea0f7..f48a031dc346a307aa33aebc62686709a18673f6 100644 (file)
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -509,6 +509,11 @@ set by Git if the remote helper has the 'option' capability.
        Indicate that only the objects wanted need to be fetched, not
        their dependents.
 
+'option atomic' {'true'|'false'}::
+       When pushing, request the remote server to update refs in a single atomic
+       transaction.  If successful, all refs will be updated, or none will.  If the
+       remote side does not support this capability, the push will fail.
+
 SEE ALSO
 --------
 linkgit:git-remote[1]

diff --git a/remote-curl.c b/remote-curl.c
index 1612e7f52d7930ea2d5fdb0a3df876a597694463..350d92a074ed82a99060cebb1a4a3fb0a98cfd8e 100644 (file)
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -40,7 +40,8 @@ struct options {
                push_cert : 2,
                deepen_relative : 1,
                from_promisor : 1,
-               no_dependents : 1;
+               no_dependents : 1,
+               atomic : 1;
 };
 static struct options options;
 static struct string_list cas_options = STRING_LIST_INIT_DUP;
@@ -148,6 +149,14 @@ static int set_option(const char *name, const char *value)
                else
                        return -1;
                return 0;
+       } else if (!strcmp(name, "atomic")) {
+               if (!strcmp(value, "true"))
+                       options.atomic = 1;
+               else if (!strcmp(value, "false"))
+                       options.atomic = 0;
+               else
+                       return -1;
+               return 0;
        } else if (!strcmp(name, "push-option")) {
                if (*value != '"')
                        string_list_append(&options.push_options, value);
@@ -1196,6 +1205,8 @@ static int push_git(struct discovery *heads, int nr_spec, const char **specs)
                argv_array_push(&args, "--signed=yes");
        else if (options.push_cert == SEND_PACK_PUSH_CERT_IF_ASKED)
                argv_array_push(&args, "--signed=if-asked");
+       if (options.atomic)
+               argv_array_push(&args, "--atomic");
        if (options.verbosity == 0)
                argv_array_push(&args, "--quiet");
        else if (options.verbosity > 1)

diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh
index 92bac4325733e270133b270095f213c9db0a0108..4c970787b0ec1fcc3d18cdcab0f5c8fd15f16afd 100755 (executable)
--- a/t/t5541-http-push-smart.sh
+++ b/t/t5541-http-push-smart.sh
@@ -184,11 +184,12 @@ test_expect_success 'push --atomic also prevents branch creation, reports collat
        test_config -C "$d" http.receivepack true &&
        up="$HTTPD_URL"/smart/atomic-branches.git &&
 
-       # Tell "$up" about two branches for now
+       # Tell "$up" about three branches for now
        test_commit atomic1 &&
        test_commit atomic2 &&
        git branch collateral &&
-       git push "$up" master collateral &&
+       git branch other &&
+       git push "$up" master collateral other &&
 
        # collateral is a valid push, but should be failed by atomic push
        git checkout collateral &&
@@ -226,6 +227,41 @@ test_expect_success 'push --atomic also prevents branch creation, reports collat
        grep "^ ! .*rejected.* collateral -> collateral .*atomic push failed" output
 '
 
+test_expect_success 'push --atomic fails on server-side errors' '
+       # Use previously set up repository
+       d=$HTTPD_DOCUMENT_ROOT_PATH/atomic-branches.git &&
+       test_config -C "$d" http.receivepack true &&
+       up="$HTTPD_URL"/smart/atomic-branches.git &&
+
+       # break ref updates for other on the remote site
+       mkdir "$d/refs/heads/other.lock" &&
+
+       # add the new commit to other
+       git branch -f other collateral &&
+
+       # --atomic should cause entire push to be rejected
+       test_must_fail git push --atomic "$up" atomic other 2>output  &&
+
+       # the new branch should not have been created upstream
+       test_must_fail git -C "$d" show-ref --verify refs/heads/atomic &&
+
+       # upstream should still reflect atomic2, the last thing we pushed
+       # successfully
+       git rev-parse atomic2 >expected &&
+       # ...to other.
+       git -C "$d" rev-parse refs/heads/other >actual &&
+       test_cmp expected actual &&
+
+       # the new branch should not have been created upstream
+       test_must_fail git -C "$d" show-ref --verify refs/heads/atomic &&
+
+       # the failed refs should be indicated to the user
+       grep "^ ! .*rejected.* other -> other .*atomic transaction failed" output &&
+
+       # the collateral failure refs should be indicated to the user
+       grep "^ ! .*rejected.* atomic -> atomic .*atomic transaction failed" output
+'
+
 test_expect_success 'push --all can push to empty repo' '
        d=$HTTPD_DOCUMENT_ROOT_PATH/empty-all.git &&
        git init --bare "$d" &&

diff --git a/transport-helper.c b/transport-helper.c
index 9e1279b928bb3eadf110e43c091b4a3ba789fe3b..a9d690297e1fe33abdafed6126f0b4541ce35af5 100644 (file)
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -854,6 +854,10 @@ static void set_common_push_options(struct transport *transport,
                        die(_("helper %s does not support --signed=if-asked"), name);
        }
 
+       if (flags & TRANSPORT_PUSH_ATOMIC)
+               if (set_helper_option(transport, TRANS_OPT_ATOMIC, "true") != 0)
+                       die(_("helper %s does not support --atomic"), name);
+
        if (flags & TRANSPORT_PUSH_OPTIONS) {
                struct string_list_item *item;
                for_each_string_list_item(item, transport->push_options)

diff --git a/transport.h b/transport.h
index 0b5f7806f625d888175e1c633ec2b59d8d07803b..e0131daab987f582801972fe84aa412d235c89f7 100644 (file)
--- a/transport.h
+++ b/transport.h
@@ -208,6 +208,9 @@ void transport_check_allowed(const char *type);
 /* Filter objects for partial clone and fetch */
 #define TRANS_OPT_LIST_OBJECTS_FILTER "filter"
 
+/* Request atomic (all-or-nothing) updates when pushing */
+#define TRANS_OPT_ATOMIC "atomic"
+
 /**
  * Returns 0 if the option was used, non-zero otherwise. Prints a
  * message to stderr if the option is not used.