Re: [PATCH v2 27/29] tree-walk.h API: add a get_tree_entry_path() function
- From
Elijah Newren <newren@gmail.com>
- Date
- Mar 16, 2021, 06:50 UTC
- Message-ID
- <CABPp-BF3SOzz+C1YW73kYgz2P8h2pSgYSmc8FJMOygAHVdoeFw@mail.gmail.com>
- In-Reply-To
- <20210316021312.13927-28-avarab@gmail.com>
On Mon, Mar 15, 2021 at 7:13 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
Show 8 quoted lines
>
> Add a get_tree_entry_path() variant in addition to
> get_tree_entry_path_{mode,type,all}(). This is for those callers that
> need neither the mode nor "enum object_type" parameters filled for
> them.
>
> There's callers here which doesn't need the "struct object_id" filled
> either, and provides a throwaway variable for us.As with the previous round, this paragraph does not parse well. My suggestion on the last round was:
There are callers here which don't need the "struct object_id" filled; forcing callers to pass one just requires they create a throwaway variable.
you don't have to take it, but the paragraph needs rewording one way or another.
Show 150 quoted lines
>
> See the following commits for the introduction of such code that's
> being modified here:
>
> - shift_tree(): 68faf68938e (A new merge stragety 'subtree'.,
> 2007-02-15) for the shift_tree()
>
> - tree_has_path(): 96e7ffbdc31 (merge-recursive: check for directory
> level conflicts, 2018-04-19)
>
> - init_notes(): fd53c9eb445 (Speed up git notes lookup, 2009-10-09)
>
> - diagnose_invalid_oid_path(): 009fee4774d (Detailed diagnosis when
> parsing an object name fails., 2009-12-07)
>
> Those could potentially be refactored too, but I've got to stop at
> some point, and right now I'm focusing downstream code that depends on
> "mode" (or "enum object_type").
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> match-trees.c | 4 +---
> merge-recursive.c | 6 ++----
> notes.c | 3 +--
> object-name.c | 3 +--
> tree-walk.c | 11 +++++++++++
> tree-walk.h | 3 +++
> 6 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/match-trees.c b/match-trees.c
> index 2afa4968109..25bfb46fb02 100644
> --- a/match-trees.c
> +++ b/match-trees.c
> @@ -288,12 +288,10 @@ void shift_tree(struct repository *r,
>
> if (add_score < del_score) {
> /* We need to pick a subtree of two */
> - unsigned short mode;
> -
> if (!*del_prefix)
> return;
>
> - if (get_tree_entry_mode(r, hash2, del_prefix, shifted, &mode))
> + if (get_tree_entry_path(r, hash2, del_prefix, shifted))
> die("cannot find path %s in tree %s",
> del_prefix, oid_to_hex(hash2));
> return;
> diff --git a/merge-recursive.c b/merge-recursive.c
> index bbbb68e15bc..83d2b8b8440 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1884,11 +1884,9 @@ static int tree_has_path(struct repository *r, struct tree *tree,
> const char *path)
> {
> struct object_id hashy;
> - unsigned short mode_o;
> -
> - return !get_tree_entry_mode(r,
> + return !get_tree_entry_path(r,
> &tree->object.oid, path,
> - &hashy, &mode_o);
> + &hashy);
> }
>
> /*
> diff --git a/notes.c b/notes.c
> index ef138606146..aa46cb2b09e 100644
> --- a/notes.c
> +++ b/notes.c
> @@ -994,7 +994,6 @@ void init_notes(struct notes_tree *t, const char *notes_ref,
> combine_notes_fn combine_notes, int flags)
> {
> struct object_id oid, object_oid;
> - unsigned short mode;
> struct leaf_node root_tree;
>
> if (!t)
> @@ -1021,7 +1020,7 @@ void init_notes(struct notes_tree *t, const char *notes_ref,
> return;
> if (flags & NOTES_INIT_WRITABLE && read_ref(notes_ref, &object_oid))
> die("Cannot use notes ref %s", notes_ref);
> - if (get_tree_entry_mode(the_repository, &object_oid, "", &oid, &mode))
> + if (get_tree_entry_path(the_repository, &object_oid, "", &oid))
> die("Failed to read notes tree referenced by %s (%s)",
> notes_ref, oid_to_hex(&object_oid));
>
> diff --git a/object-name.c b/object-name.c
> index 7e3b2d6d739..9ff5f83c1ff 100644
> --- a/object-name.c
> +++ b/object-name.c
> @@ -1693,7 +1693,6 @@ static void diagnose_invalid_oid_path(struct repository *r,
> int object_name_len)
> {
> struct object_id oid;
> - unsigned short mode;
>
> if (!prefix)
> prefix = "";
> @@ -1704,7 +1703,7 @@ static void diagnose_invalid_oid_path(struct repository *r,
> if (is_missing_file_error(errno)) {
> char *fullname = xstrfmt("%s%s", prefix, filename);
>
> - if (!get_tree_entry_mode(r, tree_oid, fullname, &oid, &mode)) {
> + if (!get_tree_entry_path(r, tree_oid, fullname, &oid)) {
> die(_("path '%s' exists, but not '%s'\n"
> "hint: Did you mean '%.*s:%s' aka '%.*s:./%s'?"),
> fullname,
> diff --git a/tree-walk.c b/tree-walk.c
> index a90dbf87af4..fa846535dfb 100644
> --- a/tree-walk.c
> +++ b/tree-walk.c
> @@ -632,6 +632,17 @@ int get_tree_entry_all(struct repository *r,
> return retval;
> }
>
> +int get_tree_entry_path(struct repository *r,
> + const struct object_id *tree_oid,
> + const char *name,
> + struct object_id *oid)
> +{
> + unsigned short mode;
> + enum object_type object_type;
> + return get_tree_entry_all(r, tree_oid, name, oid,
> + &mode, &object_type);
> +}
> +
> int get_tree_entry_mode(struct repository *r,
> const struct object_id *tree_oid,
> const char *name,
> diff --git a/tree-walk.h b/tree-walk.h
> index 55ef88ef2e5..efcd7ccd10e 100644
> --- a/tree-walk.h
> +++ b/tree-walk.h
> @@ -193,10 +193,13 @@ struct traverse_info {
> * "struct name_entry" you'd like. You always need a pointer to an
> * appropriate variable to fill in (NULL won't do!):
> *
> + * get_tree_entry_path(): <no extra argument, just get the common 'path'>
> * get_tree_entry_mode(): unsigned int mode
> * get_tree_entry_type(): enum object_type
> * get_tree_entry_all(): unsigned int mode, enum object_type
> */
> +int get_tree_entry_path(struct repository *, const struct object_id *, const char *,
> + struct object_id *);
> int get_tree_entry_mode(struct repository *, const struct object_id *, const char *,
> struct object_id *,
> unsigned short *);
> --
> 2.31.0.rc2.211.g1d0b8788b3
>