On Sat, Mar 6, 2021 at 11:35 AM Ævar Arnfjörð Bjarmason
<avarab@xxxxxxxxx> 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@xxxxxxxxx>
> ---
> 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.
