From patchwork Fri Apr 29 09:56:44 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tao Klerks X-Patchwork-Id: 12831700 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A1273C433EF for ; Fri, 29 Apr 2022 09:57:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1357157AbiD2KAM (ORCPT ); Fri, 29 Apr 2022 06:00:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52544 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1357135AbiD2KAL (ORCPT ); Fri, 29 Apr 2022 06:00:11 -0400 Received: from mail-wr1-x42a.google.com (mail-wr1-x42a.google.com [IPv6:2a00:1450:4864:20::42a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0661A45059 for ; Fri, 29 Apr 2022 02:56:51 -0700 (PDT) Received: by mail-wr1-x42a.google.com with SMTP id e24so10050689wrc.9 for ; Fri, 29 Apr 2022 02:56:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=aEQZs1LrNlUjnojn61Xpx+bfSR6yPe5q/LVV/LYDMi4=; b=TY2jgDpT3DAzDyjozKjYCQ+ahY1RKfp4A4m1qexxINLn0Mue/t6kj+L6HhZbyF/Ukt UyqGIr4i0yfKxm8vDG1N+IwdKvYzS0I8HNVmGDnOVeySs16X2cUoAwuLhhlidqa+gVgb uV1G3PLra/4yAXz7RAwzSg2j1rTDOfAJ16GT+ti4v/t1/QtP6XKX67rlqRzqoWrnd/45 BpDiIA3kr328dlMdphR3g/xuPnAFmJKxTzfWtGFr9iAQxLzIsD87MLWAOmYlm8gC53DV B6A1Y6eIugoI5hA6Jn/S1Llgdq0wWuWndi6TYrAH0b33edXm499LooUCcDu7A0J/PibI wQCg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=aEQZs1LrNlUjnojn61Xpx+bfSR6yPe5q/LVV/LYDMi4=; b=MlxTEmPBETtD7pTnnVwAc9SZ3mpOGxNiOA3hFtgiYFDG9kzLzeLtOEcCHmt65IkP+q D8FrvtxFr5ZOGAeNH9sQIV5oF3WrqQrND/PUQxlKwbKusSPCdG/nYWRtOLLZ1grilvxQ Rnylud8S4YiEBFvu2Dy9GqKQ0rmJpufQ7kTaAcCLcs39s6CDaKY5CWV0Afhg9F/eJxie XXohz2Mm/sOsYztnMvwPv+QTM8zku4+GeQ7VJTle+6QE1sriX/2oEFk/DLma7tUnV8Lh E059fTnJSddqDJPM3CGzvZiaXXy/kjOIR3BofMrdMlxIVWfdyrK7XRHEhynZTWTbut4H 2Aig== X-Gm-Message-State: AOAM532jGYs/si+XdHxIKeKRbvQAcd8JRqqztzoruL1zxWNpXpW9kEZZ iCcWjNBSJ1TOOJW9qNpSgq2cakY1Emk= X-Google-Smtp-Source: ABdhPJx384QIeiT0r8l51ojNHh/AUf5vfw8ssWhtTYtvDPVLHq+x0u8OImUZT4JZYZ/g1DIAJsxZRQ== X-Received: by 2002:a5d:64eb:0:b0:20a:ecf8:ac9f with SMTP id g11-20020a5d64eb000000b0020aecf8ac9fmr10654673wri.342.1651226209725; Fri, 29 Apr 2022 02:56:49 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id e12-20020a056000178c00b0020aaf8d351bsm1930708wrg.103.2022.04.29.02.56.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 29 Apr 2022 02:56:49 -0700 (PDT) Message-Id: <5b08edcdeefdaf2bd7bbc111bec82281b2f2b313.1651226207.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Fri, 29 Apr 2022 09:56:44 +0000 Subject: [PATCH v5 1/3] branch: new autosetupmerge option 'simple' for matching branches Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Tao Klerks , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Eric Sunshine , Josh Steadmon , Tao Klerks , Tao Klerks Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Tao Klerks From: Tao Klerks With the default push.default option, "simple", beginners are protected from accidentally pushing to the "wrong" branch in centralized workflows: if the remote tracking branch they would push to does not have the same name as the local branch, and they try to do a "default push", they get an error and explanation with options. There is a particular centralized workflow where this often happens: a user branches to a new local topic branch from an existing remote branch, eg with "checkout -b feature1 origin/master". With the default branch.autosetupmerge configuration (value "true"), git will automatically add origin/master as the upstream tracking branch. When the user pushes with a default "git push", with the intention of pushing their (new) topic branch to the remote, they get an error, and (amongst other things) a suggestion to run "git push origin HEAD". If they follow this suggestion the push succeeds, but on subsequent default pushes they continue to get an error - so eventually they figure out to add "-u" to change the tracking branch, or they spelunk the push.default config doc as proposed and set it to "current", or some GUI tooling does one or the other of these things for them. When one of their coworkers later works on the same topic branch, they don't get any of that "weirdness". They just "git checkout feature1" and everything works exactly as they expect, with the shared remote branch set up as remote tracking branch, and push and pull working out of the box. The "stable state" for this way of working is that local branches have the same-name remote tracking branch (origin/feature1 in this example), and multiple people can work on that remote feature branch at the same time, trusting "git pull" to merge or rebase as required for them to be able to push their interim changes to that same feature branch on that same remote. (merging from the upstream "master" branch, and merging back to it, are separate more involved processes in this flow). There is a problem in this flow/way of working, however, which is that the first user, when they first branched from origin/master, ended up with the "wrong" remote tracking branch (different from the stable state). For a while, before they pushed (and maybe longer, if they don't use -u/--set-upstream), their "git pull" wasn't getting other users' changes to the feature branch - it was getting any changes from the remote "master" branch instead (a completely different class of changes!) An experienced git user might say "well yeah, that's what it means to have the remote tracking branch set to origin/master!" - but the original user above didn't *ask* to have the remote master branch added as remote tracking branch - that just happened automatically when they branched their feature branch. They didn't necessarily even notice or understand the meaning of the "set up to track 'origin/master'" message when they created the branch - especially if they are using a GUI. Looking at how to fix this, you might think "OK, so disable auto setup of remote tracking - set branch.autosetupmerge to false" - but that will inconvenience the *second* user in this story - the one who just wanted to start working on the topic branch. The first and second users swap roles at different points in time of course - they should both have a sane configuration that does the right thing in both situations. Make this "branches have the same name locally as on the remote" workflow less painful / more obvious by introducing a new branch.autosetupmerge option called "simple", to match the same-name "push.default" option that makes similar assumptions. This new option automatically sets up tracking in a *subset* of the current default situations: when the original ref is a remote tracking branch *and* has the same branch name on the remote (as the new local branch name). Update the error displayed when the 'push.default=simple' configuration rejects a mismatching-upstream-name default push, to offer this new branch.autosetupmerge option that will prevent this class of error. With this new configuration, in the example situation above, the first user does *not* get origin/master set up as the tracking branch for the new local branch. If they "git pull" in their new local-only branch, they get an error explaining there is no upstream branch - which makes sense and is helpful. If they "git push", they get an error explaining how to push *and* suggesting they specify --set-upstream - which is exactly the right thing to do for them. This new option is likely not appropriate for users intentionally implementing a "triangular workflow" with a shared upstream tracking branch, that they "git pull" in and a "private" feature branch that they push/force-push to just for remote safe-keeping until they are ready to push up to the shared branch explicitly/separately. Such users are likely to prefer keeping the current default merge.autosetupmerge=true behavior, and change their push.default to "current". Also extend the existing branch tests with three new cases testing this option - the obvious matching-name and non-matching-name cases, and also a non-matching-ref-type case. The matching-name case needs to temporarily create an independent repo to fetch from, as the general strategy of using the local repo as the remote in these tests precludes locally branching with the same name as in the "remote". Signed-off-by: Tao Klerks --- Documentation/config/branch.txt | 4 +++- Documentation/git-branch.txt | 18 ++++++++++------- branch.c | 27 ++++++++++++++++++++++++- branch.h | 1 + builtin/push.c | 20 ++++++++++++++----- config.c | 3 +++ t/t3200-branch.sh | 35 +++++++++++++++++++++++++++++++++ 7 files changed, 94 insertions(+), 14 deletions(-) diff --git a/Documentation/config/branch.txt b/Documentation/config/branch.txt index 1e0c7af014b..8df10d07129 100644 --- a/Documentation/config/branch.txt +++ b/Documentation/config/branch.txt @@ -9,7 +9,9 @@ branch.autoSetupMerge:: automatic setup is done when the starting point is either a local branch or remote-tracking branch; `inherit` -- if the starting point has a tracking configuration, it is copied to the new - branch. This option defaults to true. + branch; `simple` -- automatic setup is done only when the starting point + is a remote-tracking branch and the new branch has the same name as the + remote branch. This option defaults to true. branch.autoSetupRebase:: When a new branch is created with 'git branch', 'git switch' or 'git checkout' diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt index c8b4f9ce3c7..ae82378349d 100644 --- a/Documentation/git-branch.txt +++ b/Documentation/git-branch.txt @@ -221,13 +221,17 @@ The exact upstream branch is chosen depending on the optional argument: itself as the upstream; `--track=inherit` means to copy the upstream configuration of the start-point branch. + -`--track=direct` is the default when the start point is a remote-tracking branch. -Set the branch.autoSetupMerge configuration variable to `false` if you -want `git switch`, `git checkout` and `git branch` to always behave as if `--no-track` -were given. Set it to `always` if you want this behavior when the -start-point is either a local or remote-tracking branch. Set it to -`inherit` if you want to copy the tracking configuration from the -branch point. +The branch.autoSetupMerge configuration variable specifies how `git switch`, +`git checkout` and `git branch` should behave when neither `--track` nor +`--no-track` are specified: ++ +The default option, `true`, behaves as though `--track=direct` +were given whenever the start-point is a remote-tracking branch. +`false` behaves as if `--no-track` were given. `always` behaves as though +`--track=direct` were given. `inherit` behaves as though `--track=inherit` +were given. `simple` behaves as though `--track=direct` were given only when +the start-point is a remote-tracking branch and the new branch has the same +name as the remote branch. + See linkgit:git-pull[1] and linkgit:git-config[1] for additional discussion on how the `branch..remote` and `branch..merge` options are used. diff --git a/branch.c b/branch.c index 01ecb816d5c..962aa7c8609 100644 --- a/branch.c +++ b/branch.c @@ -44,9 +44,9 @@ static int find_tracked_branch(struct remote *remote, void *priv) string_list_clear(tracking->srcs, 0); break; } + /* remote_find_tracking() searches by src if present */ tracking->spec.src = NULL; } - return 0; } @@ -264,15 +264,23 @@ static void setup_tracking(const char *new_ref, const char *orig_ref, if (!tracking.matches) switch (track) { + /* If ref is not remote, still use local */ case BRANCH_TRACK_ALWAYS: case BRANCH_TRACK_EXPLICIT: case BRANCH_TRACK_OVERRIDE: + /* Remote matches not evaluated */ case BRANCH_TRACK_INHERIT: break; + /* Otherwise, if no remote don't track */ default: goto cleanup; } + /* + * This check does not apply to BRANCH_TRACK_INHERIT; + * that supports multiple entries in tracking_srcs but + * leaves tracking.matches at 0. + */ if (tracking.matches > 1) { int status = die_message(_("not tracking: ambiguous information for ref '%s'"), orig_ref); @@ -307,6 +315,21 @@ static void setup_tracking(const char *new_ref, const char *orig_ref, exit(status); } + if (track == BRANCH_TRACK_SIMPLE) { + /* + * Only track if remote branch name matches. + * Reaching into items[0].string is safe because + * we know there is at least one and not more than + * one entry (because only BRANCH_TRACK_INHERIT can + * produce more than one entry). + */ + const char *tracked_branch; + if (!skip_prefix(tracking.srcs->items[0].string, + "refs/heads/", &tracked_branch) || + strcmp(tracked_branch, new_ref)) + return; + } + if (tracking.srcs->nr < 1) string_list_append(tracking.srcs, orig_ref); if (install_branch_config_multiple_remotes(config_flags, new_ref, @@ -603,6 +626,8 @@ static int submodule_create_branch(struct repository *r, /* Default for "git checkout". Do not pass --track. */ case BRANCH_TRACK_REMOTE: /* Default for "git branch". Do not pass --track. */ + case BRANCH_TRACK_SIMPLE: + /* Config-driven only. Do not pass --track. */ break; } diff --git a/branch.h b/branch.h index 04df2aa5b51..560b6b96a8f 100644 --- a/branch.h +++ b/branch.h @@ -12,6 +12,7 @@ enum branch_track { BRANCH_TRACK_EXPLICIT, BRANCH_TRACK_OVERRIDE, BRANCH_TRACK_INHERIT, + BRANCH_TRACK_SIMPLE, }; extern enum branch_track git_branch_track; diff --git a/builtin/push.c b/builtin/push.c index cad997965a7..447f91f5b47 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -2,6 +2,7 @@ * "git push" */ #include "cache.h" +#include "branch.h" #include "config.h" #include "refs.h" #include "refspec.h" @@ -151,7 +152,8 @@ static NORETURN void die_push_simple(struct branch *branch, * upstream to a non-branch, we should probably be showing * them the big ugly fully qualified ref. */ - const char *advice_maybe = ""; + const char *advice_pushdefault_maybe = ""; + const char *advice_automergesimple_maybe = ""; const char *short_upstream = branch->merge[0]->src; skip_prefix(short_upstream, "refs/heads/", &short_upstream); @@ -161,9 +163,16 @@ static NORETURN void die_push_simple(struct branch *branch, * push.default. */ if (push_default == PUSH_DEFAULT_UNSPECIFIED) - advice_maybe = _("\n" + advice_pushdefault_maybe = _("\n" "To choose either option permanently, " - "see push.default in 'git help config'."); + "see push.default in 'git help config'.\n"); + if (git_branch_track != BRANCH_TRACK_SIMPLE) + advice_automergesimple_maybe = _("\n" + "To avoid automatically configuring " + "upstream branches when their name\n" + "doesn't match the local branch, see option " + "'simple' of branch.autosetupmerge\n" + "in 'git help config'.\n"); die(_("The upstream branch of your current branch does not match\n" "the name of your current branch. To push to the upstream branch\n" "on the remote, use\n" @@ -173,9 +182,10 @@ static NORETURN void die_push_simple(struct branch *branch, "To push to the branch of the same name on the remote, use\n" "\n" " git push %s HEAD\n" - "%s"), + "%s%s"), remote->name, short_upstream, - remote->name, advice_maybe); + remote->name, advice_pushdefault_maybe, + advice_automergesimple_maybe); } static const char message_detached_head_die[] = diff --git a/config.c b/config.c index a5e11aad7fe..8dbeb1932e5 100644 --- a/config.c +++ b/config.c @@ -1781,6 +1781,9 @@ static int git_default_branch_config(const char *var, const char *value) } else if (value && !strcmp(value, "inherit")) { git_branch_track = BRANCH_TRACK_INHERIT; return 0; + } else if (value && !strcmp(value, "simple")) { + git_branch_track = BRANCH_TRACK_SIMPLE; + return 0; } git_branch_track = git_config_bool(var, value); return 0; diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index e12db593615..9723c2827cc 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -886,6 +886,41 @@ test_expect_success 'branch from tag w/--track causes failure' ' test_must_fail git branch --track my11 foobar ' +test_expect_success 'simple tracking works when remote branch name matches' ' + test_when_finished "rm -rf otherserver" && + git init otherserver && + test_commit -C otherserver my_commit 1 && + git -C otherserver branch feature && + test_config branch.autosetupmerge simple && + test_config remote.otherserver.url otherserver && + test_config remote.otherserver.fetch refs/heads/*:refs/remotes/otherserver/* && + git fetch otherserver && + git branch feature otherserver/feature && + test_cmp_config otherserver branch.feature.remote && + test_cmp_config refs/heads/feature branch.feature.merge +' + +test_expect_success 'simple tracking skips when remote branch name does not match' ' + test_config branch.autosetupmerge simple && + test_config remote.local.url . && + test_config remote.local.fetch refs/heads/*:refs/remotes/local/* && + git fetch local && + git branch my-other local/main && + test_cmp_config "" --default "" branch.my-other.remote && + test_cmp_config "" --default "" branch.my-other.merge +' + +test_expect_success 'simple tracking skips when remote ref is not a branch' ' + test_config branch.autosetupmerge simple && + test_config remote.localtags.url . && + test_config remote.localtags.fetch refs/tags/*:refs/remotes/localtags/* && + git tag mytag12 main && + git fetch localtags && + git branch mytag12 localtags/mytag12 && + test_cmp_config "" --default "" branch.mytag12.remote && + test_cmp_config "" --default "" branch.mytag12.merge +' + test_expect_success '--set-upstream-to fails on multiple branches' ' echo "fatal: too many arguments to set new upstream" >expect && test_must_fail git branch --set-upstream-to main a b c 2>err && From patchwork Fri Apr 29 09:56:45 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tao Klerks X-Patchwork-Id: 12831701 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id BE28EC433EF for ; Fri, 29 Apr 2022 09:57:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1357170AbiD2KAU (ORCPT ); Fri, 29 Apr 2022 06:00:20 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52296 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1357145AbiD2KAL (ORCPT ); Fri, 29 Apr 2022 06:00:11 -0400 Received: from mail-wr1-x436.google.com (mail-wr1-x436.google.com [IPv6:2a00:1450:4864:20::436]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C365D45AC5 for ; Fri, 29 Apr 2022 02:56:52 -0700 (PDT) Received: by mail-wr1-x436.google.com with SMTP id c11so244956wrn.8 for ; Fri, 29 Apr 2022 02:56:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=V02L8s8EBQzkdw4TBhkPUpsVl9AfSAtv1XWp0fHySVg=; b=gX9jrYeoU9I7UP4wUdlyzy7q3zejszqGA/jMnj6eMLwuRKXoAKVtOfDcGSnIzx8OWO zlUywXsW9GGUq1fZSzSFcsebZKS2KQTVFi8f56JUUVVMEdEfTLksLX2subw7Om9MM6ap 24e/X0rDfpIXWLd4ECi9yImjQTjdKnDw23pojP+lcZwH7HobfkA3A68r+CecJyDfjOMG /GGDG5W5Q5Cbqkne/rBoac4tmcOouT4AWvndXq5KT4j4UNdnkBTvQfl+IWS70Hy236r0 QhkI2zEECTtRVGkVJHSs5jmnPt+nXp+9PZe/wRl8L0LNGaVXN6a7L7jpoCNyMvFBsLl9 +lIw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=V02L8s8EBQzkdw4TBhkPUpsVl9AfSAtv1XWp0fHySVg=; b=t3uKaqi1h0ICY7gvMqOLvjk6W6ZB8Qzh5rHmSTIraHrozGDUoQV5ISOtJ+2Xu8YXyU BVfwHWBiR7QmisuZv8nqF9dCjVtoatCOYtZQ4nuZSt/jXrnK8K7k2GElzoZkNaQqP5T3 kBTUpZw7GWNuvEOmO3tE3VN8ioOvasbMy7PHMG9Z/CArifLCmyVpbJ5pq7ROYM2+Qbb+ npo9HiAA3C2tfx1OwSrEZThk9B05Vy1C6ghRiwUIztBubwj/BEBuIBItXftolrYP1d/v +a6g3yH7wNp30/I+dCTB+gyE49XsqlTwscKGzcbQePO7+To3epcvZ2gocQiNE8mZ1z4I 0HqA== X-Gm-Message-State: AOAM5306kkX7T24iOsBDIisSiCBkR7B+qbk1aGVboTnwgM242nt9SdNj ksIbpc25RpPGHNP57gL1lEhE7zMPqQ4= X-Google-Smtp-Source: ABdhPJzvswCGx2rwmxfC9CBvkyY83M4HTGBoza5zGWpbaL8p6Pu4gvDXkdlU7Gb+460sFocDbceiyA== X-Received: by 2002:a5d:408d:0:b0:20a:ce37:1306 with SMTP id o13-20020a5d408d000000b0020ace371306mr26946228wrp.215.1651226210932; Fri, 29 Apr 2022 02:56:50 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id r204-20020a1c44d5000000b003941901f2b0sm2540336wma.41.2022.04.29.02.56.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 29 Apr 2022 02:56:50 -0700 (PDT) Message-Id: <31184c3a65d64826a7f546fcc04f6efc6f2d017f.1651226207.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Fri, 29 Apr 2022 09:56:45 +0000 Subject: [PATCH v5 2/3] push: default to single remote even when not named origin Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Tao Klerks , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Eric Sunshine , Josh Steadmon , Tao Klerks , Tao Klerks Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Tao Klerks From: Tao Klerks With "push.default=current" configured, a simple "git push" will push to the same-name branch on the current branch's branch..pushRemote, or remote.pushDefault, or origin. If none of these are defined, the push will fail with error "fatal: No configured push destination". The same "default to origin if no config" behavior applies with "push.default=matching". Other commands use "origin" as a default when there are multiple options, but default to the single remote when there is only one - for example, "git checkout ". This "assume the single remote if there is only one" behavior is more friendly/useful than a defaulting behavior that only uses the name "origin" no matter what. Update "git push" to also default to the single remote (and finally fall back to "origin" as default if there are several), for "push.default=current" and for other current and future remote-defaulting push behaviors. This change also modifies the behavior of ls-remote in a consistent way, so defaulting not only supplies 'origin', but any single configured remote also. Document the change in behavior, correct incorrect assumptions in related tests, and add test cases reflecting this new single-remote-defaulting behavior. Signed-off-by: Tao Klerks --- Documentation/config/branch.txt | 5 +-- remote.c | 2 ++ t/t5512-ls-remote.sh | 17 +++++++-- t/t5528-push-default.sh | 63 ++++++++++++++++++++++++++++++++- 4 files changed, 81 insertions(+), 6 deletions(-) diff --git a/Documentation/config/branch.txt b/Documentation/config/branch.txt index 8df10d07129..445341a906b 100644 --- a/Documentation/config/branch.txt +++ b/Documentation/config/branch.txt @@ -40,8 +40,9 @@ branch..remote:: may be overridden with `remote.pushDefault` (for all branches). The remote to push to, for the current branch, may be further overridden by `branch..pushRemote`. If no remote is - configured, or if you are not on any branch, it defaults to - `origin` for fetching and `remote.pushDefault` for pushing. + configured, or if you are not on any branch and there is more than + one remote defined in the repository, it defaults to `origin` for + fetching and `remote.pushDefault` for pushing. Additionally, `.` (a period) is the current local repository (a dot-repository), see `branch..merge`'s final note below. diff --git a/remote.c b/remote.c index 42a4e7106e1..930fdc9c2f6 100644 --- a/remote.c +++ b/remote.c @@ -543,6 +543,8 @@ static const char *remotes_remote_for_branch(struct remote_state *remote_state, } if (explicit) *explicit = 0; + if (remote_state->remotes_nr == 1) + return remote_state->remotes[0]->name; return "origin"; } diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh index f53f58895a1..20d063fb9ae 100755 --- a/t/t5512-ls-remote.sh +++ b/t/t5512-ls-remote.sh @@ -15,6 +15,10 @@ generate_references () { done } +test_expect_success 'dies when no remote found' ' + test_must_fail git ls-remote +' + test_expect_success setup ' >file && git add file && @@ -30,7 +34,8 @@ test_expect_success setup ' git show-ref -d >refs && sed -e "s/ / /" refs >>expected.all && - git remote add self "$(pwd)/.git" + git remote add self "$(pwd)/.git" && + git remote add self2 "." ' test_expect_success 'ls-remote --tags .git' ' @@ -83,11 +88,17 @@ test_expect_success 'ls-remote --sort="-refname" --tags self' ' test_cmp expect actual ' -test_expect_success 'dies when no remote specified and no default remotes found' ' +test_expect_success 'dies when no remote specified, multiple remotes found, and no default specified' ' test_must_fail git ls-remote ' -test_expect_success 'use "origin" when no remote specified' ' +test_expect_success 'succeeds when no remote specified but only one found' ' + test_when_finished git remote add self2 "." && + git remote remove self2 && + git ls-remote +' + +test_expect_success 'use "origin" when no remote specified and multiple found' ' URL="$(pwd)/.git" && echo "From $URL" >exp_err && diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh index f280e00eb79..0d6c9869ed3 100755 --- a/t/t5528-push-default.sh +++ b/t/t5528-push-default.sh @@ -94,13 +94,74 @@ test_expect_success '"upstream" does not push when remotes do not match' ' test_must_fail git push parent2 ' -test_expect_success 'push from/to new branch with upstream, matching and simple' ' +test_expect_success '"current" does not push when multiple remotes and none origin' ' + git checkout main && + test_config push.default current && + test_commit current-multi && + test_must_fail git push +' + +test_expect_success '"current" pushes when remote explicitly specified' ' + git checkout main && + test_config push.default current && + test_commit current-specified && + git push parent1 +' + +test_expect_success '"current" pushes to origin when no remote specified among multiple' ' + git checkout main && + test_config remote.origin.url repo1 && + test_config remote.origin.fetch "+refs/heads/*:refs/remotes/origin/*" && + test_commit current-origin && + test_push_success current main +' + +test_expect_success '"current" pushes to single remote even when not specified' ' + git checkout main && + test_when_finished git remote add parent1 repo1 && + git remote remove parent1 && + test_commit current-implied && + test_push_success current main repo2 +' + +test_expect_success 'push from/to new branch with non-defaulted remote fails with upstream, matching, current and simple ' ' git checkout -b new-branch && test_push_failure simple && test_push_failure matching && + test_push_failure upstream && + test_push_failure current +' + +test_expect_success 'push from/to new branch fails with upstream and simple ' ' + git checkout -b new-branch-1 && + test_config branch.new-branch-1.remote parent1 && + test_push_failure simple && test_push_failure upstream ' +# The behavior here is surprising but not entirely wrong: +# - the current branch is used to determine the target remote +# - the "matching" push default pushes matching branches, *ignoring* the +# current new branch as it does not have upstream tracking +# - the default push succeeds +# +# A previous test expected this to fail, but for the wrong reasons: +# it expected a fail becaause the branch is new and cannot be pushed, but +# in fact it was failing because of an ambiguous remote +# +test_expect_failure 'push from/to new branch fails with matching ' ' + git checkout -b new-branch-2 && + test_config branch.new-branch-2.remote parent1 && + test_push_failure matching +' + +test_expect_success 'push from/to branch with tracking fails with nothing ' ' + git checkout -b tracked-branch && + test_config branch.tracked-branch.remote parent1 && + test_config branch.tracked-branch.merge refs/heads/tracked-branch && + test_push_failure nothing +' + test_expect_success '"matching" fails if none match' ' git init --bare empty && test_must_fail git push empty : 2>actual && From patchwork Fri Apr 29 09:56:46 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tao Klerks X-Patchwork-Id: 12831702 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 15503C433F5 for ; Fri, 29 Apr 2022 09:57:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1357174AbiD2KAY (ORCPT ); Fri, 29 Apr 2022 06:00:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52330 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235489AbiD2KAM (ORCPT ); Fri, 29 Apr 2022 06:00:12 -0400 Received: from mail-wr1-x432.google.com (mail-wr1-x432.google.com [IPv6:2a00:1450:4864:20::432]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F3B274756C for ; Fri, 29 Apr 2022 02:56:53 -0700 (PDT) Received: by mail-wr1-x432.google.com with SMTP id x18so10122004wrc.0 for ; Fri, 29 Apr 2022 02:56:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=94CNW39yx0FK5+icdSaliaZmEjJBiy7AXXxBJV0lNsQ=; b=XAkJi3clhs7BBCXFWsraM0qOY04Gk3qrxXxpdjliQKLojswloaJUxSa9VIzEtm9u2y 2NatpAiNQXwoecTLgYSnaPHk435lFhXerwEHIMyHZevthTF7He9VtXSHtaN1TS9rrCof ZWAP3AM6Jzya3f79t2ilV1IAEOI6N7yqinxpAnBuyehivRbwoHxcKw4fOszC5LcEegEW KVxA71iTwpKV2sjiBjfvx4iPdeteUoabUlYuxCA2V7kZsJ1ipJEKk8UdtH9h+YCOX7a+ DWml3g8eKxQMPscgJgvrpwvALPQGMKga2fi8IwNjWWeDYNUmKQLpN6PGvuicl84vW9Vd 3ADw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=94CNW39yx0FK5+icdSaliaZmEjJBiy7AXXxBJV0lNsQ=; b=wdLG/wVStpmhvikTpFSOFd8xAaBtBMBe1oaI3vld75q9W0+72Lezua9eTB1Tap3J5g md2sqc+P2TehkuyrJIKKxb/+SkGOH9XRvEfomT+bXIB1S7ERwJ1vPGLTlv12J+eJp094 1qdRWL9PQgHEoG+8QJz+JeWEb5UeTuokOVt+r8NNkjpgsUZ3Ff4hDI+mnUDHoBWuMJxj vdkXA61nUU8//2G5Sl89GtwD6pqEVYmaPhtG6+y9MeUIJ8YtxGc9zfvQh2GriKqantwJ dunoZ5M7t4cOEVcP35jv8lo8UBGr+Ysy02bNUN3C3r/pTdkRO2l+3JH2Xv0MhiEASRnW 7kiQ== X-Gm-Message-State: AOAM533hV2mugVdtxgZj5DPTsfpWPuIokLMrw67G+dC2DI3EW/xF7d40 /T0Vh7cYFHkgGXqghzXPA6NuoVgAzr0= X-Google-Smtp-Source: ABdhPJwJbF7Au8RqNvRLSMV4TnffMPjDTJjprQBsqzOfoW/QP4+SvxHKLMa5mUQjLFP7HV5Qg62PXA== X-Received: by 2002:adf:f187:0:b0:20a:dfb0:766a with SMTP id h7-20020adff187000000b0020adfb0766amr18270246wro.517.1651226212187; Fri, 29 Apr 2022 02:56:52 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id q3-20020adfab03000000b0020ad57b8ddesm2036309wrc.101.2022.04.29.02.56.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 29 Apr 2022 02:56:51 -0700 (PDT) Message-Id: <41c88e51ac6baf3ddaf08f2335015b4fa69fadf6.1651226207.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Fri, 29 Apr 2022 09:56:46 +0000 Subject: [PATCH v5 3/3] push: new config option "push.autoSetupRemote" supports "simple" push Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Tao Klerks , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , Eric Sunshine , Josh Steadmon , Tao Klerks , Tao Klerks Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Tao Klerks From: Tao Klerks In some "simple" centralized workflows, users expect remote tracking branch names to match local branch names. "git push" pushes to the remote version/instance of the branch, and "git pull" pulls any changes to the remote branch (changes made by the same user in another place, or by other users). This expectation is supported by the push.default default option "simple" which refuses a default push for a mismatching tracking branch name, and by the new branch.autosetupmerge option, "simple", which only sets up remote tracking for same-name remote branches. When a new branch has been created by the user and has not yet been pushed (and push.default is not set to "current"), the user is prompted with a "The current branch %s has no upstream branch" error, and instructions on how to push and add tracking. This error is helpful in that following the advice once per branch "resolves" the issue for that branch forever, but inconvenient in that for the "simple" centralized workflow, this is always the right thing to do, so it would be better to just do it. Support this workflow with a new config setting, push.autoSetupRemote, which will cause a default push, when there is no remote tracking branch configured, to push to the same-name on the remote and --set-upstream. Also add a hint offering this new option when the "The current branch %s has no upstream branch" error is encountered, and add corresponding tests. Signed-off-by: Tao Klerks --- Documentation/config/push.txt | 11 +++++++++ builtin/push.c | 44 ++++++++++++++++++++++++++++------- t/t5528-push-default.sh | 14 +++++++++++ transport.h | 1 + 4 files changed, 62 insertions(+), 8 deletions(-) diff --git a/Documentation/config/push.txt b/Documentation/config/push.txt index 632033638c4..e32801e6c91 100644 --- a/Documentation/config/push.txt +++ b/Documentation/config/push.txt @@ -1,3 +1,14 @@ +push.autoSetupRemote:: + If set to "true" assume `--set-upstream` on default push when no + upstream tracking exists for the current branch; this option + takes effect with push.default options 'simple', 'upstream', + and 'current'. It is useful if by default you want new branches + to be pushed to the default remote (like the behavior of + 'push.default=current') and you also want the upstream tracking + to be set. Workflows most likely to benefit from this option are + 'simple' central workflows where all branches are expected to + have the same name on the remote. + push.default:: Defines the action `git push` should take if no refspec is given (whether from the command-line, config, or elsewhere). diff --git a/builtin/push.c b/builtin/push.c index 447f91f5b47..86b44f8aa71 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -195,16 +195,32 @@ static const char message_detached_head_die[] = "\n" " git push %s HEAD:\n"); -static const char *get_upstream_ref(struct branch *branch, const char *remote_name) +static const char *get_upstream_ref(int flags, struct branch *branch, const char *remote_name) { - if (!branch->merge_nr || !branch->merge || !branch->remote_name) + if (branch->merge_nr == 0 && (flags & TRANSPORT_PUSH_AUTO_UPSTREAM)) { + /* if missing, assume same; set_upstream will be defined later */ + return branch->refname; + } + + if (!branch->merge_nr || !branch->merge || !branch->remote_name) { + const char *advice_autosetup_maybe = ""; + if (!(flags & TRANSPORT_PUSH_AUTO_UPSTREAM)) { + advice_autosetup_maybe = _("\n" + "To have this happen automatically for " + "branches without a tracking\n" + "upstream, see 'push.autoSetupRemote' " + "in 'git help config'.\n"); + } die(_("The current branch %s has no upstream branch.\n" "To push the current branch and set the remote as upstream, use\n" "\n" - " git push --set-upstream %s %s\n"), + " git push --set-upstream %s %s\n" + "%s"), branch->name, remote_name, - branch->name); + branch->name, + advice_autosetup_maybe); + } if (branch->merge_nr != 1) die(_("The current branch %s has multiple upstream branches, " "refusing to push."), branch->name); @@ -212,7 +228,7 @@ static const char *get_upstream_ref(struct branch *branch, const char *remote_na return branch->merge[0]->src; } -static void setup_default_push_refspecs(struct remote *remote) +static void setup_default_push_refspecs(int *flags, struct remote *remote) { struct branch *branch; const char *dst; @@ -244,7 +260,7 @@ static void setup_default_push_refspecs(struct remote *remote) case PUSH_DEFAULT_SIMPLE: if (!same_remote) break; - if (strcmp(branch->refname, get_upstream_ref(branch, remote->name))) + if (strcmp(branch->refname, get_upstream_ref(*flags, branch, remote->name))) die_push_simple(branch, remote); break; @@ -254,13 +270,21 @@ static void setup_default_push_refspecs(struct remote *remote) "your current branch '%s', without telling me what to push\n" "to update which remote branch."), remote->name, branch->name); - dst = get_upstream_ref(branch, remote->name); + dst = get_upstream_ref(*flags, branch, remote->name); break; case PUSH_DEFAULT_CURRENT: break; } + /* + * this is a default push - if auto-upstream is enabled and there is + * no upstream defined, then set it (with options 'simple', 'upstream', + * and 'current'). + */ + if ((*flags & TRANSPORT_PUSH_AUTO_UPSTREAM) && branch->merge_nr == 0) + *flags |= TRANSPORT_PUSH_SET_UPSTREAM; + refspec_appendf(&rs, "%s:%s", branch->refname, dst); } @@ -411,7 +435,7 @@ static int do_push(int flags, if (remote->push.nr) { push_refspec = &remote->push; } else if (!(flags & TRANSPORT_PUSH_MIRROR)) - setup_default_push_refspecs(remote); + setup_default_push_refspecs(&flags, remote); } errs = 0; url_nr = push_url_of_remote(remote, &url); @@ -482,6 +506,10 @@ static int git_push_config(const char *k, const char *v, void *cb) else *flags &= ~TRANSPORT_PUSH_FOLLOW_TAGS; return 0; + } else if (!strcmp(k, "push.autosetupremote")) { + if (git_config_bool(k, v)) + *flags |= TRANSPORT_PUSH_AUTO_UPSTREAM; + return 0; } else if (!strcmp(k, "push.gpgsign")) { const char *value; if (!git_config_get_value("push.gpgsign", &value)) { diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh index 0d6c9869ed3..284e20fefda 100755 --- a/t/t5528-push-default.sh +++ b/t/t5528-push-default.sh @@ -162,6 +162,20 @@ test_expect_success 'push from/to branch with tracking fails with nothing ' ' test_push_failure nothing ' +test_expect_success 'push from/to new branch succeeds with upstream if push.autoSetupRemote' ' + git checkout -b new-branch-a && + test_config push.autoSetupRemote true && + test_config branch.new-branch-a.remote parent1 && + test_push_success upstream new-branch-a +' + +test_expect_success 'push from/to new branch succeeds with simple if push.autoSetupRemote' ' + git checkout -b new-branch-c && + test_config push.autoSetupRemote true && + test_config branch.new-branch-c.remote parent1 && + test_push_success simple new-branch-c +' + test_expect_success '"matching" fails if none match' ' git init --bare empty && test_must_fail git push empty : 2>actual && diff --git a/transport.h b/transport.h index 12bc08fc339..b5bf7b3e704 100644 --- a/transport.h +++ b/transport.h @@ -145,6 +145,7 @@ struct transport { #define TRANSPORT_PUSH_OPTIONS (1<<14) #define TRANSPORT_RECURSE_SUBMODULES_ONLY (1<<15) #define TRANSPORT_PUSH_FORCE_IF_INCLUDES (1<<16) +#define TRANSPORT_PUSH_AUTO_UPSTREAM (1<<17) int transport_summary_width(const struct ref *refs);