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

Re: [PATCH 7/7] submodule-config: cleanup backward compatibility helpers

From
BWBrandon Williams <bmwill@google.com>
Date
Jun 22, 2018, 17:09 UTC
Message-ID
<20180622170958.GD244185@google.com>
In-Reply-To
<20180622162656.19338-8-ao2@ao2.it>
On 06/22, Antonio Ospite wrote:
Show 7 quoted lines
> Use one callback per configuration setting to handle the generic options
> which have to be supported for backward compatibility.
> 
> This removes some duplication and some support code at the cost of
> parsing the .gitmodules file twice when calling the fetch command.
> 
> Signed-off-by: Antonio Ospite <ao2@ao2.it>

I'm not sure how I feel about this patch, I'm leaning towards not needing it but I like the idea of eliminating the duplicate code. One way you could get rid of having to read the .gitmodules file twice would be something like the following but I don't really think its needed:

diff --git a/submodule-config.c b/submodule-config.c
index ce204fb53..7cc1864b5 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -681,19 +681,24 @@ void submodule_free(struct repository *r)
 		submodule_cache_clear(r->submodule_cache);
 }
 
-struct fetch_config {
-	int *max_children;
+struct fetch_clone_config {
+	int *fetchjobs;
 	int *recurse_submodules;
 };
 
-static int gitmodules_fetch_config(const char *var, const char *value, void *cb)
+static int gitmodules_fetch_clone_config(const char *var, const char *value,
+					 void *cb)
 {
-	struct fetch_config *config = cb;
+	struct fetch_clone_config *config = cb;
 	if (!strcmp(var, "submodule.fetchjobs")) {
-		*(config->max_children) = parse_submodule_fetchjobs(var, value);
+		if (config->fetchjobs)
+			*(config->fetchjobs) =
+				parse_submodule_fetchjobs(var, value);
 		return 0;
 	} else if (!strcmp(var, "fetch.recursesubmodules")) {
-		*(config ->recurse_submodules) = parse_fetch_recurse_submodules_arg(var, value);
+		if (config->recurse_submodules)
+			*(config->recurse_submodules) =
+				parse_fetch_recurse_submodules_arg(var, value);
 		return 0;
 	}
 
@@ -702,23 +707,20 @@ static int gitmodules_fetch_config(const char *var, const char *value, void *cb)
 
 void fetch_config_from_gitmodules(int *max_children, int *recurse_submodules)
 {
-	struct fetch_config config = {
-		.max_children = max_children,
-		.recurse_submodules = recurse_submodules
+	struct fetch_clone_config config = {
+		.fetchjobs = max_children,
+		.recurse_submodules = recurse_submodules,
 	};
-	config_from_gitmodules(gitmodules_fetch_config, the_repository, &config);
-}
-
-static int gitmodules_update_clone_config(const char *var, const char *value,
-					  void *cb)
-{
-	int *max_jobs = cb;
-	if (!strcmp(var, "submodule.fetchjobs"))
-		*max_jobs = parse_submodule_fetchjobs(var, value);
-	return 0;
+	config_from_gitmodules(gitmodules_fetch_clone_config, the_repository,
+			       &config);
 }
 
 void update_clone_config_from_gitmodules(int *max_jobs)
 {
-	config_from_gitmodules(gitmodules_update_clone_config, the_repository, &max_jobs);
+	struct fetch_clone_config config = {
+		.fetchjobs = max_jobs,
+		.recurse_submodules = NULL,
+	};
+	config_from_gitmodules(gitmodules_fetch_clone_config, the_repository,
+			       &config);
 }
-- 
Brandon Williams
Previous: Antonio OspiteNext: Antonio Ospite
Message 7 of 15 in “Restrict the usage of config_from_gitmodules()”
  1. 0/7 Restrict the usage of config_from_gitmodules()Antonio Ospite, Jun 22, 2018
  2. 5/7 submodule-config: pass repository as argument to config_from_gitmodulesAntonio Ospite, Jun 22, 2018
  3. 2/7 submodule-config: add helper function to get 'fetch' config from .gitmodulesAntonio Ospite, Jun 22, 2018
  4. Brandon WilliamsJun 22, 2018
  5. 1/7 config: move config_from_gitmodules to submodule-config.cAntonio Ospite, Jun 22, 2018
  6. 7/7 submodule-config: cleanup backward compatibility helpersAntonio Ospite, Jun 22, 2018
  7. Brandon WilliamsJun 22, 2018
  8. 4/7 submodule-config: make 'config_from_gitmodules' privateAntonio Ospite, Jun 22, 2018
  9. 3/7 submodule-config: add helper to get 'update-clone' config from .gitmodulesAntonio Ospite, Jun 22, 2018
  10. Brandon WilliamsJun 22, 2018
  11. 6/7 submodule-config: reuse config_from_gitmodules in repo_read_gitmodulesAntonio Ospite, Jun 22, 2018
  12. Brandon WilliamsJun 22, 2018
  13. Brandon WilliamsJun 22, 2018
  14. Antonio OspiteJun 22, 2018
  15. Brandon WilliamsJun 22, 2018

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.