Commit 9da9965b authored by Junio C Hamano's avatar Junio C Hamano

Merge branch 'hv/submodule-not-yet-pushed-fix' into maint

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:
  submodule_needs_pushing(): explain the behaviour when we cannot answer
  batch check whether submodule needs pushing into one call
  serialize collection of refs that contain submodule changes
  serialize collection of changed submodules
parents 0f47d3d7 250ab24a
......@@ -500,27 +500,67 @@ static int has_remote(const char *refname, const struct object_id *oid,
return 1;
static int submodule_needs_pushing(const char *path, const unsigned char sha1[20])
static int append_sha1_to_argv(const unsigned char sha1[20], void *data)
if (add_submodule_odb(path) || !lookup_commit_reference(sha1))
struct argv_array *argv = data;
argv_array_push(argv, sha1_to_hex(sha1));
return 0;
static int check_has_commit(const unsigned char sha1[20], void *data)
int *has_commit = data;
if (!lookup_commit_reference(sha1))
*has_commit = 0;
return 0;
static int submodule_has_commits(const char *path, struct sha1_array *commits)
int has_commit = 1;
if (add_submodule_odb(path))
return 0;
sha1_array_for_each_unique(commits, check_has_commit, &has_commit);
return has_commit;
static int submodule_needs_pushing(const char *path, struct sha1_array *commits)
if (!submodule_has_commits(path, commits))
* NOTE: We do consider it safe to return "no" here. The
* correct answer would be "We do not know" instead of
* "No push needed", but it is quite hard to change
* the submodule pointer without having the submodule
* around. If a user did however change the submodules
* without having the submodule around, this indicates
* an expert who knows what they are doing or a
* maintainer integrating work from other people. In
* both cases it should be safe to skip this check.
return 0;
if (for_each_remote_ref_submodule(path, has_remote, NULL) > 0) {
struct child_process cp = CHILD_PROCESS_INIT;
const char *argv[] = {"rev-list", NULL, "--not", "--remotes", "-n", "1" , NULL};
struct strbuf buf = STRBUF_INIT;
int needs_pushing = 0;
argv[1] = sha1_to_hex(sha1);
cp.argv = argv;
argv_array_push(&cp.args, "rev-list");
sha1_array_for_each_unique(commits, append_sha1_to_argv, &cp.args);
argv_array_pushl(&cp.args, "--not", "--remotes", "-n", "1" , NULL);
cp.git_cmd = 1;
cp.no_stdin = 1;
cp.out = -1;
cp.dir = path;
if (start_command(&cp))
die("Could not run 'git rev-list %s --not --remotes -n 1' command in submodule %s",
sha1_to_hex(sha1), path);
die("Could not run 'git rev-list <commits> --not --remotes -n 1' command in submodule %s",
if (strbuf_read(&buf, cp.out, 41))
needs_pushing = 1;
......@@ -532,19 +572,34 @@ static int submodule_needs_pushing(const char *path, const unsigned char sha1[20
return 0;
static struct sha1_array *submodule_commits(struct string_list *submodules,
const char *path)
struct string_list_item *item;
item = string_list_insert(submodules, path);
if (item->util)
return (struct sha1_array *) item->util;
/* NEEDSWORK: should we have sha1_array_init()? */
item->util = xcalloc(1, sizeof(struct sha1_array));
return (struct sha1_array *) item->util;
static void collect_submodules_from_diff(struct diff_queue_struct *q,
struct diff_options *options,
void *data)
int i;
struct string_list *needs_pushing = data;
struct string_list *submodules = data;
for (i = 0; i < q->nr; i++) {
struct diff_filepair *p = q->queue[i];
struct sha1_array *commits;
if (!S_ISGITLINK(p->two->mode))
if (submodule_needs_pushing(p->two->path, p->two->oid.hash))
string_list_insert(needs_pushing, p->two->path);
commits = submodule_commits(submodules, p->two->path);
sha1_array_append(commits, p->two->oid.hash);
......@@ -560,32 +615,48 @@ static void find_unpushed_submodule_commits(struct commit *commit,
diff_tree_combined_merge(commit, 1, &rev);
int find_unpushed_submodules(unsigned char new_sha1[20],
static void free_submodules_sha1s(struct string_list *submodules)
struct string_list_item *item;
for_each_string_list_item(item, submodules)
sha1_array_clear((struct sha1_array *) item->util);
string_list_clear(submodules, 1);
int find_unpushed_submodules(struct sha1_array *commits,
const char *remotes_name, struct string_list *needs_pushing)
struct rev_info rev;
struct commit *commit;
const char *argv[] = {NULL, NULL, "--not", "NULL", NULL};
int argc = ARRAY_SIZE(argv) - 1;
char *sha1_copy;
struct strbuf remotes_arg = STRBUF_INIT;
struct string_list submodules = STRING_LIST_INIT_DUP;
struct string_list_item *submodule;
struct argv_array argv = ARGV_ARRAY_INIT;
strbuf_addf(&remotes_arg, "--remotes=%s", remotes_name);
init_revisions(&rev, NULL);
sha1_copy = xstrdup(sha1_to_hex(new_sha1));
argv[1] = sha1_copy;
argv[3] = remotes_arg.buf;
setup_revisions(argc, argv, &rev, NULL);
/* argv.argv[0] will be ignored by setup_revisions */
argv_array_push(&argv, "find_unpushed_submodules");
sha1_array_for_each_unique(commits, append_sha1_to_argv, &argv);
argv_array_push(&argv, "--not");
argv_array_pushf(&argv, "--remotes=%s", remotes_name);
setup_revisions(argv.argc, argv.argv, &rev, NULL);
if (prepare_revision_walk(&rev))
die("revision walk setup failed");
while ((commit = get_revision(&rev)) != NULL)
find_unpushed_submodule_commits(commit, needs_pushing);
find_unpushed_submodule_commits(commit, &submodules);
for_each_string_list_item(submodule, &submodules) {
struct sha1_array *commits = (struct sha1_array *) submodule->util;
if (submodule_needs_pushing(submodule->string, commits))
string_list_insert(needs_pushing, submodule->string);
return needs_pushing->nr;
......@@ -612,12 +683,12 @@ static int push_submodule(const char *path)
return 1;
int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name)
int push_unpushed_submodules(struct sha1_array *commits, const char *remotes_name)
int i, ret = 1;
struct string_list needs_pushing = STRING_LIST_INIT_DUP;
if (!find_unpushed_submodules(new_sha1, remotes_name, &needs_pushing))
if (!find_unpushed_submodules(commits, remotes_name, &needs_pushing))
return 1;
for (i = 0; i <; i++) {
......@@ -3,6 +3,7 @@
struct diff_options;
struct argv_array;
struct sha1_array;
enum {
......@@ -62,9 +63,9 @@ int submodule_uses_gitfile(const char *path);
int ok_to_remove_submodule(const char *path);
int merge_submodule(unsigned char result[20], const char *path, const unsigned char base[20],
const unsigned char a[20], const unsigned char b[20], int search);
int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name,
int find_unpushed_submodules(struct sha1_array *commits, const char *remotes_name,
struct string_list *needs_pushing);
int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);
int push_unpushed_submodules(struct sha1_array *commits, const char *remotes_name);
void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
int parallel_submodules(void);
......@@ -949,23 +949,36 @@ int transport_push(struct transport *transport,
if ((flags & TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND) && !is_bare_repository()) {
struct ref *ref = remote_refs;
struct sha1_array commits = SHA1_ARRAY_INIT;
for (; ref; ref = ref->next)
if (!is_null_oid(&ref->new_oid) &&
die ("Failed to push all needed submodules!");
if (!is_null_oid(&ref->new_oid))
sha1_array_append(&commits, ref->new_oid.hash);
if (!push_unpushed_submodules(&commits, transport->remote->name)) {
die("Failed to push all needed submodules!");
TRANSPORT_RECURSE_SUBMODULES_CHECK)) && !is_bare_repository()) {
struct ref *ref = remote_refs;
struct string_list needs_pushing = STRING_LIST_INIT_DUP;
struct sha1_array commits = SHA1_ARRAY_INIT;
for (; ref; ref = ref->next)
if (!is_null_oid(&ref->new_oid) &&
transport->remote->name, &needs_pushing))
if (!is_null_oid(&ref->new_oid))
sha1_array_append(&commits, ref->new_oid.hash);
if (find_unpushed_submodules(&commits, transport->remote->name,
&needs_pushing)) {
string_list_clear(&needs_pushing, 0);
push_ret = transport->push_refs(transport, remote_refs, flags);
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment