Merge branch 'ds/more-index-cleanups'

Cleaning various codepaths up.

* ds/more-index-cleanups:
  t1092: test interesting sparse-checkout scenarios
  test-lib: test_region looks for trace2 regions
  sparse-checkout: load sparse-checkout patterns
  name-hash: use trace2 regions for init
  repository: add repo reference to index_state
  fsmonitor: de-duplicate BUG()s around dirty bits
  cache-tree: extract subtree_pos()
  cache-tree: simplify verify_cache() prototype
  cache-tree: clean up cache_tree_update()
This commit is contained in:
Junio C Hamano 2021-02-10 14:48:32 -08:00
commit 2f794620f5
15 changed files with 408 additions and 53 deletions

View File

@ -821,9 +821,6 @@ static int merge_working_tree(const struct checkout_opts *opts,
}
}
if (!active_cache_tree)
active_cache_tree = cache_tree();
if (!cache_tree_fully_valid(active_cache_tree))
cache_tree_update(&the_index, WRITE_TREE_SILENT | WRITE_TREE_REPAIR);

View File

@ -22,11 +22,6 @@ static char const * const builtin_sparse_checkout_usage[] = {
NULL
};
static char *get_sparse_checkout_filename(void)
{
return git_pathdup("info/sparse-checkout");
}
static void write_patterns_to_file(FILE *fp, struct pattern_list *pl)
{
int i;

View File

@ -45,7 +45,7 @@ static int subtree_name_cmp(const char *one, int onelen,
return memcmp(one, two, onelen);
}
static int subtree_pos(struct cache_tree *it, const char *path, int pathlen)
int cache_tree_subtree_pos(struct cache_tree *it, const char *path, int pathlen)
{
struct cache_tree_sub **down = it->down;
int lo, hi;
@ -72,7 +72,7 @@ static struct cache_tree_sub *find_subtree(struct cache_tree *it,
int create)
{
struct cache_tree_sub *down;
int pos = subtree_pos(it, path, pathlen);
int pos = cache_tree_subtree_pos(it, path, pathlen);
if (0 <= pos)
return it->down[pos];
if (!create)
@ -123,7 +123,7 @@ static int do_invalidate_path(struct cache_tree *it, const char *path)
it->entry_count = -1;
if (!*slash) {
int pos;
pos = subtree_pos(it, path, namelen);
pos = cache_tree_subtree_pos(it, path, namelen);
if (0 <= pos) {
cache_tree_free(&it->down[pos]->cache_tree);
free(it->down[pos]);
@ -151,16 +151,15 @@ void cache_tree_invalidate_path(struct index_state *istate, const char *path)
istate->cache_changed |= CACHE_TREE_CHANGED;
}
static int verify_cache(struct cache_entry **cache,
int entries, int flags)
static int verify_cache(struct index_state *istate, int flags)
{
int i, funny;
unsigned i, funny;
int silent = flags & WRITE_TREE_SILENT;
/* Verify that the tree is merged */
funny = 0;
for (i = 0; i < entries; i++) {
const struct cache_entry *ce = cache[i];
for (i = 0; i < istate->cache_nr; i++) {
const struct cache_entry *ce = istate->cache[i];
if (ce_stage(ce)) {
if (silent)
return -1;
@ -180,13 +179,13 @@ static int verify_cache(struct cache_entry **cache,
* stage 0 entries.
*/
funny = 0;
for (i = 0; i < entries - 1; i++) {
for (i = 0; i + 1 < istate->cache_nr; i++) {
/* path/file always comes after path because of the way
* the cache is sorted. Also path can appear only once,
* which means conflicting one would immediately follow.
*/
const struct cache_entry *this_ce = cache[i];
const struct cache_entry *next_ce = cache[i + 1];
const struct cache_entry *this_ce = istate->cache[i];
const struct cache_entry *next_ce = istate->cache[i + 1];
const char *this_name = this_ce->name;
const char *next_name = next_ce->name;
int this_len = ce_namelen(this_ce);
@ -436,16 +435,20 @@ static int update_one(struct cache_tree *it,
int cache_tree_update(struct index_state *istate, int flags)
{
struct cache_tree *it = istate->cache_tree;
struct cache_entry **cache = istate->cache;
int entries = istate->cache_nr;
int skip, i = verify_cache(cache, entries, flags);
int skip, i;
i = verify_cache(istate, flags);
if (i)
return i;
if (!istate->cache_tree)
istate->cache_tree = cache_tree();
trace_performance_enter();
trace2_region_enter("cache_tree", "update", the_repository);
i = update_one(it, cache, entries, "", 0, &skip, flags);
i = update_one(istate->cache_tree, istate->cache, istate->cache_nr,
"", 0, &skip, flags);
trace2_region_leave("cache_tree", "update", the_repository);
trace_performance_leave("cache_tree_update");
if (i < 0)
@ -635,9 +638,6 @@ static int write_index_as_tree_internal(struct object_id *oid,
cache_tree_valid = 0;
}
if (!index_state->cache_tree)
index_state->cache_tree = cache_tree();
if (!cache_tree_valid && cache_tree_update(index_state, flags) < 0)
return WRITE_TREE_UNMERGED_INDEX;

View File

@ -27,6 +27,8 @@ void cache_tree_free(struct cache_tree **);
void cache_tree_invalidate_path(struct index_state *, const char *);
struct cache_tree_sub *cache_tree_sub(struct cache_tree *, const char *);
int cache_tree_subtree_pos(struct cache_tree *it, const char *path, int pathlen);
void cache_tree_write(struct strbuf *, struct cache_tree *root);
struct cache_tree *cache_tree_read(const char *buffer, unsigned long size);

View File

@ -328,6 +328,7 @@ struct index_state {
struct ewah_bitmap *fsmonitor_dirty;
struct mem_pool *ce_mem_pool;
struct progress *progress;
struct repository *repo;
};
/* Name hashing */

17
dir.c
View File

@ -2998,6 +2998,23 @@ void setup_standard_excludes(struct dir_struct *dir)
}
}
char *get_sparse_checkout_filename(void)
{
return git_pathdup("info/sparse-checkout");
}
int get_sparse_checkout_patterns(struct pattern_list *pl)
{
int res;
char *sparse_filename = get_sparse_checkout_filename();
pl->use_cone_patterns = core_sparse_checkout_cone;
res = add_patterns_from_file_to_list(sparse_filename, "", 0, pl, NULL);
free(sparse_filename);
return res;
}
int remove_path(const char *name)
{
char *slash;

2
dir.h
View File

@ -448,6 +448,8 @@ int is_empty_dir(const char *dir);
void setup_standard_excludes(struct dir_struct *dir);
char *get_sparse_checkout_filename(void);
int get_sparse_checkout_patterns(struct pattern_list *pl);
/* Constants for remove_dir_recursively: */

View File

@ -13,14 +13,19 @@
struct trace_key trace_fsmonitor = TRACE_KEY_INIT(FSMONITOR);
static void assert_index_minimum(struct index_state *istate, size_t pos)
{
if (pos > istate->cache_nr)
BUG("fsmonitor_dirty has more entries than the index (%"PRIuMAX" > %u)",
(uintmax_t)pos, istate->cache_nr);
}
static void fsmonitor_ewah_callback(size_t pos, void *is)
{
struct index_state *istate = (struct index_state *)is;
struct cache_entry *ce;
if (pos >= istate->cache_nr)
BUG("fsmonitor_dirty has more entries than the index (%"PRIuMAX" >= %u)",
(uintmax_t)pos, istate->cache_nr);
assert_index_minimum(istate, pos + 1);
ce = istate->cache[pos];
ce->ce_flags &= ~CE_FSMONITOR_VALID;
@ -82,10 +87,8 @@ int read_fsmonitor_extension(struct index_state *istate, const void *data,
}
istate->fsmonitor_dirty = fsmonitor_dirty;
if (!istate->split_index &&
istate->fsmonitor_dirty->bit_size > istate->cache_nr)
BUG("fsmonitor_dirty has more entries than the index (%"PRIuMAX" > %u)",
(uintmax_t)istate->fsmonitor_dirty->bit_size, istate->cache_nr);
if (!istate->split_index)
assert_index_minimum(istate, istate->fsmonitor_dirty->bit_size);
trace_printf_key(&trace_fsmonitor, "read fsmonitor extension successful");
return 0;
@ -110,10 +113,8 @@ void write_fsmonitor_extension(struct strbuf *sb, struct index_state *istate)
uint32_t ewah_size = 0;
int fixup = 0;
if (!istate->split_index &&
istate->fsmonitor_dirty->bit_size > istate->cache_nr)
BUG("fsmonitor_dirty has more entries than the index (%"PRIuMAX" > %u)",
(uintmax_t)istate->fsmonitor_dirty->bit_size, istate->cache_nr);
if (!istate->split_index)
assert_index_minimum(istate, istate->fsmonitor_dirty->bit_size);
put_be32(&hdr_version, INDEX_EXTENSION_VERSION2);
strbuf_add(sb, &hdr_version, sizeof(uint32_t));
@ -335,9 +336,7 @@ void tweak_fsmonitor(struct index_state *istate)
}
/* Mark all previously saved entries as dirty */
if (istate->fsmonitor_dirty->bit_size > istate->cache_nr)
BUG("fsmonitor_dirty has more entries than the index (%"PRIuMAX" > %u)",
(uintmax_t)istate->fsmonitor_dirty->bit_size, istate->cache_nr);
assert_index_minimum(istate, istate->fsmonitor_dirty->bit_size);
ewah_each_bit(istate->fsmonitor_dirty, fsmonitor_ewah_callback, istate);
refresh_fsmonitor(istate);

View File

@ -7,6 +7,7 @@
*/
#include "cache.h"
#include "thread-utils.h"
#include "trace2.h"
struct dir_entry {
struct hashmap_entry ent;
@ -577,6 +578,7 @@ static void lazy_init_name_hash(struct index_state *istate)
if (istate->name_hash_initialized)
return;
trace_performance_enter();
trace2_region_enter("index", "name-hash-init", istate->repo);
hashmap_init(&istate->name_hash, cache_entry_cmp, NULL, istate->cache_nr);
hashmap_init(&istate->dir_hash, dir_entry_cmp, NULL, istate->cache_nr);
@ -597,6 +599,7 @@ static void lazy_init_name_hash(struct index_state *istate)
}
istate->name_hash_initialized = 1;
trace2_region_leave("index", "name-hash-init", istate->repo);
trace_performance_leave("initialize name hash");
}

View File

@ -264,6 +264,12 @@ int repo_read_index(struct repository *repo)
if (!repo->index)
repo->index = xcalloc(1, sizeof(*repo->index));
/* Complete the double-reference */
if (!repo->index->repo)
repo->index->repo = repo;
else if (repo->index->repo != repo)
BUG("repo's index should point back at itself");
return read_index_from(repo->index, repo->index_file, repo->gitdir);
}

View File

@ -679,9 +679,6 @@ static int do_recursive_merge(struct repository *r,
static struct object_id *get_cache_tree_oid(struct index_state *istate)
{
if (!istate->cache_tree)
istate->cache_tree = cache_tree();
if (!cache_tree_fully_valid(istate->cache_tree))
if (cache_tree_update(istate, 0)) {
error(_("unable to update cache tree"));

View File

@ -303,8 +303,7 @@ test_expect_success 'progress generates traces' '
"Working hard" <in 2>stderr &&
# t0212/parse_events.perl intentionally omits regions and data.
grep -e "region_enter" -e "\"category\":\"progress\"" trace.event &&
grep -e "region_leave" -e "\"category\":\"progress\"" trace.event &&
test_region progress "Working hard" trace.event &&
grep "\"key\":\"total_objects\",\"value\":\"40\"" trace.event &&
grep "\"key\":\"total_bytes\",\"value\":\"409600\"" trace.event
'

View File

@ -0,0 +1,301 @@
#!/bin/sh
test_description='compare full workdir to sparse workdir'
. ./test-lib.sh
test_expect_success 'setup' '
git init initial-repo &&
(
cd initial-repo &&
echo a >a &&
echo "after deep" >e &&
echo "after folder1" >g &&
echo "after x" >z &&
mkdir folder1 folder2 deep x &&
mkdir deep/deeper1 deep/deeper2 &&
mkdir deep/deeper1/deepest &&
echo "after deeper1" >deep/e &&
echo "after deepest" >deep/deeper1/e &&
cp a folder1 &&
cp a folder2 &&
cp a x &&
cp a deep &&
cp a deep/deeper1 &&
cp a deep/deeper2 &&
cp a deep/deeper1/deepest &&
cp -r deep/deeper1/deepest deep/deeper2 &&
git add . &&
git commit -m "initial commit" &&
git checkout -b base &&
for dir in folder1 folder2 deep
do
git checkout -b update-$dir &&
echo "updated $dir" >$dir/a &&
git commit -a -m "update $dir" || return 1
done &&
git checkout -b rename-base base &&
echo >folder1/larger-content <<-\EOF &&
matching
lines
help
inexact
renames
EOF
cp folder1/larger-content folder2/ &&
cp folder1/larger-content deep/deeper1/ &&
git add . &&
git commit -m "add interesting rename content" &&
git checkout -b rename-out-to-out rename-base &&
mv folder1/a folder2/b &&
mv folder1/larger-content folder2/edited-content &&
echo >>folder2/edited-content &&
git add . &&
git commit -m "rename folder1/... to folder2/..." &&
git checkout -b rename-out-to-in rename-base &&
mv folder1/a deep/deeper1/b &&
mv folder1/larger-content deep/deeper1/edited-content &&
echo >>deep/deeper1/edited-content &&
git add . &&
git commit -m "rename folder1/... to deep/deeper1/..." &&
git checkout -b rename-in-to-out rename-base &&
mv deep/deeper1/a folder1/b &&
mv deep/deeper1/larger-content folder1/edited-content &&
echo >>folder1/edited-content &&
git add . &&
git commit -m "rename deep/deeper1/... to folder1/..." &&
git checkout -b deepest base &&
echo "updated deepest" >deep/deeper1/deepest/a &&
git commit -a -m "update deepest" &&
git checkout -f base &&
git reset --hard
)
'
init_repos () {
rm -rf full-checkout sparse-checkout sparse-index &&
# create repos in initial state
cp -r initial-repo full-checkout &&
git -C full-checkout reset --hard &&
cp -r initial-repo sparse-checkout &&
git -C sparse-checkout reset --hard &&
git -C sparse-checkout sparse-checkout init --cone &&
# initialize sparse-checkout definitions
git -C sparse-checkout sparse-checkout set deep
}
run_on_sparse () {
(
cd sparse-checkout &&
$* >../sparse-checkout-out 2>../sparse-checkout-err
)
}
run_on_all () {
(
cd full-checkout &&
$* >../full-checkout-out 2>../full-checkout-err
) &&
run_on_sparse $*
}
test_all_match () {
run_on_all $* &&
test_cmp full-checkout-out sparse-checkout-out &&
test_cmp full-checkout-err sparse-checkout-err
}
test_expect_success 'status with options' '
init_repos &&
test_all_match git status --porcelain=v2 &&
test_all_match git status --porcelain=v2 -z -u &&
test_all_match git status --porcelain=v2 -uno &&
run_on_all "touch README.md" &&
test_all_match git status --porcelain=v2 &&
test_all_match git status --porcelain=v2 -z -u &&
test_all_match git status --porcelain=v2 -uno &&
test_all_match git add README.md &&
test_all_match git status --porcelain=v2 &&
test_all_match git status --porcelain=v2 -z -u &&
test_all_match git status --porcelain=v2 -uno
'
test_expect_success 'add, commit, checkout' '
init_repos &&
write_script edit-contents <<-\EOF &&
echo text >>$1
EOF
run_on_all "../edit-contents README.md" &&
test_all_match git add README.md &&
test_all_match git status --porcelain=v2 &&
test_all_match git commit -m "Add README.md" &&
test_all_match git checkout HEAD~1 &&
test_all_match git checkout - &&
run_on_all "../edit-contents README.md" &&
test_all_match git add -A &&
test_all_match git status --porcelain=v2 &&
test_all_match git commit -m "Extend README.md" &&
test_all_match git checkout HEAD~1 &&
test_all_match git checkout - &&
run_on_all "../edit-contents deep/newfile" &&
test_all_match git status --porcelain=v2 -uno &&
test_all_match git status --porcelain=v2 &&
test_all_match git add . &&
test_all_match git status --porcelain=v2 &&
test_all_match git commit -m "add deep/newfile" &&
test_all_match git checkout HEAD~1 &&
test_all_match git checkout -
'
test_expect_success 'checkout and reset --hard' '
init_repos &&
test_all_match git checkout update-folder1 &&
test_all_match git status --porcelain=v2 &&
test_all_match git checkout update-deep &&
test_all_match git status --porcelain=v2 &&
test_all_match git checkout -b reset-test &&
test_all_match git reset --hard deepest &&
test_all_match git reset --hard update-folder1 &&
test_all_match git reset --hard update-folder2
'
test_expect_success 'diff --staged' '
init_repos &&
write_script edit-contents <<-\EOF &&
echo text >>README.md
EOF
run_on_all "../edit-contents" &&
test_all_match git diff &&
test_all_match git diff --staged &&
test_all_match git add README.md &&
test_all_match git diff &&
test_all_match git diff --staged
'
test_expect_success 'diff with renames' '
init_repos &&
for branch in rename-out-to-out rename-out-to-in rename-in-to-out
do
test_all_match git checkout rename-base &&
test_all_match git checkout $branch -- .&&
test_all_match git diff --staged --no-renames &&
test_all_match git diff --staged --find-renames || return 1
done
'
test_expect_success 'log with pathspec outside sparse definition' '
init_repos &&
test_all_match git log -- a &&
test_all_match git log -- folder1/a &&
test_all_match git log -- folder2/a &&
test_all_match git log -- deep/a &&
test_all_match git log -- deep/deeper1/a &&
test_all_match git log -- deep/deeper1/deepest/a &&
test_all_match git checkout update-folder1 &&
test_all_match git log -- folder1/a
'
test_expect_success 'blame with pathspec inside sparse definition' '
init_repos &&
test_all_match git blame a &&
test_all_match git blame deep/a &&
test_all_match git blame deep/deeper1/a &&
test_all_match git blame deep/deeper1/deepest/a
'
# TODO: blame currently does not support blaming files outside of the
# sparse definition. It complains that the file doesn't exist locally.
test_expect_failure 'blame with pathspec outside sparse definition' '
init_repos &&
test_all_match git blame folder1/a &&
test_all_match git blame folder2/a &&
test_all_match git blame deep/deeper2/a &&
test_all_match git blame deep/deeper2/deepest/a
'
# TODO: reset currently does not behave as expected when in a
# sparse-checkout.
test_expect_failure 'checkout and reset (mixed)' '
init_repos &&
test_all_match git checkout -b reset-test update-deep &&
test_all_match git reset deepest &&
test_all_match git reset update-folder1 &&
test_all_match git reset update-folder2
'
test_expect_success 'merge' '
init_repos &&
test_all_match git checkout -b merge update-deep &&
test_all_match git merge -m "folder1" update-folder1 &&
test_all_match git rev-parse HEAD^{tree} &&
test_all_match git merge -m "folder2" update-folder2 &&
test_all_match git rev-parse HEAD^{tree}
'
test_expect_success 'merge with outside renames' '
init_repos &&
for type in out-to-out out-to-in in-to-out
do
test_all_match git reset --hard &&
test_all_match git checkout -f -b merge-$type update-deep &&
test_all_match git merge -m "$type" rename-$type &&
test_all_match git rev-parse HEAD^{tree} || return 1
done
'
test_expect_success 'clean' '
init_repos &&
echo bogus >>.gitignore &&
run_on_all cp ../.gitignore . &&
test_all_match git add .gitignore &&
test_all_match git commit -m ignore-bogus-files &&
run_on_sparse mkdir folder1 &&
run_on_all touch folder1/bogus &&
test_all_match git status --porcelain=v2 &&
test_all_match git clean -f &&
test_all_match git status --porcelain=v2 &&
test_all_match git clean -xf &&
test_all_match git status --porcelain=v2 &&
test_all_match git clean -xdf &&
test_all_match git status --porcelain=v2 &&
test_path_is_dir sparse-checkout/folder1
'
test_done

View File

@ -1683,3 +1683,45 @@ test_subcommand () {
grep "\[$expr\]"
fi
}
# Check that the given command was invoked as part of the
# trace2-format trace on stdin.
#
# test_region [!] <category> <label> git <command> <args>...
#
# For example, to look for trace2_region_enter("index", "do_read_index", repo)
# in an invocation of "git checkout HEAD~1", run
#
# GIT_TRACE2_EVENT="$(pwd)/trace.txt" GIT_TRACE2_EVENT_NESTING=10 \
# git checkout HEAD~1 &&
# test_region index do_read_index <trace.txt
#
# If the first parameter passed is !, this instead checks that
# the given region was not entered.
#
test_region () {
local expect_exit=0
if test "$1" = "!"
then
expect_exit=1
shift
fi
grep -e '"region_enter".*"category":"'"$1"'","label":"'"$2"\" "$3"
exitcode=$?
if test $exitcode != $expect_exit
then
return 1
fi
grep -e '"region_leave".*"category":"'"$1"'","label":"'"$2"\" "$3"
exitcode=$?
if test $exitcode != $expect_exit
then
return 1
fi
return 0
}

View File

@ -1549,14 +1549,10 @@ static void mark_new_skip_worktree(struct pattern_list *pl,
static void populate_from_existing_patterns(struct unpack_trees_options *o,
struct pattern_list *pl)
{
char *sparse = git_pathdup("info/sparse-checkout");
pl->use_cone_patterns = core_sparse_checkout_cone;
if (add_patterns_from_file_to_list(sparse, "", 0, pl, NULL) < 0)
if (get_sparse_checkout_patterns(pl) < 0)
o->skip_sparse_checkout = 1;
else
o->pl = pl;
free(sparse);
}
@ -1726,8 +1722,6 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
if (!ret) {
if (git_env_bool("GIT_TEST_CHECK_CACHE_TREE", 0))
cache_tree_verify(the_repository, &o->result);
if (!o->result.cache_tree)
o->result.cache_tree = cache_tree();
if (!cache_tree_fully_valid(o->result.cache_tree))
cache_tree_update(&o->result,
WRITE_TREE_SILENT |