From e89f151db13684924feb0cd0a0ca3a13c1d71516 Mon Sep 17 00:00:00 2001 From: Glen Choo Date: Fri, 28 Jan 2022 16:04:41 -0800 Subject: branch: move --set-upstream-to behavior to dwim_and_setup_tracking() This commit is preparation for a future commit that will simplify create_branch() so that it always creates a branch. This will allow create_branch() to accept a dry_run parameter (which is needed for "git branch --recurse-submodules"). create_branch() used to always create a branch, but 4fc5006676 (Add branch --set-upstream, 2010-01-18) changed it to also be able to set tracking information without creating a branch. Refactor the code that sets tracking information into its own functions dwim_branch_start() and dwim_and_setup_tracking(). Also change an invocation of create_branch() in cmd_branch() in builtin/branch.c to use dwim_and_setup_tracking(), since that invocation is only for setting tracking information (in "git branch --set-upstream-to"). As of this commit, create_branch() is no longer invoked in a way that does not create branches. Helped-by: Jonathan Tan Signed-off-by: Glen Choo Reviewed-by: Jonathan Tan Signed-off-by: Junio C Hamano --- branch.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 67 insertions(+), 20 deletions(-) (limited to 'branch.c') diff --git a/branch.c b/branch.c index a4e4631ef1..f3a31930fb 100644 --- a/branch.c +++ b/branch.c @@ -218,9 +218,11 @@ static int inherit_tracking(struct tracking *tracking, const char *orig_ref) } /* - * This is called when new_ref is branched off of orig_ref, and tries - * to infer the settings for branch..{remote,merge} from the - * config. + * Used internally to set the branch..{remote,merge} config + * settings so that branch 'new_ref' tracks 'orig_ref'. Unlike + * dwim_and_setup_tracking(), this does not do DWIM, i.e. "origin/main" + * will not be expanded to "refs/remotes/origin/main", so it is not safe + * for 'orig_ref' to be raw user input. */ static void setup_tracking(const char *new_ref, const char *orig_ref, enum branch_track track, int quiet) @@ -341,31 +343,37 @@ N_("\n" "will track its remote counterpart, you may want to use\n" "\"git push -u\" to set the upstream config as you push."); -void create_branch(struct repository *r, - const char *name, const char *start_name, - int force, int clobber_head_ok, int reflog, - int quiet, enum branch_track track) +/** + * DWIMs a user-provided ref to determine the starting point for a + * branch and validates it, where: + * + * - r is the repository to validate the branch for + * + * - start_name is the ref that we would like to test. This is + * expanded with DWIM and assigned to out_real_ref. + * + * - track is the tracking mode of the new branch. If tracking is + * explicitly requested, start_name must be a branch (because + * otherwise start_name cannot be tracked) + * + * - out_oid is an out parameter containing the object_id of start_name + * + * - out_real_ref is an out parameter containing the full, 'real' form + * of start_name e.g. refs/heads/main instead of main + * + */ +static void dwim_branch_start(struct repository *r, const char *start_name, + enum branch_track track, char **out_real_ref, + struct object_id *out_oid) { struct commit *commit; struct object_id oid; char *real_ref; - struct strbuf ref = STRBUF_INIT; - int forcing = 0; - int dont_change_ref = 0; int explicit_tracking = 0; if (track == BRANCH_TRACK_EXPLICIT || track == BRANCH_TRACK_OVERRIDE) explicit_tracking = 1; - if ((track == BRANCH_TRACK_OVERRIDE || clobber_head_ok) - ? validate_branchname(name, &ref) - : validate_new_branchname(name, &ref, force)) { - if (!force) - dont_change_ref = 1; - else - forcing = 1; - } - real_ref = NULL; if (get_oid_mb(start_name, &oid)) { if (explicit_tracking) { @@ -402,7 +410,37 @@ void create_branch(struct repository *r, if ((commit = lookup_commit_reference(r, &oid)) == NULL) die(_("Not a valid branch point: '%s'."), start_name); - oidcpy(&oid, &commit->object.oid); + if (out_real_ref) { + *out_real_ref = real_ref; + real_ref = NULL; + } + if (out_oid) + oidcpy(out_oid, &commit->object.oid); + + FREE_AND_NULL(real_ref); +} + +void create_branch(struct repository *r, + const char *name, const char *start_name, + int force, int clobber_head_ok, int reflog, + int quiet, enum branch_track track) +{ + struct object_id oid; + char *real_ref; + struct strbuf ref = STRBUF_INIT; + int forcing = 0; + int dont_change_ref = 0; + + if ((track == BRANCH_TRACK_OVERRIDE || clobber_head_ok) + ? validate_branchname(name, &ref) + : validate_new_branchname(name, &ref, force)) { + if (!force) + dont_change_ref = 1; + else + forcing = 1; + } + + dwim_branch_start(r, start_name, track, &real_ref, &oid); if (reflog) log_all_ref_updates = LOG_REFS_NORMAL; @@ -436,6 +474,15 @@ void create_branch(struct repository *r, free(real_ref); } +void dwim_and_setup_tracking(struct repository *r, const char *new_ref, + const char *orig_ref, enum branch_track track, + int quiet) +{ + char *real_orig_ref; + dwim_branch_start(r, orig_ref, track, &real_orig_ref, NULL); + setup_tracking(new_ref, real_orig_ref, track, quiet); +} + void remove_merge_branch_state(struct repository *r) { unlink(git_path_merge_head(r)); -- cgit v1.2.3 From bc0893cf3b0ee376ef5b6ed293b1525480a9d720 Mon Sep 17 00:00:00 2001 From: Glen Choo Date: Fri, 28 Jan 2022 16:04:42 -0800 Subject: branch: make create_branch() always create a branch With the previous commit, there are no more invocations of create_branch() that do not create a branch because: * BRANCH_TRACK_OVERRIDE is no longer passed * clobber_head_ok = true and force = false is never passed Assert these situations, delete dead code and ensure that we're handling clobber_head_ok and force correctly by introducing tests for `git branch --force`. As a result, create_branch() now always creates a branch. Helped-by: Jonathan Tan Signed-off-by: Glen Choo Reviewed-by: Jonathan Tan Signed-off-by: Junio C Hamano --- branch.c | 57 +++++++++++++++++++++++++++------------------------------ 1 file changed, 27 insertions(+), 30 deletions(-) (limited to 'branch.c') diff --git a/branch.c b/branch.c index f3a31930fb..df24021f27 100644 --- a/branch.c +++ b/branch.c @@ -429,15 +429,19 @@ void create_branch(struct repository *r, char *real_ref; struct strbuf ref = STRBUF_INIT; int forcing = 0; - int dont_change_ref = 0; - - if ((track == BRANCH_TRACK_OVERRIDE || clobber_head_ok) - ? validate_branchname(name, &ref) - : validate_new_branchname(name, &ref, force)) { - if (!force) - dont_change_ref = 1; - else - forcing = 1; + struct ref_transaction *transaction; + struct strbuf err = STRBUF_INIT; + char *msg; + + if (track == BRANCH_TRACK_OVERRIDE) + BUG("'track' cannot be BRANCH_TRACK_OVERRIDE. Did you mean to call dwim_and_setup_tracking()?"); + if (clobber_head_ok && !force) + BUG("'clobber_head_ok' can only be used with 'force'"); + + if (clobber_head_ok ? + validate_branchname(name, &ref) : + validate_new_branchname(name, &ref, force)) { + forcing = 1; } dwim_branch_start(r, start_name, track, &real_ref, &oid); @@ -445,27 +449,20 @@ void create_branch(struct repository *r, if (reflog) log_all_ref_updates = LOG_REFS_NORMAL; - if (!dont_change_ref) { - struct ref_transaction *transaction; - struct strbuf err = STRBUF_INIT; - char *msg; - - if (forcing) - msg = xstrfmt("branch: Reset to %s", start_name); - else - msg = xstrfmt("branch: Created from %s", start_name); - - transaction = ref_transaction_begin(&err); - if (!transaction || - ref_transaction_update(transaction, ref.buf, - &oid, forcing ? NULL : null_oid(), - 0, msg, &err) || - ref_transaction_commit(transaction, &err)) - die("%s", err.buf); - ref_transaction_free(transaction); - strbuf_release(&err); - free(msg); - } + if (forcing) + msg = xstrfmt("branch: Reset to %s", start_name); + else + msg = xstrfmt("branch: Created from %s", start_name); + transaction = ref_transaction_begin(&err); + if (!transaction || + ref_transaction_update(transaction, ref.buf, + &oid, forcing ? NULL : null_oid(), + 0, msg, &err) || + ref_transaction_commit(transaction, &err)) + die("%s", err.buf); + ref_transaction_free(transaction); + strbuf_release(&err); + free(msg); if (real_ref && track) setup_tracking(ref.buf + 11, real_ref, track, quiet); -- cgit v1.2.3 From 3f3e76082bc29ff647dff16de9f0145a4d582825 Mon Sep 17 00:00:00 2001 From: Glen Choo Date: Fri, 28 Jan 2022 16:04:43 -0800 Subject: branch: add a dry_run parameter to create_branch() Add a dry_run parameter to create_branch() such that dry_run = 1 will validate a new branch without trying to create it. This will be used in `git branch --recurse-submodules` to ensure that the new branch can be created in all submodules. Signed-off-by: Glen Choo Reviewed-by: Jonathan Tan Signed-off-by: Junio C Hamano --- branch.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'branch.c') diff --git a/branch.c b/branch.c index df24021f27..02d46a69b8 100644 --- a/branch.c +++ b/branch.c @@ -423,7 +423,7 @@ static void dwim_branch_start(struct repository *r, const char *start_name, void create_branch(struct repository *r, const char *name, const char *start_name, int force, int clobber_head_ok, int reflog, - int quiet, enum branch_track track) + int quiet, enum branch_track track, int dry_run) { struct object_id oid; char *real_ref; @@ -445,6 +445,8 @@ void create_branch(struct repository *r, } dwim_branch_start(r, start_name, track, &real_ref, &oid); + if (dry_run) + goto cleanup; if (reflog) log_all_ref_updates = LOG_REFS_NORMAL; @@ -467,6 +469,7 @@ void create_branch(struct repository *r, if (real_ref && track) setup_tracking(ref.buf + 11, real_ref, track, quiet); +cleanup: strbuf_release(&ref); free(real_ref); } -- cgit v1.2.3 From 961b130d20c9aea322b94a639a63ec8cca9f14fc Mon Sep 17 00:00:00 2001 From: Glen Choo Date: Fri, 28 Jan 2022 16:04:45 -0800 Subject: branch: add --recurse-submodules option for branch creation To improve the submodules UX, we would like to teach Git to handle branches in submodules. Start this process by teaching "git branch" the --recurse-submodules option so that "git branch --recurse-submodules topic" will create the `topic` branch in the superproject and its submodules. Although this commit does not introduce breaking changes, it does not work well with existing --recurse-submodules commands because "git branch --recurse-submodules" writes to the submodule ref store, but most commands only consider the superproject gitlink and ignore the submodule ref store. For example, "git checkout --recurse-submodules" will check out the commits in the superproject gitlinks (and put the submodules in detached HEAD) instead of checking out the submodule branches. Because of this, this commit introduces a new configuration value, `submodule.propagateBranches`. The plan is for Git commands to prioritize submodule ref store information over superproject gitlinks if this value is true. Because "git branch --recurse-submodules" writes to submodule ref stores, for the sake of clarity, it will not function unless this configuration value is set. This commit also includes changes that support working with submodules from a superproject commit because "branch --recurse-submodules" (and future commands) need to read .gitmodules and gitlinks from the superproject commit, but submodules are typically read from the filesystem's .gitmodules and the index's gitlinks. These changes are: * add a submodules_of_tree() helper that gives the relevant information of an in-tree submodule (e.g. path and oid) and initializes the repository * add is_tree_submodule_active() by adding a treeish_name parameter to is_submodule_active() * add the "submoduleNotUpdated" advice to advise users to update the submodules in their trees Incidentally, fix an incorrect usage string that combined the 'list' usage of git branch (-l) with the 'create' usage; this string has been incorrect since its inception, a8dfd5eac4 (Make builtin-branch.c use parse_options., 2007-10-07). Helped-by: Jonathan Tan Signed-off-by: Glen Choo Reviewed-by: Jonathan Tan Signed-off-by: Junio C Hamano --- branch.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 141 insertions(+) (limited to 'branch.c') diff --git a/branch.c b/branch.c index 02d46a69b8..70026b3c79 100644 --- a/branch.c +++ b/branch.c @@ -8,6 +8,8 @@ #include "sequencer.h" #include "commit.h" #include "worktree.h" +#include "submodule-config.h" +#include "run-command.h" struct tracking { struct refspec_item spec; @@ -483,6 +485,145 @@ void dwim_and_setup_tracking(struct repository *r, const char *new_ref, setup_tracking(new_ref, real_orig_ref, track, quiet); } +/** + * Creates a branch in a submodule by calling + * create_branches_recursively() in a child process. The child process + * is necessary because install_branch_config_multiple_remotes() (which + * is called by setup_tracking()) does not support writing configs to + * submodules. + */ +static int submodule_create_branch(struct repository *r, + const struct submodule *submodule, + const char *name, const char *start_oid, + const char *tracking_name, int force, + int reflog, int quiet, + enum branch_track track, int dry_run) +{ + int ret = 0; + struct child_process child = CHILD_PROCESS_INIT; + struct strbuf child_err = STRBUF_INIT; + struct strbuf out_buf = STRBUF_INIT; + char *out_prefix = xstrfmt("submodule '%s': ", submodule->name); + child.git_cmd = 1; + child.err = -1; + child.stdout_to_stderr = 1; + + prepare_other_repo_env(&child.env_array, r->gitdir); + /* + * submodule_create_branch() is indirectly invoked by "git + * branch", but we cannot invoke "git branch" in the child + * process. "git branch" accepts a branch name and start point, + * where the start point is assumed to provide both the OID + * (start_oid) and the branch to use for tracking + * (tracking_name). But when recursing through submodules, + * start_oid and tracking name need to be specified separately + * (see create_branches_recursively()). + */ + strvec_pushl(&child.args, "submodule--helper", "create-branch", NULL); + if (dry_run) + strvec_push(&child.args, "--dry-run"); + if (force) + strvec_push(&child.args, "--force"); + if (quiet) + strvec_push(&child.args, "--quiet"); + if (reflog) + strvec_push(&child.args, "--create-reflog"); + if (track == BRANCH_TRACK_ALWAYS || track == BRANCH_TRACK_EXPLICIT) + strvec_push(&child.args, "--track"); + + strvec_pushl(&child.args, name, start_oid, tracking_name, NULL); + + if ((ret = start_command(&child))) + return ret; + ret = finish_command(&child); + strbuf_read(&child_err, child.err, 0); + strbuf_add_lines(&out_buf, out_prefix, child_err.buf, child_err.len); + + if (ret) + fprintf(stderr, "%s", out_buf.buf); + else + printf("%s", out_buf.buf); + + strbuf_release(&child_err); + strbuf_release(&out_buf); + return ret; +} + +void create_branches_recursively(struct repository *r, const char *name, + const char *start_commitish, + const char *tracking_name, int force, + int reflog, int quiet, enum branch_track track, + int dry_run) +{ + int i = 0; + char *branch_point = NULL; + struct object_id super_oid; + struct submodule_entry_list submodule_entry_list; + + /* Perform dwim on start_commitish to get super_oid and branch_point. */ + dwim_branch_start(r, start_commitish, BRANCH_TRACK_NEVER, + &branch_point, &super_oid); + + /* + * If we were not given an explicit name to track, then assume we are at + * the top level and, just like the non-recursive case, the tracking + * name is the branch point. + */ + if (!tracking_name) + tracking_name = branch_point; + + submodules_of_tree(r, &super_oid, &submodule_entry_list); + /* + * Before creating any branches, first check that the branch can + * be created in every submodule. + */ + for (i = 0; i < submodule_entry_list.entry_nr; i++) { + if (submodule_entry_list.entries[i].repo == NULL) { + if (advice_enabled(ADVICE_SUBMODULES_NOT_UPDATED)) + advise(_("You may try updating the submodules using 'git checkout %s && git submodule update --init'"), + start_commitish); + die(_("submodule '%s': unable to find submodule"), + submodule_entry_list.entries[i].submodule->name); + } + + if (submodule_create_branch( + submodule_entry_list.entries[i].repo, + submodule_entry_list.entries[i].submodule, name, + oid_to_hex(&submodule_entry_list.entries[i] + .name_entry->oid), + tracking_name, force, reflog, quiet, track, 1)) + die(_("submodule '%s': cannot create branch '%s'"), + submodule_entry_list.entries[i].submodule->name, + name); + } + + create_branch(the_repository, name, start_commitish, force, 0, reflog, quiet, + BRANCH_TRACK_NEVER, dry_run); + if (dry_run) + return; + /* + * NEEDSWORK If tracking was set up in the superproject but not the + * submodule, users might expect "git branch --recurse-submodules" to + * fail or give a warning, but this is not yet implemented because it is + * tedious to determine whether or not tracking was set up in the + * superproject. + */ + setup_tracking(name, tracking_name, track, quiet); + + for (i = 0; i < submodule_entry_list.entry_nr; i++) { + if (submodule_create_branch( + submodule_entry_list.entries[i].repo, + submodule_entry_list.entries[i].submodule, name, + oid_to_hex(&submodule_entry_list.entries[i] + .name_entry->oid), + tracking_name, force, reflog, quiet, track, 0)) + die(_("submodule '%s': cannot create branch '%s'"), + submodule_entry_list.entries[i].submodule->name, + name); + repo_clear(submodule_entry_list.entries[i].repo); + } +} + void remove_merge_branch_state(struct repository *r) { unlink(git_path_merge_head(r)); -- cgit v1.2.3 From 679e3693aba0c17af60c031f7eef68f2296b8dad Mon Sep 17 00:00:00 2001 From: Glen Choo Date: Fri, 28 Jan 2022 16:04:46 -0800 Subject: branch.c: use 'goto cleanup' in setup_tracking() to fix memory leaks Signed-off-by: Glen Choo Reviewed-by: Jonathan Tan Signed-off-by: Junio C Hamano --- branch.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'branch.c') diff --git a/branch.c b/branch.c index 70026b3c79..47251669e1 100644 --- a/branch.c +++ b/branch.c @@ -239,7 +239,7 @@ static void setup_tracking(const char *new_ref, const char *orig_ref, if (track != BRANCH_TRACK_INHERIT) for_each_remote(find_tracked_branch, &tracking); else if (inherit_tracking(&tracking, orig_ref)) - return; + goto cleanup; if (!tracking.matches) switch (track) { @@ -249,7 +249,7 @@ static void setup_tracking(const char *new_ref, const char *orig_ref, case BRANCH_TRACK_INHERIT: break; default: - return; + goto cleanup; } if (tracking.matches > 1) @@ -262,7 +262,8 @@ static void setup_tracking(const char *new_ref, const char *orig_ref, tracking.remote, tracking.srcs) < 0) exit(-1); - string_list_clear(tracking.srcs, 0); +cleanup: + string_list_clear(&tracking_srcs, 0); } int read_branch_desc(struct strbuf *buf, const char *branch_name) -- cgit v1.2.3