git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 5/7] submodule: return target of submodule symref

From
Glen Choo via GitGitGadget <gitgitgadget@gmail.com>
Date
Oct 20, 2022, 20:20 UTC
Message-ID
<6f769cb80ad0bd7df98b8c7d207922f8631133f2.1666297239.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1321.v2.git.git.1666297238.gitgitgadget@gmail.com>
From: Glen Choo <chooglen@google.com>

resolve_gitlink_ref() can tell us which oid the submodule ref is pointing to, but in a future commit, we would also like to know the symbolic ref target if we are checking a symbolic ref. Teach resolve_gitlink_ref() to "return" the symbolic ref's target via an "out" parameter.

This changes resolve_gitlink_ref()'s signature so that new callers trying to use the old signature will be stopped by the compiler. If we returned the target instead (just like refs_resolve_ref_unsafe()), we would be more consistent with refs_resolve_ref_unsafe(), but callers expecting the old signature will get the opposite return value from what they expect (since exit code 0 means success, but NULL pointer means failure). We should do this refactor once we think that nobody will try to use the old signature.

Signed-off-by: Glen Choo <chooglen@google.com>
---
 builtin/submodule--helper.c |  8 +++++---
 builtin/update-index.c      |  4 ++--
 combine-diff.c              |  3 ++-
 diff-lib.c                  |  2 +-
 dir.c                       |  2 +-
 object-file.c               |  2 +-
 read-cache.c                |  4 ++--
 refs.c                      | 10 ++++++----
 refs.h                      |  5 ++++-
 unpack-trees.c              |  3 ++-
 10 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 1ce3458a29c..138b133790f 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2531,7 +2531,8 @@ static int update_submodule(struct update_data *update_data)
 
 	if (update_data->just_cloned)
 		oidcpy(&update_data->suboid, null_oid());
-	else if (resolve_gitlink_ref(update_data->sm_path, "HEAD", &update_data->suboid))
+	else if (resolve_gitlink_ref(update_data->sm_path, "HEAD",
+				     &update_data->suboid, NULL))
 		return die_message(_("Unable to find current revision in submodule path '%s'"),
 				   update_data->displaypath);
 
@@ -2558,7 +2559,8 @@ static int update_submodule(struct update_data *update_data)
 						   update_data->sm_path);
 		}
 
-		if (resolve_gitlink_ref(update_data->sm_path, remote_ref, &update_data->oid))
+		if (resolve_gitlink_ref(update_data->sm_path, remote_ref,
+					&update_data->oid, NULL))
 			return die_message(_("Unable to find %s revision in submodule path '%s'"),
 					   remote_ref, update_data->sm_path);
 
@@ -3303,7 +3305,7 @@ static void die_on_repo_without_commits(const char *path)
 	strbuf_addstr(&sb, path);
 	if (is_nonbare_repository_dir(&sb)) {
 		struct object_id oid;
-		if (resolve_gitlink_ref(path, "HEAD", &oid) < 0)
+		if (resolve_gitlink_ref(path, "HEAD", &oid, NULL) < 0)
 			die(_("'%s' does not have a commit checked out"), path);
 	}
 	strbuf_release(&sb);
diff --git a/builtin/update-index.c b/builtin/update-index.c
index b62249905f1..19a21a4586c 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -339,7 +339,7 @@ static int process_directory(const char *path, int len, struct stat *st)
 		if (S_ISGITLINK(ce->ce_mode)) {
 
 			/* Do nothing to the index if there is no HEAD! */
-			if (resolve_gitlink_ref(path, "HEAD", &oid) < 0)
+			if (resolve_gitlink_ref(path, "HEAD", &oid, NULL) < 0)
 				return 0;
 
 			return add_one_path(ce, path, len, st);
@@ -365,7 +365,7 @@ static int process_directory(const char *path, int len, struct stat *st)
 	}
 
 	/* No match - should we add it as a gitlink? */
-	if (!resolve_gitlink_ref(path, "HEAD", &oid))
+	if (!resolve_gitlink_ref(path, "HEAD", &oid, NULL))
 		return add_one_path(NULL, path, len, st);
 
 	/* Error out. */
diff --git a/combine-diff.c b/combine-diff.c
index b0ece954808..88efcaeefa7 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1060,7 +1060,8 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 			elem->mode = canon_mode(st.st_mode);
 		} else if (S_ISDIR(st.st_mode)) {
 			struct object_id oid;
-			if (resolve_gitlink_ref(elem->path, "HEAD", &oid) < 0)
+			if (resolve_gitlink_ref(elem->path, "HEAD", &oid,
+						NULL) < 0)
 				result = grab_blob(opt->repo, &elem->oid,
 						   elem->mode, &result_size,
 						   NULL, NULL);
diff --git a/diff-lib.c b/diff-lib.c
index 2edea41a234..ac94b6234ca 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -53,7 +53,7 @@ static int check_removed(const struct index_state *istate, const struct cache_en
 		 * a directory --- the blob was removed!
 		 */
 		if (!S_ISGITLINK(ce->ce_mode) &&
-		    resolve_gitlink_ref(ce->name, "HEAD", &sub))
+		    resolve_gitlink_ref(ce->name, "HEAD", &sub, NULL))
 			return 1;
 	}
 	return 0;
diff --git a/dir.c b/dir.c
index d604d1bab98..81d232424c1 100644
--- a/dir.c
+++ b/dir.c
@@ -3251,7 +3251,7 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)
 	struct object_id submodule_head;
 
 	if ((flag & REMOVE_DIR_KEEP_NESTED_GIT) &&
-	    !resolve_gitlink_ref(path->buf, "HEAD", &submodule_head)) {
+	    !resolve_gitlink_ref(path->buf, "HEAD", &submodule_head, NULL)) {
 		/* Do not descend and nuke a nested git work tree. */
 		if (kept_up)
 			*kept_up = 1;
diff --git a/object-file.c b/object-file.c
index 5e309602346..a3c6580c16e 100644
--- a/object-file.c
+++ b/object-file.c
@@ -2522,7 +2522,7 @@ int index_path(struct index_state *istate, struct object_id *oid,
 		strbuf_release(&sb);
 		break;
 	case S_IFDIR:
-		return resolve_gitlink_ref(path, "HEAD", oid);
+		return resolve_gitlink_ref(path, "HEAD", oid, NULL);
 	default:
 		return error(_("%s: unsupported file type"), path);
 	}
diff --git a/read-cache.c b/read-cache.c
index 32024029274..4c1bf33ef48 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -285,7 +285,7 @@ static int ce_compare_gitlink(const struct cache_entry *ce)
 	 *
 	 * If so, we consider it always to match.
 	 */
-	if (resolve_gitlink_ref(ce->name, "HEAD", &oid) < 0)
+	if (resolve_gitlink_ref(ce->name, "HEAD", &oid, NULL) < 0)
 		return 0;
 	return !oideq(&oid, &ce->oid);
 }
@@ -781,7 +781,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 
 	namelen = strlen(path);
 	if (S_ISDIR(st_mode)) {
-		if (resolve_gitlink_ref(path, "HEAD", &oid) < 0)
+		if (resolve_gitlink_ref(path, "HEAD", &oid, NULL) < 0)
 			return error(_("'%s' does not have a commit checked out"), path);
 		while (namelen && path[namelen-1] == '/')
 			namelen--;
diff --git a/refs.c b/refs.c
index 1491ae937eb..a32a25ccb69 100644
--- a/refs.c
+++ b/refs.c
@@ -1904,19 +1904,21 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
 }
 
 int resolve_gitlink_ref(const char *submodule, const char *refname,
-			struct object_id *oid)
+			struct object_id *oid, const char **target_out)
 {
 	struct ref_store *refs;
 	int flags;
+	const char *target;
 
 	refs = get_submodule_ref_store(submodule);
 
 	if (!refs)
 		return -1;
-
-	if (!refs_resolve_ref_unsafe(refs, refname, 0, oid, &flags) ||
-	    is_null_oid(oid))
+	target = refs_resolve_ref_unsafe(refs, refname, 0, oid, &flags);
+	if (!target || is_null_oid(oid))
 		return -1;
+	if (target_out)
+		*target_out = target;
 	return 0;
 }
 
diff --git a/refs.h b/refs.h
index 8958717a17d..d5c32fac000 100644
--- a/refs.h
+++ b/refs.h
@@ -137,9 +137,12 @@ int peel_iterated_oid(const struct object_id *base, struct object_id *peeled);
  * submodule (which must be non-NULL). If the resolution is
  * successful, return 0 and set oid to the name of the object;
  * otherwise, return a non-zero value.
+ *
+ * FIXME: Return "target" just like refs_resolve_ref_unsafe(). This will be
+ * safe to do once we merge resolve_gitlink_ref() into master.
  */
 int resolve_gitlink_ref(const char *submodule, const char *refname,
-			struct object_id *oid);
+			struct object_id *oid, const char **target);
 
 /*
  * Return true iff abbrev_name is a possible abbreviation for
diff --git a/unpack-trees.c b/unpack-trees.c
index bae812156c4..db12bfcaffd 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -2288,7 +2288,8 @@ static int verify_clean_subdirectory(const struct cache_entry *ce,
 
 	if (S_ISGITLINK(ce->ce_mode)) {
 		struct object_id oid;
-		int sub_head = resolve_gitlink_ref(ce->name, "HEAD", &oid);
+		int sub_head =
+			resolve_gitlink_ref(ce->name, "HEAD", &oid, NULL);
 		/*
 		 * If we are not going to update the submodule, then
 		 * we don't care.

-- 
gitgitgadget
Previous: Glen Choo via GitGitGadgetNext: Glen Choo via GitGitGadget
Message 23 of 56 in “clone, submodule update: check out submodule branches”
  1. 0/6 clone, submodule update: check out submodule branchesGlen Choo via GitGitGadget, Aug 29, 2022
  2. 1/6 clone: teach --detach optionGlen Choo via GitGitGadget, Aug 29, 2022
  3. Philippe BlainAug 30, 2022
  4. 2/6 repo-settings: add submodule_propagate_branchesGlen Choo via GitGitGadget, Aug 29, 2022
  5. Philippe BlainAug 30, 2022
  6. 3/6 t5617: drop references to remote-tracking branchesGlen Choo via GitGitGadget, Aug 29, 2022
  7. Philippe BlainAug 30, 2022
  8. 4/6 submodule: return target of submodule symrefGlen Choo via GitGitGadget, Aug 29, 2022
  9. Jonathan TanSep 1, 2022
  10. Glen ChooSep 1, 2022
  11. 6/6 clone, submodule update: check out branchesGlen Choo via GitGitGadget, Aug 29, 2022
  12. Philippe BlainAug 30, 2022
  13. Glen ChooAug 30, 2022
  14. Jonathan TanSep 1, 2022
  15. 5/6 submodule--helper: refactor up-to-date criterionGlen Choo via GitGitGadget, Aug 29, 2022
  16. 0/7 clone, submodule update: check out submodule branchesGlen Choo via GitGitGadget, Oct 20, 2022
  17. 1/7 clone: teach --detach optionGlen Choo via GitGitGadget, Oct 20, 2022
  18. 2/7 repo-settings: add submodule_propagate_branchesGlen Choo via GitGitGadget, Oct 20, 2022
  19. Jonathan TanOct 25, 2022
  20. 3/7 submodule--helper clone: create named branchGlen Choo via GitGitGadget, Oct 20, 2022
  21. Jonathan TanOct 25, 2022
  22. 4/7 t5617: drop references to remote-tracking branchesGlen Choo via GitGitGadget, Oct 20, 2022
  23. 5/7 submodule: return target of submodule symrefGlen Choo via GitGitGadget, Oct 20, 2022
  24. 6/7 submodule update: refactor update targetsGlen Choo via GitGitGadget, Oct 20, 2022
  25. 7/7 clone, submodule update: create and check out branchesGlen Choo via GitGitGadget, Oct 20, 2022
  26. Jonathan TanOct 25, 2022
  27. Glen ChooOct 25, 2022
  28. Junio C HamanoOct 20, 2022
  29. Glen ChooOct 20, 2022
  30. Junio C HamanoOct 21, 2022
  31. 0/8 clone, submodule update: check out submodule branchesGlen Choo via GitGitGadget, Oct 28, 2022
  32. 1/8 clone: teach --detach optionGlen Choo via GitGitGadget, Oct 28, 2022
  33. Junio C HamanoOct 28, 2022
  34. Junio C HamanoOct 28, 2022
  35. Glen ChooOct 28, 2022
  36. Taylor BlauOct 30, 2022
  37. Glen ChooOct 31, 2022
  38. Philippe BlainNov 8, 2022
  39. 2/8 repo-settings: add submodule_propagate_branchesGlen Choo via GitGitGadget, Oct 28, 2022
  40. 3/8 submodule--helper clone: create named branchGlen Choo via GitGitGadget, Oct 28, 2022
  41. 5/8 submodule: return target of submodule symrefGlen Choo via GitGitGadget, Oct 28, 2022
  42. Junio C HamanoOct 28, 2022
  43. Glen ChooOct 28, 2022
  44. 4/8 t5617: drop references to remote-tracking branchesGlen Choo via GitGitGadget, Oct 28, 2022
  45. 8/8 clone, submodule update: create and check out branchesGlen Choo via GitGitGadget, Oct 28, 2022
  46. Philippe BlainNov 8, 2022
  47. Jonathan TanNov 15, 2022
  48. Glen ChooNov 22, 2022
  49. Jonathan TanNov 23, 2022
  50. Junio C HamanoNov 23, 2022
  51. 6/8 submodule update: refactor update targetsGlen Choo via GitGitGadget, Oct 28, 2022
  52. 7/8 submodule--helper: remove update_data.suboidGlen Choo via GitGitGadget, Oct 28, 2022
  53. Jonathan TanNov 14, 2022
  54. Taylor BlauOct 30, 2022
  55. Philippe BlainNov 8, 2022
  56. Glen ChooNov 8, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.