Merge branch 'ab/fsck-transfer-updates'
authorJunio C Hamano <gitster@pobox.com>
Fri, 17 Aug 2018 20:09:54 +0000 (13:09 -0700)
committerJunio C Hamano <gitster@pobox.com>
Fri, 17 Aug 2018 20:09:54 +0000 (13:09 -0700)
The test performed at the receiving end of "git push" to prevent
bad objects from entering repository can be customized via
receive.fsck.* configuration variables; we now have gained a
counterpart to do the same on the "git fetch" side, with
fetch.fsck.* configuration variables.

* ab/fsck-transfer-updates:
fsck: test and document unknown fsck.<msg-id> values
fsck: add stress tests for fsck.skipList
fsck: test & document {fetch,receive}.fsck.* config fallback
fetch: implement fetch.fsck.*
transfer.fsckObjects tests: untangle confusing setup
config doc: elaborate on fetch.fsckObjects security
config doc: elaborate on what transfer.fsckObjects does
config doc: unify the description of fsck.* and receive.fsck.*
config doc: don't describe *.fetchObjects twice
receive.fsck.<msg-id> tests: remove dead code

1  2 
Documentation/config.txt
fetch-pack.c
diff --combined Documentation/config.txt
index fd8d27e7618b2c3d29ed0d61ba63b707571752d7,4cead6119a962418572634db45382267bed16099..c0eb78e344ec2412e7ad1d5acfcfe64c629451e5
@@@ -344,16 -344,6 +344,16 @@@ advice.*:
                Advice shown when you used linkgit:git-checkout[1] to
                move to the detach HEAD state, to instruct how to create
                a local branch after the fact.
 +      checkoutAmbiguousRemoteBranchName::
 +              Advice shown when the argument to
 +              linkgit:git-checkout[1] ambiguously resolves to a
 +              remote tracking branch on more than one remote in
 +              situations where an unambiguous argument would have
 +              otherwise caused a remote-tracking branch to be
 +              checked out. See the `checkout.defaultRemote`
 +              configuration variable for how to set a given remote
 +              to used by default in some situations where this
 +              advice would be printed.
        amWorkDir::
                Advice that shows the location of the patch file when
                linkgit:git-am[1] fails to apply it.
@@@ -917,17 -907,9 +917,17 @@@ core.notesRef:
  This setting defaults to "refs/notes/commits", and it can be overridden by
  the `GIT_NOTES_REF` environment variable.  See linkgit:git-notes[1].
  
 -core.commitGraph::
 -      Enable git commit graph feature. Allows reading from the
 -      commit-graph file.
 +gc.commitGraph::
 +      If true, then gc will rewrite the commit-graph file when
 +      linkgit:git-gc[1] is run. When using linkgit:git-gc[1]
 +      '--auto' the commit-graph will be updated if housekeeping is
 +      required. Default is false. See linkgit:git-commit-graph[1]
 +      for details.
 +
 +core.useReplaceRefs::
 +      If set to `false`, behave as if the `--no-replace-objects`
 +      option was given on the command line. See linkgit:git[1] and
 +      linkgit:git-replace[1] for more information.
  
  core.sparseCheckout::
        Enable "sparse checkout" feature. See section "Sparse checkout" in
@@@ -1119,22 -1101,6 +1119,22 @@@ browser.<tool>.path:
        browse HTML help (see `-w` option in linkgit:git-help[1]) or a
        working repository in gitweb (see linkgit:git-instaweb[1]).
  
 +checkout.defaultRemote::
 +      When you run 'git checkout <something>' and only have one
 +      remote, it may implicitly fall back on checking out and
 +      tracking e.g. 'origin/<something>'. This stops working as soon
 +      as you have more than one remote with a '<something>'
 +      reference. This setting allows for setting the name of a
 +      preferred remote that should always win when it comes to
 +      disambiguation. The typical use-case is to set this to
 +      `origin`.
 ++
 +Currently this is used by linkgit:git-checkout[1] when 'git checkout
 +<something>' will checkout the '<something>' branch on another remote,
 +and by linkgit:git-worktree[1] when 'git worktree add' refers to a
 +remote branch. This setting might be used for other checkout-like
 +commands or functionality in the future.
 +
  clean.requireForce::
        A boolean to make git-clean do nothing unless given -f,
        -i or -n.   Defaults to true.
@@@ -1183,11 -1149,6 +1183,11 @@@ diff.colorMoved:
        true the default color mode will be used. When set to false,
        moved lines are not colored.
  
 +diff.colorMovedWS::
 +      When moved lines are colored using e.g. the `diff.colorMoved` setting,
 +      this option controls the `<mode>` how spaces are treated
 +      for details of valid modes see '--color-moved-ws' in linkgit:git-diff[1].
 +
  color.diff.<slot>::
        Use customized color for diff colorization.  `<slot>` specifies
        which part of the patch to use the specified color, and is one
@@@ -1502,10 -1463,19 +1502,19 @@@ fetch.recurseSubmodules:
  
  fetch.fsckObjects::
        If it is set to true, git-fetch-pack will check all fetched
-       objects. It will abort in the case of a malformed object or a
-       broken link. The result of an abort are only dangling objects.
-       Defaults to false. If not set, the value of `transfer.fsckObjects`
-       is used instead.
+       objects. See `transfer.fsckObjects` for what's
+       checked. Defaults to false. If not set, the value of
+       `transfer.fsckObjects` is used instead.
+ fetch.fsck.<msg-id>::
+       Acts like `fsck.<msg-id>`, but is used by
+       linkgit:git-fetch-pack[1] instead of linkgit:git-fsck[1]. See
+       the `fsck.<msg-id>` documentation for details.
+ fetch.fsck.skipList::
+       Acts like `fsck.skipList`, but is used by
+       linkgit:git-fetch-pack[1] instead of linkgit:git-fsck[1]. See
+       the `fsck.skipList` documentation for details.
  
  fetch.unpackLimit::
        If the number of objects fetched over the Git native
@@@ -1536,15 -1506,6 +1545,15 @@@ fetch.output:
        `full` and `compact`. Default value is `full`. See section
        OUTPUT in linkgit:git-fetch[1] for detail.
  
 +fetch.negotiationAlgorithm::
 +      Control how information about the commits in the local repository is
 +      sent when negotiating the contents of the packfile to be sent by the
 +      server. Set to "skipping" to use an algorithm that skips commits in an
 +      effort to converge faster, but may result in a larger-than-necessary
 +      packfile; any other value instructs Git to use the default algorithm
 +      that never skips commits (unless the server has acknowledged it or one
 +      of its descendants).
 +
  format.attach::
        Enable multipart/mixed attachments as the default for
        'format-patch'.  The value can also be a double quoted string
@@@ -1644,15 -1605,42 +1653,42 @@@ filter.<driver>.smudge:
        linkgit:gitattributes[5] for details.
  
  fsck.<msg-id>::
-       Allows overriding the message type (error, warn or ignore) of a
-       specific message ID such as `missingEmail`.
- +
- For convenience, fsck prefixes the error/warning with the message ID,
- e.g.  "missingEmail: invalid author/committer line - missing email" means
- that setting `fsck.missingEmail = ignore` will hide that issue.
- +
- This feature is intended to support working with legacy repositories
- which cannot be repaired without disruptive changes.
+       During fsck git may find issues with legacy data which
+       wouldn't be generated by current versions of git, and which
+       wouldn't be sent over the wire if `transfer.fsckObjects` was
+       set. This feature is intended to support working with legacy
+       repositories containing such data.
+ +
+ Setting `fsck.<msg-id>` will be picked up by linkgit:git-fsck[1], but
+ to accept pushes of such data set `receive.fsck.<msg-id>` instead, or
+ to clone or fetch it set `fetch.fsck.<msg-id>`.
+ +
+ The rest of the documentation discusses `fsck.*` for brevity, but the
+ same applies for the corresponding `receive.fsck.*` and
+ `fetch.<msg-id>.*`. variables.
+ +
+ Unlike variables like `color.ui` and `core.editor` the
+ `receive.fsck.<msg-id>` and `fetch.fsck.<msg-id>` variables will not
+ fall back on the `fsck.<msg-id>` configuration if they aren't set. To
+ uniformly configure the same fsck settings in different circumstances
+ all three of them they must all set to the same values.
+ +
+ When `fsck.<msg-id>` is set, errors can be switched to warnings and
+ vice versa by configuring the `fsck.<msg-id>` setting where the
+ `<msg-id>` is the fsck message ID and the value is one of `error`,
+ `warn` or `ignore`. For convenience, fsck prefixes the error/warning
+ with the message ID, e.g. "missingEmail: invalid author/committer line
+ - missing email" means that setting `fsck.missingEmail = ignore` will
+ hide that issue.
+ +
+ In general, it is better to enumerate existing objects with problems
+ with `fsck.skipList`, instead of listing the kind of breakages these
+ problematic objects share to be ignored, as doing the latter will
+ allow new instances of the same breakages go unnoticed.
+ +
+ Setting an unknown `fsck.<msg-id>` value will cause fsck to die, but
+ doing the same for `receive.fsck.<msg-id>` and `fetch.fsck.<msg-id>`
+ will only cause git to warn.
  
  fsck.skipList::
        The path to a sorted list of object names (i.e. one SHA-1 per
        should be accepted despite early commits containing errors that
        can be safely ignored such as invalid committer email addresses.
        Note: corrupt objects cannot be skipped with this setting.
+ +
+ Like `fsck.<msg-id>` this variable has corresponding
+ `receive.fsck.skipList` and `fetch.fsck.skipList` variants.
+ +
+ Unlike variables like `color.ui` and `core.editor` the
+ `receive.fsck.skipList` and `fetch.fsck.skipList` variables will not
+ fall back on the `fsck.skipList` configuration if they aren't set. To
+ uniformly configure the same fsck settings in different circumstances
+ all three of them they must all set to the same values.
  
  gc.aggressiveDepth::
        The depth parameter used in the delta compression
@@@ -1884,16 -1881,6 +1929,16 @@@ gpg.program:
        signed, and the program is expected to send the result to its
        standard output.
  
 +gpg.format::
 +      Specifies which key format to use when signing with `--gpg-sign`.
 +      Default is "openpgp" and another possible value is "x509".
 +
 +gpg.<format>.program::
 +      Use this to customize the program used for the signing format you
 +      chose. (see `gpg.program` and `gpg.format`) `gpg.program` can still
 +      be used as a legacy synonym for `gpg.openpgp.program`. The default
 +      value for `gpg.x509.program` is "gpgsm".
 +
  gui.commitMsgWidth::
        Defines how wide the commit message window is in the
        linkgit:git-gui[1]. "75" is the default.
@@@ -2947,32 -2934,21 +2992,21 @@@ receive.certNonceSlop:
  
  receive.fsckObjects::
        If it is set to true, git-receive-pack will check all received
-       objects. It will abort in the case of a malformed object or a
-       broken link. The result of an abort are only dangling objects.
-       Defaults to false. If not set, the value of `transfer.fsckObjects`
-       is used instead.
+       objects. See `transfer.fsckObjects` for what's checked.
+       Defaults to false. If not set, the value of
+       `transfer.fsckObjects` is used instead.
  
  receive.fsck.<msg-id>::
-       When `receive.fsckObjects` is set to true, errors can be switched
-       to warnings and vice versa by configuring the `receive.fsck.<msg-id>`
-       setting where the `<msg-id>` is the fsck message ID and the value
-       is one of `error`, `warn` or `ignore`. For convenience, fsck prefixes
-       the error/warning with the message ID, e.g. "missingEmail: invalid
-       author/committer line - missing email" means that setting
-       `receive.fsck.missingEmail = ignore` will hide that issue.
- +
- This feature is intended to support working with legacy repositories
- which would not pass pushing when `receive.fsckObjects = true`, allowing
- the host to accept repositories with certain known issues but still catch
- other issues.
+       Acts like `fsck.<msg-id>`, but is used by
+       linkgit:git-receive-pack[1] instead of
+       linkgit:git-fsck[1]. See the `fsck.<msg-id>` documentation for
+       details.
  
  receive.fsck.skipList::
-       The path to a sorted list of object names (i.e. one SHA-1 per
-       line) that are known to be broken in a non-fatal way and should
-       be ignored. This feature is useful when an established project
-       should be accepted despite early commits containing errors that
-       can be safely ignored such as invalid committer email addresses.
-       Note: corrupt objects cannot be skipped with this setting.
+       Acts like `fsck.skipList`, but is used by
+       linkgit:git-receive-pack[1] instead of
+       linkgit:git-fsck[1]. See the `fsck.skipList` documentation for
+       details.
  
  receive.keepAlive::
        After receiving the pack from the client, `receive-pack` may
@@@ -3447,6 -3423,40 +3481,40 @@@ transfer.fsckObjects:
        When `fetch.fsckObjects` or `receive.fsckObjects` are
        not set, the value of this variable is used instead.
        Defaults to false.
+ +
+ When set, the fetch or receive will abort in the case of a malformed
+ object or a link to a nonexistent object. In addition, various other
+ issues are checked for, including legacy issues (see `fsck.<msg-id>`),
+ and potential security issues like the existence of a `.GIT` directory
+ or a malicious `.gitmodules` file (see the release notes for v2.2.1
+ and v2.17.1 for details). Other sanity and security checks may be
+ added in future releases.
+ +
+ On the receiving side, failing fsckObjects will make those objects
+ unreachable, see "QUARANTINE ENVIRONMENT" in
+ linkgit:git-receive-pack[1]. On the fetch side, malformed objects will
+ instead be left unreferenced in the repository.
+ +
+ Due to the non-quarantine nature of the `fetch.fsckObjects`
+ implementation it can not be relied upon to leave the object store
+ clean like `receive.fsckObjects` can.
+ +
+ As objects are unpacked they're written to the object store, so there
+ can be cases where malicious objects get introduced even though the
+ "fetch" failed, only to have a subsequent "fetch" succeed because only
+ new incoming objects are checked, not those that have already been
+ written to the object store. That difference in behavior should not be
+ relied upon. In the future, such objects may be quarantined for
+ "fetch" as well.
+ +
+ For now, the paranoid need to find some way to emulate the quarantine
+ environment if they'd like the same protection as "push". E.g. in the
+ case of an internal mirror do the mirroring in two steps, one to fetch
+ the untrusted objects, and then do a second "push" (which will use the
+ quarantine) to another internal repo, and have internal clients
+ consume this pushed-to repository, or embargo internal fetches and
+ only allow them once a full "fsck" has run (and no new fetches have
+ happened in the meantime).
  
  transfer.hideRefs::
        String(s) `receive-pack` and `upload-pack` use to decide which
diff --combined fetch-pack.c
index f80a7acdf3b3edd7425a56434c9fae6adcc4f9bd,aea2f6cf263b56ec392745764cddf24ddcd88a69..88a078e9befd281cf5f03e9e64615b14ca768a35
  #include "connect.h"
  #include "transport.h"
  #include "version.h"
 -#include "prio-queue.h"
  #include "sha1-array.h"
  #include "oidset.h"
  #include "packfile.h"
  #include "object-store.h"
  #include "connected.h"
 +#include "fetch-negotiator.h"
+ #include "fsck.h"
  
  static int transfer_unpack_limit = -1;
  static int fetch_unpack_limit = -1;
@@@ -35,11 -36,17 +36,12 @@@ static int agent_supported
  static int server_supports_filtering;
  static struct lock_file shallow_lock;
  static const char *alternate_shallow_file;
 +static char *negotiation_algorithm;
+ static struct strbuf fsck_msg_types = STRBUF_INIT;
  
  /* Remember to update object flag allocation in object.h */
  #define COMPLETE      (1U << 0)
 -#define COMMON                (1U << 1)
 -#define COMMON_REF    (1U << 2)
 -#define SEEN          (1U << 3)
 -#define POPPED                (1U << 4)
 -#define ALTERNATE     (1U << 5)
 -
 -static int marked;
 +#define ALTERNATE     (1U << 1)
  
  /*
   * After sending this many "have"s if we do not get any new ACK , we
@@@ -47,7 -54,8 +49,7 @@@
   */
  #define MAX_IN_VAIN 256
  
 -static struct prio_queue rev_list = { compare_commits_by_commit_date };
 -static int non_common_revs, multi_ack, use_sideband;
 +static int multi_ack, use_sideband;
  /* Allow specifying sha1 if it is a ref tip. */
  #define ALLOW_TIP_SHA1        01
  /* Allow request of a sha1 if it is reachable from a ref (possibly hidden ref). */
@@@ -79,7 -87,7 +81,7 @@@ static void cache_one_alternate(const c
                                void *vcache)
  {
        struct alternate_object_cache *cache = vcache;
 -      struct object *obj = parse_object(oid);
 +      struct object *obj = parse_object(the_repository, oid);
  
        if (!obj || (obj->flags & ALTERNATE))
                return;
@@@ -89,9 -97,7 +91,9 @@@
        cache->items[cache->nr++] = obj;
  }
  
 -static void for_each_cached_alternate(void (*cb)(struct object *))
 +static void for_each_cached_alternate(struct fetch_negotiator *negotiator,
 +                                    void (*cb)(struct fetch_negotiator *,
 +                                               struct object *))
  {
        static int initialized;
        static struct alternate_object_cache cache;
        }
  
        for (i = 0; i < cache.nr; i++)
 -              cb(cache.items[i]);
 -}
 -
 -static void rev_list_push(struct commit *commit, int mark)
 -{
 -      if (!(commit->object.flags & mark)) {
 -              commit->object.flags |= mark;
 -
 -              if (parse_commit(commit))
 -                      return;
 -
 -              prio_queue_put(&rev_list, commit);
 -
 -              if (!(commit->object.flags & COMMON))
 -                      non_common_revs++;
 -      }
 +              cb(negotiator, cache.items[i]);
  }
  
 -static int rev_list_insert_ref(const char *refname, const struct object_id *oid)
 +static int rev_list_insert_ref(struct fetch_negotiator *negotiator,
 +                             const char *refname,
 +                             const struct object_id *oid)
  {
 -      struct object *o = deref_tag(parse_object(oid), refname, 0);
 +      struct object *o = deref_tag(the_repository,
 +                                   parse_object(the_repository, oid),
 +                                   refname, 0);
  
        if (o && o->type == OBJ_COMMIT)
 -              rev_list_push((struct commit *)o, SEEN);
 +              negotiator->add_tip(negotiator, (struct commit *)o);
  
        return 0;
  }
  static int rev_list_insert_ref_oid(const char *refname, const struct object_id *oid,
                                   int flag, void *cb_data)
  {
 -      return rev_list_insert_ref(refname, oid);
 -}
 -
 -static int clear_marks(const char *refname, const struct object_id *oid,
 -                     int flag, void *cb_data)
 -{
 -      struct object *o = deref_tag(parse_object(oid), refname, 0);
 -
 -      if (o && o->type == OBJ_COMMIT)
 -              clear_commit_marks((struct commit *)o,
 -                                 COMMON | COMMON_REF | SEEN | POPPED);
 -      return 0;
 -}
 -
 -/*
 -   This function marks a rev and its ancestors as common.
 -   In some cases, it is desirable to mark only the ancestors (for example
 -   when only the server does not yet know that they are common).
 -*/
 -
 -static void mark_common(struct commit *commit,
 -              int ancestors_only, int dont_parse)
 -{
 -      if (commit != NULL && !(commit->object.flags & COMMON)) {
 -              struct object *o = (struct object *)commit;
 -
 -              if (!ancestors_only)
 -                      o->flags |= COMMON;
 -
 -              if (!(o->flags & SEEN))
 -                      rev_list_push(commit, SEEN);
 -              else {
 -                      struct commit_list *parents;
 -
 -                      if (!ancestors_only && !(o->flags & POPPED))
 -                              non_common_revs--;
 -                      if (!o->parsed && !dont_parse)
 -                              if (parse_commit(commit))
 -                                      return;
 -
 -                      for (parents = commit->parents;
 -                                      parents;
 -                                      parents = parents->next)
 -                              mark_common(parents->item, 0, dont_parse);
 -              }
 -      }
 -}
 -
 -/*
 -  Get the next rev to send, ignoring the common.
 -*/
 -
 -static const struct object_id *get_rev(void)
 -{
 -      struct commit *commit = NULL;
 -
 -      while (commit == NULL) {
 -              unsigned int mark;
 -              struct commit_list *parents;
 -
 -              if (rev_list.nr == 0 || non_common_revs == 0)
 -                      return NULL;
 -
 -              commit = prio_queue_get(&rev_list);
 -              parse_commit(commit);
 -              parents = commit->parents;
 -
 -              commit->object.flags |= POPPED;
 -              if (!(commit->object.flags & COMMON))
 -                      non_common_revs--;
 -
 -              if (commit->object.flags & COMMON) {
 -                      /* do not send "have", and ignore ancestors */
 -                      commit = NULL;
 -                      mark = COMMON | SEEN;
 -              } else if (commit->object.flags & COMMON_REF)
 -                      /* send "have", and ignore ancestors */
 -                      mark = COMMON | SEEN;
 -              else
 -                      /* send "have", also for its ancestors */
 -                      mark = SEEN;
 -
 -              while (parents) {
 -                      if (!(parents->item->object.flags & SEEN))
 -                              rev_list_push(parents->item, mark);
 -                      if (mark & COMMON)
 -                              mark_common(parents->item, 1, 0);
 -                      parents = parents->next;
 -              }
 -      }
 -
 -      return &commit->object.oid;
 +      return rev_list_insert_ref(cb_data, refname, oid);
  }
  
  enum ack_type {
@@@ -192,10 -300,9 +194,10 @@@ static void send_request(struct fetch_p
                write_or_die(fd, buf->buf, buf->len);
  }
  
 -static void insert_one_alternate_object(struct object *obj)
 +static void insert_one_alternate_object(struct fetch_negotiator *negotiator,
 +                                      struct object *obj)
  {
 -      rev_list_insert_ref(NULL, &obj->oid);
 +      rev_list_insert_ref(negotiator, NULL, &obj->oid);
  }
  
  #define INITIAL_FLUSH 16
@@@ -218,24 -325,7 +220,24 @@@ static int next_flush(int stateless_rpc
        return count;
  }
  
 -static int find_common(struct fetch_pack_args *args,
 +static void mark_tips(struct fetch_negotiator *negotiator,
 +                    const struct oid_array *negotiation_tips)
 +{
 +      int i;
 +
 +      if (!negotiation_tips) {
 +              for_each_ref(rev_list_insert_ref_oid, negotiator);
 +              return;
 +      }
 +
 +      for (i = 0; i < negotiation_tips->nr; i++)
 +              rev_list_insert_ref(negotiator, NULL,
 +                                  &negotiation_tips->oid[i]);
 +      return;
 +}
 +
 +static int find_common(struct fetch_negotiator *negotiator,
 +                     struct fetch_pack_args *args,
                       int fd[2], struct object_id *result_oid,
                       struct ref *refs)
  {
  
        if (args->stateless_rpc && multi_ack == 1)
                die(_("--stateless-rpc requires multi_ack_detailed"));
 -      if (marked)
 -              for_each_ref(clear_marks, NULL);
 -      marked = 1;
  
 -      for_each_ref(rev_list_insert_ref_oid, NULL);
 -      for_each_cached_alternate(insert_one_alternate_object);
 +      mark_tips(negotiator, args->negotiation_tips);
 +      for_each_cached_alternate(negotiator, insert_one_alternate_object);
  
        fetching = 0;
        for ( ; refs ; refs = refs->next) {
                 * interested in the case we *know* the object is
                 * reachable and we have already scanned it.
                 */
 -              if (((o = lookup_object(remote->hash)) != NULL) &&
 +              if (((o = lookup_object(the_repository, remote->hash)) != NULL) &&
                                (o->flags & COMPLETE)) {
                        continue;
                }
                        if (skip_prefix(line, "unshallow ", &arg)) {
                                if (get_oid_hex(arg, &oid))
                                        die(_("invalid unshallow line: %s"), line);
 -                              if (!lookup_object(oid.hash))
 +                              if (!lookup_object(the_repository, oid.hash))
                                        die(_("object not found: %s"), line);
                                /* make sure that it is parsed as shallow */
 -                              if (!parse_object(&oid))
 +                              if (!parse_object(the_repository, &oid))
                                        die(_("error in object: %s"), line);
                                if (unregister_shallow(&oid))
                                        die(_("no shallow found: %s"), line);
        retval = -1;
        if (args->no_dependents)
                goto done;
 -      while ((oid = get_rev())) {
 +      while ((oid = negotiator->next(negotiator))) {
                packet_buf_write(&req_buf, "have %s\n", oid_to_hex(oid));
                print_verbose(args, "have %s", oid_to_hex(oid));
                in_vain++;
                                case ACK_ready:
                                case ACK_continue: {
                                        struct commit *commit =
 -                                              lookup_commit(result_oid);
 +                                              lookup_commit(the_repository,
 +                                                            result_oid);
 +                                      int was_common;
 +
                                        if (!commit)
                                                die(_("invalid commit %s"), oid_to_hex(result_oid));
 +                                      was_common = negotiator->ack(negotiator, commit);
                                        if (args->stateless_rpc
                                         && ack == ACK_common
 -                                       && !(commit->object.flags & COMMON)) {
 +                                       && !was_common) {
                                                /* We need to replay the have for this object
                                                 * on the next RPC request so the peer knows
                                                 * it is in common with us.
                                        } else if (!args->stateless_rpc
                                                   || ack != ACK_common)
                                                in_vain = 0;
 -                                      mark_common(commit, 0, 1);
                                        retval = 0;
                                        got_continue = 1;
 -                                      if (ack == ACK_ready) {
 -                                              clear_prio_queue(&rev_list);
 +                                      if (ack == ACK_ready)
                                                got_ready = 1;
 -                                      }
                                        break;
                                        }
                                }
                                print_verbose(args, _("giving up"));
                                break; /* give up */
                        }
 +                      if (got_ready)
 +                              break;
                }
        }
  done:
@@@ -483,14 -573,14 +485,14 @@@ static struct commit_list *complete
  
  static int mark_complete(const struct object_id *oid)
  {
 -      struct object *o = parse_object(oid);
 +      struct object *o = parse_object(the_repository, oid);
  
        while (o && o->type == OBJ_TAG) {
                struct tag *t = (struct tag *) o;
                if (!t->tagged)
                        break; /* broken repository */
                o->flags |= COMPLETE;
 -              o = parse_object(&t->tagged->oid);
 +              o = parse_object(the_repository, &t->tagged->oid);
        }
        if (o && o->type == OBJ_COMMIT) {
                struct commit *commit = (struct commit *)o;
@@@ -621,8 -711,7 +623,8 @@@ static void filter_refs(struct fetch_pa
        *refs = newlist;
  }
  
 -static void mark_alternate_complete(struct object *obj)
 +static void mark_alternate_complete(struct fetch_negotiator *unused,
 +                                  struct object *obj)
  {
        mark_complete(&obj->oid);
  }
@@@ -649,21 -738,12 +651,21 @@@ static int add_loose_objects_to_set(con
        return 0;
  }
  
 -static int everything_local(struct fetch_pack_args *args,
 -                          struct ref **refs,
 -                          struct ref **sought, int nr_sought)
 +/*
 + * Mark recent commits available locally and reachable from a local ref as
 + * COMPLETE. If args->no_dependents is false, also mark COMPLETE remote refs as
 + * COMMON_REF (otherwise, we are not planning to participate in negotiation, and
 + * thus do not need COMMON_REF marks).
 + *
 + * The cutoff time for recency is determined by this heuristic: it is the
 + * earliest commit time of the objects in refs that are commits and that we know
 + * the commit time of.
 + */
 +static void mark_complete_and_common_ref(struct fetch_negotiator *negotiator,
 +                                       struct fetch_pack_args *args,
 +                                       struct ref **refs)
  {
        struct ref *ref;
 -      int retval;
        int old_save_commit_buffer = save_commit_buffer;
        timestamp_t cutoff = 0;
        struct oidset loose_oid_set = OIDSET_INIT;
  
                if (!has_object_file_with_flags(&ref->old_oid, flags))
                        continue;
 -              o = parse_object(&ref->old_oid);
 +              o = parse_object(the_repository, &ref->old_oid);
                if (!o)
                        continue;
  
        if (!args->no_dependents) {
                if (!args->deepen) {
                        for_each_ref(mark_complete_oid, NULL);
 -                      for_each_cached_alternate(mark_alternate_complete);
 +                      for_each_cached_alternate(NULL, mark_alternate_complete);
                        commit_list_sort_by_date(&complete);
                        if (cutoff)
                                mark_recent_complete_commits(args, cutoff);
                 * Don't mark them common yet; the server has to be told so first.
                 */
                for (ref = *refs; ref; ref = ref->next) {
 -                      struct object *o = deref_tag(lookup_object(ref->old_oid.hash),
 +                      struct object *o = deref_tag(the_repository,
 +                                                   lookup_object(the_repository,
 +                                                   ref->old_oid.hash),
                                                     NULL, 0);
  
                        if (!o || o->type != OBJ_COMMIT || !(o->flags & COMPLETE))
                                continue;
  
 -                      if (!(o->flags & SEEN)) {
 -                              rev_list_push((struct commit *)o, COMMON_REF | SEEN);
 -
 -                              mark_common((struct commit *)o, 1, 1);
 -                      }
 +                      negotiator->known_common(negotiator,
 +                                               (struct commit *)o);
                }
        }
  
 -      filter_refs(args, refs, sought, nr_sought);
 +      save_commit_buffer = old_save_commit_buffer;
 +}
 +
 +/*
 + * Returns 1 if every object pointed to by the given remote refs is available
 + * locally and reachable from a local ref, and 0 otherwise.
 + */
 +static int everything_local(struct fetch_pack_args *args,
 +                          struct ref **refs)
 +{
 +      struct ref *ref;
 +      int retval;
  
        for (retval = 1, ref = *refs; ref ; ref = ref->next) {
                const struct object_id *remote = &ref->old_oid;
                struct object *o;
  
 -              o = lookup_object(remote->hash);
 +              o = lookup_object(the_repository, remote->hash);
                if (!o || !(o->flags & COMPLETE)) {
                        retval = 0;
                        print_verbose(args, "want %s (%s)", oid_to_hex(remote),
                              ref->name);
        }
  
 -      save_commit_buffer = old_save_commit_buffer;
 -
        return retval;
  }
  
@@@ -867,7 -939,8 +869,8 @@@ static int get_pack(struct fetch_pack_a
                         */
                        argv_array_push(&cmd.args, "--fsck-objects");
                else
-                       argv_array_push(&cmd.args, "--strict");
+                       argv_array_pushf(&cmd.args, "--strict%s",
+                                        fsck_msg_types.buf);
        }
  
        cmd.in = demux.out;
@@@ -913,8 -986,6 +916,8 @@@ static struct ref *do_fetch_pack(struc
        struct object_id oid;
        const char *agent_feature;
        int agent_len;
 +      struct fetch_negotiator negotiator;
 +      fetch_negotiator_init(&negotiator, negotiation_algorithm);
  
        sort_ref_list(&ref, ref_compare_name);
        QSORT(sought, nr_sought, cmp_ref_by_name);
        if (!server_supports("deepen-relative") && args->deepen_relative)
                die(_("Server does not support --deepen"));
  
 -      if (everything_local(args, &ref, sought, nr_sought)) {
 +      mark_complete_and_common_ref(&negotiator, args, &ref);
 +      filter_refs(args, &ref, sought, nr_sought);
 +      if (everything_local(args, &ref)) {
                packet_flush(fd[1]);
                goto all_done;
        }
 -      if (find_common(args, fd, &oid, ref) < 0)
 +      if (find_common(&negotiator, args, fd, &oid, ref) < 0)
                if (!args->keep_pack)
                        /* When cloning, it is not unusual to have
                         * no common commit.
                die(_("git fetch-pack: fetch failed."));
  
   all_done:
 +      negotiator.release(&negotiator);
        return ref;
  }
  
@@@ -1055,7 -1123,7 +1058,7 @@@ static void add_wants(const struct ref 
                 * interested in the case we *know* the object is
                 * reachable and we have already scanned it.
                 */
 -              if (((o = lookup_object(remote->hash)) != NULL) &&
 +              if (((o = lookup_object(the_repository, remote->hash)) != NULL) &&
                    (o->flags & COMPLETE)) {
                        continue;
                }
@@@ -1078,15 -1146,13 +1081,15 @@@ static void add_common(struct strbuf *r
        }
  }
  
 -static int add_haves(struct strbuf *req_buf, int *haves_to_send, int *in_vain)
 +static int add_haves(struct fetch_negotiator *negotiator,
 +                   struct strbuf *req_buf,
 +                   int *haves_to_send, int *in_vain)
  {
        int ret = 0;
        int haves_added = 0;
        const struct object_id *oid;
  
 -      while ((oid = get_rev())) {
 +      while ((oid = negotiator->next(negotiator))) {
                packet_buf_write(req_buf, "have %s\n", oid_to_hex(oid));
                if (++haves_added >= *haves_to_send)
                        break;
        return ret;
  }
  
 -static int send_fetch_request(int fd_out, const struct fetch_pack_args *args,
 +static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
 +                            const struct fetch_pack_args *args,
                              const struct ref *wants, struct oidset *common,
                              int *haves_to_send, int *in_vain)
  {
                add_common(&req_buf, common);
  
                /* Add initial haves */
 -              ret = add_haves(&req_buf, haves_to_send, in_vain);
 +              ret = add_haves(negotiator, &req_buf, haves_to_send, in_vain);
        }
  
        /* Send request */
@@@ -1185,13 -1250,13 +1188,13 @@@ static int process_section_header(struc
        int ret;
  
        if (packet_reader_peek(reader) != PACKET_READ_NORMAL)
 -              die("error reading section header '%s'", section);
 +              die(_("error reading section header '%s'"), section);
  
        ret = !strcmp(reader->line, section);
  
        if (!peek) {
                if (!ret)
 -                      die("expected '%s', received '%s'",
 +                      die(_("expected '%s', received '%s'"),
                            section, reader->line);
                packet_reader_read(reader);
        }
        return ret;
  }
  
 -static int process_acks(struct packet_reader *reader, struct oidset *common)
 +static int process_acks(struct fetch_negotiator *negotiator,
 +                      struct packet_reader *reader,
 +                      struct oidset *common)
  {
        /* received */
        int received_ready = 0;
                        if (!get_oid_hex(arg, &oid)) {
                                struct commit *commit;
                                oidset_insert(common, &oid);
 -                              commit = lookup_commit(&oid);
 -                              mark_common(commit, 0, 1);
 +                              commit = lookup_commit(the_repository, &oid);
 +                              negotiator->ack(negotiator, commit);
                        }
                        continue;
                }
  
                if (!strcmp(reader->line, "ready")) {
 -                      clear_prio_queue(&rev_list);
                        received_ready = 1;
                        continue;
                }
  
 -              die("unexpected acknowledgment line: '%s'", reader->line);
 +              die(_("unexpected acknowledgment line: '%s'"), reader->line);
        }
  
        if (reader->status != PACKET_READ_FLUSH &&
            reader->status != PACKET_READ_DELIM)
 -              die("error processing acks: %d", reader->status);
 +              die(_("error processing acks: %d"), reader->status);
  
        /* return 0 if no common, 1 if there are common, or 2 if ready */
        return received_ready ? 2 : (received_ack ? 1 : 0);
@@@ -1258,10 -1322,10 +1261,10 @@@ static void receive_shallow_info(struc
                if (skip_prefix(reader->line, "unshallow ", &arg)) {
                        if (get_oid_hex(arg, &oid))
                                die(_("invalid unshallow line: %s"), reader->line);
 -                      if (!lookup_object(oid.hash))
 +                      if (!lookup_object(the_repository, oid.hash))
                                die(_("object not found: %s"), reader->line);
                        /* make sure that it is parsed as shallow */
 -                      if (!parse_object(&oid))
 +                      if (!parse_object(the_repository, &oid))
                                die(_("error in object: %s"), reader->line);
                        if (unregister_shallow(&oid))
                                die(_("no shallow found: %s"), reader->line);
  
        if (reader->status != PACKET_READ_FLUSH &&
            reader->status != PACKET_READ_DELIM)
 -              die("error processing shallow info: %d", reader->status);
 +              die(_("error processing shallow info: %d"), reader->status);
  
        setup_alternate_shallow(&shallow_lock, &alternate_shallow_file, NULL);
        args->deepen = 1;
  }
  
 -static void receive_wanted_refs(struct packet_reader *reader, struct ref *refs)
 +static void receive_wanted_refs(struct packet_reader *reader,
 +                              struct ref **sought, int nr_sought)
  {
        process_section_header(reader, "wanted-refs", 0);
        while (packet_reader_read(reader) == PACKET_READ_NORMAL) {
                struct object_id oid;
                const char *end;
 -              struct ref *r = NULL;
 +              int i;
  
                if (parse_oid_hex(reader->line, &oid, &end) || *end++ != ' ')
 -                      die("expected wanted-ref, got '%s'", reader->line);
 +                      die(_("expected wanted-ref, got '%s'"), reader->line);
  
 -              for (r = refs; r; r = r->next) {
 -                      if (!strcmp(end, r->name)) {
 -                              oidcpy(&r->old_oid, &oid);
 +              for (i = 0; i < nr_sought; i++) {
 +                      if (!strcmp(end, sought[i]->name)) {
 +                              oidcpy(&sought[i]->old_oid, &oid);
                                break;
                        }
                }
  
 -              if (!r)
 -                      die("unexpected wanted-ref: '%s'", reader->line);
 +              if (i == nr_sought)
 +                      die(_("unexpected wanted-ref: '%s'"), reader->line);
        }
  
        if (reader->status != PACKET_READ_DELIM)
 -              die("error processing wanted refs: %d", reader->status);
 +              die(_("error processing wanted refs: %d"), reader->status);
  }
  
  enum fetch_state {
@@@ -1325,8 -1388,6 +1328,8 @@@ static struct ref *do_fetch_pack_v2(str
        struct packet_reader reader;
        int in_vain = 0;
        int haves_to_send = INITIAL_FLUSH;
 +      struct fetch_negotiator negotiator;
 +      fetch_negotiator_init(&negotiator, negotiation_algorithm);
        packet_reader_init(&reader, fd[0], NULL, 0,
                           PACKET_READ_CHOMP_NEWLINE);
  
                        if (args->depth > 0 || args->deepen_since || args->deepen_not)
                                args->deepen = 1;
  
 -                      if (marked)
 -                              for_each_ref(clear_marks, NULL);
 -                      marked = 1;
 -
 -                      for_each_ref(rev_list_insert_ref_oid, NULL);
 -                      for_each_cached_alternate(insert_one_alternate_object);
 -
                        /* Filter 'ref' by 'sought' and those that aren't local */
 -                      if (everything_local(args, &ref, sought, nr_sought))
 +                      mark_complete_and_common_ref(&negotiator, args, &ref);
 +                      filter_refs(args, &ref, sought, nr_sought);
 +                      if (everything_local(args, &ref))
                                state = FETCH_DONE;
                        else
                                state = FETCH_SEND_REQUEST;
 +
 +                      mark_tips(&negotiator, args->negotiation_tips);
 +                      for_each_cached_alternate(&negotiator,
 +                                                insert_one_alternate_object);
                        break;
                case FETCH_SEND_REQUEST:
 -                      if (send_fetch_request(fd[1], args, ref, &common,
 +                      if (send_fetch_request(&negotiator, fd[1], args, ref,
 +                                             &common,
                                               &haves_to_send, &in_vain))
                                state = FETCH_GET_PACK;
                        else
                        break;
                case FETCH_PROCESS_ACKS:
                        /* Process ACKs/NAKs */
 -                      switch (process_acks(&reader, &common)) {
 +                      switch (process_acks(&negotiator, &reader, &common)) {
                        case 2:
                                state = FETCH_GET_PACK;
                                break;
                                receive_shallow_info(args, &reader);
  
                        if (process_section_header(&reader, "wanted-refs", 1))
 -                              receive_wanted_refs(&reader, ref);
 +                              receive_wanted_refs(&reader, sought, nr_sought);
  
                        /* get the pack */
                        process_section_header(&reader, "packfile", 0);
                }
        }
  
 +      negotiator.release(&negotiator);
        oidset_clear(&common);
        return ref;
  }
  
+ static int fetch_pack_config_cb(const char *var, const char *value, void *cb)
+ {
+       if (strcmp(var, "fetch.fsck.skiplist") == 0) {
+               const char *path;
+               if (git_config_pathname(&path, var, value))
+                       return 1;
+               strbuf_addf(&fsck_msg_types, "%cskiplist=%s",
+                       fsck_msg_types.len ? ',' : '=', path);
+               free((char *)path);
+               return 0;
+       }
+       if (skip_prefix(var, "fetch.fsck.", &var)) {
+               if (is_valid_msg_type(var, value))
+                       strbuf_addf(&fsck_msg_types, "%c%s=%s",
+                               fsck_msg_types.len ? ',' : '=', var, value);
+               else
+                       warning("Skipping unknown msg id '%s'", var);
+               return 0;
+       }
+       return git_default_config(var, value, cb);
+ }
  static void fetch_pack_config(void)
  {
        git_config_get_int("fetch.unpacklimit", &fetch_unpack_limit);
        git_config_get_bool("repack.usedeltabaseoffset", &prefer_ofs_delta);
        git_config_get_bool("fetch.fsckobjects", &fetch_fsck_objects);
        git_config_get_bool("transfer.fsckobjects", &transfer_fsck_objects);
 +      git_config_get_string("fetch.negotiationalgorithm",
 +                            &negotiation_algorithm);
  
-       git_config(git_default_config, NULL);
+       git_config(fetch_pack_config_cb, NULL);
  }
  
  static void fetch_pack_setup(void)
@@@ -1449,12 -1532,13 +1477,12 @@@ static int remove_duplicates_in_refs(st
  }
  
  static void update_shallow(struct fetch_pack_args *args,
 -                         struct ref *refs,
 +                         struct ref **sought, int nr_sought,
                           struct shallow_info *si)
  {
        struct oid_array ref = OID_ARRAY_INIT;
        int *status;
        int i;
 -      struct ref *r;
  
        if (args->deepen && alternate_shallow_file) {
                if (*alternate_shallow_file == '\0') { /* --unshallow */
        remove_nonexistent_theirs_shallow(si);
        if (!si->nr_ours && !si->nr_theirs)
                return;
 -      for (r = refs; r; r = r->next)
 -              oid_array_append(&ref, &r->old_oid);
 +      for (i = 0; i < nr_sought; i++)
 +              oid_array_append(&ref, &sought[i]->old_oid);
        si->ref = &ref;
  
        if (args->update_shallow) {
         * remote is also shallow, check what ref is safe to update
         * without updating .git/shallow
         */
 -      status = xcalloc(ref.nr, sizeof(*status));
 +      status = xcalloc(nr_sought, sizeof(*status));
        assign_shallow_commits_to_refs(si, NULL, status);
        if (si->nr_ours || si->nr_theirs) {
 -              for (r = refs, i = 0; r; r = r->next, i++)
 +              for (i = 0; i < nr_sought; i++)
                        if (status[i])
 -                              r->status = REF_STATUS_REJECT_SHALLOW;
 +                              sought[i]->status = REF_STATUS_REJECT_SHALLOW;
        }
        free(status);
        oid_array_clear(&ref);
@@@ -1599,7 -1683,7 +1627,7 @@@ struct ref *fetch_pack(struct fetch_pac
                args->connectivity_checked = 1;
        }
  
 -      update_shallow(args, ref_cpy, &si);
 +      update_shallow(args, sought, nr_sought, &si);
  cleanup:
        clear_shallow_info(&si);
        return ref_cpy;