Merge branch 'jk/index-pack-maint'
authorJunio C Hamano <gitster@pobox.com>
Wed, 13 Jun 2018 19:50:46 +0000 (12:50 -0700)
committerJunio C Hamano <gitster@pobox.com>
Wed, 13 Jun 2018 19:50:46 +0000 (12:50 -0700)
"index-pack --strict" has been taught to make sure that it runs the
final object integrity checks after making the freshly indexed
packfile available to itself.

* jk/index-pack-maint:
index-pack: correct install_packed_git() args
index-pack: handle --strict checks of non-repo packs
prepare_commit_graft: treat non-repository as a noop

1  2 
builtin/index-pack.c
commit.c
t/t5300-pack-object.sh
t/t7415-submodule-names.sh
diff --combined builtin/index-pack.c
index 4ab31ed388bb860110f5bd33ddf6d707f0cfa41f,3030c88d384742c86677316c517f529a8ff3bd72..74fe2973e1256e092652fce28ba2036cc3203af9
@@@ -9,11 -9,10 +9,11 @@@
  #include "tree.h"
  #include "progress.h"
  #include "fsck.h"
 -#include "exec_cmd.h"
 +#include "exec-cmd.h"
  #include "streaming.h"
  #include "thread-utils.h"
  #include "packfile.h"
 +#include "object-store.h"
  
  static const char index_pack_usage[] =
  "git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--verify] [--strict] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
@@@ -60,7 -59,7 +60,7 @@@ struct ofs_delta_entry 
  };
  
  struct ref_delta_entry {
 -      unsigned char sha1[20];
 +      struct object_id oid;
        int obj_no;
  };
  
@@@ -223,7 -222,7 +223,7 @@@ static unsigned check_object(struct obj
  
        if (!(obj->flags & FLAG_CHECKED)) {
                unsigned long size;
 -              int type = sha1_object_info(obj->oid.hash, &size);
 +              int type = oid_object_info(the_repository, &obj->oid, &size);
                if (type <= 0)
                        die(_("did not receive expected object %s"),
                              oid_to_hex(&obj->oid));
@@@ -673,18 -672,18 +673,18 @@@ static void find_ofs_delta_children(off
        *last_index = last;
  }
  
 -static int compare_ref_delta_bases(const unsigned char *sha1,
 -                                 const unsigned char *sha2,
 +static int compare_ref_delta_bases(const struct object_id *oid1,
 +                                 const struct object_id *oid2,
                                   enum object_type type1,
                                   enum object_type type2)
  {
        int cmp = type1 - type2;
        if (cmp)
                return cmp;
 -      return hashcmp(sha1, sha2);
 +      return oidcmp(oid1, oid2);
  }
  
 -static int find_ref_delta(const unsigned char *sha1, enum object_type type)
 +static int find_ref_delta(const struct object_id *oid, enum object_type type)
  {
        int first = 0, last = nr_ref_deltas;
  
                struct ref_delta_entry *delta = &ref_deltas[next];
                int cmp;
  
 -              cmp = compare_ref_delta_bases(sha1, delta->sha1,
 +              cmp = compare_ref_delta_bases(oid, &delta->oid,
                                              type, objects[delta->obj_no].type);
                if (!cmp)
                        return next;
        return -first-1;
  }
  
 -static void find_ref_delta_children(const unsigned char *sha1,
 +static void find_ref_delta_children(const struct object_id *oid,
                                    int *first_index, int *last_index,
                                    enum object_type type)
  {
 -      int first = find_ref_delta(sha1, type);
 +      int first = find_ref_delta(oid, type);
        int last = first;
        int end = nr_ref_deltas - 1;
  
                *last_index = -1;
                return;
        }
 -      while (first > 0 && !hashcmp(ref_deltas[first - 1].sha1, sha1))
 +      while (first > 0 && !oidcmp(&ref_deltas[first - 1].oid, oid))
                --first;
 -      while (last < end && !hashcmp(ref_deltas[last + 1].sha1, sha1))
 +      while (last < end && !oidcmp(&ref_deltas[last + 1].oid, oid))
                ++last;
        *first_index = first;
        *last_index = last;
@@@ -773,7 -772,7 +773,7 @@@ static int check_collison(struct object
  
        memset(&data, 0, sizeof(data));
        data.entry = entry;
 -      data.st = open_istream(entry->idx.oid.hash, &type, &size, NULL);
 +      data.st = open_istream(&entry->idx.oid, &type, &size, NULL);
        if (!data.st)
                return -1;
        if (size != entry->size || type != entry->type)
@@@ -812,12 -811,12 +812,12 @@@ static void sha1_object(const void *dat
                enum object_type has_type;
                unsigned long has_size;
                read_lock();
 -              has_type = sha1_object_info(oid->hash, &has_size);
 +              has_type = oid_object_info(the_repository, oid, &has_size);
                if (has_type < 0)
                        die(_("cannot read existing object info %s"), oid_to_hex(oid));
                if (has_type != type || has_size != size)
                        die(_("SHA1 COLLISION FOUND WITH %s !"), oid_to_hex(oid));
 -              has_data = read_sha1_file(oid->hash, &has_type, &has_size);
 +              has_data = read_object_file(oid, &has_type, &has_size);
                read_unlock();
                if (!data)
                        data = new_data = get_data_from_pack(obj_entry);
                        if (obj->type == OBJ_COMMIT) {
                                struct commit *commit = (struct commit *) obj;
                                if (detach_commit_buffer(commit, NULL) != data)
 -                                      die("BUG: parse_object_buffer transmogrified our buffer");
 +                                      BUG("parse_object_buffer transmogrified our buffer");
                        }
                        obj->flags |= FLAG_CHECKED;
                }
@@@ -996,7 -995,7 +996,7 @@@ static struct base_data *find_unresolve
                                                  struct base_data *prev_base)
  {
        if (base->ref_last == -1 && base->ofs_last == -1) {
 -              find_ref_delta_children(base->obj->idx.oid.hash,
 +              find_ref_delta_children(&base->obj->idx.oid,
                                        &base->ref_first, &base->ref_last,
                                        OBJ_REF_DELTA);
  
  
                if (!compare_and_swap_type(&child->real_type, OBJ_REF_DELTA,
                                           base->obj->real_type))
 -                      die("BUG: child->real_type != OBJ_REF_DELTA");
 +                      BUG("child->real_type != OBJ_REF_DELTA");
  
                resolve_delta(child, base, result);
                if (base->ref_first == base->ref_last && base->ofs_last == -1)
@@@ -1080,7 -1079,7 +1080,7 @@@ static int compare_ref_delta_entry(cons
        const struct ref_delta_entry *delta_a = a;
        const struct ref_delta_entry *delta_b = b;
  
 -      return hashcmp(delta_a->sha1, delta_b->sha1);
 +      return oidcmp(&delta_a->oid, &delta_b->oid);
  }
  
  static void resolve_base(struct object_entry *obj)
@@@ -1146,7 -1145,7 +1146,7 @@@ static void parse_pack_objects(unsigne
                        ofs_delta++;
                } else if (obj->type == OBJ_REF_DELTA) {
                        ALLOC_GROW(ref_deltas, nr_ref_deltas + 1, ref_deltas_alloc);
 -                      hashcpy(ref_deltas[nr_ref_deltas].sha1, ref_delta_oid.hash);
 +                      oidcpy(&ref_deltas[nr_ref_deltas].oid, &ref_delta_oid);
                        ref_deltas[nr_ref_deltas].obj_no = i;
                        nr_ref_deltas++;
                } else if (!data) {
@@@ -1274,7 -1273,7 +1274,7 @@@ static void conclude_pack(int fix_thin_
                            nr_objects - nr_objects_initial);
                stop_progress_msg(&progress, msg.buf);
                strbuf_release(&msg);
 -              hashclose(f, tail_hash, 0);
 +              finalize_hashfile(f, tail_hash, 0);
                hashcpy(read_hash, pack_hash);
                fixup_pack_header_footer(output_fd, pack_hash,
                                         curr_pack, nr_objects,
@@@ -1378,15 -1377,14 +1378,15 @@@ static void fix_unresolved_deltas(struc
  
                if (objects[d->obj_no].real_type != OBJ_REF_DELTA)
                        continue;
 -              base_obj->data = read_sha1_file(d->sha1, &type, &base_obj->size);
 +              base_obj->data = read_object_file(&d->oid, &type,
 +                                                &base_obj->size);
                if (!base_obj->data)
                        continue;
  
 -              if (check_sha1_signature(d->sha1, base_obj->data,
 +              if (check_object_signature(&d->oid, base_obj->data,
                                base_obj->size, type_name(type)))
 -                      die(_("local object %s is corrupt"), sha1_to_hex(d->sha1));
 -              base_obj->obj = append_obj_to_pack(f, d->sha1,
 +                      die(_("local object %s is corrupt"), oid_to_hex(&d->oid));
 +              base_obj->obj = append_obj_to_pack(f, d->oid.hash,
                                        base_obj->data, base_obj->size, type);
                find_unresolved_deltas(base_obj);
                display_progress(progress, nr_resolved_deltas);
@@@ -1482,8 -1480,12 +1482,12 @@@ static void final(const char *final_pac
        } else
                chmod(final_index_name, 0444);
  
-       if (do_fsck_object)
-               add_packed_git(final_index_name, strlen(final_index_name), 0);
+       if (do_fsck_object) {
+               struct packed_git *p;
+               p = add_packed_git(final_index_name, strlen(final_index_name), 0);
+               if (p)
 -                      install_packed_git(p);
++                      install_packed_git(the_repository, p);
+       }
  
        if (!from_stdin) {
                printf("%s\n", sha1_to_hex(hash));
@@@ -1549,13 -1551,12 +1553,13 @@@ static void read_v2_anomalous_offsets(s
  {
        const uint32_t *idx1, *idx2;
        uint32_t i;
 +      const uint32_t hashwords = the_hash_algo->rawsz / sizeof(uint32_t);
  
        /* The address of the 4-byte offset table */
        idx1 = (((const uint32_t *)p->index_data)
                + 2 /* 8-byte header */
                + 256 /* fan out */
 -              + 5 * p->num_objects /* 20-byte SHA-1 table */
 +              + hashwords * p->num_objects /* object ID table */
                + p->num_objects /* CRC32 table */
                );
  
@@@ -1600,7 -1601,7 +1604,7 @@@ static void read_idx_option(struct pack
        /*
         * Get rid of the idx file as we do not need it anymore.
         * NEEDSWORK: extract this bit from free_pack_by_name() in
 -       * sha1_file.c, perhaps?  It shouldn't matter very much as we
 +       * sha1-file.c, perhaps?  It shouldn't matter very much as we
         * know we haven't installed this pack (hence we never have
         * read anything from it).
         */
diff --combined commit.c
index b0e57cc4400964ad7dca8d37c5d4c10bf3979b51,9e49c221ed4726008f06882c4d1ea69d47a2f623..0030e79940ff8564b5c8c5ebab462eb1bd745174
+++ b/commit.c
@@@ -1,7 -1,6 +1,7 @@@
  #include "cache.h"
  #include "tag.h"
  #include "commit.h"
 +#include "commit-graph.h"
  #include "pkt-line.h"
  #include "utf8.h"
  #include "diff.h"
@@@ -13,7 -12,6 +13,7 @@@
  #include "prio-queue.h"
  #include "sha1-lookup.h"
  #include "wt-status.h"
 +#include "advice.h"
  
  static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **);
  
@@@ -178,15 -176,6 +178,15 @@@ static int read_graft_file(const char *
        struct strbuf buf = STRBUF_INIT;
        if (!fp)
                return -1;
 +      if (advice_graft_file_deprecated)
 +              advise(_("Support for <GIT_DIR>/info/grafts is deprecated\n"
 +                       "and will be removed in a future Git version.\n"
 +                       "\n"
 +                       "Please use \"git replace --convert-graft-file\"\n"
 +                       "to convert the grafts into replace refs.\n"
 +                       "\n"
 +                       "Turn this message off by running\n"
 +                       "\"git config advice.graftFileDeprecated false\""));
        while (!strbuf_getwholeline(&buf, fp, '\n')) {
                /* The format is just "Commit Parent1 Parent2 ...\n" */
                struct commit_graft *graft = read_graft_line(&buf);
@@@ -207,6 -196,9 +207,9 @@@ static void prepare_commit_graft(void
  
        if (commit_graft_prepared)
                return;
+       if (!startup_info->have_repository)
+               return;
        graft_file = get_graft_file();
        read_graft_file(graft_file);
        /* make sure shallows are read */
@@@ -277,7 -269,7 +280,7 @@@ const void *get_commit_buffer(const str
        if (!ret) {
                enum object_type type;
                unsigned long size;
 -              ret = read_sha1_file(commit->object.oid.hash, &type, &size);
 +              ret = read_object_file(&commit->object.oid, &type, &size);
                if (!ret)
                        die("cannot read commit object %s",
                            oid_to_hex(&commit->object.oid));
@@@ -306,22 -298,6 +309,22 @@@ void free_commit_buffer(struct commit *
        }
  }
  
 +struct tree *get_commit_tree(const struct commit *commit)
 +{
 +      if (commit->maybe_tree || !commit->object.parsed)
 +              return commit->maybe_tree;
 +
 +      if (commit->graph_pos == COMMIT_NOT_FROM_GRAPH)
 +              BUG("commit has NULL tree, but was not loaded from commit-graph");
 +
 +      return get_commit_tree_in_graph(commit);
 +}
 +
 +struct object_id *get_commit_tree_oid(const struct commit *commit)
 +{
 +      return &get_commit_tree(commit)->object.oid;
 +}
 +
  const void *detach_commit_buffer(struct commit *commit, unsigned long *sizep)
  {
        struct commit_buffer *v = buffer_slab_peek(&buffer_slab, commit);
@@@ -358,10 -334,10 +361,10 @@@ int parse_commit_buffer(struct commit *
        if (tail <= bufptr + tree_entry_len + 1 || memcmp(bufptr, "tree ", 5) ||
                        bufptr[tree_entry_len] != '\n')
                return error("bogus commit object %s", oid_to_hex(&item->object.oid));
 -      if (get_sha1_hex(bufptr + 5, parent.hash) < 0)
 +      if (get_oid_hex(bufptr + 5, &parent) < 0)
                return error("bad tree pointer in commit %s",
                             oid_to_hex(&item->object.oid));
 -      item->tree = lookup_tree(&parent);
 +      item->maybe_tree = lookup_tree(&parent);
        bufptr += tree_entry_len + 1; /* "tree " + "hex sha1" + "\n" */
        pptr = &item->parents;
  
                struct commit *new_parent;
  
                if (tail <= bufptr + parent_entry_len + 1 ||
 -                  get_sha1_hex(bufptr + 7, parent.hash) ||
 +                  get_oid_hex(bufptr + 7, &parent) ||
                    bufptr[parent_entry_len] != '\n')
                        return error("bad parents in commit %s", oid_to_hex(&item->object.oid));
                bufptr += parent_entry_len + 1;
@@@ -410,9 -386,7 +413,9 @@@ int parse_commit_gently(struct commit *
                return -1;
        if (item->object.parsed)
                return 0;
 -      buffer = read_sha1_file(item->object.oid.hash, &type, &size);
 +      if (parse_commit_in_graph(item))
 +              return 0;
 +      buffer = read_object_file(&item->object.oid, &type, &size);
        if (!buffer)
                return quiet_on_missing ? -1 :
                        error("Could not read %s",
@@@ -1235,7 -1209,7 +1238,7 @@@ static void handle_signed_tag(struct co
        desc = merge_remote_util(parent);
        if (!desc || !desc->obj)
                return;
 -      buf = read_sha1_file(desc->obj->oid.hash, &type, &size);
 +      buf = read_object_file(&desc->obj->oid, &type, &size);
        if (!buf || type != OBJ_TAG)
                goto free_return;
        len = parse_signature(buf, size);
@@@ -1317,19 -1291,17 +1320,19 @@@ struct commit_extra_header *read_commit
        return extra;
  }
  
 -void for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void *data)
 +int for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void *data)
  {
        struct commit_extra_header *extra, *to_free;
 +      int res = 0;
  
        to_free = read_commit_extra_headers(commit, NULL);
 -      for (extra = to_free; extra; extra = extra->next) {
 +      for (extra = to_free; !res && extra; extra = extra->next) {
                if (strcmp(extra->key, "mergetag"))
                        continue; /* not a merge tag */
 -              fn(commit, extra, data);
 +              res = fn(commit, extra, data);
        }
        free_commit_extra_headers(to_free);
 +      return res;
  }
  
  static inline int standard_header_field(const char *field, size_t len)
@@@ -1548,7 -1520,7 +1551,7 @@@ int commit_tree_extended(const char *ms
        int encoding_is_utf8;
        struct strbuf buffer;
  
 -      assert_sha1_type(tree->hash, OBJ_TREE);
 +      assert_oid_type(tree, OBJ_TREE);
  
        if (memchr(msg, '\0', msg_len))
                return error("a NUL byte in commit log message not allowed.");
diff --combined t/t5300-pack-object.sh
index 87a590c4a952d743c2d9f8b0daac76d448f51e8e,88ec5d78489ee3394e9c719f851546d7ae12e320..2336d09dcc45f3b808c5e5d6a7c43c8129e8cecc
@@@ -16,8 -16,8 +16,8 @@@ test_expect_success 
       perl -e "print \"a\" x 4096;" > a &&
       perl -e "print \"b\" x 4096;" > b &&
       perl -e "print \"c\" x 4096;" > c &&
 -     test-genrandom "seed a" 2097152 > a_big &&
 -     test-genrandom "seed b" 2097152 > b_big &&
 +     test-tool genrandom "seed a" 2097152 > a_big &&
 +     test-tool genrandom "seed b" 2097152 > b_big &&
       git update-index --add a a_big b b_big c &&
       cat c >d && echo foo >>d && git update-index --add d &&
       tree=$(git write-tree) &&
@@@ -311,8 -311,8 +311,8 @@@ test_expect_success 'unpacking with --s
        rm -f .git/index &&
        tail -n 10 LIST | git update-index --index-info &&
        ST=$(git write-tree) &&
 -      PACK5=$( git rev-list --objects "$LIST" "$LI" "$ST" | \
 -              git pack-objects test-5 ) &&
 +      git rev-list --objects "$LIST" "$LI" "$ST" >actual &&
 +      PACK5=$( git pack-objects test-5 <actual ) &&
        PACK6=$( (
                        echo "$LIST"
                        echo "$LI"
@@@ -358,8 -358,8 +358,8 @@@ test_expect_success 'index-pack with --
        rm -f .git/index &&
        tail -n 10 LIST | git update-index --index-info &&
        ST=$(git write-tree) &&
 -      PACK5=$( git rev-list --objects "$LIST" "$LI" "$ST" | \
 -              git pack-objects test-5 ) &&
 +      git rev-list --objects "$LIST" "$LI" "$ST" >actual &&
 +      PACK5=$( git pack-objects test-5 <actual ) &&
        PACK6=$( (
                        echo "$LIST"
                        echo "$LI"
@@@ -421,6 -421,12 +421,12 @@@ test_expect_success 'index-pack <pack> 
        test_path_is_file foo.idx
  '
  
+ test_expect_success 'index-pack --strict <pack> works in non-repo' '
+       rm -f foo.idx &&
+       nongit git index-pack --strict ../foo.pack &&
+       test_path_is_file foo.idx
+ '
  test_expect_success !PTHREADS,C_LOCALE_OUTPUT 'index-pack --threads=N or pack.threads=N warns when no pthreads' '
        test_must_fail git index-pack --threads=2 2>err &&
        grep ^warning: err >warnings &&
@@@ -457,11 -463,6 +463,11 @@@ test_expect_success !PTHREADS,C_LOCALE_
        grep -F "no threads support, ignoring pack.threads" err
  '
  
 +test_expect_success 'pack-objects in too-many-packs mode' '
 +      GIT_TEST_FULL_IN_PACK_ARRAY=1 git repack -ad &&
 +      git fsck
 +'
 +
  #
  # WARNING!
  #
  
  test_expect_success \
      'fake a SHA1 hash collision' \
 -    'test -f  .git/objects/c8/2de19312b6c3695c0c18f70709a6c535682a67 &&
 -     cp -f    .git/objects/9d/235ed07cd19811a6ceb342de82f190e49c9f68 \
 -              .git/objects/c8/2de19312b6c3695c0c18f70709a6c535682a67'
 +    'long_a=$(git hash-object a | sed -e "s!^..!&/!") &&
 +     long_b=$(git hash-object b | sed -e "s!^..!&/!") &&
 +     test -f  .git/objects/$long_b &&
 +     cp -f    .git/objects/$long_a \
 +              .git/objects/$long_b'
  
  test_expect_success \
      'make sure index-pack detects the SHA1 collision' \
index 3c0f1a102a09e5c03cd6e270bbbf32d870b5c093,4157e1a134e15678e8b271a036230ad6a8ffc257..b68c5f5e8510ddbf4e10f6cb608d9e789bef09b4
@@@ -122,6 -122,16 +122,16 @@@ test_expect_success 'transfer.fsckObjec
        test_must_fail git -C dst.git index-pack --strict --stdin <odd.pack
  '
  
+ test_expect_success 'index-pack --strict works for non-repo pack' '
+       rm -rf dst.git &&
+       git init --bare dst.git &&
+       cp odd.pack dst.git &&
+       test_must_fail git -C dst.git index-pack --strict odd.pack 2>output &&
+       # Make sure we fail due to bad gitmodules content, not because we
+       # could not read the blob in the first place.
+       grep gitmodulesName output
+ '
  test_expect_success 'fsck detects symlinked .gitmodules file' '
        git init symlink &&
        (
                tricky="[foo]bar=true" &&
                content=$(git hash-object -w ../.gitmodules) &&
                target=$(printf "$tricky" | git hash-object -w --stdin) &&
 -              tree=$(
 -                      {
 -                              printf "100644 blob $content\t$tricky\n" &&
 -                              printf "120000 blob $target\t.gitmodules\n"
 -                      } | git mktree
 -              ) &&
 -              commit=$(git commit-tree $tree) &&
 +              {
 +                      printf "100644 blob $content\t$tricky\n" &&
 +                      printf "120000 blob $target\t.gitmodules\n"
 +              } | git mktree &&
  
                # Check not only that we fail, but that it is due to the
                # symlink detector; this grep string comes from the config
        )
  '
  
 +test_expect_success 'fsck detects non-blob .gitmodules' '
 +      git init non-blob &&
 +      (
 +              cd non-blob &&
 +
 +              # As above, make the funny tree directly to avoid index
 +              # restrictions.
 +              mkdir subdir &&
 +              cp ../.gitmodules subdir/file &&
 +              git add subdir/file &&
 +              git commit -m ok &&
 +              git ls-tree HEAD | sed s/subdir/.gitmodules/ | git mktree &&
 +
 +              test_must_fail git fsck 2>output &&
 +              grep gitmodulesBlob output
 +      )
 +'
 +
  test_done