Paul-Sebastian Ungureanu <ungureanupaulsebastian@xxxxxxxxx> writes:
> Add `get_tree_entry_gently()`, `find_tree_entry_gently()`
> and `get_tree_entry_follow_symlinks_gently()`, which will
> make `get_oid()` to be more gently.
>
> Since `get_tree_entry()` is used in more than 20 places,
> adding a new parameter will make this commit harder to read.
> In every place it is called there will need to be an additional
> 0 parameter at the end of the call. The solution to avoid this is
> to rename the function in `get_tree_entry_gently()` which gets
> an additional `flags` variable. A new `get_tree_entry()`
> will call `get_tree_entry_gently()` with `flags` being 0.
> This way, no additional changes will be needed.
And that is the right way to introduce a new feature to existing API
with many callers in general.
I wonder if the GENTLY option should apply to update_tree_entry()
the same way as it would to the other codepaths that currently die
to express "we were handed this string by the caller and told to
give back object ID the string represents, and we found no good
answer". In this one (and the "bad ref" one), the existing failures
in these two codepaths are not "we got a string and that does not
resolve to an object name", but "we didn't have the data to work on
to begin with (either a corrupt tree object or a corrupt ref").
In other words, it's not like "We were given HEAD:no-such-path and
there is no such path in that tree"; it is "We tried to read HEAD:
tree for no-such-path in it, but the tree was corrupt and we couldn't
even tell if such a path is or is not in it", no?
> Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@xxxxxxxxx>
> ---
> sha1-name.c | 2 +-
> tree-walk.c | 108 +++++++++++++++++++++++++++++++++++++++++++---------
> tree-walk.h | 3 +-
> 3 files changed, 94 insertions(+), 19 deletions(-)
>
> diff --git a/sha1-name.c b/sha1-name.c
> index 60d9ef3c7..d741e1129 100644
> --- a/sha1-name.c
> +++ b/sha1-name.c
> @@ -1721,7 +1721,7 @@ static int get_oid_with_context_1(const char *name,
> if (flags & GET_OID_FOLLOW_SYMLINKS) {
> ret = get_tree_entry_follow_symlinks(&tree_oid,
> filename, oid, &oc->symlink_path,
> - &oc->mode);
> + &oc->mode, flags);
> } else {
> ret = get_tree_entry(&tree_oid, filename, oid,
> &oc->mode);
> diff --git a/tree-walk.c b/tree-walk.c
> index 8f5090862..2925eaec2 100644
> --- a/tree-walk.c
> +++ b/tree-walk.c
> @@ -491,7 +491,9 @@ struct dir_state {
> struct object_id oid;
> };
>
> -static int find_tree_entry(struct tree_desc *t, const char *name, struct object_id *result, unsigned *mode)
> +static int find_tree_entry(struct tree_desc *t, const char *name,
> + struct object_id *result, unsigned *mode,
> + int flags)
> {
> int namelen = strlen(name);
> while (t->size) {
> @@ -501,7 +503,11 @@ static int find_tree_entry(struct tree_desc *t, const char *name, struct object_
>
> oid = tree_entry_extract(t, &entry, mode);
> entrylen = tree_entry_len(&t->entry);
> - update_tree_entry(t);
> +
> + if (!(flags & GET_OID_GENTLY))
> + update_tree_entry(t);
> + else if (update_tree_entry_gently(t))
> + return -1;
> if (entrylen > namelen)
> continue;
> cmp = memcmp(name, entry, entrylen);
> @@ -521,19 +527,28 @@ static int find_tree_entry(struct tree_desc *t, const char *name, struct object_
> oidcpy(result, oid);
> return 0;
> }
> - return get_tree_entry(oid, name + entrylen, result, mode);
> + return get_tree_entry_gently(oid, name + entrylen, result, mode, flags);
> }
> return -1;
> }
>
> -int get_tree_entry(const struct object_id *tree_oid, const char *name, struct object_id *oid, unsigned *mode)
> +int get_tree_entry_gently(const struct object_id *tree_oid, const char *name,
> + struct object_id *oid, unsigned *mode, int flags)
> {
> int retval;
> void *tree;
> unsigned long size;
> struct object_id root;
>
> - tree = read_object_with_reference(tree_oid, tree_type, &size, &root);
> + if (!(flags & GET_OID_GENTLY)) {
> + tree = read_object_with_reference(tree_oid, tree_type, &size, &root);
> + } else {
> + struct object_info oi = OBJECT_INFO_INIT;
> +
> + oi.contentp = tree;
> + if (oid_object_info_extended(the_repository, tree_oid, &oi, 0) < 0)
> + return -1;
> + }
> if (!tree)
> return -1;
>
> @@ -547,13 +562,27 @@ int get_tree_entry(const struct object_id *tree_oid, const char *name, struct ob
> retval = -1;
> } else {
> struct tree_desc t;
> - init_tree_desc(&t, tree, size);
> - retval = find_tree_entry(&t, name, oid, mode);
> + if (!(flags & GET_OID_GENTLY)) {
> + init_tree_desc(&t, tree, size);
> + } else {
> + if (init_tree_desc_gently(&t, tree, size)) {
> + retval = -1;
> + goto done;
> + }
> + }
> + retval = find_tree_entry(&t, name, oid, mode, flags);
> }
> +done:
> free(tree);
> return retval;
> }
>
> +int get_tree_entry(const struct object_id *tree_oid, const char *name,
> + struct object_id *oid, unsigned *mode)
> +{
> + return get_tree_entry_gently(tree_oid, name, oid, mode, 0);
> +}
> +
> /*
> * This is Linux's built-in max for the number of symlinks to follow.
> * That limit, of course, does not affect git, but it's a reasonable
> @@ -576,7 +605,7 @@ int get_tree_entry(const struct object_id *tree_oid, const char *name, struct ob
> * See the code for enum follow_symlink_result for a description of
> * the return values.
> */
> -enum follow_symlinks_result get_tree_entry_follow_symlinks(struct object_id *tree_oid, const char *name, struct object_id *result, struct strbuf *result_path, unsigned *mode)
> +enum follow_symlinks_result get_tree_entry_follow_symlinks(struct object_id *tree_oid, const char *name, struct object_id *result, struct strbuf *result_path, unsigned *mode, int flags)
> {
> int retval = MISSING_OBJECT;
> struct dir_state *parents = NULL;
> @@ -600,9 +629,21 @@ enum follow_symlinks_result get_tree_entry_follow_symlinks(struct object_id *tre
> void *tree;
> struct object_id root;
> unsigned long size;
> - tree = read_object_with_reference(¤t_tree_oid,
> - tree_type, &size,
> - &root);
> + if (!(flags & GET_OID_GENTLY)) {
> + tree = read_object_with_reference(¤t_tree_oid,
> + tree_type, &size,
> + &root);
> + } else {
> + struct object_info oi = OBJECT_INFO_INIT;
> +
> + oi.contentp = tree;
> + if (oid_object_info_extended(the_repository,
> + ¤t_tree_oid, &oi, 0) < 0) {
> + retval = MISSING_OBJECT;
> + goto done;
> + }
> + }
> +
> if (!tree)
> goto done;
>
> @@ -622,7 +663,14 @@ enum follow_symlinks_result get_tree_entry_follow_symlinks(struct object_id *tre
> goto done;
>
> /* descend */
> - init_tree_desc(&t, tree, size);
> + if (!(flags & GET_OID_GENTLY)) {
> + init_tree_desc(&t, tree, size);
> + } else {
> + if (init_tree_desc_gently(&t, tree, size)) {
> + retval = MISSING_OBJECT;
> + goto done;
> + }
> + }
> }
>
> /* Handle symlinks to e.g. a//b by removing leading slashes */
> @@ -656,7 +704,15 @@ enum follow_symlinks_result get_tree_entry_follow_symlinks(struct object_id *tre
> free(parent->tree);
> parents_nr--;
> parent = &parents[parents_nr - 1];
> - init_tree_desc(&t, parent->tree, parent->size);
> + if (!(flags & GET_OID_GENTLY)) {
> + init_tree_desc(&t, parent->tree, parent->size);
> + } else {
> + if (init_tree_desc_gently(&t, parent->tree,
> + parent->size)) {
> + retval = MISSING_OBJECT;
> + goto done;
> + }
> + }
> strbuf_remove(&namebuf, 0, remainder ? 3 : 2);
> continue;
> }
> @@ -670,7 +726,7 @@ enum follow_symlinks_result get_tree_entry_follow_symlinks(struct object_id *tre
>
> /* Look up the first (or only) path component in the tree. */
> find_result = find_tree_entry(&t, namebuf.buf,
> - ¤t_tree_oid, mode);
> + ¤t_tree_oid, mode, flags);
> if (find_result) {
> goto done;
> }
> @@ -713,8 +769,19 @@ enum follow_symlinks_result get_tree_entry_follow_symlinks(struct object_id *tre
> */
> retval = DANGLING_SYMLINK;
>
> - contents = read_object_file(¤t_tree_oid, &type,
> - &link_len);
> + if (!(flags & GET_OID_GENTLY)) {
> + contents = read_object_file(¤t_tree_oid,
> + &type, &link_len);
> + } else {
> + struct object_info oi = OBJECT_INFO_INIT;
> + oi.contentp = (void*) contents;
> +
> + if (oid_object_info_extended(the_repository,
> + ¤t_tree_oid, &oi, 0) < 0) {
> + retval = MISSING_OBJECT;
> + goto done;
> + }
> + }
>
> if (!contents)
> goto done;
> @@ -735,7 +802,14 @@ enum follow_symlinks_result get_tree_entry_follow_symlinks(struct object_id *tre
> contents_start = contents;
>
> parent = &parents[parents_nr - 1];
> - init_tree_desc(&t, parent->tree, parent->size);
> + if (!(flags & GET_OID_GENTLY)) {
> + init_tree_desc(&t, parent->tree, parent->size);
> + } else {
> + if (init_tree_desc_gently(&t, parent->tree, parent->size)) {
> + retval = MISSING_OBJECT;
> + goto done;
> + }
> + }
> strbuf_splice(&namebuf, 0, len,
> contents_start, link_len);
> if (remainder)
> diff --git a/tree-walk.h b/tree-walk.h
> index 805f58f00..6f043af6e 100644
> --- a/tree-walk.h
> +++ b/tree-walk.h
> @@ -64,7 +64,7 @@ enum follow_symlinks_result {
> */
> };
>
> -enum follow_symlinks_result get_tree_entry_follow_symlinks(struct object_id *tree_oid, const char *name, struct object_id *result, struct strbuf *result_path, unsigned *mode);
> +enum follow_symlinks_result get_tree_entry_follow_symlinks(struct object_id *tree_oid, const char *name, struct object_id *result, struct strbuf *result_path, unsigned *mode, int flags);
>
> struct traverse_info {
> const char *traverse_path;
> @@ -79,6 +79,7 @@ struct traverse_info {
> int show_all_errors;
> };
>
> +int get_tree_entry_gently(const struct object_id *, const char *, struct object_id *, unsigned *, int);
> int get_tree_entry(const struct object_id *, const char *, struct object_id *, unsigned *);
> extern char *make_traverse_path(char *path, const struct traverse_info *info, const struct name_entry *n);
> extern void setup_traverse_info(struct traverse_info *info, const char *base);
