[05/10] submodule: store OIDs in changed_submodule_names

Commit Message

Stefan Beller Oct. 25, 2018, 11:32 p.m. UTC

'calculate_changed_submodule_paths' uses a local list to compute the
changed submodules, and then produces the result by copying appropriate
items into the result list.

Instead use the result list directly and prune items afterwards
using string_list_remove_empty_items.

By doing so we'll have access to the util pointer for longer that
contains the commits that we need to fetch, which will be
useful in a later patch.

Signed-off-by: Stefan Beller <[e-mail]>
Reviewed-by: Jonathan Tan <[e-mail]>
---
 submodule.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

Comments

Jonathan Tan Oct. 26, 2018, 6:57 p.m. UTC | #1

> Reviewed-by: Jonathan Tan <[e-mail]>

Probably better not to include such lines, especially since the review
by me is not yet complete.

Having said that, patches 1-5 look good to me. Patches 1-3 are identical
to the previous version, which I have already reviewed. In patch 4,
Stefan made the code change I suggested [1].

In this patch, compared to the previous version which I have already
reviewed [2], the code is unchanged aside from some variable renaming. I
suggested a commit title change which Stefan has done.

[1] https://public-inbox.org/git/[e-mail]/
[2] https://public-inbox.org/git/[e-mail]/

Patch

diff --git a/submodule.c b/submodule.c
index 6fb0b9d783..e4b494af7b 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1127,8 +1127,7 @@  static void calculate_changed_submodule_paths(
 	struct string_list *changed_submodule_names)
 {
 	struct argv_array argv = ARGV_ARRAY_INIT;
-	struct string_list changed_submodules = STRING_LIST_INIT_DUP;
-	const struct string_list_item *name;
+	struct string_list_item *name;
 
 	/* No need to check if there are no submodules configured */
 	if (!submodule_from_path(the_repository, NULL, NULL))
@@ -1145,9 +1144,9 @@  static void calculate_changed_submodule_paths(
 	 * Collect all submodules (whether checked out or not) for which new
 	 * commits have been recorded upstream in "changed_submodule_names".
 	 */
-	collect_changed_submodules(&changed_submodules, &argv);
+	collect_changed_submodules(changed_submodule_names, &argv);
 
-	for_each_string_list_item(name, &changed_submodules) {
+	for_each_string_list_item(name, changed_submodule_names) {
 		struct oid_array *commits = name->util;
 		const struct submodule *submodule;
 		const char *path = NULL;
@@ -1161,12 +1160,14 @@  static void calculate_changed_submodule_paths(
 		if (!path)
 			continue;
 
-		if (!submodule_has_commits(path, commits))
-			string_list_append(changed_submodule_names,
-					   name->string);
+		if (submodule_has_commits(path, commits)) {
+			oid_array_clear(commits);
+			*name->string = '\0';
+		}
 	}
 
-	free_submodules_oids(&changed_submodules);
+	string_list_remove_empty_items(changed_submodule_names, 1);
+
 	argv_array_clear(&argv);
 	oid_array_clear(&ref_tips_before_fetch);
 	oid_array_clear(&ref_tips_after_fetch);
@@ -1376,7 +1377,7 @@  int fetch_populated_submodules(struct repository *r,
 
 	argv_array_clear(&spf.args);
 out:
-	string_list_clear(&spf.changed_submodule_names, 1);
+	free_submodules_oids(&spf.changed_submodule_names);
 	return spf.result;
 }