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

Re: [PATCH 06/10] repository: repo_submodule_init to take a submodule struct

From
JTJonathan Tan <jonathantanmy@google.com>
Date
Oct 26, 2018, 19:15 UTC
Message-ID
<20181026191553.108916-1-jonathantanmy@google.com>
In-Reply-To
<20181025233231.102245-7-sbeller@google.com>
Show 12 quoted lines
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 7da8fef31a..ba7634258a 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -418,7 +418,10 @@ static int grep_submodule(struct grep_opt *opt, struct repository *superproject,
>  			  const struct object_id *oid,
>  			  const char *filename, const char *path)
>  {
> -	struct repository submodule;
> +	struct repository subrepo;
> +	const struct submodule *sub = submodule_from_path(superproject,
> +							  &null_oid, path);
[snip]
> -	if (repo_submodule_init(&submodule, superproject, path)) {
> +	if (repo_submodule_init(&subrepo, superproject, sub)) {

The last argument to repo_submodule_init is now "submodule_from_path(superproject, &null_oid, path)" instead of "path", and looking forward into the patch, we do not need a NULL check because repo_submodule_init() tolerates NULL in that argument.

Let's see if the rest of the code follows the pattern - a call to submodule_from_path() with the 3 expected arguments (repo, null OID, path).

Show 15 quoted lines
> diff --git a/builtin/ls-files.c b/builtin/ls-files.c
> index 7f9919a362..4d1649c1b3 100644
> --- a/builtin/ls-files.c
> +++ b/builtin/ls-files.c
> @@ -206,17 +206,19 @@ static void show_files(struct repository *repo, struct dir_struct *dir);
>  static void show_submodule(struct repository *superproject,
>  			   struct dir_struct *dir, const char *path)
>  {
> -	struct repository submodule;
> +	struct repository subrepo;
> +	const struct submodule *sub = submodule_from_path(superproject,
> +							  &null_oid, path);
>  
> -	if (repo_submodule_init(&submodule, superproject, path))
> +	if (repo_submodule_init(&subrepo, superproject, sub))
So far so good.
Show 10 quoted lines
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 5f8a804a6e..015aa1471f 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -2037,7 +2037,7 @@ static int ensure_core_worktree(int argc, const char **argv, const char *prefix)
>  	if (!sub)
>  		BUG("We could get the submodule handle before?");
>  
> -	if (repo_submodule_init(&subrepo, the_repository, path))
> +	if (repo_submodule_init(&subrepo, the_repository, sub))

The definition of "sub" is not quoted here in this e-mail, but it is indeed "submodule_from_path(the_repository, &null_oid, path)". ("the_repository" in the invocation to submodule_from_path() is correct because the 2nd argument to the invocation of repo_submodule_init() is "the_repository".)

Show 12 quoted lines
> -int repo_submodule_init(struct repository *submodule,
> +int repo_submodule_init(struct repository *subrepo,
>  			struct repository *superproject,
> -			const char *path)
> +			const struct submodule *sub)
>  {
> -	const struct submodule *sub;
>  	struct strbuf gitdir = STRBUF_INIT;
>  	struct strbuf worktree = STRBUF_INIT;
>  	int ret = 0;
>  
> -	sub = submodule_from_path(superproject, &null_oid, path);
As expected, this line is removed.
Show 9 quoted lines
>  	if (!sub) {
>  		ret = -1;
>  		goto out;
>  	}
>  
> -	strbuf_repo_worktree_path(&gitdir, superproject, "%s/.git", path);
> -	strbuf_repo_worktree_path(&worktree, superproject, "%s", path);
> +	strbuf_repo_worktree_path(&gitdir, superproject, "%s/.git", sub->path);
> +	strbuf_repo_worktree_path(&worktree, superproject, "%s", sub->path);

path and sub->path are the same, so this is fine. (This can be seen from cache_put_path() and cache_lookup_path() in submodule-config.c.)

Show 8 quoted lines
> -	submodule->submodule_prefix = xstrfmt("%s%s/",
> -					      superproject->submodule_prefix ?
> -					      superproject->submodule_prefix :
> -					      "", path);
> +	subrepo->submodule_prefix = xstrfmt("%s%s/",
> +					    superproject->submodule_prefix ?
> +					    superproject->submodule_prefix :
> +					    "", sub->path);
Likewise.
Show 11 quoted lines
> +/*
> + * Initialize the repository 'subrepo' as the submodule given by the
> + * struct submodule 'sub' in parent repository 'superproject'.
> + * Return 0 upon success and a non-zero value upon failure, which may happen
> + * if the submodule is not found, or 'sub' is NULL.
> + */
> +struct submodule;
> +int repo_submodule_init(struct repository *subrepo,
>  			struct repository *superproject,
> -			const char *path);
> +			const struct submodule *sub);
Here is where it says that the last argument can be NULL.
Show 20 quoted lines
> diff --git a/t/helper/test-submodule-nested-repo-config.c b/t/helper/test-submodule-nested-repo-config.c
> index a31e2a9bea..bc97929bbc 100644
> --- a/t/helper/test-submodule-nested-repo-config.c
> +++ b/t/helper/test-submodule-nested-repo-config.c
> @@ -10,19 +10,21 @@ static void die_usage(int argc, const char **argv, const char *msg)
>  
>  int cmd__submodule_nested_repo_config(int argc, const char **argv)
>  {
> -	struct repository submodule;
> +	struct repository subrepo;
> +	const struct submodule *sub;
>  
>  	if (argc < 3)
>  		die_usage(argc, argv, "Wrong number of arguments.");
>  
>  	setup_git_directory();
>  
> -	if (repo_submodule_init(&submodule, the_repository, argv[1])) {
> +	sub = submodule_from_path(the_repository, &null_oid, argv[1]);
> +	if (repo_submodule_init(&subrepo, the_repository, sub)) {
The expected pattern.
This patch looks good to me.
Previous: Stefan BellerNext: Stefan Beller
Message 9 of 19 in “Resending sb/submodule-recursive-fetch-gets-the-tip”
  1. 00/10 Resending sb/submodule-recursive-fetch-gets-the-tipStefan Beller, Oct 25, 2018
  2. 01/10 sha1-array: provide oid_array_filterStefan Beller, Oct 25, 2018
  3. 02/10 submodule.c: fix indentationStefan Beller, Oct 25, 2018
  4. 03/10 submodule.c: sort changed_submodule_names before searching itStefan Beller, Oct 25, 2018
  5. 04/10 submodule.c: tighten scope of changed_submodule_names structStefan Beller, Oct 25, 2018
  6. 05/10 submodule: store OIDs in changed_submodule_namesStefan Beller, Oct 25, 2018
  7. Jonathan TanOct 26, 2018
  8. 06/10 repository: repo_submodule_init to take a submodule structStefan Beller, Oct 25, 2018
  9. Jonathan TanOct 26, 2018
  10. Stefan BellerOct 26, 2018
  11. 07/10 submodule: migrate get_next_submodule to use repository structsStefan Beller, Oct 25, 2018
  12. Jonathan TanOct 26, 2018
  13. 08/10 submodule.c: fetch in submodules git directory instead of in worktreeStefan Beller, Oct 25, 2018
  14. 09/10 fetch: try fetching submodules if needed objects were not fetchedStefan Beller, Oct 25, 2018
  15. Jonathan TanOct 26, 2018
  16. Stefan BellerNov 29, 2018
  17. 10/10 builtin/fetch: check for submodule updates in any ref updateStefan Beller, Oct 25, 2018
  18. Jonathan TanOct 26, 2018
  19. Junio C HamanoOct 29, 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.