| Message ID | 20210306193458.20633-7-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Move the read_tree() function to its only user | expand |
On Sat, Mar 6, 2021 at 11:35 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > Every caller or the read_tree_recursive() function hardcoded a s/or/of/? > starting point of "" in the tree. So let's simply remove that > parameter. Interesting. It appears that read_tree_recursive() was the last function to call read_tree_recursive() with a starting point other than "", but that was changed in commit ffd31f661d ("Reimplement read_tree_recursive() using tree_entry_interesting()", 2011-03-25). The last caller of read_tree_recursive() other than itself to call it with base != "", was ebfbdb340a ("Git archive and trailing "/" in prefix", 2009-10-08). > It might be useful in the future to get this functionality back, > there's no reason we won't have a read_tree_recursive() use-case that > would want to start in a subdirectory. This paragraph is very hard to parse. > But if and when that happens we can just add something like a > read_tree_recursive_subdir() and have both read_tree_recursive() and > that function be a thin wrapper for read_tree_1(). Starting with "But if and when that happens" suggests this shouldn't be an independent paragraph. > In the meantime there's no reason to keep around what amounts to dead > code just in case we need it in the future. Makes sense to me. > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > archive.c | 8 ++++---- > builtin/checkout.c | 2 +- > builtin/log.c | 4 ++-- > builtin/ls-files.c | 2 +- > builtin/ls-tree.c | 2 +- > merge-recursive.c | 2 +- > tree.c | 2 -- > tree.h | 1 - > 8 files changed, 10 insertions(+), 13 deletions(-) > > diff --git a/archive.c b/archive.c > index 5919d9e5050..9394f170f7f 100644 > --- a/archive.c > +++ b/archive.c > @@ -316,8 +316,8 @@ 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, > + err = read_tree_recursive(args->repo, args->tree, > + 0, &args->pathspec, > queue_or_write_archive_entry, > &context); > if (err == READ_TREE_RECURSIVE) > @@ -405,8 +405,8 @@ 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, > + ret = read_tree_recursive(args->repo, args->tree, > + 0, &ctx.pathspec, > reject_entry, &ctx); > clear_pathspec(&ctx.pathspec); > return ret != 0; > diff --git a/builtin/checkout.c b/builtin/checkout.c > index 2d6550bc3c8..21b742c0f07 100644 > --- a/builtin/checkout.c > +++ b/builtin/checkout.c > @@ -155,7 +155,7 @@ 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, > + read_tree_recursive(the_repository, tree, 0, > pathspec, update_some, NULL); > > /* update the index with the given tree's info > diff --git a/builtin/log.c b/builtin/log.c > index f67b67d80ed..ffa3fb8c286 100644 > --- a/builtin/log.c > +++ b/builtin/log.c > @@ -681,8 +681,8 @@ 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, > + read_tree_recursive(the_repository, (struct tree *)o, > + 0, &match_all, show_tree_object, > rev.diffopt.file); > rev.shown_one = 1; > break; > diff --git a/builtin/ls-files.c b/builtin/ls-files.c > index c0349a7b206..2c609428eea 100644 > --- a/builtin/ls-files.c > +++ b/builtin/ls-files.c > @@ -483,7 +483,7 @@ void overlay_tree_on_index(struct index_state *istate, > PATHSPEC_PREFER_CWD, prefix, matchbuf); > } else > memset(&pathspec, 0, sizeof(pathspec)); > - if (read_tree_recursive(the_repository, tree, "", 0, 1, > + if (read_tree_recursive(the_repository, tree, 1, > &pathspec, read_one_entry_quick, istate)) > die("unable to read tree entries %s", tree_name); > > diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c > index 7cad3f24ebd..7d3fb2e6d0f 100644 > --- a/builtin/ls-tree.c > +++ b/builtin/ls-tree.c > @@ -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, > + return !!read_tree_recursive(the_repository, tree, 0, > &pathspec, show_tree, NULL); > } > diff --git a/merge-recursive.c b/merge-recursive.c > index b052974f191..fa7602ff0f2 100644 > --- a/merge-recursive.c > +++ b/merge-recursive.c > @@ -473,7 +473,7 @@ 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, > + read_tree_recursive(opt->repo, tree, 0, > &match_all, save_files_dirs, opt); > } > > diff --git a/tree.c b/tree.c > index c1bde9314d0..285633892c2 100644 > --- a/tree.c > +++ b/tree.c > @@ -82,14 +82,12 @@ static int read_tree_1(struct repository *r, > > 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) > { > struct strbuf sb = STRBUF_INIT; > int ret; > > - strbuf_add(&sb, base, baselen); > ret = read_tree_1(r, tree, &sb, stage, pathspec, fn, context); > strbuf_release(&sb); > return ret; > diff --git a/tree.h b/tree.h > index 34549c86c9f..9a0fd3221e3 100644 > --- a/tree.h > +++ b/tree.h > @@ -33,7 +33,6 @@ typedef int (*read_tree_fn_t)(const struct object_id *, struct strbuf *, const c > > 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); > > -- > 2.31.0.rc0.126.g04f22c5b82 Looks good.
diff --git a/archive.c b/archive.c index 5919d9e5050..9394f170f7f 100644 --- a/archive.c +++ b/archive.c @@ -316,8 +316,8 @@ 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, + err = read_tree_recursive(args->repo, args->tree, + 0, &args->pathspec, queue_or_write_archive_entry, &context); if (err == READ_TREE_RECURSIVE) @@ -405,8 +405,8 @@ 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, + ret = read_tree_recursive(args->repo, args->tree, + 0, &ctx.pathspec, reject_entry, &ctx); clear_pathspec(&ctx.pathspec); return ret != 0; diff --git a/builtin/checkout.c b/builtin/checkout.c index 2d6550bc3c8..21b742c0f07 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -155,7 +155,7 @@ 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, + read_tree_recursive(the_repository, tree, 0, pathspec, update_some, NULL); /* update the index with the given tree's info diff --git a/builtin/log.c b/builtin/log.c index f67b67d80ed..ffa3fb8c286 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -681,8 +681,8 @@ 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, + read_tree_recursive(the_repository, (struct tree *)o, + 0, &match_all, show_tree_object, rev.diffopt.file); rev.shown_one = 1; break; diff --git a/builtin/ls-files.c b/builtin/ls-files.c index c0349a7b206..2c609428eea 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -483,7 +483,7 @@ void overlay_tree_on_index(struct index_state *istate, PATHSPEC_PREFER_CWD, prefix, matchbuf); } else memset(&pathspec, 0, sizeof(pathspec)); - if (read_tree_recursive(the_repository, tree, "", 0, 1, + if (read_tree_recursive(the_repository, tree, 1, &pathspec, read_one_entry_quick, istate)) die("unable to read tree entries %s", tree_name); diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c index 7cad3f24ebd..7d3fb2e6d0f 100644 --- a/builtin/ls-tree.c +++ b/builtin/ls-tree.c @@ -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, + return !!read_tree_recursive(the_repository, tree, 0, &pathspec, show_tree, NULL); } diff --git a/merge-recursive.c b/merge-recursive.c index b052974f191..fa7602ff0f2 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -473,7 +473,7 @@ 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, + read_tree_recursive(opt->repo, tree, 0, &match_all, save_files_dirs, opt); } diff --git a/tree.c b/tree.c index c1bde9314d0..285633892c2 100644 --- a/tree.c +++ b/tree.c @@ -82,14 +82,12 @@ static int read_tree_1(struct repository *r, 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) { struct strbuf sb = STRBUF_INIT; int ret; - strbuf_add(&sb, base, baselen); ret = read_tree_1(r, tree, &sb, stage, pathspec, fn, context); strbuf_release(&sb); return ret; diff --git a/tree.h b/tree.h index 34549c86c9f..9a0fd3221e3 100644 --- a/tree.h +++ b/tree.h @@ -33,7 +33,6 @@ typedef int (*read_tree_fn_t)(const struct object_id *, struct strbuf *, const c 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);
Every caller or the read_tree_recursive() function hardcoded a starting point of "" in the tree. So let's simply remove that parameter. It might be useful in the future to get this functionality back, there's no reason we won't have a read_tree_recursive() use-case that would want to start in a subdirectory. But if and when that happens we can just add something like a read_tree_recursive_subdir() and have both read_tree_recursive() and that function be a thin wrapper for read_tree_1(). In the meantime there's no reason to keep around what amounts to dead code just in case we need it in the future. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- archive.c | 8 ++++---- builtin/checkout.c | 2 +- builtin/log.c | 4 ++-- builtin/ls-files.c | 2 +- builtin/ls-tree.c | 2 +- merge-recursive.c | 2 +- tree.c | 2 -- tree.h | 1 - 8 files changed, 10 insertions(+), 13 deletions(-)