diff options
| author | Junio C Hamano <gitster@pobox.com> | 2025-07-07 14:12:55 -0700 |
|---|---|---|
| committer | Junio C Hamano <gitster@pobox.com> | 2025-07-07 14:12:56 -0700 |
| commit | 7310e539ada2b368e1c7243734ee7316639814f2 (patch) | |
| tree | ae96aaaa39d398cd5a3f63c593cc9826be78d25c | |
| parent | 8b6f19ccfc3aefbd0f22f6b7d56ad6a3fc5e4f37 (diff) | |
| parent | ca62f524c1eaef606b5c312de53ef7c4d9eefa4f (diff) | |
| download | git-7310e539ada2b368e1c7243734ee7316639814f2.tar.gz | |
Merge branch 'jk/submodule-remote-lookup-cleanup'
Updating submodules from the upstream did not work well when
submodule's HEAD is detached, which has been improved.
* jk/submodule-remote-lookup-cleanup:
submodule: look up remotes by URL first
submodule: move get_default_remote_submodule()
submodule--helper: improve logic for fallback remote name
remote: remove the_repository from some functions
dir: move starts_with_dot(_dot)_slash to dir.h
remote: fix tear down of struct remote
remote: remove branch->merge_name and fix branch_release()
| -rw-r--r-- | branch.c | 4 | ||||
| -rw-r--r-- | builtin/pull.c | 2 | ||||
| -rw-r--r-- | builtin/submodule--helper.c | 106 | ||||
| -rw-r--r-- | dir.h | 23 | ||||
| -rw-r--r-- | remote.c | 141 | ||||
| -rw-r--r-- | remote.h | 8 | ||||
| -rw-r--r-- | submodule-config.c | 12 | ||||
| -rwxr-xr-x | t/t7406-submodule-update.sh | 61 |
8 files changed, 226 insertions, 131 deletions
@@ -230,7 +230,7 @@ static int inherit_tracking(struct tracking *tracking, const char *orig_ref) return -1; } - if (branch->merge_nr < 1 || !branch->merge_name || !branch->merge_name[0]) { + if (branch->merge_nr < 1 || !branch->merge || !branch->merge[0] || !branch->merge[0]->src) { warning(_("asked to inherit tracking from '%s', but no merge configuration is set"), bare_ref); return -1; @@ -238,7 +238,7 @@ static int inherit_tracking(struct tracking *tracking, const char *orig_ref) tracking->remote = branch->remote_name; for (i = 0; i < branch->merge_nr; i++) - string_list_append(tracking->srcs, branch->merge_name[i]); + string_list_append(tracking->srcs, branch->merge[i]->src); return 0; } diff --git a/builtin/pull.c b/builtin/pull.c index 6e72a2e9a4..c593f324fe 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -490,7 +490,7 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs } else fprintf_ln(stderr, _("Your configuration specifies to merge with the ref '%s'\n" "from the remote, but no such ref was fetched."), - *curr_branch->merge_name); + curr_branch->merge[0]->src); exit(1); } diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index afef77a490..c725979201 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -41,61 +41,9 @@ typedef void (*each_submodule_fn)(const struct cache_entry *list_item, void *cb_data); -static int repo_get_default_remote(struct repository *repo, char **default_remote) -{ - char *dest = NULL; - struct strbuf sb = STRBUF_INIT; - struct ref_store *store = get_main_ref_store(repo); - const char *refname = refs_resolve_ref_unsafe(store, "HEAD", 0, NULL, - NULL); - - if (!refname) - return die_message(_("No such ref: %s"), "HEAD"); - - /* detached HEAD */ - if (!strcmp(refname, "HEAD")) { - *default_remote = xstrdup("origin"); - return 0; - } - - if (!skip_prefix(refname, "refs/heads/", &refname)) - return die_message(_("Expecting a full ref name, got %s"), - refname); - - strbuf_addf(&sb, "branch.%s.remote", refname); - if (repo_config_get_string(repo, sb.buf, &dest)) - *default_remote = xstrdup("origin"); - else - *default_remote = dest; - - strbuf_release(&sb); - return 0; -} - -static int get_default_remote_submodule(const char *module_path, char **default_remote) -{ - struct repository subrepo; - int ret; - - if (repo_submodule_init(&subrepo, the_repository, module_path, - null_oid(the_hash_algo)) < 0) - return die_message(_("could not get a repository handle for submodule '%s'"), - module_path); - ret = repo_get_default_remote(&subrepo, default_remote); - repo_clear(&subrepo); - - return ret; -} - static char *get_default_remote(void) { - char *default_remote; - int code = repo_get_default_remote(the_repository, &default_remote); - - if (code) - exit(code); - - return default_remote; + return xstrdup(repo_default_remote(the_repository)); } static char *resolve_relative_url(const char *rel_url, const char *up_path, int quiet) @@ -122,6 +70,46 @@ static char *resolve_relative_url(const char *rel_url, const char *up_path, int return resolved_url; } +static int get_default_remote_submodule(const char *module_path, char **default_remote) +{ + const struct submodule *sub; + struct repository subrepo; + const char *remote_name = NULL; + char *url = NULL; + + sub = submodule_from_path(the_repository, null_oid(the_hash_algo), module_path); + if (sub && sub->url) { + url = xstrdup(sub->url); + + /* Possibly a url relative to parent */ + if (starts_with_dot_dot_slash(url) || + starts_with_dot_slash(url)) { + char *oldurl = url; + + url = resolve_relative_url(oldurl, NULL, 1); + free(oldurl); + } + } + + if (repo_submodule_init(&subrepo, the_repository, module_path, + null_oid(the_hash_algo)) < 0) + return die_message(_("could not get a repository handle for submodule '%s'"), + module_path); + + /* Look up by URL first */ + if (url) + remote_name = repo_remote_from_url(&subrepo, url); + if (!remote_name) + remote_name = repo_default_remote(&subrepo); + + *default_remote = xstrdup(remote_name); + + repo_clear(&subrepo); + free(url); + + return 0; +} + /* the result should be freed by the caller. */ static char *get_submodule_displaypath(const char *path, const char *prefix, const char *super_prefix) @@ -438,18 +426,6 @@ cleanup: return ret; } -static int starts_with_dot_slash(const char *const path) -{ - return path_match_flags(path, PATH_MATCH_STARTS_WITH_DOT_SLASH | - PATH_MATCH_XPLATFORM); -} - -static int starts_with_dot_dot_slash(const char *const path) -{ - return path_match_flags(path, PATH_MATCH_STARTS_WITH_DOT_DOT_SLASH | - PATH_MATCH_XPLATFORM); -} - struct init_cb { const char *prefix; const char *super_prefix; @@ -676,4 +676,27 @@ static inline int starts_with_dot_dot_slash_native(const char *const path) return path_match_flags(path, what | PATH_MATCH_NATIVE); } +/** + * starts_with_dot_slash: convenience wrapper for + * patch_match_flags() with PATH_MATCH_STARTS_WITH_DOT_SLASH and + * PATH_MATCH_XPLATFORM. + */ +static inline int starts_with_dot_slash(const char *const path) +{ + const enum path_match_flags what = PATH_MATCH_STARTS_WITH_DOT_SLASH; + + return path_match_flags(path, what | PATH_MATCH_XPLATFORM); +} + +/** + * starts_with_dot_dot_slash: convenience wrapper for + * patch_match_flags() with PATH_MATCH_STARTS_WITH_DOT_DOT_SLASH and + * PATH_MATCH_XPLATFORM. + */ +static inline int starts_with_dot_dot_slash(const char *const path) +{ + const enum path_match_flags what = PATH_MATCH_STARTS_WITH_DOT_DOT_SLASH; + + return path_match_flags(path, what | PATH_MATCH_XPLATFORM); +} #endif @@ -165,6 +165,9 @@ static void remote_clear(struct remote *remote) strvec_clear(&remote->url); strvec_clear(&remote->pushurl); + refspec_clear(&remote->push); + refspec_clear(&remote->fetch); + free((char *)remote->receivepack); free((char *)remote->uploadpack); FREE_AND_NULL(remote->http_proxy); @@ -174,9 +177,15 @@ static void remote_clear(struct remote *remote) static void add_merge(struct branch *branch, const char *name) { - ALLOC_GROW(branch->merge_name, branch->merge_nr + 1, + struct refspec_item *merge; + + ALLOC_GROW(branch->merge, branch->merge_nr + 1, branch->merge_alloc); - branch->merge_name[branch->merge_nr++] = name; + + merge = xcalloc(1, sizeof(*merge)); + merge->src = xstrdup(name); + + branch->merge[branch->merge_nr++] = merge; } struct branches_hash_key { @@ -247,15 +256,23 @@ static struct branch *make_branch(struct remote_state *remote_state, return ret; } +static void merge_clear(struct branch *branch) +{ + for (int i = 0; i < branch->merge_nr; i++) { + refspec_item_clear(branch->merge[i]); + free(branch->merge[i]); + } + FREE_AND_NULL(branch->merge); + branch->merge_nr = 0; +} + static void branch_release(struct branch *branch) { free((char *)branch->name); free((char *)branch->refname); free(branch->remote_name); free(branch->pushremote_name); - for (int i = 0; i < branch->merge_nr; i++) - refspec_item_clear(branch->merge[i]); - free(branch->merge); + merge_clear(branch); } static struct rewrite *make_rewrite(struct rewrites *r, @@ -317,11 +334,10 @@ static void warn_about_deprecated_remote_type(const char *type, type, remote->name, remote->name, remote->name); } -static void read_remotes_file(struct remote_state *remote_state, - struct remote *remote) +static void read_remotes_file(struct repository *repo, struct remote *remote) { struct strbuf buf = STRBUF_INIT; - FILE *f = fopen_or_warn(repo_git_path_append(the_repository, &buf, + FILE *f = fopen_or_warn(repo_git_path_append(repo, &buf, "remotes/%s", remote->name), "r"); if (!f) @@ -337,7 +353,7 @@ static void read_remotes_file(struct remote_state *remote_state, strbuf_rtrim(&buf); if (skip_prefix(buf.buf, "URL:", &v)) - add_url_alias(remote_state, remote, + add_url_alias(repo->remote_state, remote, skip_spaces(v)); else if (skip_prefix(buf.buf, "Push:", &v)) refspec_append(&remote->push, skip_spaces(v)); @@ -350,12 +366,11 @@ out: strbuf_release(&buf); } -static void read_branches_file(struct remote_state *remote_state, - struct remote *remote) +static void read_branches_file(struct repository *repo, struct remote *remote) { char *frag, *to_free = NULL; struct strbuf buf = STRBUF_INIT; - FILE *f = fopen_or_warn(repo_git_path_append(the_repository, &buf, + FILE *f = fopen_or_warn(repo_git_path_append(repo, &buf, "branches/%s", remote->name), "r"); if (!f) @@ -382,9 +397,9 @@ static void read_branches_file(struct remote_state *remote_state, if (frag) *(frag++) = '\0'; else - frag = to_free = repo_default_branch_name(the_repository, 0); + frag = to_free = repo_default_branch_name(repo, 0); - add_url_alias(remote_state, remote, buf.buf); + add_url_alias(repo->remote_state, remote, buf.buf); refspec_appendf(&remote->fetch, "refs/heads/%s:refs/heads/%s", frag, remote->name); @@ -429,7 +444,7 @@ static int handle_config(const char *key, const char *value, } else if (!strcmp(subkey, "merge")) { if (!value) return config_error_nonbool(key); - add_merge(branch, xstrdup(value)); + add_merge(branch, value); } return 0; } @@ -681,7 +696,7 @@ const char *pushremote_for_branch(struct branch *branch, int *explicit) branch, explicit); } -static struct remote *remotes_remote_get(struct remote_state *remote_state, +static struct remote *remotes_remote_get(struct repository *repo, const char *name); char *remote_ref_for_branch(struct branch *branch, int for_push) @@ -692,7 +707,7 @@ char *remote_ref_for_branch(struct branch *branch, int for_push) if (branch) { if (!for_push) { if (branch->merge_nr) { - return xstrdup(branch->merge_name[0]); + return xstrdup(branch->merge[0]->src); } } else { char *dst; @@ -700,7 +715,7 @@ char *remote_ref_for_branch(struct branch *branch, int for_push) the_repository->remote_state, branch, NULL); struct remote *remote = remotes_remote_get( - the_repository->remote_state, remote_name); + the_repository, remote_name); if (remote && remote->push.nr && (dst = apply_refspecs(&remote->push, @@ -757,10 +772,11 @@ loop_cleanup: } static struct remote * -remotes_remote_get_1(struct remote_state *remote_state, const char *name, +remotes_remote_get_1(struct repository *repo, const char *name, const char *(*get_default)(struct remote_state *, struct branch *, int *)) { + struct remote_state *remote_state = repo->remote_state; struct remote *ret; int name_given = 0; @@ -774,9 +790,9 @@ remotes_remote_get_1(struct remote_state *remote_state, const char *name, #ifndef WITH_BREAKING_CHANGES if (valid_remote_nick(name) && have_git_dir()) { if (!valid_remote(ret)) - read_remotes_file(remote_state, ret); + read_remotes_file(repo, ret); if (!valid_remote(ret)) - read_branches_file(remote_state, ret); + read_branches_file(repo, ret); } #endif /* WITH_BREAKING_CHANGES */ if (name_given && !valid_remote(ret)) @@ -790,35 +806,33 @@ remotes_remote_get_1(struct remote_state *remote_state, const char *name, } static inline struct remote * -remotes_remote_get(struct remote_state *remote_state, const char *name) +remotes_remote_get(struct repository *repo, const char *name) { - return remotes_remote_get_1(remote_state, name, - remotes_remote_for_branch); + return remotes_remote_get_1(repo, name, remotes_remote_for_branch); } struct remote *remote_get(const char *name) { read_config(the_repository, 0); - return remotes_remote_get(the_repository->remote_state, name); + return remotes_remote_get(the_repository, name); } struct remote *remote_get_early(const char *name) { read_config(the_repository, 1); - return remotes_remote_get(the_repository->remote_state, name); + return remotes_remote_get(the_repository, name); } static inline struct remote * -remotes_pushremote_get(struct remote_state *remote_state, const char *name) +remotes_pushremote_get(struct repository *repo, const char *name) { - return remotes_remote_get_1(remote_state, name, - remotes_pushremote_for_branch); + return remotes_remote_get_1(repo, name, remotes_pushremote_for_branch); } struct remote *pushremote_get(const char *name) { read_config(the_repository, 0); - return remotes_pushremote_get(the_repository->remote_state, name); + return remotes_pushremote_get(the_repository, name); } int remote_is_configured(struct remote *remote, int in_repo) @@ -1722,7 +1736,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, } } -static void set_merge(struct remote_state *remote_state, struct branch *ret) +static void set_merge(struct repository *repo, struct branch *ret) { struct remote *remote; char *ref; @@ -1731,52 +1745,80 @@ static void set_merge(struct remote_state *remote_state, struct branch *ret) if (!ret) return; /* no branch */ - if (ret->merge) + if (ret->set_merge) return; /* already run */ if (!ret->remote_name || !ret->merge_nr) { /* * no merge config; let's make sure we don't confuse callers * with a non-zero merge_nr but a NULL merge */ - ret->merge_nr = 0; + merge_clear(ret); return; } + ret->set_merge = 1; - remote = remotes_remote_get(remote_state, ret->remote_name); + remote = remotes_remote_get(repo, ret->remote_name); - CALLOC_ARRAY(ret->merge, ret->merge_nr); for (i = 0; i < ret->merge_nr; i++) { - ret->merge[i] = xcalloc(1, sizeof(**ret->merge)); - ret->merge[i]->src = xstrdup(ret->merge_name[i]); if (!remote_find_tracking(remote, ret->merge[i]) || strcmp(ret->remote_name, ".")) continue; - if (repo_dwim_ref(the_repository, ret->merge_name[i], - strlen(ret->merge_name[i]), &oid, &ref, + if (repo_dwim_ref(repo, ret->merge[i]->src, + strlen(ret->merge[i]->src), &oid, &ref, 0) == 1) ret->merge[i]->dst = ref; else - ret->merge[i]->dst = xstrdup(ret->merge_name[i]); + ret->merge[i]->dst = xstrdup(ret->merge[i]->src); } } -struct branch *branch_get(const char *name) +static struct branch *repo_branch_get(struct repository *repo, const char *name) { struct branch *ret; - read_config(the_repository, 0); + read_config(repo, 0); if (!name || !*name || !strcmp(name, "HEAD")) - ret = the_repository->remote_state->current_branch; + ret = repo->remote_state->current_branch; else - ret = make_branch(the_repository->remote_state, name, + ret = make_branch(repo->remote_state, name, strlen(name)); - set_merge(the_repository->remote_state, ret); + set_merge(repo, ret); return ret; } +struct branch *branch_get(const char *name) +{ + return repo_branch_get(the_repository, name); +} + +const char *repo_default_remote(struct repository *repo) +{ + struct branch *branch; + + read_config(repo, 0); + branch = repo_branch_get(repo, "HEAD"); + + return remotes_remote_for_branch(repo->remote_state, branch, NULL); +} + +const char *repo_remote_from_url(struct repository *repo, const char *url) +{ + read_config(repo, 0); + + for (int i = 0; i < repo->remote_state->remotes_nr; i++) { + struct remote *remote = repo->remote_state->remotes[i]; + if (!remote) + continue; + + if (remote_has_url(remote, url)) + return remote->name; + } + return NULL; +} + int branch_has_merge_config(struct branch *branch) { - return branch && !!branch->merge; + return branch && branch->set_merge; } int branch_merge_matches(struct branch *branch, @@ -1841,13 +1883,14 @@ static const char *tracking_for_push_dest(struct remote *remote, return ret; } -static const char *branch_get_push_1(struct remote_state *remote_state, +static const char *branch_get_push_1(struct repository *repo, struct branch *branch, struct strbuf *err) { + struct remote_state *remote_state = repo->remote_state; struct remote *remote; remote = remotes_remote_get( - remote_state, + repo, remotes_pushremote_for_branch(remote_state, branch, NULL)); if (!remote) return error_buf(err, @@ -1914,7 +1957,7 @@ const char *branch_get_push(struct branch *branch, struct strbuf *err) if (!branch->push_tracking_ref) branch->push_tracking_ref = branch_get_push_1( - the_repository->remote_state, branch, err); + the_repository, branch, err); return branch->push_tracking_ref; } @@ -9,6 +9,7 @@ struct option; struct transport_ls_refs_options; +struct repository; /** * The API gives access to the configuration related to remotes. It handles @@ -315,8 +316,8 @@ struct branch { char *pushremote_name; - /* An array of the "merge" lines in the configuration. */ - const char **merge_name; + /* True if set_merge() has been called to finalize the merge array */ + int set_merge; /** * An array of the struct refspecs used for the merge lines. That is, @@ -338,6 +339,9 @@ const char *remote_for_branch(struct branch *branch, int *explicit); const char *pushremote_for_branch(struct branch *branch, int *explicit); char *remote_ref_for_branch(struct branch *branch, int for_push); +const char *repo_default_remote(struct repository *repo); +const char *repo_remote_from_url(struct repository *repo, const char *url); + /* returns true if the given branch has merge configuration given. */ int branch_has_merge_config(struct branch *branch); diff --git a/submodule-config.c b/submodule-config.c index 8630e27947..d64438b2a1 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -235,18 +235,6 @@ in_component: return 0; } -static int starts_with_dot_slash(const char *const path) -{ - return path_match_flags(path, PATH_MATCH_STARTS_WITH_DOT_SLASH | - PATH_MATCH_XPLATFORM); -} - -static int starts_with_dot_dot_slash(const char *const path) -{ - return path_match_flags(path, PATH_MATCH_STARTS_WITH_DOT_DOT_SLASH | - PATH_MATCH_XPLATFORM); -} - static int submodule_url_is_relative(const char *url) { return starts_with_dot_slash(url) || starts_with_dot_dot_slash(url); diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index ab76d4b6dc..3adab12091 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -1137,6 +1137,67 @@ test_expect_success 'setup clean recursive superproject' ' git clone --recurse-submodules top top-clean ' +test_expect_success 'submodule update with multiple remotes' ' + test_when_finished "rm -fr top-cloned" && + cp -r top-clean top-cloned && + + # Create a commit in each repo, starting with bottom + test_commit -C bottom multiple_remote_commit && + # Create middle commit + git -C middle/bottom fetch && + git -C middle/bottom checkout -f FETCH_HEAD && + git -C middle add bottom && + git -C middle commit -m "multiple_remote_commit" && + # Create top commit + git -C top/middle fetch && + git -C top/middle checkout -f FETCH_HEAD && + git -C top add middle && + git -C top commit -m "multiple_remote_commit" && + + # rename the submodule remote + git -C top-cloned/middle remote rename origin upstream && + + # Add another remote + git -C top-cloned/middle remote add other bogus && + + # Make the update of "middle" a no-op, otherwise we error out + # because of its unmerged state + test_config -C top-cloned submodule.middle.update !true && + git -C top-cloned submodule update --recursive 2>actual.err && + cat >expect.err <<-\EOF && + EOF + test_cmp expect.err actual.err +' + +test_expect_success 'submodule update with renamed remote' ' + test_when_finished "rm -fr top-cloned" && + cp -r top-clean top-cloned && + + # Create a commit in each repo, starting with bottom + test_commit -C bottom rename_commit && + # Create middle commit + git -C middle/bottom fetch && + git -C middle/bottom checkout -f FETCH_HEAD && + git -C middle add bottom && + git -C middle commit -m "rename_commit" && + # Create top commit + git -C top/middle fetch && + git -C top/middle checkout -f FETCH_HEAD && + git -C top add middle && + git -C top commit -m "rename_commit" && + + # rename the submodule remote + git -C top-cloned/middle remote rename origin upstream && + + # Make the update of "middle" a no-op, otherwise we error out + # because of its unmerged state + test_config -C top-cloned submodule.middle.update !true && + git -C top-cloned submodule update --recursive 2>actual.err && + cat >expect.err <<-\EOF && + EOF + test_cmp expect.err actual.err +' + test_expect_success 'submodule update should skip unmerged submodules' ' test_when_finished "rm -fr top-cloned" && cp -r top-clean top-cloned && |
