Merge branch 'bp/refresh-cache-ent-rehash-fix'
authorJunio C Hamano <gitster@pobox.com>
Wed, 21 Mar 2018 18:30:11 +0000 (11:30 -0700)
committerJunio C Hamano <gitster@pobox.com>
Wed, 21 Mar 2018 18:30:11 +0000 (11:30 -0700)
The codepath to replace an existing entry in the index had a bug in
updating the name hash structure, which has been fixed.

* bp/refresh-cache-ent-rehash-fix:
Fix bugs preventing adding updated cache entries to the name hash

1  2 
read-cache.c
diff --combined read-cache.c
index d05eb725b533d3b70daca910df60d27de9ce947c,96fa833111893544cc31504b1b17791417518e12..a5dc7c7e6718972b158aebfd36b4625f54887341
@@@ -62,6 -62,7 +62,7 @@@ static void replace_index_entry(struct 
        replace_index_entry_in_base(istate, old, ce);
        remove_name_hash(istate, old);
        free(old);
+       ce->ce_flags &= ~CE_HASHED;
        set_index_entry(istate, nr, ce);
        ce->ce_flags |= CE_UPDATE_IN_BASE;
        mark_fsmonitor_invalid(istate, ce);
  
  void rename_index_entry_at(struct index_state *istate, int nr, const char *new_name)
  {
 -      struct cache_entry *old = istate->cache[nr], *new;
 +      struct cache_entry *old_entry = istate->cache[nr], *new_entry;
        int namelen = strlen(new_name);
  
 -      new = xmalloc(cache_entry_size(namelen));
 -      copy_cache_entry(new, old);
 -      new->ce_flags &= ~CE_HASHED;
 -      new->ce_namelen = namelen;
 -      new->index = 0;
 -      memcpy(new->name, new_name, namelen + 1);
 +      new_entry = xmalloc(cache_entry_size(namelen));
 +      copy_cache_entry(new_entry, old_entry);
 +      new_entry->ce_flags &= ~CE_HASHED;
 +      new_entry->ce_namelen = namelen;
 +      new_entry->index = 0;
 +      memcpy(new_entry->name, new_name, namelen + 1);
  
 -      cache_tree_invalidate_path(istate, old->name);
 -      untracked_cache_remove_from_index(istate, old->name);
 +      cache_tree_invalidate_path(istate, old_entry->name);
 +      untracked_cache_remove_from_index(istate, old_entry->name);
        remove_index_entry_at(istate, nr);
 -      add_index_entry(istate, new, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);
 +      add_index_entry(istate, new_entry, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);
  }
  
  void fill_stat_data(struct stat_data *sd, struct stat *st)
@@@ -615,26 -616,26 +616,26 @@@ static struct cache_entry *create_alias
                                           struct cache_entry *alias)
  {
        int len;
 -      struct cache_entry *new;
 +      struct cache_entry *new_entry;
  
        if (alias->ce_flags & CE_ADDED)
                die("Will not add file alias '%s' ('%s' already exists in index)", ce->name, alias->name);
  
        /* Ok, create the new entry using the name of the existing alias */
        len = ce_namelen(alias);
 -      new = xcalloc(1, cache_entry_size(len));
 -      memcpy(new->name, alias->name, len);
 -      copy_cache_entry(new, ce);
 +      new_entry = xcalloc(1, cache_entry_size(len));
 +      memcpy(new_entry->name, alias->name, len);
 +      copy_cache_entry(new_entry, ce);
        save_or_free_index_entry(istate, ce);
 -      return new;
 +      return new_entry;
  }
  
  void set_object_name_for_intent_to_add_entry(struct cache_entry *ce)
  {
 -      unsigned char sha1[20];
 -      if (write_sha1_file("", 0, blob_type, sha1))
 +      struct object_id oid;
 +      if (write_object_file("", 0, blob_type, &oid))
                die("cannot create an empty blob in the object database");
 -      hashcpy(ce->oid.hash, sha1);
 +      oidcpy(&ce->oid, &oid);
  }
  
  int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags)
@@@ -1217,8 -1218,9 +1218,8 @@@ int add_index_entry(struct index_state 
        /* Add it in.. */
        istate->cache_nr++;
        if (istate->cache_nr > pos + 1)
 -              memmove(istate->cache + pos + 1,
 -                      istate->cache + pos,
 -                      (istate->cache_nr - pos - 1) * sizeof(ce));
 +              MOVE_ARRAY(istate->cache + pos + 1, istate->cache + pos,
 +                         istate->cache_nr - pos - 1);
        set_index_entry(istate, pos, ce);
        istate->cache_changed |= CE_ENTRY_ADDED;
        return 0;
@@@ -1324,7 -1326,8 +1325,8 @@@ static struct cache_entry *refresh_cach
  
        size = ce_size(ce);
        updated = xmalloc(size);
-       memcpy(updated, ce, size);
+       copy_cache_entry(updated, ce);
+       memcpy(updated->name, ce->name, ce->ce_namelen + 1);
        fill_stat_cache_info(updated, &st);
        /*
         * If ignore_valid is not set, we should leave CE_VALID bit
@@@ -1371,7 -1374,6 +1373,7 @@@ int refresh_index(struct index_state *i
        const char *typechange_fmt;
        const char *added_fmt;
        const char *unmerged_fmt;
 +      uint64_t start = getnanotime();
  
        modified_fmt = (in_porcelain ? "M\t%s\n" : "%s: needs update\n");
        deleted_fmt = (in_porcelain ? "D\t%s\n" : "%s: needs update\n");
        added_fmt = (in_porcelain ? "A\t%s\n" : "%s needs update\n");
        unmerged_fmt = (in_porcelain ? "U\t%s\n" : "%s: needs merge\n");
        for (i = 0; i < istate->cache_nr; i++) {
 -              struct cache_entry *ce, *new;
 +              struct cache_entry *ce, *new_entry;
                int cache_errno = 0;
                int changed = 0;
                int filtered = 0;
                if (filtered)
                        continue;
  
 -              new = refresh_cache_ent(istate, ce, options, &cache_errno, &changed);
 -              if (new == ce)
 +              new_entry = refresh_cache_ent(istate, ce, options, &cache_errno, &changed);
 +              if (new_entry == ce)
                        continue;
 -              if (!new) {
 +              if (!new_entry) {
                        const char *fmt;
  
                        if (really && cache_errno == EINVAL) {
                        continue;
                }
  
 -              replace_index_entry(istate, i, new);
 +              replace_index_entry(istate, i, new_entry);
        }
 +      trace_performance_since(start, "refresh index");
        return has_errors;
  }
  
@@@ -1546,8 -1547,8 +1548,8 @@@ int verify_ce_order
  
  static int verify_hdr(struct cache_header *hdr, unsigned long size)
  {
 -      git_SHA_CTX c;
 -      unsigned char sha1[20];
 +      git_hash_ctx c;
 +      unsigned char hash[GIT_MAX_RAWSZ];
        int hdr_version;
  
        if (hdr->hdr_signature != htonl(CACHE_SIGNATURE))
        if (!verify_index_checksum)
                return 0;
  
 -      git_SHA1_Init(&c);
 -      git_SHA1_Update(&c, hdr, size - 20);
 -      git_SHA1_Final(sha1, &c);
 -      if (hashcmp(sha1, (unsigned char *)hdr + size - 20))
 +      the_hash_algo->init_fn(&c);
 +      the_hash_algo->update_fn(&c, hdr, size - the_hash_algo->rawsz);
 +      the_hash_algo->final_fn(hash, &c);
 +      if (hashcmp(hash, (unsigned char *)hdr + size - the_hash_algo->rawsz))
                return error("bad index file sha1 signature");
        return 0;
  }
@@@ -1604,7 -1605,7 +1606,7 @@@ int hold_locked_index(struct lock_file 
  
  int read_index(struct index_state *istate)
  {
 -      return read_index_from(istate, get_index_file());
 +      return read_index_from(istate, get_index_file(), get_git_dir());
  }
  
  static struct cache_entry *cache_entry_from_ondisk(struct ondisk_cache_entry *ondisk,
@@@ -1792,7 -1793,7 +1794,7 @@@ int do_read_index(struct index_state *i
                die_errno("cannot stat the open index");
  
        mmap_size = xsize_t(st.st_size);
 -      if (mmap_size < sizeof(struct cache_header) + 20)
 +      if (mmap_size < sizeof(struct cache_header) + the_hash_algo->rawsz)
                die("index file smaller than expected");
  
        mmap = xmmap(NULL, mmap_size, PROT_READ, MAP_PRIVATE, fd, 0);
        if (verify_hdr(hdr, mmap_size) < 0)
                goto unmap;
  
 -      hashcpy(istate->sha1, (const unsigned char *)hdr + mmap_size - 20);
 +      hashcpy(istate->sha1, (const unsigned char *)hdr + mmap_size - the_hash_algo->rawsz);
        istate->version = ntohl(hdr->hdr_version);
        istate->cache_nr = ntohl(hdr->hdr_entries);
        istate->cache_alloc = alloc_nr(istate->cache_nr);
        istate->timestamp.sec = st.st_mtime;
        istate->timestamp.nsec = ST_MTIME_NSEC(st);
  
 -      while (src_offset <= mmap_size - 20 - 8) {
 +      while (src_offset <= mmap_size - the_hash_algo->rawsz - 8) {
                /* After an array of active_nr index entries,
                 * there can be arbitrary number of extended
                 * sections, each of which is prefixed with
@@@ -1864,27 -1865,26 +1866,27 @@@ unmap
   * This way, shared index can be removed if they have not been used
   * for some time.
   */
 -static void freshen_shared_index(char *base_sha1_hex, int warn)
 +static void freshen_shared_index(const char *shared_index, int warn)
  {
 -      char *shared_index = git_pathdup("sharedindex.%s", base_sha1_hex);
        if (!check_and_freshen_file(shared_index, 1) && warn)
                warning("could not freshen shared index '%s'", shared_index);
 -      free(shared_index);
  }
  
 -int read_index_from(struct index_state *istate, const char *path)
 +int read_index_from(struct index_state *istate, const char *path,
 +                  const char *gitdir)
  {
 +      uint64_t start = getnanotime();
        struct split_index *split_index;
        int ret;
        char *base_sha1_hex;
 -      const char *base_path;
 +      char *base_path;
  
        /* istate->initialized covers both .git/index and .git/sharedindex.xxx */
        if (istate->initialized)
                return istate->cache_nr;
  
        ret = do_read_index(istate, path, 0);
 +      trace_performance_since(start, "read cache %s", path);
  
        split_index = istate->split_index;
        if (!split_index || is_null_sha1(split_index->base_sha1)) {
                split_index->base = xcalloc(1, sizeof(*split_index->base));
  
        base_sha1_hex = sha1_to_hex(split_index->base_sha1);
 -      base_path = git_path("sharedindex.%s", base_sha1_hex);
 +      base_path = xstrfmt("%s/sharedindex.%s", gitdir, base_sha1_hex);
        ret = do_read_index(split_index->base, base_path, 1);
        if (hashcmp(split_index->base_sha1, split_index->base->sha1))
                die("broken index, expect %s in %s, got %s",
                    base_sha1_hex, base_path,
                    sha1_to_hex(split_index->base->sha1));
  
 -      freshen_shared_index(base_sha1_hex, 0);
 +      freshen_shared_index(base_path, 0);
        merge_base_index(istate);
        post_read_index_from(istate);
 +      trace_performance_since(start, "read cache %s", base_path);
 +      free(base_path);
        return ret;
  }
  
@@@ -1961,11 -1959,11 +1963,11 @@@ int unmerged_index(const struct index_s
  static unsigned char write_buffer[WRITE_BUFFER_SIZE];
  static unsigned long write_buffer_len;
  
 -static int ce_write_flush(git_SHA_CTX *context, int fd)
 +static int ce_write_flush(git_hash_ctx *context, int fd)
  {
        unsigned int buffered = write_buffer_len;
        if (buffered) {
 -              git_SHA1_Update(context, write_buffer, buffered);
 +              the_hash_algo->update_fn(context, write_buffer, buffered);
                if (write_in_full(fd, write_buffer, buffered) < 0)
                        return -1;
                write_buffer_len = 0;
        return 0;
  }
  
 -static int ce_write(git_SHA_CTX *context, int fd, void *data, unsigned int len)
 +static int ce_write(git_hash_ctx *context, int fd, void *data, unsigned int len)
  {
        while (len) {
                unsigned int buffered = write_buffer_len;
        return 0;
  }
  
 -static int write_index_ext_header(git_SHA_CTX *context, int fd,
 +static int write_index_ext_header(git_hash_ctx *context, int fd,
                                  unsigned int ext, unsigned int sz)
  {
        ext = htonl(ext);
                (ce_write(context, fd, &sz, 4) < 0)) ? -1 : 0;
  }
  
 -static int ce_flush(git_SHA_CTX *context, int fd, unsigned char *sha1)
 +static int ce_flush(git_hash_ctx *context, int fd, unsigned char *hash)
  {
        unsigned int left = write_buffer_len;
  
        if (left) {
                write_buffer_len = 0;
 -              git_SHA1_Update(context, write_buffer, left);
 +              the_hash_algo->update_fn(context, write_buffer, left);
        }
  
 -      /* Flush first if not enough space for SHA1 signature */
 -      if (left + 20 > WRITE_BUFFER_SIZE) {
 +      /* Flush first if not enough space for hash signature */
 +      if (left + the_hash_algo->rawsz > WRITE_BUFFER_SIZE) {
                if (write_in_full(fd, write_buffer, left) < 0)
                        return -1;
                left = 0;
        }
  
 -      /* Append the SHA1 signature at the end */
 -      git_SHA1_Final(write_buffer + left, context);
 -      hashcpy(sha1, write_buffer + left);
 -      left += 20;
 +      /* Append the hash signature at the end */
 +      the_hash_algo->final_fn(write_buffer + left, context);
 +      hashcpy(hash, write_buffer + left);
 +      left += the_hash_algo->rawsz;
        return (write_in_full(fd, write_buffer, left) < 0) ? -1 : 0;
  }
  
@@@ -2104,7 -2102,7 +2106,7 @@@ static void copy_cache_entry_to_ondisk(
        }
  }
  
 -static int ce_write_entry(git_SHA_CTX *c, int fd, struct cache_entry *ce,
 +static int ce_write_entry(git_hash_ctx *c, int fd, struct cache_entry *ce,
                          struct strbuf *previous_name, struct ondisk_cache_entry *ondisk)
  {
        int size;
@@@ -2171,7 -2169,7 +2173,7 @@@ static int verify_index_from(const stru
        int fd;
        ssize_t n;
        struct stat st;
 -      unsigned char sha1[20];
 +      unsigned char hash[GIT_MAX_RAWSZ];
  
        if (!istate->initialized)
                return 0;
        if (fstat(fd, &st))
                goto out;
  
 -      if (st.st_size < sizeof(struct cache_header) + 20)
 +      if (st.st_size < sizeof(struct cache_header) + the_hash_algo->rawsz)
                goto out;
  
 -      n = pread_in_full(fd, sha1, 20, st.st_size - 20);
 -      if (n != 20)
 +      n = pread_in_full(fd, hash, the_hash_algo->rawsz, st.st_size - the_hash_algo->rawsz);
 +      if (n != the_hash_algo->rawsz)
                goto out;
  
 -      if (hashcmp(istate->sha1, sha1))
 +      if (hashcmp(istate->sha1, hash))
                goto out;
  
        close(fd);
@@@ -2238,9 -2236,8 +2240,9 @@@ void update_index_if_able(struct index_
  static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
                          int strip_extensions)
  {
 +      uint64_t start = getnanotime();
        int newfd = tempfile->fd;
 -      git_SHA_CTX c;
 +      git_hash_ctx c;
        struct cache_header hdr;
        int i, err = 0, removed, extended, hdr_version;
        struct cache_entry **cache = istate->cache;
        struct stat st;
        struct ondisk_cache_entry_extended ondisk;
        struct strbuf previous_name_buf = STRBUF_INIT, *previous_name;
 -      int drop_cache_tree = 0;
 +      int drop_cache_tree = istate->drop_cache_tree;
  
        for (i = removed = extended = 0; i < entries; i++) {
                if (cache[i]->ce_flags & CE_REMOVE)
        hdr.hdr_version = htonl(hdr_version);
        hdr.hdr_entries = htonl(entries - removed);
  
 -      git_SHA1_Init(&c);
 +      the_hash_algo->init_fn(&c);
        if (ce_write(&c, newfd, &hdr, sizeof(hdr)) < 0)
                return -1;
  
                return -1;
        istate->timestamp.sec = (unsigned int)st.st_mtime;
        istate->timestamp.nsec = ST_MTIME_NSEC(st);
 +      trace_performance_since(start, "write index, changed mask = %x", istate->cache_changed);
        return 0;
  }
  
@@@ -2478,21 -2474,32 +2480,21 @@@ static int clean_shared_index_files(con
  }
  
  static int write_shared_index(struct index_state *istate,
 -                            struct lock_file *lock, unsigned flags)
 +                            struct tempfile **temp)
  {
 -      struct tempfile *temp;
        struct split_index *si = istate->split_index;
        int ret;
  
 -      temp = mks_tempfile(git_path("sharedindex_XXXXXX"));
 -      if (!temp) {
 -              hashclr(si->base_sha1);
 -              return do_write_locked_index(istate, lock, flags);
 -      }
        move_cache_to_base_index(istate);
 -      ret = do_write_index(si->base, temp, 1);
 -      if (ret) {
 -              delete_tempfile(&temp);
 +      ret = do_write_index(si->base, *temp, 1);
 +      if (ret)
                return ret;
 -      }
 -      ret = adjust_shared_perm(get_tempfile_path(temp));
 +      ret = adjust_shared_perm(get_tempfile_path(*temp));
        if (ret) {
 -              int save_errno = errno;
 -              error("cannot fix permission bits on %s", get_tempfile_path(temp));
 -              delete_tempfile(&temp);
 -              errno = save_errno;
 +              error("cannot fix permission bits on %s", get_tempfile_path(*temp));
                return ret;
        }
 -      ret = rename_tempfile(&temp,
 +      ret = rename_tempfile(temp,
                              git_path("sharedindex.%s", sha1_to_hex(si->base->sha1)));
        if (!ret) {
                hashcpy(si->base_sha1, si->base->sha1);
@@@ -2538,12 -2545,6 +2540,12 @@@ int write_locked_index(struct index_sta
        int new_shared_index, ret;
        struct split_index *si = istate->split_index;
  
 +      if ((flags & SKIP_IF_UNCHANGED) && !istate->cache_changed) {
 +              if (flags & COMMIT_LOCK)
 +                      rollback_lock_file(lock);
 +              return 0;
 +      }
 +
        if (istate->fsmonitor_last_update)
                fill_fsmonitor_bitmap(istate);
  
        new_shared_index = istate->cache_changed & SPLIT_INDEX_ORDERED;
  
        if (new_shared_index) {
 -              ret = write_shared_index(istate, lock, flags);
 +              struct tempfile *temp;
 +              int saved_errno;
 +
 +              temp = mks_tempfile(git_path("sharedindex_XXXXXX"));
 +              if (!temp) {
 +                      hashclr(si->base_sha1);
 +                      ret = do_write_locked_index(istate, lock, flags);
 +                      goto out;
 +              }
 +              ret = write_shared_index(istate, &temp);
 +
 +              saved_errno = errno;
 +              if (is_tempfile_active(temp))
 +                      delete_tempfile(&temp);
 +              errno = saved_errno;
 +
                if (ret)
                        goto out;
        }
        ret = write_split_index(istate, lock, flags);
  
        /* Freshen the shared index only if the split-index was written */
 -      if (!ret && !new_shared_index)
 -              freshen_shared_index(sha1_to_hex(si->base_sha1), 1);
 +      if (!ret && !new_shared_index) {
 +              const char *shared_index = git_path("sharedindex.%s",
 +                                                  sha1_to_hex(si->base_sha1));
 +              freshen_shared_index(shared_index, 1);
 +      }
  
  out:
        if (flags & COMMIT_LOCK)