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

[PATCH 1/3] submodule-config: passing name reference for .gitmodule blobs

From
Heiko Voigt <hvoigt@hvoigt.net>
Date
Jul 28, 2016, 12:49 UTC
Message-ID
<20160728124910.GA2530@sandbox>
In-Reply-To
<xmqqmvl4xdlm.fsf@gitster.mtv.corp.google.com>

Commit 959b5455 (submodule: implement a config API for lookup of .gitmodules values, 2015-08-18) implemented the initial version of the submodule config cache. During development of that initial version we extracted the function gitmodule_sha1_from_commit(). During that process we missed that the strbuf rev was still used in config_from() and now is left empty. Lets fix this by also returning this string.

This means that now when reading .gitmodules from revisions, the error messages also contain a reference to the blob they are from.

Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
Here you go. Now including a test.
 submodule-config.c          | 26 +++++++++++++++++---------
 t/t7411-submodule-config.sh | 11 +++++++++++
 2 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/submodule-config.c b/submodule-config.c
index 1210b26..853989e 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -349,9 +349,9 @@ static int parse_config(const char *var, const char *value, void *data)
 }
 
 static int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
-				      unsigned char *gitmodules_sha1)
+				      unsigned char *gitmodules_sha1,
+				      struct strbuf *rev)
 {
-	struct strbuf rev = STRBUF_INIT;
 	int ret = 0;
 
 	if (is_null_sha1(commit_sha1)) {
@@ -359,11 +359,10 @@ static int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
 		return 1;
 	}
 
-	strbuf_addf(&rev, "%s:.gitmodules", sha1_to_hex(commit_sha1));
-	if (get_sha1(rev.buf, gitmodules_sha1) >= 0)
+	strbuf_addf(rev, "%s:.gitmodules", sha1_to_hex(commit_sha1));
+	if (get_sha1(rev->buf, gitmodules_sha1) >= 0)
 		ret = 1;
 
-	strbuf_release(&rev);
 	return ret;
 }
 
@@ -375,6 +374,7 @@ static const struct submodule *config_from(struct submodule_cache *cache,
 		const unsigned char *commit_sha1, const char *key,
 		enum lookup_type lookup_type)
 {
+	struct strbuf rev = STRBUF_INIT;
 	unsigned long config_size;
 	char *config;
 	unsigned char sha1[20];
@@ -397,8 +397,10 @@ static const struct submodule *config_from(struct submodule_cache *cache,
 		return entry->config;
 	}
 
-	if (!gitmodule_sha1_from_commit(commit_sha1, sha1))
+	if (!gitmodule_sha1_from_commit(commit_sha1, sha1, &rev)) {
+		strbuf_release(&rev);
 		return NULL;
+	}
 
 	switch (lookup_type) {
 	case lookup_name:
@@ -408,14 +410,19 @@ static const struct submodule *config_from(struct submodule_cache *cache,
 		submodule = cache_lookup_path(cache, sha1, key);
 		break;
 	}
-	if (submodule)
+	if (submodule) {
+		strbuf_release(&rev);
 		return submodule;
+	}
 
 	config = read_sha1_file(sha1, &type, &config_size);
-	if (!config)
+	if (!config) {
+		strbuf_release(&rev);
 		return NULL;
+	}
 
 	if (type != OBJ_BLOB) {
+		strbuf_release(&rev);
 		free(config);
 		return NULL;
 	}
@@ -425,8 +432,9 @@ static const struct submodule *config_from(struct submodule_cache *cache,
 	parameter.commit_sha1 = commit_sha1;
 	parameter.gitmodules_sha1 = sha1;
 	parameter.overwrite = 0;
-	git_config_from_mem(parse_config, "submodule-blob", "",
+	git_config_from_mem(parse_config, "submodule-blob", rev.buf,
 			config, config_size, &parameter);
+	strbuf_release(&rev);
 	free(config);
 
 	switch (lookup_type) {
diff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh
index fc97c33..400e2b1 100755
--- a/t/t7411-submodule-config.sh
+++ b/t/t7411-submodule-config.sh
@@ -82,6 +82,17 @@ test_expect_success 'error in one submodule config lets continue' '
 	)
 '
 
+test_expect_success 'error message contains blob reference' '
+	(cd super &&
+		sha1=$(git rev-parse HEAD) &&
+		test-submodule-config \
+			HEAD b \
+			HEAD submodule \
+				2>actual_err &&
+		grep "submodule-blob $sha1:.gitmodules" actual_err >/dev/null
+	)
+'
+
 cat >super/expect_url <<EOF
 Submodule url: 'git@somewhere.else.net:a.git' for path 'b'
 Submodule url: 'git@somewhere.else.net:submodule.git' for path 'submodule'
-- 
2.0.2.832.g083c931
Previous: Junio C HamanoNext: Stefan Beller
Message 13 of 23 in “submodule-config: use explicit empty string instead of strbuf in config_from()”
  1. submodule-config: use explicit empty string instead of strbuf in config_from()René Scharfe, Jul 19, 2016
  2. Junio C HamanoJul 19, 2016
  3. Stefan BellerJul 19, 2016
  4. Heiko VoigtJul 20, 2016
  5. René ScharfeJul 21, 2016
  6. Heiko VoigtJul 25, 2016
  7. Junio C HamanoJul 25, 2016
  8. 2/2 submodule-config: combine error checking if clausesHeiko Voigt, Jul 26, 2016
  9. Stefan BellerJul 26, 2016
  10. 1/2 fix passing a name for config from submodulesHeiko Voigt, Jul 26, 2016
  11. Stefan BellerJul 26, 2016
  12. Junio C HamanoJul 26, 2016
  13. 1/3 submodule-config: passing name reference for .gitmodule blobsHeiko Voigt, Jul 28, 2016
  14. Stefan BellerJul 28, 2016
  15. 2/3 submodule-config: combine early return code into one gotoHeiko Voigt, Jul 28, 2016
  16. 3/3 submodule-config: fix test binary crashing when no arguments givenHeiko Voigt, Jul 28, 2016
  17. Heiko VoigtJul 28, 2016
  18. document how to reference previous commitsHeiko Voigt, Jul 28, 2016
  19. Junio C HamanoJul 28, 2016
  20. Stefan BellerJul 28, 2016
  21. Heiko VoigtAug 17, 2016
  22. Junio C HamanoAug 17, 2016
  23. Junio C HamanoJul 28, 2016

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.