git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] use repo_get_oid_with_flags()
@ 2025-09-10 17:16 René Scharfe
  2025-09-15  6:16 ` Patrick Steinhardt
  0 siblings, 1 reply; 2+ messages in thread
From: René Scharfe @ 2025-09-10 17:16 UTC (permalink / raw)
  To: Git List; +Cc: Patrick Steinhardt

get_oid_with_context() allows specifying flags and reports object
details via a passed-in struct object_context.  Some callers just want
to specify flags, but don't need any details back.  Convert them to
repo_get_oid_with_flags(), which provides just that and frees them from
dealing with the context structure.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 builtin/ls-tree.c     |  7 ++-----
 builtin/rev-parse.c   |  7 ++-----
 builtin/stash.c       | 14 +++++---------
 list-objects-filter.c |  9 +++------
 object-name.c         | 30 +++++-------------------------
 5 files changed, 17 insertions(+), 50 deletions(-)

diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 5d55731ca3..ec6940fc7c 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -373,7 +373,6 @@ int cmd_ls_tree(int argc,
 		OPT_END()
 	};
 	struct ls_tree_cmdmode_to_fmt *m2f = ls_tree_cmdmode_format;
-	struct object_context obj_context = {0};
 	int ret;
 
 	repo_config(the_repository, git_default_config, NULL);
@@ -405,9 +404,8 @@ int cmd_ls_tree(int argc,
 			ls_tree_usage, ls_tree_options);
 	if (argc < 1)
 		usage_with_options(ls_tree_usage, ls_tree_options);
-	if (get_oid_with_context(the_repository, argv[0],
-				 GET_OID_HASH_ANY, &oid,
-				 &obj_context))
+	if (repo_get_oid_with_flags(the_repository, argv[0], &oid,
+				    GET_OID_HASH_ANY))
 		die("Not a valid object name %s", argv[0]);
 
 	/*
@@ -447,6 +445,5 @@ int cmd_ls_tree(int argc,
 
 	ret = !!read_tree(the_repository, tree, &options.pathspec, fn, &options);
 	clear_pathspec(&options.pathspec);
-	object_context_release(&obj_context);
 	return ret;
 }
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 44ff1b8342..9da92b990d 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -708,7 +708,6 @@ int cmd_rev_parse(int argc,
 	struct object_id oid;
 	unsigned int flags = 0;
 	const char *name = NULL;
-	struct object_context unused;
 	struct strbuf buf = STRBUF_INIT;
 	int seen_end_of_options = 0;
 	enum format_type format = FORMAT_DEFAULT;
@@ -1141,9 +1140,8 @@ int cmd_rev_parse(int argc,
 			name++;
 			type = REVERSED;
 		}
-		if (!get_oid_with_context(the_repository, name,
-					  flags, &oid, &unused)) {
-			object_context_release(&unused);
+		if (!repo_get_oid_with_flags(the_repository, name, &oid,
+					     flags)) {
 			if (output_algo)
 				repo_oid_to_algop(the_repository, &oid,
 						  output_algo, &oid);
@@ -1153,7 +1151,6 @@ int cmd_rev_parse(int argc,
 				show_rev(type, &oid, name);
 			continue;
 		}
-		object_context_release(&unused);
 		if (verify)
 			die_no_single_rev(quiet);
 		if (has_dashdash)
diff --git a/builtin/stash.c b/builtin/stash.c
index f5ddee5c7f..87acefb82f 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -1089,7 +1089,6 @@ static int store_stash(int argc, const char **argv, const char *prefix,
 	int quiet = 0;
 	const char *stash_msg = NULL;
 	struct object_id obj;
-	struct object_context dummy = {0};
 	struct option options[] = {
 		OPT__QUIET(&quiet, N_("be quiet")),
 		OPT_STRING('m', "message", &stash_msg, "message",
@@ -1109,9 +1108,8 @@ static int store_stash(int argc, const char **argv, const char *prefix,
 		return -1;
 	}
 
-	if (get_oid_with_context(the_repository,
-				 argv[0], quiet ? GET_OID_QUIETLY : 0, &obj,
-				 &dummy)) {
+	if (repo_get_oid_with_flags(the_repository, argv[0], &obj,
+				    quiet ? GET_OID_QUIETLY : 0)) {
 		if (!quiet)
 			fprintf_ln(stderr, _("Cannot update %s with %s"),
 					     ref_stash, argv[0]);
@@ -1122,7 +1120,6 @@ static int store_stash(int argc, const char **argv, const char *prefix,
 	ret = do_store_stash(&obj, stash_msg, quiet);
 
 out:
-	object_context_release(&dummy);
 	return ret;
 }
 
@@ -2235,7 +2232,6 @@ static int do_export_stash(struct repository *r,
 			   const char **argv)
 {
 	struct object_id base;
-	struct object_context unused;
 	struct commit *prev;
 	struct commit_list *items = NULL, **iter = &items, *cur;
 	int res = 0;
@@ -2269,9 +2265,9 @@ static int do_export_stash(struct repository *r,
 			struct commit *stash;
 
 			if (parse_stash_revision(&revision, argv[i], 1) ||
-			    get_oid_with_context(r, revision.buf,
-						 GET_OID_QUIETLY | GET_OID_GENTLY,
-						 &oid, &unused)) {
+			    repo_get_oid_with_flags(r, revision.buf, &oid,
+						    GET_OID_QUIETLY |
+						    GET_OID_GENTLY)) {
 				res = error(_("unable to find stash entry %s"), argv[i]);
 				goto out;
 			}
diff --git a/list-objects-filter.c b/list-objects-filter.c
index 7ecd4d9ef5..acd65ebb73 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -524,12 +524,11 @@ static void filter_sparse_oid__init(
 	struct filter *filter)
 {
 	struct filter_sparse_data *d = xcalloc(1, sizeof(*d));
-	struct object_context oc;
 	struct object_id sparse_oid;
 
-	if (get_oid_with_context(the_repository,
-				 filter_options->sparse_oid_name,
-				 GET_OID_BLOB, &sparse_oid, &oc))
+	if (repo_get_oid_with_flags(the_repository,
+				    filter_options->sparse_oid_name,
+				    &sparse_oid, GET_OID_BLOB))
 		die(_("unable to access sparse blob in '%s'"),
 		    filter_options->sparse_oid_name);
 	if (add_patterns_from_blob_to_list(&sparse_oid, "", 0, &d->pl) < 0)
@@ -544,8 +543,6 @@ static void filter_sparse_oid__init(
 	filter->filter_data = d;
 	filter->filter_object_fn = filter_sparse;
 	filter->free_fn = filter_sparse_free;
-
-	object_context_release(&oc);
 }
 
 /*
diff --git a/object-name.c b/object-name.c
index 732056ff5e..52d87348d1 100644
--- a/object-name.c
+++ b/object-name.c
@@ -1858,55 +1858,35 @@ int repo_get_oid_committish(struct repository *r,
 			    const char *name,
 			    struct object_id *oid)
 {
-	struct object_context unused;
-	int ret = get_oid_with_context(r, name, GET_OID_COMMITTISH,
-				       oid, &unused);
-	object_context_release(&unused);
-	return ret;
+	return repo_get_oid_with_flags(r, name, oid, GET_OID_COMMITTISH);
 }
 
 int repo_get_oid_treeish(struct repository *r,
 			 const char *name,
 			 struct object_id *oid)
 {
-	struct object_context unused;
-	int ret = get_oid_with_context(r, name, GET_OID_TREEISH,
-				       oid, &unused);
-	object_context_release(&unused);
-	return ret;
+	return repo_get_oid_with_flags(r, name, oid, GET_OID_TREEISH);
 }
 
 int repo_get_oid_commit(struct repository *r,
 			const char *name,
 			struct object_id *oid)
 {
-	struct object_context unused;
-	int ret = get_oid_with_context(r, name, GET_OID_COMMIT,
-				       oid, &unused);
-	object_context_release(&unused);
-	return ret;
+	return repo_get_oid_with_flags(r, name, oid, GET_OID_COMMIT);
 }
 
 int repo_get_oid_tree(struct repository *r,
 		      const char *name,
 		      struct object_id *oid)
 {
-	struct object_context unused;
-	int ret = get_oid_with_context(r, name, GET_OID_TREE,
-				       oid, &unused);
-	object_context_release(&unused);
-	return ret;
+	return repo_get_oid_with_flags(r, name, oid, GET_OID_TREE);
 }
 
 int repo_get_oid_blob(struct repository *r,
 		      const char *name,
 		      struct object_id *oid)
 {
-	struct object_context unused;
-	int ret = get_oid_with_context(r, name, GET_OID_BLOB,
-				       oid, &unused);
-	object_context_release(&unused);
-	return ret;
+	return repo_get_oid_with_flags(r, name, oid, GET_OID_BLOB);
 }
 
 /* Must be called only when object_name:filename doesn't exist. */
-- 
2.51.0

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] use repo_get_oid_with_flags()
  2025-09-10 17:16 [PATCH] use repo_get_oid_with_flags() René Scharfe
@ 2025-09-15  6:16 ` Patrick Steinhardt
  0 siblings, 0 replies; 2+ messages in thread
From: Patrick Steinhardt @ 2025-09-15  6:16 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git List

On Wed, Sep 10, 2025 at 07:16:30PM +0200, René Scharfe wrote:

Nit: it would probably make sense to add something like a `treewide: `
prefix to the patch subject.

> get_oid_with_context() allows specifying flags and reports object
> details via a passed-in struct object_context.  Some callers just want
> to specify flags, but don't need any details back.  Convert them to
> repo_get_oid_with_flags(), which provides just that and frees them from
> dealing with the context structure.

Makes sense. `repo_get_oid_with_flags()` is a mere wrapper around
`get_oid_with_context()` with an object context. So these two would be
equivalent to one another.

One thing that is a bit weird though is that `repo_get_oid_with_flags()`
returns an `int` even though it directly returns the result from
`get_oid_with_context()`, which returns an `enum get_oid_result`. That
shouldn't impact correctness though, and it's not a new problem.

> diff --git a/object-name.c b/object-name.c
> index 732056ff5e..52d87348d1 100644
> --- a/object-name.c
> +++ b/object-name.c
> @@ -1858,55 +1858,35 @@ int repo_get_oid_committish(struct repository *r,
>  			    const char *name,
>  			    struct object_id *oid)
>  {
> -	struct object_context unused;
> -	int ret = get_oid_with_context(r, name, GET_OID_COMMITTISH,
> -				       oid, &unused);
> -	object_context_release(&unused);
> -	return ret;
> +	return repo_get_oid_with_flags(r, name, oid, GET_OID_COMMITTISH);
>  }

The same is true for all these functions. It would feel more sensible if
those all returned `enum get_oid_result`. But as said, that's not a new
problem and doesn't need fixing in your patch.

Other than that all the conversions look correct to me, thanks!

Patrick

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-09-15  6:16 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-10 17:16 [PATCH] use repo_get_oid_with_flags() René Scharfe
2025-09-15  6:16 ` Patrick Steinhardt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).