From dcc0a86f2f2f036b3a379f41a4b754f8bb73ed6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Sat, 20 Mar 2021 23:37:44 +0100 Subject: [PATCH 1/8] show tests: add test for "git show " MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add missing tests for showing a tree with "git show". Let's test for showing a tree, two trees, and that doing so doesn't recurse. The only tests for this code added in 5d7eeee2ac6 (git-show: grok blobs, trees and tags, too, 2006-12-14) were the tests in t7701-repack-unpack-unreachable.sh added in ccc1297226b (repack: modify behavior of -A option to leave unreferenced objects unpacked, 2008-05-09). Let's add this common mode of operation to the "show" tests themselves. It's more obvious, and the tests in t7701-repack-unpack-unreachable.sh happily pass if we start buggily emitting trees recursively. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- t/t7007-show.sh | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/t/t7007-show.sh b/t/t7007-show.sh index 42d3db6246..d6cc69e0f2 100755 --- a/t/t7007-show.sh +++ b/t/t7007-show.sh @@ -38,6 +38,45 @@ test_expect_success 'showing two commits' ' test_cmp expect actual.filtered ' +test_expect_success 'showing a tree' ' + cat >expected <<-EOF && + tree main1: + + main1.t + EOF + git show main1: >actual && + test_cmp expected actual +' + +test_expect_success 'showing two trees' ' + cat >expected <<-EOF && + tree main1^{tree} + + main1.t + + tree main2^{tree} + + main1.t + main2.t + EOF + git show main1^{tree} main2^{tree} >actual && + test_cmp expected actual +' + +test_expect_success 'showing a trees is not recursive' ' + git worktree add not-recursive main1 && + mkdir not-recursive/a && + test_commit -C not-recursive a/file && + cat >expected <<-EOF && + tree HEAD^{tree} + + a/ + main1.t + EOF + git -C not-recursive show HEAD^{tree} >actual && + test_cmp expected actual +' + test_expect_success 'showing a range walks (linear)' ' cat >expect <<-EOF && commit $(git rev-parse main3) From 8de78218c53480ded696c751f922b0622697a8b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Sat, 20 Mar 2021 23:37:45 +0100 Subject: [PATCH 2/8] ls-files tests: add meaningful --with-tree tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add tests for "ls-files --with-tree". There was effectively no coverage for any normal usage of this command, only the tests added in 54e1abce90e (Add test case for ls-files --with-tree, 2007-10-03) for an obscure bug. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- t/t3060-ls-files-with-tree.sh | 41 +++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/t/t3060-ls-files-with-tree.sh b/t/t3060-ls-files-with-tree.sh index 52ed665fcd..b257c792a4 100755 --- a/t/t3060-ls-files-with-tree.sh +++ b/t/t3060-ls-files-with-tree.sh @@ -47,6 +47,12 @@ test_expect_success setup ' git add . ' +test_expect_success 'usage' ' + test_expect_code 128 git ls-files --with-tree=HEAD -u && + test_expect_code 128 git ls-files --with-tree=HEAD -s && + test_expect_code 128 git ls-files --recurse-submodules --with-tree=HEAD +' + test_expect_success 'git ls-files --with-tree should succeed from subdir' ' # We have to run from a sub-directory to trigger prune_path # Then we finally get to run our --with-tree test @@ -60,4 +66,39 @@ test_expect_success \ 'git ls-files --with-tree should add entries from named tree.' \ 'test_cmp expected output' +test_expect_success 'no duplicates in --with-tree output' ' + git ls-files --with-tree=HEAD >actual && + sort -u actual >expected && + test_cmp expected actual +' + +test_expect_success 'setup: output in a conflict' ' + test_create_repo conflict && + test_commit -C conflict BASE file && + test_commit -C conflict A file foo && + git -C conflict reset --hard BASE && + test_commit -C conflict B file bar +' + +test_expect_success 'output in a conflict' ' + test_must_fail git -C conflict merge A B && + cat >expected <<-\EOF && + file + file + file + file + EOF + git -C conflict ls-files --with-tree=HEAD >actual && + test_cmp expected actual +' + +test_expect_success 'output with removed .git/index' ' + cat >expected <<-\EOF && + file + EOF + rm conflict/.git/index && + git -C conflict ls-files --with-tree=HEAD >actual && + test_cmp expected actual +' + test_done From eefadd18e10fc0c4c808faa8ee077f375f28050c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Sat, 20 Mar 2021 23:37:46 +0100 Subject: [PATCH 3/8] tree.c API: move read_tree() into builtin/ls-files.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since the read_tree() API was added around the same time as read_tree_recursive() in 94537c78a82 (Move "read_tree()" to "tree.c"[...], 2005-04-22) and b12ec373b8e ([PATCH] Teach read-tree about commit objects, 2005-04-20) things have gradually migrated over to the read_tree_recursive() version. Now builtin/ls-files.c is the last user of this code, let's move all the relevant code there. This allows for subsequent simplification of it, and an eventual move to read_tree_recursive(). Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/ls-files.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++ cache.h | 2 +- tree.c | 89 --------------------------------------------- tree.h | 5 --- 4 files changed, 92 insertions(+), 95 deletions(-) diff --git a/builtin/ls-files.c b/builtin/ls-files.c index f6f9e483b2..a445862281 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -12,6 +12,7 @@ #include "dir.h" #include "builtin.h" #include "tree.h" +#include "cache-tree.h" #include "parse-options.h" #include "resolve-undo.h" #include "string-list.h" @@ -420,6 +421,96 @@ static int get_common_prefix_len(const char *common_prefix) return common_prefix_len; } +static int read_one_entry_opt(struct index_state *istate, + const struct object_id *oid, + const char *base, int baselen, + const char *pathname, + unsigned mode, int stage, int opt) +{ + int len; + struct cache_entry *ce; + + if (S_ISDIR(mode)) + return READ_TREE_RECURSIVE; + + len = strlen(pathname); + ce = make_empty_cache_entry(istate, baselen + len); + + ce->ce_mode = create_ce_mode(mode); + ce->ce_flags = create_ce_flags(stage); + ce->ce_namelen = baselen + len; + memcpy(ce->name, base, baselen); + memcpy(ce->name + baselen, pathname, len+1); + oidcpy(&ce->oid, oid); + return add_index_entry(istate, ce, opt); +} + +static int read_one_entry(const struct object_id *oid, struct strbuf *base, + const char *pathname, unsigned mode, int stage, + void *context) +{ + struct index_state *istate = context; + return read_one_entry_opt(istate, oid, base->buf, base->len, pathname, + mode, stage, + ADD_CACHE_OK_TO_ADD|ADD_CACHE_SKIP_DFCHECK); +} + +/* + * This is used when the caller knows there is no existing entries at + * the stage that will conflict with the entry being added. + */ +static int read_one_entry_quick(const struct object_id *oid, struct strbuf *base, + const char *pathname, unsigned mode, int stage, + void *context) +{ + struct index_state *istate = context; + return read_one_entry_opt(istate, oid, base->buf, base->len, pathname, + mode, stage, + ADD_CACHE_JUST_APPEND); +} + + +static int read_tree(struct repository *r, struct tree *tree, int stage, + struct pathspec *match, struct index_state *istate) +{ + read_tree_fn_t fn = NULL; + int i, err; + + /* + * Currently the only existing callers of this function all + * call it with stage=1 and after making sure there is nothing + * at that stage; we could always use read_one_entry_quick(). + * + * But when we decide to straighten out git-read-tree not to + * use unpack_trees() in some cases, this will probably start + * to matter. + */ + + /* + * See if we have cache entry at the stage. If so, + * do it the original slow way, otherwise, append and then + * sort at the end. + */ + for (i = 0; !fn && i < istate->cache_nr; i++) { + const struct cache_entry *ce = istate->cache[i]; + if (ce_stage(ce) == stage) + fn = read_one_entry; + } + + if (!fn) + fn = read_one_entry_quick; + err = read_tree_recursive(r, tree, "", 0, stage, match, fn, istate); + if (fn == read_one_entry || err) + return err; + + /* + * Sort the cache entry -- we need to nuke the cache tree, though. + */ + cache_tree_free(&istate->cache_tree); + QSORT(istate->cache, istate->cache_nr, cmp_cache_name_compare); + return 0; +} + /* * Read the tree specified with --with-tree option * (typically, HEAD) into stage #1 and then diff --git a/cache.h b/cache.h index d928149614..bb317abc91 100644 --- a/cache.h +++ b/cache.h @@ -803,7 +803,7 @@ static inline int index_pos_to_insert_pos(uintmax_t pos) #define ADD_CACHE_OK_TO_ADD 1 /* Ok to add */ #define ADD_CACHE_OK_TO_REPLACE 2 /* Ok to replace file/directory */ #define ADD_CACHE_SKIP_DFCHECK 4 /* Ok to skip DF conflict checks */ -#define ADD_CACHE_JUST_APPEND 8 /* Append only; tree.c::read_tree() */ +#define ADD_CACHE_JUST_APPEND 8 /* Append only */ #define ADD_CACHE_NEW_ONLY 16 /* Do not replace existing ones */ #define ADD_CACHE_KEEP_CACHE_TREE 32 /* Do not invalidate cache-tree */ #define ADD_CACHE_RENORMALIZE 64 /* Pass along HASH_RENORMALIZE */ diff --git a/tree.c b/tree.c index a52479812c..a6c12f2745 100644 --- a/tree.c +++ b/tree.c @@ -11,54 +11,6 @@ const char *tree_type = "tree"; -static int read_one_entry_opt(struct index_state *istate, - const struct object_id *oid, - const char *base, int baselen, - const char *pathname, - unsigned mode, int stage, int opt) -{ - int len; - struct cache_entry *ce; - - if (S_ISDIR(mode)) - return READ_TREE_RECURSIVE; - - len = strlen(pathname); - ce = make_empty_cache_entry(istate, baselen + len); - - ce->ce_mode = create_ce_mode(mode); - ce->ce_flags = create_ce_flags(stage); - ce->ce_namelen = baselen + len; - memcpy(ce->name, base, baselen); - memcpy(ce->name + baselen, pathname, len+1); - oidcpy(&ce->oid, oid); - return add_index_entry(istate, ce, opt); -} - -static int read_one_entry(const struct object_id *oid, struct strbuf *base, - const char *pathname, unsigned mode, int stage, - void *context) -{ - struct index_state *istate = context; - return read_one_entry_opt(istate, oid, base->buf, base->len, pathname, - mode, stage, - ADD_CACHE_OK_TO_ADD|ADD_CACHE_SKIP_DFCHECK); -} - -/* - * This is used when the caller knows there is no existing entries at - * the stage that will conflict with the entry being added. - */ -static int read_one_entry_quick(const struct object_id *oid, struct strbuf *base, - const char *pathname, unsigned mode, int stage, - void *context) -{ - struct index_state *istate = context; - return read_one_entry_opt(istate, oid, base->buf, base->len, pathname, - mode, stage, - ADD_CACHE_JUST_APPEND); -} - static int read_tree_1(struct repository *r, struct tree *tree, struct strbuf *base, int stage, const struct pathspec *pathspec, @@ -154,47 +106,6 @@ int cmp_cache_name_compare(const void *a_, const void *b_) ce2->name, ce2->ce_namelen, ce_stage(ce2)); } -int read_tree(struct repository *r, struct tree *tree, int stage, - struct pathspec *match, struct index_state *istate) -{ - read_tree_fn_t fn = NULL; - int i, err; - - /* - * Currently the only existing callers of this function all - * call it with stage=1 and after making sure there is nothing - * at that stage; we could always use read_one_entry_quick(). - * - * But when we decide to straighten out git-read-tree not to - * use unpack_trees() in some cases, this will probably start - * to matter. - */ - - /* - * See if we have cache entry at the stage. If so, - * do it the original slow way, otherwise, append and then - * sort at the end. - */ - for (i = 0; !fn && i < istate->cache_nr; i++) { - const struct cache_entry *ce = istate->cache[i]; - if (ce_stage(ce) == stage) - fn = read_one_entry; - } - - if (!fn) - fn = read_one_entry_quick; - err = read_tree_recursive(r, tree, "", 0, stage, match, fn, istate); - if (fn == read_one_entry || err) - return err; - - /* - * Sort the cache entry -- we need to nuke the cache tree, though. - */ - cache_tree_free(&istate->cache_tree); - QSORT(istate->cache, istate->cache_nr, cmp_cache_name_compare); - return 0; -} - struct tree *lookup_tree(struct repository *r, const struct object_id *oid) { struct object *obj = lookup_object(r, oid); diff --git a/tree.h b/tree.h index 3eb0484cbf..6b0b1dc211 100644 --- a/tree.h +++ b/tree.h @@ -38,9 +38,4 @@ int read_tree_recursive(struct repository *r, const char *base, int baselen, int stage, const struct pathspec *pathspec, read_tree_fn_t fn, void *context); - -int read_tree(struct repository *r, struct tree *tree, - int stage, struct pathspec *pathspec, - struct index_state *istate); - #endif /* TREE_H */ From fcc7c12f1139d5b1fd657a4e89425f07dbc78d8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Sat, 20 Mar 2021 23:37:47 +0100 Subject: [PATCH 4/8] ls-files: don't needlessly pass around stage variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that read_tree() has been moved to ls-files.c we can get rid of the stage != 1 case that'll never happen. Let's not use read_tree_recursive() as a pass-through to pass "stage = 1" either. For now we'll pass an unused "stage = 0" for consistency with other read_tree_recursive() callers, that argument will be removed in a follow-up commit. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/ls-files.c | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/builtin/ls-files.c b/builtin/ls-files.c index a445862281..3149a2769a 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -425,7 +425,7 @@ static int read_one_entry_opt(struct index_state *istate, const struct object_id *oid, const char *base, int baselen, const char *pathname, - unsigned mode, int stage, int opt) + unsigned mode, int opt) { int len; struct cache_entry *ce; @@ -437,7 +437,7 @@ static int read_one_entry_opt(struct index_state *istate, ce = make_empty_cache_entry(istate, baselen + len); ce->ce_mode = create_ce_mode(mode); - ce->ce_flags = create_ce_flags(stage); + ce->ce_flags = create_ce_flags(1); ce->ce_namelen = baselen + len; memcpy(ce->name, base, baselen); memcpy(ce->name + baselen, pathname, len+1); @@ -451,7 +451,7 @@ static int read_one_entry(const struct object_id *oid, struct strbuf *base, { struct index_state *istate = context; return read_one_entry_opt(istate, oid, base->buf, base->len, pathname, - mode, stage, + mode, ADD_CACHE_OK_TO_ADD|ADD_CACHE_SKIP_DFCHECK); } @@ -465,26 +465,17 @@ static int read_one_entry_quick(const struct object_id *oid, struct strbuf *base { struct index_state *istate = context; return read_one_entry_opt(istate, oid, base->buf, base->len, pathname, - mode, stage, + mode, ADD_CACHE_JUST_APPEND); } -static int read_tree(struct repository *r, struct tree *tree, int stage, +static int read_tree(struct repository *r, struct tree *tree, struct pathspec *match, struct index_state *istate) { read_tree_fn_t fn = NULL; int i, err; - /* - * Currently the only existing callers of this function all - * call it with stage=1 and after making sure there is nothing - * at that stage; we could always use read_one_entry_quick(). - * - * But when we decide to straighten out git-read-tree not to - * use unpack_trees() in some cases, this will probably start - * to matter. - */ /* * See if we have cache entry at the stage. If so, @@ -493,13 +484,13 @@ static int read_tree(struct repository *r, struct tree *tree, int stage, */ for (i = 0; !fn && i < istate->cache_nr; i++) { const struct cache_entry *ce = istate->cache[i]; - if (ce_stage(ce) == stage) + if (ce_stage(ce) == 1) fn = read_one_entry; } if (!fn) fn = read_one_entry_quick; - err = read_tree_recursive(r, tree, "", 0, stage, match, fn, istate); + err = read_tree_recursive(r, tree, "", 0, 0, match, fn, istate); if (fn == read_one_entry || err) return err; @@ -549,7 +540,7 @@ void overlay_tree_on_index(struct index_state *istate, PATHSPEC_PREFER_CWD, prefix, matchbuf); } else memset(&pathspec, 0, sizeof(pathspec)); - if (read_tree(the_repository, tree, 1, &pathspec, istate)) + if (read_tree(the_repository, tree, &pathspec, istate)) die("unable to read tree entries %s", tree_name); for (i = 0; i < istate->cache_nr; i++) { From 9614ad3ce09937f3124db09cc8c6dd2777a15515 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Sat, 20 Mar 2021 23:37:48 +0100 Subject: [PATCH 5/8] ls-files: refactor away read_tree() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactor away the read_tree() function into its only user, overlay_tree_on_index(). First, change read_one_entry_opt() to use the strbuf parameter read_tree_recursive() passes down in place. This finishes up a partial refactoring started in 6a0b0b6de99 (tree.c: update read_tree_recursive callback to pass strbuf as base, 2014-11-30). Moving the rest into overlay_tree_on_index() makes this index juggling we're doing easier to read. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/ls-files.c | 77 ++++++++++++++++++++-------------------------- 1 file changed, 34 insertions(+), 43 deletions(-) diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 3149a2769a..aa153423b8 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -423,7 +423,7 @@ static int get_common_prefix_len(const char *common_prefix) static int read_one_entry_opt(struct index_state *istate, const struct object_id *oid, - const char *base, int baselen, + struct strbuf *base, const char *pathname, unsigned mode, int opt) { @@ -434,13 +434,13 @@ static int read_one_entry_opt(struct index_state *istate, return READ_TREE_RECURSIVE; len = strlen(pathname); - ce = make_empty_cache_entry(istate, baselen + len); + ce = make_empty_cache_entry(istate, base->len + len); ce->ce_mode = create_ce_mode(mode); ce->ce_flags = create_ce_flags(1); - ce->ce_namelen = baselen + len; - memcpy(ce->name, base, baselen); - memcpy(ce->name + baselen, pathname, len+1); + ce->ce_namelen = base->len + len; + memcpy(ce->name, base->buf, base->len); + memcpy(ce->name + base->len, pathname, len+1); oidcpy(&ce->oid, oid); return add_index_entry(istate, ce, opt); } @@ -450,7 +450,7 @@ static int read_one_entry(const struct object_id *oid, struct strbuf *base, void *context) { struct index_state *istate = context; - return read_one_entry_opt(istate, oid, base->buf, base->len, pathname, + return read_one_entry_opt(istate, oid, base, pathname, mode, ADD_CACHE_OK_TO_ADD|ADD_CACHE_SKIP_DFCHECK); } @@ -464,42 +464,8 @@ static int read_one_entry_quick(const struct object_id *oid, struct strbuf *base void *context) { struct index_state *istate = context; - return read_one_entry_opt(istate, oid, base->buf, base->len, pathname, - mode, - ADD_CACHE_JUST_APPEND); -} - - -static int read_tree(struct repository *r, struct tree *tree, - struct pathspec *match, struct index_state *istate) -{ - read_tree_fn_t fn = NULL; - int i, err; - - - /* - * See if we have cache entry at the stage. If so, - * do it the original slow way, otherwise, append and then - * sort at the end. - */ - for (i = 0; !fn && i < istate->cache_nr; i++) { - const struct cache_entry *ce = istate->cache[i]; - if (ce_stage(ce) == 1) - fn = read_one_entry; - } - - if (!fn) - fn = read_one_entry_quick; - err = read_tree_recursive(r, tree, "", 0, 0, match, fn, istate); - if (fn == read_one_entry || err) - return err; - - /* - * Sort the cache entry -- we need to nuke the cache tree, though. - */ - cache_tree_free(&istate->cache_tree); - QSORT(istate->cache, istate->cache_nr, cmp_cache_name_compare); - return 0; + return read_one_entry_opt(istate, oid, base, pathname, + mode, ADD_CACHE_JUST_APPEND); } /* @@ -518,6 +484,8 @@ void overlay_tree_on_index(struct index_state *istate, struct pathspec pathspec; struct cache_entry *last_stage0 = NULL; int i; + read_tree_fn_t fn = NULL; + int err; if (get_oid(tree_name, &oid)) die("tree-ish %s not found.", tree_name); @@ -540,9 +508,32 @@ void overlay_tree_on_index(struct index_state *istate, PATHSPEC_PREFER_CWD, prefix, matchbuf); } else memset(&pathspec, 0, sizeof(pathspec)); - if (read_tree(the_repository, tree, &pathspec, istate)) + + /* + * See if we have cache entry at the stage. If so, + * do it the original slow way, otherwise, append and then + * sort at the end. + */ + for (i = 0; !fn && i < istate->cache_nr; i++) { + const struct cache_entry *ce = istate->cache[i]; + if (ce_stage(ce) == 1) + fn = read_one_entry; + } + + if (!fn) + fn = read_one_entry_quick; + err = read_tree_recursive(the_repository, tree, "", 0, 1, &pathspec, fn, istate); + if (err) die("unable to read tree entries %s", tree_name); + /* + * Sort the cache entry -- we need to nuke the cache tree, though. + */ + if (fn == read_one_entry_quick) { + cache_tree_free(&istate->cache_tree); + QSORT(istate->cache, istate->cache_nr, cmp_cache_name_compare); + } + for (i = 0; i < istate->cache_nr; i++) { struct cache_entry *ce = istate->cache[i]; switch (ce_stage(ce)) { From 7367d88261e5df7b1458cc02217f0e302bc2e127 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Sat, 20 Mar 2021 23:37:49 +0100 Subject: [PATCH 6/8] archive: stop passing "stage" through read_tree_recursive() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "stage" variable being passed around in the archive code has only ever been an elaborate way to hardcode the value "0". This code was added in its original form in e4fbbfe9ecc (Add git-zip-tree, 2006-08-26), at which point a hardcoded "0" would be passed down through read_tree_recursive() to write_zip_entry(). It was then diligently added to the "struct directory" in ed22b4173bd (archive: support filtering paths with glob, 2014-09-21), but we were still not doing anything except passing it around as-is. Let's stop doing that in the code internal to archive.c, we'll still feed "0" to read_tree_recursive() itself, but won't use it. That we're providing it at all to read_tree_recursive() will be changed in a follow-up commit. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- archive.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/archive.c b/archive.c index 5919d9e505..4f27133154 100644 --- a/archive.c +++ b/archive.c @@ -107,7 +107,6 @@ struct directory { struct object_id oid; int baselen, len; unsigned mode; - int stage; char path[FLEX_ARRAY]; }; @@ -138,7 +137,7 @@ static int check_attr_export_subst(const struct attr_check *check) } static int write_archive_entry(const struct object_id *oid, const char *base, - int baselen, const char *filename, unsigned mode, int stage, + int baselen, const char *filename, unsigned mode, void *context) { static struct strbuf path = STRBUF_INIT; @@ -197,7 +196,7 @@ static int write_archive_entry(const struct object_id *oid, const char *base, static void queue_directory(const unsigned char *sha1, struct strbuf *base, const char *filename, - unsigned mode, int stage, struct archiver_context *c) + unsigned mode, struct archiver_context *c) { struct directory *d; size_t len = st_add4(base->len, 1, strlen(filename), 1); @@ -205,7 +204,6 @@ static void queue_directory(const unsigned char *sha1, d->up = c->bottom; d->baselen = base->len; d->mode = mode; - d->stage = stage; c->bottom = d; d->len = xsnprintf(d->path, len, "%.*s%s/", (int)base->len, base->buf, filename); hashcpy(d->oid.hash, sha1); @@ -224,7 +222,7 @@ static int write_directory(struct archiver_context *c) write_directory(c) || write_archive_entry(&d->oid, d->path, d->baselen, d->path + d->baselen, d->mode, - d->stage, c) != READ_TREE_RECURSIVE; + c) != READ_TREE_RECURSIVE; free(d); return ret ? -1 : 0; } @@ -256,14 +254,14 @@ static int queue_or_write_archive_entry(const struct object_id *oid, if (check_attr_export_ignore(check)) return 0; queue_directory(oid->hash, base, filename, - mode, stage, c); + mode, c); return READ_TREE_RECURSIVE; } if (write_directory(c)) return -1; return write_archive_entry(oid, base->buf, base->len, filename, mode, - stage, context); + context); } struct extra_file_info { @@ -377,8 +375,8 @@ struct path_exists_context { }; static int reject_entry(const struct object_id *oid, struct strbuf *base, - const char *filename, unsigned mode, - int stage, void *context) + const char *filename, unsigned mode, int stage, + void *context) { int ret = -1; struct path_exists_context *ctx = context; From 6c9fc42e9f1bf27a3830ef594b7f5f9147af8361 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Sat, 20 Mar 2021 23:37:50 +0100 Subject: [PATCH 7/8] tree.h API: expose read_tree_1() as read_tree_at() Rename the static read_tree_1() function to read_tree_at(). This function works just like read_tree_recursive(), except you provide your own strbuf. This step doesn't make much sense now, but in follow-up commits I'll remove the base/baselen/stage arguments to read_tree_recursive(). At that point an anticipated in-tree user[1] for the old read_tree_recursive() couldn't provide a path to start the traversal. Let's give them a function to do so with an API that makes more sense for them, by taking a strbuf we should be able to avoid more casting and/or reallocations in the future. 1. https://lore.kernel.org/git/xmqqft106sok.fsf@gitster.g Signed-off-by: Junio C Hamano --- tree.c | 17 +++++++++-------- tree.h | 6 ++++++ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/tree.c b/tree.c index a6c12f2745..f67c215305 100644 --- a/tree.c +++ b/tree.c @@ -11,10 +11,11 @@ const char *tree_type = "tree"; -static int read_tree_1(struct repository *r, - struct tree *tree, struct strbuf *base, - int stage, const struct pathspec *pathspec, - read_tree_fn_t fn, void *context) +int read_tree_at(struct repository *r, + struct tree *tree, struct strbuf *base, + int stage, + const struct pathspec *pathspec, + read_tree_fn_t fn, void *context) { struct tree_desc desc; struct name_entry entry; @@ -71,9 +72,9 @@ static int read_tree_1(struct repository *r, len = tree_entry_len(&entry); strbuf_add(base, entry.path, len); strbuf_addch(base, '/'); - retval = read_tree_1(r, lookup_tree(r, &oid), - base, stage, pathspec, - fn, context); + retval = read_tree_at(r, lookup_tree(r, &oid), + base, stage, pathspec, + fn, context); strbuf_setlen(base, oldlen); if (retval) return -1; @@ -91,7 +92,7 @@ int read_tree_recursive(struct repository *r, int ret; strbuf_add(&sb, base, baselen); - ret = read_tree_1(r, tree, &sb, stage, pathspec, fn, context); + ret = read_tree_at(r, tree, &sb, stage, pathspec, fn, context); strbuf_release(&sb); return ret; } diff --git a/tree.h b/tree.h index 6b0b1dc211..123fc41efe 100644 --- a/tree.h +++ b/tree.h @@ -33,6 +33,12 @@ int cmp_cache_name_compare(const void *a_, const void *b_); #define READ_TREE_RECURSIVE 1 typedef int (*read_tree_fn_t)(const struct object_id *, struct strbuf *, const char *, unsigned int, int, void *); +int read_tree_at(struct repository *r, + struct tree *tree, struct strbuf *base, + int stage, + const struct pathspec *pathspec, + read_tree_fn_t fn, void *context); + int read_tree_recursive(struct repository *r, struct tree *tree, const char *base, int baselen, From 47957485b3b731a7860e0554d2bd12c0dce1c75a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Sat, 20 Mar 2021 23:37:51 +0100 Subject: [PATCH 8/8] tree.h API: simplify read_tree_recursive() signature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplify the signature of read_tree_recursive() to omit the "base", "baselen" and "stage" arguments. No callers of it use these parameters for anything anymore. The last function to call read_tree_recursive() with a non-"" path was read_tree_recursive() itself, but that was changed in ffd31f661d5 (Reimplement read_tree_recursive() using tree_entry_interesting(), 2011-03-25). The last user of the "stage" parameter went away in the last commit, and even that use was mere boilerplate. So let's remove those and rename the read_tree_recursive() function to just read_tree(). We had another read_tree() function that I've refactored away in preceding commits, since all in-tree users read trees recursively with a callback we can change the name to signify that this is the norm. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- archive.c | 18 +++++++++--------- builtin/checkout.c | 8 ++++---- builtin/log.c | 8 ++++---- builtin/ls-files.c | 6 +++--- builtin/ls-tree.c | 6 +++--- merge-recursive.c | 6 +++--- tree.c | 19 +++++++------------ tree.h | 13 ++++++------- 8 files changed, 39 insertions(+), 45 deletions(-) diff --git a/archive.c b/archive.c index 4f27133154..4921dc8a69 100644 --- a/archive.c +++ b/archive.c @@ -229,7 +229,7 @@ static int write_directory(struct archiver_context *c) static int queue_or_write_archive_entry(const struct object_id *oid, struct strbuf *base, const char *filename, - unsigned mode, int stage, void *context) + unsigned mode, void *context) { struct archiver_context *c = context; @@ -314,10 +314,10 @@ int write_archive_entries(struct archiver_args *args, git_attr_set_direction(GIT_ATTR_INDEX); } - err = read_tree_recursive(args->repo, args->tree, "", - 0, 0, &args->pathspec, - queue_or_write_archive_entry, - &context); + err = read_tree(args->repo, args->tree, + &args->pathspec, + queue_or_write_archive_entry, + &context); if (err == READ_TREE_RECURSIVE) err = 0; while (context.bottom) { @@ -375,7 +375,7 @@ struct path_exists_context { }; static int reject_entry(const struct object_id *oid, struct strbuf *base, - const char *filename, unsigned mode, int stage, + const char *filename, unsigned mode, void *context) { int ret = -1; @@ -403,9 +403,9 @@ static int path_exists(struct archiver_args *args, const char *path) ctx.args = args; parse_pathspec(&ctx.pathspec, 0, 0, "", paths); ctx.pathspec.recursive = 1; - ret = read_tree_recursive(args->repo, args->tree, "", - 0, 0, &ctx.pathspec, - reject_entry, &ctx); + ret = read_tree(args->repo, args->tree, + &ctx.pathspec, + reject_entry, &ctx); clear_pathspec(&ctx.pathspec); return ret != 0; } diff --git a/builtin/checkout.c b/builtin/checkout.c index 2d6550bc3c..0e66390520 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -114,7 +114,7 @@ static int post_checkout_hook(struct commit *old_commit, struct commit *new_comm } static int update_some(const struct object_id *oid, struct strbuf *base, - const char *pathname, unsigned mode, int stage, void *context) + const char *pathname, unsigned mode, void *context) { int len; struct cache_entry *ce; @@ -155,8 +155,8 @@ static int update_some(const struct object_id *oid, struct strbuf *base, static int read_tree_some(struct tree *tree, const struct pathspec *pathspec) { - read_tree_recursive(the_repository, tree, "", 0, 0, - pathspec, update_some, NULL); + read_tree(the_repository, tree, + pathspec, update_some, NULL); /* update the index with the given tree's info * for all args, expanding wildcards, and exit @@ -322,7 +322,7 @@ static void mark_ce_for_checkout_overlay(struct cache_entry *ce, * If it comes from the tree-ish, we already know it * matches the pathspec and could just stamp * CE_MATCHED to it from update_some(). But we still - * need ps_matched and read_tree_recursive (and + * need ps_matched and read_tree (and * eventually tree_entry_interesting) cannot fill * ps_matched yet. Once it can, we can avoid calling * match_pathspec() for _all_ entries when diff --git a/builtin/log.c b/builtin/log.c index f67b67d80e..980de59063 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -599,7 +599,7 @@ static int show_tag_object(const struct object_id *oid, struct rev_info *rev) static int show_tree_object(const struct object_id *oid, struct strbuf *base, - const char *pathname, unsigned mode, int stage, void *context) + const char *pathname, unsigned mode, void *context) { FILE *file = context; fprintf(file, "%s%s\n", pathname, S_ISDIR(mode) ? "/" : ""); @@ -681,9 +681,9 @@ int cmd_show(int argc, const char **argv, const char *prefix) diff_get_color_opt(&rev.diffopt, DIFF_COMMIT), name, diff_get_color_opt(&rev.diffopt, DIFF_RESET)); - read_tree_recursive(the_repository, (struct tree *)o, "", - 0, 0, &match_all, show_tree_object, - rev.diffopt.file); + read_tree(the_repository, (struct tree *)o, + &match_all, show_tree_object, + rev.diffopt.file); rev.shown_one = 1; break; case OBJ_COMMIT: diff --git a/builtin/ls-files.c b/builtin/ls-files.c index aa153423b8..60a2913a01 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -446,7 +446,7 @@ static int read_one_entry_opt(struct index_state *istate, } static int read_one_entry(const struct object_id *oid, struct strbuf *base, - const char *pathname, unsigned mode, int stage, + const char *pathname, unsigned mode, void *context) { struct index_state *istate = context; @@ -460,7 +460,7 @@ static int read_one_entry(const struct object_id *oid, struct strbuf *base, * the stage that will conflict with the entry being added. */ static int read_one_entry_quick(const struct object_id *oid, struct strbuf *base, - const char *pathname, unsigned mode, int stage, + const char *pathname, unsigned mode, void *context) { struct index_state *istate = context; @@ -522,7 +522,7 @@ void overlay_tree_on_index(struct index_state *istate, if (!fn) fn = read_one_entry_quick; - err = read_tree_recursive(the_repository, tree, "", 0, 1, &pathspec, fn, istate); + err = read_tree(the_repository, tree, &pathspec, fn, istate); if (err) die("unable to read tree entries %s", tree_name); diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c index 7cad3f24eb..3a442631c7 100644 --- a/builtin/ls-tree.c +++ b/builtin/ls-tree.c @@ -62,7 +62,7 @@ static int show_recursive(const char *base, int baselen, const char *pathname) } static int show_tree(const struct object_id *oid, struct strbuf *base, - const char *pathname, unsigned mode, int stage, void *context) + const char *pathname, unsigned mode, void *context) { int retval = 0; int baselen; @@ -185,6 +185,6 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix) tree = parse_tree_indirect(&oid); if (!tree) die("not a tree object"); - return !!read_tree_recursive(the_repository, tree, "", 0, 0, - &pathspec, show_tree, NULL); + return !!read_tree(the_repository, tree, + &pathspec, show_tree, NULL); } diff --git a/merge-recursive.c b/merge-recursive.c index b052974f19..3d9207455b 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -453,7 +453,7 @@ static void unpack_trees_finish(struct merge_options *opt) static int save_files_dirs(const struct object_id *oid, struct strbuf *base, const char *path, - unsigned int mode, int stage, void *context) + unsigned int mode, void *context) { struct path_hashmap_entry *entry; int baselen = base->len; @@ -473,8 +473,8 @@ static void get_files_dirs(struct merge_options *opt, struct tree *tree) { struct pathspec match_all; memset(&match_all, 0, sizeof(match_all)); - read_tree_recursive(opt->repo, tree, "", 0, 0, - &match_all, save_files_dirs, opt); + read_tree(opt->repo, tree, + &match_all, save_files_dirs, opt); } static int get_tree_entry_if_blob(struct repository *r, diff --git a/tree.c b/tree.c index f67c215305..410e3b477e 100644 --- a/tree.c +++ b/tree.c @@ -13,7 +13,6 @@ const char *tree_type = "tree"; int read_tree_at(struct repository *r, struct tree *tree, struct strbuf *base, - int stage, const struct pathspec *pathspec, read_tree_fn_t fn, void *context) { @@ -39,7 +38,7 @@ int read_tree_at(struct repository *r, } switch (fn(&entry.oid, base, - entry.path, entry.mode, stage, context)) { + entry.path, entry.mode, context)) { case 0: continue; case READ_TREE_RECURSIVE: @@ -73,7 +72,7 @@ int read_tree_at(struct repository *r, strbuf_add(base, entry.path, len); strbuf_addch(base, '/'); retval = read_tree_at(r, lookup_tree(r, &oid), - base, stage, pathspec, + base, pathspec, fn, context); strbuf_setlen(base, oldlen); if (retval) @@ -82,17 +81,13 @@ int read_tree_at(struct repository *r, return 0; } -int read_tree_recursive(struct repository *r, - struct tree *tree, - const char *base, int baselen, - int stage, const struct pathspec *pathspec, - read_tree_fn_t fn, void *context) +int read_tree(struct repository *r, + struct tree *tree, + const struct pathspec *pathspec, + read_tree_fn_t fn, void *context) { struct strbuf sb = STRBUF_INIT; - int ret; - - strbuf_add(&sb, base, baselen); - ret = read_tree_at(r, tree, &sb, stage, pathspec, fn, context); + int ret = read_tree_at(r, tree, &sb, pathspec, fn, context); strbuf_release(&sb); return ret; } diff --git a/tree.h b/tree.h index 123fc41efe..6efff003e2 100644 --- a/tree.h +++ b/tree.h @@ -31,17 +31,16 @@ struct tree *parse_tree_indirect(const struct object_id *oid); int cmp_cache_name_compare(const void *a_, const void *b_); #define READ_TREE_RECURSIVE 1 -typedef int (*read_tree_fn_t)(const struct object_id *, struct strbuf *, const char *, unsigned int, int, void *); +typedef int (*read_tree_fn_t)(const struct object_id *, struct strbuf *, const char *, unsigned int, void *); int read_tree_at(struct repository *r, struct tree *tree, struct strbuf *base, - int stage, const struct pathspec *pathspec, read_tree_fn_t fn, void *context); -int read_tree_recursive(struct repository *r, - struct tree *tree, - const char *base, int baselen, - int stage, const struct pathspec *pathspec, - read_tree_fn_t fn, void *context); +int read_tree(struct repository *r, + struct tree *tree, + const struct pathspec *pathspec, + read_tree_fn_t fn, void *context); + #endif /* TREE_H */