Re: [PATCH v2 2/7] send-pack.c: add an --atomic-push command line argument

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Fixed. Thanks


On Tue, Nov 4, 2014 at 2:17 PM, Stefan Beller <sbeller@xxxxxxxxxx> wrote:
> On Mon, Nov 3, 2014 at 11:12 AM, Ronnie Sahlberg <sahlberg@xxxxxxxxxx> wrote:
>> This adds support to send-pack to to negotiate and use atomic pushes
>
> /s/to to/to/
>
>> iff the server supports it. Atomic pushes are activated by a new command
>> line flag --atomic-push.
>>
>> In order to do this we also need to change the semantics for send_pack()
>> slightly. The existing send_pack() function actually don't sent all the
>> refs back to the server when multiple refs are involved, for example
>> when using --all. Several of the failure modes for pushes can already be
>> detected locally in the send_pack client based on the information from the
>> initial server side list of all the refs as generated by receive-pack.
>> Any such refs that we thus know would fail to push are thus pruned from
>> the list of refs we send to the server to update.
>>
>> For atomic pushes, we have to deal thus with both failures that are detected
>> locally as well as failures that are reported back from the server. In order
>> to do so we treat all local failures as push failures too.
>>
>> We introduce a new status code REF_STATUS_ATOMIC_PUSH_FAILED so we can
>> flag all refs that we would normally have tried to push to the server
>> but we did not due to local failures. This is to improve the error message
>> back to the end user to flag that "these refs failed to update since the
>> atomic push operation failed."
>>
>> Signed-off-by: Ronnie Sahlberg <sahlberg@xxxxxxxxxx>
>> ---
>>  Documentation/git-send-pack.txt |  7 ++++++-
>>  builtin/send-pack.c             |  6 +++++-
>>  remote.h                        |  3 ++-
>>  send-pack.c                     | 39 ++++++++++++++++++++++++++++++++++-----
>>  send-pack.h                     |  1 +
>>  transport.c                     |  4 ++++
>>  6 files changed, 52 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
>> index 2a0de42..9296587 100644
>> --- a/Documentation/git-send-pack.txt
>> +++ b/Documentation/git-send-pack.txt
>> @@ -9,7 +9,7 @@ git-send-pack - Push objects over Git protocol to another repository
>>  SYNOPSIS
>>  --------
>>  [verse]
>> -'git send-pack' [--all] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]
>> +'git send-pack' [--all] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [--atomic-push] [<host>:]<directory> [<ref>...]
>>
>>  DESCRIPTION
>>  -----------
>> @@ -62,6 +62,11 @@ be in a separate packet, and the list must end with a flush packet.
>>         Send a "thin" pack, which records objects in deltified form based
>>         on objects not included in the pack to reduce network traffic.
>>
>> +--atomic-push::
>> +       Use an atomic transaction for updating the refs. If any of the refs
>> +       fails to update then the entire push will fail without changing any
>> +       refs.
>> +
>>  <host>::
>>         A remote host to house the repository.  When this
>>         part is specified, 'git-receive-pack' is invoked via
>> diff --git a/builtin/send-pack.c b/builtin/send-pack.c
>> index b564a77..93cb17c 100644
>> --- a/builtin/send-pack.c
>> +++ b/builtin/send-pack.c
>> @@ -13,7 +13,7 @@
>>  #include "sha1-array.h"
>>
>>  static const char send_pack_usage[] =
>> -"git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]\n"
>> +"git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [--atomic-push] [<host>:]<directory> [<ref>...]\n"
>>  "  --all and explicit <ref> specification are mutually exclusive.";
>>
>>  static struct send_pack_args args;
>> @@ -170,6 +170,10 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
>>                                 args.use_thin_pack = 1;
>>                                 continue;
>>                         }
>> +                       if (!strcmp(arg, "--atomic-push")) {
>> +                               args.use_atomic_push = 1;
>> +                               continue;
>> +                       }
>>                         if (!strcmp(arg, "--stateless-rpc")) {
>>                                 args.stateless_rpc = 1;
>>                                 continue;
>> diff --git a/remote.h b/remote.h
>> index 8b62efd..f346524 100644
>> --- a/remote.h
>> +++ b/remote.h
>> @@ -115,7 +115,8 @@ struct ref {
>>                 REF_STATUS_REJECT_SHALLOW,
>>                 REF_STATUS_UPTODATE,
>>                 REF_STATUS_REMOTE_REJECT,
>> -               REF_STATUS_EXPECTING_REPORT
>> +               REF_STATUS_EXPECTING_REPORT,
>> +               REF_STATUS_ATOMIC_PUSH_FAILED
>>         } status;
>>         char *remote_status;
>>         struct ref *peer_ref; /* when renaming */
>> diff --git a/send-pack.c b/send-pack.c
>> index 1ccc84c..08602a8 100644
>> --- a/send-pack.c
>> +++ b/send-pack.c
>> @@ -190,7 +190,7 @@ static void advertise_shallow_grafts_buf(struct strbuf *sb)
>>         for_each_commit_graft(advertise_shallow_grafts_cb, sb);
>>  }
>>
>> -static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_args *args)
>> +static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_args *args, int *atomic_push_failed)
>>  {
>>         if (!ref->peer_ref && !args->send_mirror)
>>                 return 0;
>> @@ -203,6 +203,12 @@ static int ref_update_to_be_sent(const struct ref *ref, const struct send_pack_a
>>         case REF_STATUS_REJECT_NEEDS_FORCE:
>>         case REF_STATUS_REJECT_STALE:
>>         case REF_STATUS_REJECT_NODELETE:
>> +               if (atomic_push_failed) {
>> +                       fprintf(stderr, "Atomic push failed for ref %s. "
>> +                               "Status:%d\n", ref->name, ref->status);
>> +                       *atomic_push_failed = 1;
>> +               }
>> +               /* fallthrough */
>>         case REF_STATUS_UPTODATE:
>>                 return 0;
>>         default:
>> @@ -250,7 +256,7 @@ static int generate_push_cert(struct strbuf *req_buf,
>>         strbuf_addstr(&cert, "\n");
>>
>>         for (ref = remote_refs; ref; ref = ref->next) {
>> -               if (!ref_update_to_be_sent(ref, args))
>> +               if (!ref_update_to_be_sent(ref, args, NULL))
>>                         continue;
>>                 update_seen = 1;
>>                 strbuf_addf(&cert, "%s %s %s\n",
>> @@ -297,7 +303,7 @@ int send_pack(struct send_pack_args *args,
>>         int atomic_push_supported = 0;
>>         int atomic_push = 0;
>>         unsigned cmds_sent = 0;
>> -       int ret;
>> +       int ret, atomic_push_failed = 0;
>>         struct async demux;
>>         const char *push_cert_nonce = NULL;
>>
>> @@ -332,6 +338,11 @@ int send_pack(struct send_pack_args *args,
>>                         "Perhaps you should specify a branch such as 'master'.\n");
>>                 return 0;
>>         }
>> +       if (args->use_atomic_push && !atomic_push_supported) {
>> +               fprintf(stderr, "Server does not support atomic-push.");
>> +               return -1;
>> +       }
>> +       atomic_push = atomic_push_supported && args->use_atomic_push;
>>
>>         if (status_report)
>>                 strbuf_addstr(&cap_buf, " report-status");
>> @@ -365,7 +376,8 @@ int send_pack(struct send_pack_args *args,
>>          * the pack data.
>>          */
>>         for (ref = remote_refs; ref; ref = ref->next) {
>> -               if (!ref_update_to_be_sent(ref, args))
>> +               if (!ref_update_to_be_sent(ref, args,
>> +                       args->use_atomic_push ? &atomic_push_failed : NULL))
>>                         continue;
>>
>>                 if (!ref->deletion)
>> @@ -377,6 +389,23 @@ int send_pack(struct send_pack_args *args,
>>                         ref->status = REF_STATUS_EXPECTING_REPORT;
>>         }
>>
>> +       if (atomic_push_failed) {
>> +               for (ref = remote_refs; ref; ref = ref->next) {
>> +                       if (!ref->peer_ref && !args->send_mirror)
>> +                               continue;
>> +
>> +                       switch (ref->status) {
>> +                       case REF_STATUS_EXPECTING_REPORT:
>> +                               ref->status = REF_STATUS_ATOMIC_PUSH_FAILED;
>> +                               continue;
>> +                       default:
>> +                               ; /* do nothing */
>> +                       }
>> +               }
>> +               fprintf(stderr, "Atomic push failed.");
>> +               return -1;
>> +       }
>> +
>>         /*
>>          * Finally, tell the other end!
>>          */
>> @@ -386,7 +415,7 @@ int send_pack(struct send_pack_args *args,
>>                 if (args->dry_run || args->push_cert)
>>                         continue;
>>
>> -               if (!ref_update_to_be_sent(ref, args))
>> +               if (!ref_update_to_be_sent(ref, args, NULL))
>>                         continue;
>>
>>                 old_hex = sha1_to_hex(ref->old_sha1);
>> diff --git a/send-pack.h b/send-pack.h
>> index 5635457..7486e65 100644
>> --- a/send-pack.h
>> +++ b/send-pack.h
>> @@ -11,6 +11,7 @@ struct send_pack_args {
>>                 force_update:1,
>>                 use_thin_pack:1,
>>                 use_ofs_delta:1,
>> +               use_atomic_push:1,
>>                 dry_run:1,
>>                 push_cert:1,
>>                 stateless_rpc:1;
>> diff --git a/transport.c b/transport.c
>> index f70d62f..2111986 100644
>> --- a/transport.c
>> +++ b/transport.c
>> @@ -731,6 +731,10 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
>>                                                  ref->deletion ? NULL : ref->peer_ref,
>>                                                  "remote failed to report status", porcelain);
>>                 break;
>> +       case REF_STATUS_ATOMIC_PUSH_FAILED:
>> +               print_ref_status('!', "[rejected]", ref, ref->peer_ref,
>> +                                                "atomic-push-failed", porcelain);
>> +               break;
>>         case REF_STATUS_OK:
>>                 print_ok_ref_status(ref, porcelain);
>>                 break;
>> --
>> 2.1.0.rc2.206.gedb03e5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]