Merge branch 'es/local-atomic-push-failure-with-http'
authorJunio C Hamano <gitster@pobox.com>
Thu, 25 Jul 2019 20:59:22 +0000 (13:59 -0700)
committerJunio C Hamano <gitster@pobox.com>
Thu, 25 Jul 2019 20:59:22 +0000 (13:59 -0700)
"git push --atomic" that goes over the transport-helper (namely,
the smart http transport) failed to prevent refs to be pushed when
it can locally tell that one of the ref update will fail without
having to consult the other end, which has been corrected.

* es/local-atomic-push-failure-with-http:
transport-helper: avoid var decl in for () loop control
transport-helper: enforce atomic in push_refs_with_push

1  2 
t/t5541-http-push-smart.sh
transport-helper.c
transport.c
index 2e4802e2063b87bc6c36d2e728195036e47d8edc,92bac4325733e270133b270095f213c9db0a0108..b86ddb60f2ea6ec1c177b8be5d056d064a1d7697
@@@ -177,6 -177,55 +177,55 @@@ test_expect_success 'push (chunked)' 
         test $HEAD = $(git rev-parse --verify HEAD))
  '
  
+ test_expect_success 'push --atomic also prevents branch creation, reports collateral' '
+       # Setup upstream repo - empty for now
+       d=$HTTPD_DOCUMENT_ROOT_PATH/atomic-branches.git &&
+       git init --bare "$d" &&
+       test_config -C "$d" http.receivepack true &&
+       up="$HTTPD_URL"/smart/atomic-branches.git &&
+       # Tell "$up" about two branches for now
+       test_commit atomic1 &&
+       test_commit atomic2 &&
+       git branch collateral &&
+       git push "$up" master collateral &&
+       # collateral is a valid push, but should be failed by atomic push
+       git checkout collateral &&
+       test_commit collateral1 &&
+       # Make master incompatible with upstream to provoke atomic
+       git checkout master &&
+       git reset --hard HEAD^ &&
+       # Add a new branch which should be failed by atomic push. This is a
+       # regression case.
+       git branch atomic &&
+       # --atomic should cause entire push to be rejected
+       test_must_fail git push --atomic "$up" master atomic collateral 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 &&
+       # on master...
+       git -C "$d" rev-parse refs/heads/master >actual &&
+       test_cmp expected actual &&
+       # ...and collateral.
+       git -C "$d" rev-parse refs/heads/collateral >actual &&
+       test_cmp expected actual &&
+       # the failed refs should be indicated to the user
+       grep "^ ! .*rejected.* master -> master" output &&
+       # the collateral failure refs should be indicated to the user
+       grep "^ ! .*rejected.* atomic -> atomic .*atomic push failed" output &&
+       grep "^ ! .*rejected.* collateral -> collateral .*atomic push failed" output
+ '
  test_expect_success 'push --all can push to empty repo' '
        d=$HTTPD_DOCUMENT_ROOT_PATH/empty-all.git &&
        git init --bare "$d" &&
@@@ -213,7 -262,7 +262,7 @@@ test_expect_success TTY 'push shows pro
        cd "$ROOT_PATH"/test_repo_clone &&
        test_commit noisy &&
        test_terminal git push >output 2>&1 &&
 -      test_i18ngrep "^Writing objects" output
 +      test_i18ngrep "Writing objects" output
  '
  
  test_expect_success TTY 'push --quiet silences status and progress' '
@@@ -228,7 -277,7 +277,7 @@@ test_expect_success TTY 'push --no-prog
        test_commit no-progress &&
        test_terminal git push --no-progress >output 2>&1 &&
        test_i18ngrep "^To http" output &&
 -      test_i18ngrep ! "^Writing objects" output
 +      test_i18ngrep ! "Writing objects" output
  '
  
  test_expect_success 'push --progress shows progress to non-tty' '
        test_commit progress &&
        git push --progress >output 2>&1 &&
        test_i18ngrep "^To http" output &&
 -      test_i18ngrep "^Writing objects" output
 +      test_i18ngrep "Writing objects" output
  '
  
  test_expect_success 'http push gives sane defaults to reflog' '
diff --combined transport-helper.c
index c7e17ec9cb61e6782bc255e6b90381054255c269,b11abd291cd22562217ed167107b7eb59414d9a8..6b05a88faf59ee74840b0f9fa340a5d9397a0f5d
@@@ -423,7 -423,7 +423,7 @@@ static int get_importer(struct transpor
        struct helper_data *data = transport->data;
        int cat_blob_fd, code;
        child_process_init(fastimport);
 -      fastimport->in = helper->out;
 +      fastimport->in = xdup(helper->out);
        argv_array_push(&fastimport->args, "fast-import");
        argv_array_push(&fastimport->args, debug ? "--stats" : "--quiet");
  
@@@ -853,6 -853,7 +853,7 @@@ static int push_refs_with_push(struct t
  {
        int force_all = flags & TRANSPORT_PUSH_FORCE;
        int mirror = flags & TRANSPORT_PUSH_MIRROR;
+       int atomic = flags & TRANSPORT_PUSH_ATOMIC;
        struct helper_data *data = transport->data;
        struct strbuf buf = STRBUF_INIT;
        struct ref *ref;
                case REF_STATUS_REJECT_NONFASTFORWARD:
                case REF_STATUS_REJECT_STALE:
                case REF_STATUS_REJECT_ALREADY_EXISTS:
+                       if (atomic) {
+                               string_list_clear(&cas_options, 0);
+                               return 0;
+                       } else
+                               continue;
                case REF_STATUS_UPTODATE:
                        continue;
                default:
diff --combined transport.c
index 2def5a0c356bb1cff473aaefb2d17f0b6e494e0e,453de8f7041b966a9bd0eaee20ef8f5758cec2f1..778c60bf573a714134435e438da5d390fa490484
@@@ -1226,6 -1226,20 +1226,20 @@@ int transport_push(struct repository *r
                err = push_had_errors(remote_refs);
                ret = push_ret | err;
  
+               if ((flags & TRANSPORT_PUSH_ATOMIC) && err) {
+                       struct ref *it;
+                       for (it = remote_refs; it; it = it->next)
+                               switch (it->status) {
+                               case REF_STATUS_NONE:
+                               case REF_STATUS_UPTODATE:
+                               case REF_STATUS_OK:
+                                       it->status = REF_STATUS_ATOMIC_PUSH_FAILED;
+                                       break;
+                               default:
+                                       break;
+                               }
+               }
                if (!quiet || err)
                        transport_print_push_status(transport->url, remote_refs,
                                        verbose | porcelain, porcelain,
@@@ -1380,3 -1394,100 +1394,3 @@@ char *transport_anonymize_url(const cha
  literal_copy:
        return xstrdup(url);
  }
 -
 -static void fill_alternate_refs_command(struct child_process *cmd,
 -                                      const char *repo_path)
 -{
 -      const char *value;
 -
 -      if (!git_config_get_value("core.alternateRefsCommand", &value)) {
 -              cmd->use_shell = 1;
 -
 -              argv_array_push(&cmd->args, value);
 -              argv_array_push(&cmd->args, repo_path);
 -      } else {
 -              cmd->git_cmd = 1;
 -
 -              argv_array_pushf(&cmd->args, "--git-dir=%s", repo_path);
 -              argv_array_push(&cmd->args, "for-each-ref");
 -              argv_array_push(&cmd->args, "--format=%(objectname)");
 -
 -              if (!git_config_get_value("core.alternateRefsPrefixes", &value)) {
 -                      argv_array_push(&cmd->args, "--");
 -                      argv_array_split(&cmd->args, value);
 -              }
 -      }
 -
 -      cmd->env = local_repo_env;
 -      cmd->out = -1;
 -}
 -
 -static void read_alternate_refs(const char *path,
 -                              alternate_ref_fn *cb,
 -                              void *data)
 -{
 -      struct child_process cmd = CHILD_PROCESS_INIT;
 -      struct strbuf line = STRBUF_INIT;
 -      FILE *fh;
 -
 -      fill_alternate_refs_command(&cmd, path);
 -
 -      if (start_command(&cmd))
 -              return;
 -
 -      fh = xfdopen(cmd.out, "r");
 -      while (strbuf_getline_lf(&line, fh) != EOF) {
 -              struct object_id oid;
 -              const char *p;
 -
 -              if (parse_oid_hex(line.buf, &oid, &p) || *p) {
 -                      warning(_("invalid line while parsing alternate refs: %s"),
 -                              line.buf);
 -                      break;
 -              }
 -
 -              cb(&oid, data);
 -      }
 -
 -      fclose(fh);
 -      finish_command(&cmd);
 -}
 -
 -struct alternate_refs_data {
 -      alternate_ref_fn *fn;
 -      void *data;
 -};
 -
 -static int refs_from_alternate_cb(struct object_directory *e,
 -                                void *data)
 -{
 -      struct strbuf path = STRBUF_INIT;
 -      size_t base_len;
 -      struct alternate_refs_data *cb = data;
 -
 -      if (!strbuf_realpath(&path, e->path, 0))
 -              goto out;
 -      if (!strbuf_strip_suffix(&path, "/objects"))
 -              goto out;
 -      base_len = path.len;
 -
 -      /* Is this a git repository with refs? */
 -      strbuf_addstr(&path, "/refs");
 -      if (!is_directory(path.buf))
 -              goto out;
 -      strbuf_setlen(&path, base_len);
 -
 -      read_alternate_refs(path.buf, cb->fn, cb->data);
 -
 -out:
 -      strbuf_release(&path);
 -      return 0;
 -}
 -
 -void for_each_alternate_ref(alternate_ref_fn fn, void *data)
 -{
 -      struct alternate_refs_data cb;
 -      cb.fn = fn;
 -      cb.data = data;
 -      foreach_alt_odb(refs_from_alternate_cb, &cb);
 -}