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

Re: [PATCH v2 5/7] fetch/pull: Don't recurse into a submodule when commits are already present

From
Marc Branchaud <marcnarc@xiplink.com>
Date
Mar 2, 2011, 16:08 UTC
Message-ID
<4D6E6B71.2030800@xiplink.com>
In-Reply-To
<4D6D7B49.3030005@web.de>

As I said earlier, this patch should also tweak the documentation in fetch-options.txt and config.txt.

In my suggested wording I used the phrase
	... only recurse into a populated submodule when its superproject
	retrieves a commit that updates the submodule's reference.
With this patch, that should now be
	... only recurse into a populated submodule when its superproject
	retrieves a commit that updates the submodule's reference to a
	commit that isn't already in the local submodule clone.
		M.
On 11-03-01 06:03 PM, Jens Lehmann wrote:
Show 88 quoted lines
> When looking for submodules where new commits have been recorded in the
> superproject ignore those cases where the submodules commits are already
> present locally. This can happen e.g. when the submodule has been rewound
> to an earlier state. Then there is no need to fetch the submodule again
> as the commit recorded in the newly fetched superproject commit has
> already been fetched earlier into the submodule.
> 
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> ---
>  submodule.c                 |   29 ++++++++++++++++++++++++++++-
>  t/t5526-fetch-submodules.sh |   19 +++++++++++++++++++
>  2 files changed, 47 insertions(+), 1 deletions(-)
> 
> diff --git a/submodule.c b/submodule.c
> index 3be6654..11e0ef5 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -263,6 +263,33 @@ void set_config_fetch_recurse_submodules(int value)
>  	config_fetch_recurse_submodules = value;
>  }
> 
> +static int is_submodule_commit_present(const char *path, unsigned char sha1[20])
> +{
> +	int is_present = 0;
> +	if (!add_submodule_odb(path) && lookup_commit_reference(sha1)) {
> +		/* Even if the submodule is checked out and the commit is
> +		 * present, make sure it is reachable from a ref. */
> +		struct child_process cp;
> +		const char *argv[] = {"rev-list", "-n", "1", NULL, "--not", "--all", NULL};
> +		struct strbuf buf = STRBUF_INIT;
> +
> +		argv[3] = sha1_to_hex(sha1);
> +		memset(&cp, 0, sizeof(cp));
> +		cp.argv = argv;
> +		cp.env = local_repo_env;
> +		cp.git_cmd = 1;
> +		cp.no_stdin = 1;
> +		cp.out = -1;
> +		cp.dir = path;
> +		if (!run_command(&cp) && !strbuf_read(&buf, cp.out, 1024))
> +			is_present = 1;
> +
> +		close(cp.out);
> +		strbuf_release(&buf);
> +	}
> +	return is_present;
> +}
> +
>  static void submodule_collect_changed_cb(struct diff_queue_struct *q,
>  					 struct diff_options *options,
>  					 void *data)
> @@ -280,7 +307,7 @@ static void submodule_collect_changed_cb(struct diff_queue_struct *q,
>  			 * being moved around. */
>  			struct string_list_item *path;
>  			path = unsorted_string_list_lookup(&changed_submodule_paths, p->two->path);
> -			if (!path)
> +			if (!path && !is_submodule_commit_present(p->two->path, p->two->sha1))
>  				string_list_append(&changed_submodule_paths, xstrdup(p->two->path));
>  		} else {
>  			/* Submodule is new or was moved here */
> diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
> index 09701aa..3decfae 100755
> --- a/t/t5526-fetch-submodules.sh
> +++ b/t/t5526-fetch-submodules.sh
> @@ -428,4 +428,23 @@ test_expect_success "'submodule.<sub>.fetchRecurseSubmodules=on-demand' override
>  	test_cmp expect.err.2 actual.err
>  '
> 
> +test_expect_success "don't fetch submodule when newly recorded commits are already present" '
> +	(
> +		cd submodule &&
> +		git checkout -q HEAD^^
> +	) &&
> +	head1=$(git rev-parse --short HEAD) &&
> +	git add submodule &&
> +	git commit -m "submodule rewound" &&
> +	head2=$(git rev-parse --short HEAD) &&
> +	echo "From $pwd/." > expect.err &&
> +	echo "   $head1..$head2  master     -> origin/master" >> expect.err &&
> +	(
> +		cd downstream &&
> +		git fetch >../actual.out 2>../actual.err
> +	) &&
> +	! test -s actual.out &&
> +	test_cmp expect.err actual.err
> +'
> +
>  test_done
Previous: Jens LehmannNext: Jens Lehmann
Message 11 of 16 in “Teach fetch/pull the on-demand mode and make it the default”
  1. 0/7 Teach fetch/pull the on-demand mode and make it the defaultJens Lehmann, Mar 1, 2011
  2. 1/7 fetch/pull: recurse into submodules when necessaryJens Lehmann, Mar 1, 2011
  3. Marc BranchaudMar 2, 2011
  4. 2/7 fetch/pull: Add the 'on-demand' value to the --recurse-submodules optionJens Lehmann, Mar 1, 2011
  5. Marc BranchaudMar 2, 2011
  6. Marc BranchaudMar 2, 2011
  7. 3/7 config: teach the fetch.recurseSubmodules option the 'on-demand' valueJens Lehmann, Mar 1, 2011
  8. Marc BranchaudMar 2, 2011
  9. 4/7 Submodules: Add 'on-demand' value for the 'fetchRecurseSubmodule' optionJens Lehmann, Mar 1, 2011
  10. 5/7 fetch/pull: Don't recurse into a submodule when commits are already presentJens Lehmann, Mar 1, 2011
  11. Marc BranchaudMar 2, 2011
  12. 6/7 submodule update: Don't fetch when the submodule commit is already presentJens Lehmann, Mar 1, 2011
  13. 7/7 fetch/pull: Describe --recurse-submodule restrictions in the BUGS sectionJens Lehmann, Mar 1, 2011
  14. Jens LehmannMar 2, 2011
  15. Marc BranchaudMar 2, 2011
  16. Jens LehmannMar 2, 2011

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.