Skip to content

Commit 51b8c66

Browse files
gitstergitster
committed
Merge branch 'hv/submodule-not-yet-pushed-fix' into pu
The code in "git push" to compute if any commit being pushed in the superproject binds a commit in a submodule that hasn't been pushed out was overly inefficient, making it unusable even for a small project that does not have any submodule but have a reasonable number of refs. * hv/submodule-not-yet-pushed-fix: batch check whether submodule needs pushing into one call serialize collection of refs that contain submodule changes serialize collection of changed submodules
2 parents 92d1502 + a1a385d commit 51b8c66

File tree

3 files changed

+114
-36
lines changed

3 files changed

+114
-36
lines changed

‎submodule.c

Lines changed: 90 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -500,27 +500,56 @@ static int has_remote(const char *refname, const struct object_id *oid,
500500
return 1;
501501
}
502502

503-
static int submodule_needs_pushing(const char *path, const unsigned char sha1[20])
503+
static int append_hash_to_argv(const unsigned char sha1[20], void *data)
504504
{
505-
if (add_submodule_odb(path) || !lookup_commit_reference(sha1))
505+
struct argv_array *argv = (struct argv_array *) data;
506+
argv_array_push(argv, sha1_to_hex(sha1));
507+
return 0;
508+
}
509+
510+
static int check_has_hash(const unsigned char sha1[20], void *data)
511+
{
512+
int *has_hash = (int *) data;
513+
514+
if (!lookup_commit_reference(sha1))
515+
*has_hash = 0;
516+
517+
return 0;
518+
}
519+
520+
static int submodule_has_hashes(const char *path, struct sha1_array *hashes)
521+
{
522+
int has_hash = 1;
523+
524+
if (add_submodule_odb(path))
525+
return 0;
526+
527+
sha1_array_for_each_unique(hashes, check_has_hash, &has_hash);
528+
return has_hash;
529+
}
530+
531+
static int submodule_needs_pushing(const char *path, struct sha1_array *hashes)
532+
{
533+
if (!submodule_has_hashes(path, hashes))
506534
return 0;
507535

508536
if (for_each_remote_ref_submodule(path, has_remote, NULL) > 0) {
509537
struct child_process cp = CHILD_PROCESS_INIT;
510-
const char *argv[] = {"rev-list", NULL, "--not", "--remotes", "-n", "1" , NULL};
511538
struct strbuf buf = STRBUF_INIT;
512539
int needs_pushing = 0;
513540

514-
argv[1] = sha1_to_hex(sha1);
515-
cp.argv = argv;
541+
argv_array_push(&cp.args, "rev-list");
542+
sha1_array_for_each_unique(hashes, append_hash_to_argv, &cp.args);
543+
argv_array_pushl(&cp.args, "--not", "--remotes", "-n", "1" , NULL);
544+
516545
prepare_submodule_repo_env(&cp.env_array);
517546
cp.git_cmd = 1;
518547
cp.no_stdin = 1;
519548
cp.out = -1;
520549
cp.dir = path;
521550
if (start_command(&cp))
522-
die("Could not run 'git rev-list %s --not --remotes -n 1' command in submodule %s",
523-
sha1_to_hex(sha1), path);
551+
die("Could not run 'git rev-list <hashes> --not --remotes -n 1' command in submodule %s",
552+
path);
524553
if (strbuf_read(&buf, cp.out, 41))
525554
needs_pushing = 1;
526555
finish_command(&cp);
@@ -532,19 +561,34 @@ static int submodule_needs_pushing(const char *path, const unsigned char sha1[20
532561
return 0;
533562
}
534563

564+
static struct sha1_array *get_sha1s_from_list(struct string_list *submodules,
565+
const char *path)
566+
{
567+
struct string_list_item *item;
568+
569+
item = string_list_insert(submodules, path);
570+
if (item->util)
571+
return (struct sha1_array *) item->util;
572+
573+
/* NEEDSWORK: should we have sha1_array_init()? */
574+
item->util = xcalloc(1, sizeof(struct sha1_array));
575+
return (struct sha1_array *) item->util;
576+
}
577+
535578
static void collect_submodules_from_diff(struct diff_queue_struct *q,
536579
struct diff_options *options,
537580
void *data)
538581
{
539582
int i;
540-
struct string_list *needs_pushing = data;
583+
struct string_list *submodules = data;
541584

542585
for (i = 0; i < q->nr; i++) {
543586
struct diff_filepair *p = q->queue[i];
587+
struct sha1_array *hashes;
544588
if (!S_ISGITLINK(p->two->mode))
545589
continue;
546-
if (submodule_needs_pushing(p->two->path, p->two->oid.hash))
547-
string_list_insert(needs_pushing, p->two->path);
590+
hashes = get_sha1s_from_list(submodules, p->two->path);
591+
sha1_array_append(hashes, p->two->oid.hash);
548592
}
549593
}
550594

@@ -560,32 +604,52 @@ static void find_unpushed_submodule_commits(struct commit *commit,
560604
diff_tree_combined_merge(commit, 1, &rev);
561605
}
562606

563-
int find_unpushed_submodules(unsigned char new_sha1[20],
607+
static void free_submodules_sha1s(struct string_list *submodules)
608+
{
609+
int i;
610+
for (i = 0; i < submodules->nr; i++) {
611+
struct string_list_item *item = &submodules->items[i];
612+
struct sha1_array *hashes = (struct sha1_array *) item->util;
613+
sha1_array_clear(hashes);
614+
}
615+
string_list_clear(submodules, 1);
616+
}
617+
618+
int find_unpushed_submodules(struct sha1_array *hashes,
564619
const char *remotes_name, struct string_list *needs_pushing)
565620
{
566621
struct rev_info rev;
567622
struct commit *commit;
568-
const char *argv[] = {NULL, NULL, "--not", "NULL", NULL};
569-
int argc = ARRAY_SIZE(argv) - 1;
570-
char *sha1_copy;
571-
572-
struct strbuf remotes_arg = STRBUF_INIT;
623+
int i;
624+
struct string_list submodules = STRING_LIST_INIT_DUP;
625+
struct argv_array argv = ARGV_ARRAY_INIT;
573626

574-
strbuf_addf(&remotes_arg, "--remotes=%s", remotes_name);
575627
init_revisions(&rev, NULL);
576-
sha1_copy = xstrdup(sha1_to_hex(new_sha1));
577-
argv[1] = sha1_copy;
578-
argv[3] = remotes_arg.buf;
579-
setup_revisions(argc, argv, &rev, NULL);
628+
629+
/* argv.argv[0] will be ignored by setup_revisions */
630+
argv_array_push(&argv, "find_unpushed_submodules");
631+
sha1_array_for_each_unique(hashes, append_hash_to_argv, &argv);
632+
argv_array_push(&argv, "--not");
633+
argv_array_pushf(&argv, "--remotes=%s", remotes_name);
634+
635+
setup_revisions(argv.argc, argv.argv, &rev, NULL);
580636
if (prepare_revision_walk(&rev))
581637
die("revision walk setup failed");
582638

583639
while ((commit = get_revision(&rev)) != NULL)
584-
find_unpushed_submodule_commits(commit, needs_pushing);
640+
find_unpushed_submodule_commits(commit, &submodules);
585641

586642
reset_revision_walk();
587-
free(sha1_copy);
588-
strbuf_release(&remotes_arg);
643+
argv_array_clear(&argv);
644+
645+
for (i = 0; i < submodules.nr; i++) {
646+
struct string_list_item *submodule = &submodules.items[i];
647+
struct sha1_array *hashes = (struct sha1_array *) submodule->util;
648+
649+
if (submodule_needs_pushing(submodule->string, hashes))
650+
string_list_insert(needs_pushing, submodule->string);
651+
}
652+
free_submodules_sha1s(&submodules);
589653

590654
return needs_pushing->nr;
591655
}
@@ -612,12 +676,12 @@ static int push_submodule(const char *path)
612676
return 1;
613677
}
614678

615-
int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name)
679+
int push_unpushed_submodules(struct sha1_array *hashes, const char *remotes_name)
616680
{
617681
int i, ret = 1;
618682
struct string_list needs_pushing = STRING_LIST_INIT_DUP;
619683

620-
if (!find_unpushed_submodules(new_sha1, remotes_name, &needs_pushing))
684+
if (!find_unpushed_submodules(hashes, remotes_name, &needs_pushing))
621685
return 1;
622686

623687
for (i = 0; i < needs_pushing.nr; i++) {

‎submodule.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
struct diff_options;
55
struct argv_array;
6+
struct sha1_array;
67

78
enum {
89
RECURSE_SUBMODULES_CHECK = -4,
@@ -62,9 +63,9 @@ int submodule_uses_gitfile(const char *path);
6263
int ok_to_remove_submodule(const char *path);
6364
int merge_submodule(unsigned char result[20], const char *path, const unsigned char base[20],
6465
const unsigned char a[20], const unsigned char b[20], int search);
65-
int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name,
66+
int find_unpushed_submodules(struct sha1_array *hashes, const char *remotes_name,
6667
struct string_list *needs_pushing);
67-
int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);
68+
int push_unpushed_submodules(struct sha1_array *hashes, const char *remotes_name);
6869
void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
6970
int parallel_submodules(void);
7071

‎transport.c

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -917,23 +917,36 @@ int transport_push(struct transport *transport,
917917

918918
if ((flags & TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND) && !is_bare_repository()) {
919919
struct ref *ref = remote_refs;
920+
struct sha1_array hashes = SHA1_ARRAY_INIT;
921+
920922
for (; ref; ref = ref->next)
921-
if (!is_null_oid(&ref->new_oid) &&
922-
!push_unpushed_submodules(ref->new_oid.hash,
923-
transport->remote->name))
924-
die ("Failed to push all needed submodules!");
923+
if (!is_null_oid(&ref->new_oid))
924+
sha1_array_append(&hashes, ref->new_oid.hash);
925+
926+
if (!push_unpushed_submodules(&hashes, transport->remote->name)) {
927+
sha1_array_clear(&hashes);
928+
die ("Failed to push all needed submodules!");
929+
}
930+
sha1_array_clear(&hashes);
925931
}
926932

927933
if ((flags & (TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND |
928934
TRANSPORT_RECURSE_SUBMODULES_CHECK)) && !is_bare_repository()) {
929935
struct ref *ref = remote_refs;
930936
struct string_list needs_pushing = STRING_LIST_INIT_DUP;
937+
struct sha1_array hashes = SHA1_ARRAY_INIT;
931938

932939
for (; ref; ref = ref->next)
933-
if (!is_null_oid(&ref->new_oid) &&
934-
find_unpushed_submodules(ref->new_oid.hash,
935-
transport->remote->name, &needs_pushing))
936-
die_with_unpushed_submodules(&needs_pushing);
940+
if (!is_null_oid(&ref->new_oid))
941+
sha1_array_append(&hashes, ref->new_oid.hash);
942+
943+
if (find_unpushed_submodules(&hashes, transport->remote->name,
944+
&needs_pushing)) {
945+
sha1_array_clear(&hashes);
946+
die_with_unpushed_submodules(&needs_pushing);
947+
}
948+
string_list_clear(&needs_pushing, 0);
949+
sha1_array_clear(&hashes);
937950
}
938951

939952
push_ret = transport->push_refs(transport, remote_refs, flags);

0 commit comments

Comments
 (0)