| Message ID | 20190814134629.21096-1-git@matthieu-moy.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | pull, fetch: add --set-upstream option | expand |
Hi Matthieu, This is not really a review. Just some minor nitpicks I spotted while reading through. On 14/08/19 03:46PM, Matthieu Moy wrote: > From: Corentin BOMPARD <corentin.bompard@etu.univ-lyon1.fr> > > Add the --set-upstream option to git pull/fetch > which lets the user set the upstream configuration > (branch.<current-branch-name>.merge and > branch.<current-branch-name>.remote) for the current branch. > > A typical use-case is: > > git clone http://example.com/my-public-fork > git remote add main http://example.com/project-main-repo > git pull --set-upstream main master > > or, instead of the last line: > > git fetch --set-upstream main master > git merge # or git rebase > > This functionality is analog to push --set-upstream. > > Signed-off-by: Corentin BOMPARD <corentin.bompard@etu.univ-lyon1.fr> > Signed-off-by: Nathan BERBEZIER <nathan.berbezier@etu.univ-lyon1.fr> > Signed-off-by: Pablo CHABANNE <pablo.chabanne@etu.univ-lyon1.fr> > Signed-off-by: Matthieu Moy <git@matthieu-moy.fr> > Patch-edited-by: Matthieu Moy <git@matthieu-moy.fr> > --- > This is a followup on > https://public-inbox.org/git/86zhoil3yw.fsf@univ-lyon1.fr/. It's > initially a student project, but students didn't get time to complete > it. Still, I think the feature is interesting, and I finally get time > to fix the remarks made up to now. This now looks good to me, but > obviously needs other pairs of eyes. > > Thanks, > > Documentation/fetch-options.txt | 7 ++ > builtin/fetch.c | 48 ++++++++- > builtin/pull.c | 6 ++ > t/t5553-set-upstream.sh | 178 ++++++++++++++++++++++++++++++++ > 4 files changed, 238 insertions(+), 1 deletion(-) > create mode 100755 t/t5553-set-upstream.sh > > diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt > index 3c9b4f9e09..99df1f3d4e 100644 > --- a/Documentation/fetch-options.txt > +++ b/Documentation/fetch-options.txt > @@ -169,6 +169,13 @@ ifndef::git-pull[] > Disable recursive fetching of submodules (this has the same effect as > using the `--recurse-submodules=no` option). > > +--set-upstream:: > + If the remote is fetched successfully, pull and add upstream > + (tracking) reference, used by argument-less > + linkgit:git-pull[1] and other commands. For more information, > + see `branch.<name>.merge` and `branch.<name>.remote` in > + linkgit:git-config[1]. > + > --submodule-prefix=<path>:: > Prepend <path> to paths printed in informative messages > such as "Fetching submodule foo". This option is used > diff --git a/builtin/fetch.c b/builtin/fetch.c > index 717dd14e89..5557ae1c04 100644 > --- a/builtin/fetch.c > +++ b/builtin/fetch.c > @@ -23,6 +23,7 @@ > #include "packfile.h" > #include "list-objects-filter-options.h" > #include "commit-reach.h" > +#include "branch.h" > > #define FORCED_UPDATES_DELAY_WARNING_IN_MS (10 * 1000) > > @@ -50,7 +51,7 @@ static int fetch_prune_tags_config = -1; /* unspecified */ > static int prune_tags = -1; /* unspecified */ > #define PRUNE_TAGS_BY_DEFAULT 0 /* do we prune tags by default? */ > > -static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity, deepen_relative; > +static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity, deepen_relative, set_upstream; This line is getting pretty long. I think it is a good idea to split it into two. > static int progress = -1; > static int enable_auto_gc = 1; > static int tags = TAGS_DEFAULT, unshallow, update_shallow, deepen; > @@ -123,6 +124,8 @@ static struct option builtin_fetch_options[] = { > OPT__VERBOSITY(&verbosity), > OPT_BOOL(0, "all", &all, > N_("fetch from all remotes")), > + OPT_BOOL(0, "set-upstream", &set_upstream, > + N_("set upstream for git pull/fetch")), > OPT_BOOL('a', "append", &append, > N_("append to .git/FETCH_HEAD instead of overwriting")), > OPT_STRING(0, "upload-pack", &upload_pack, N_("path"), > @@ -1367,6 +1370,49 @@ static int do_fetch(struct transport *transport, > retcode = 1; > goto cleanup; > } > + > + if (set_upstream) { > + struct branch *branch = branch_get("HEAD"); > + struct ref *rm; > + struct ref *source_ref = NULL; > + > + /* > + * We're setting the upstream configuration for the current branch. The > + * relevent upstream is the fetched branch that is meant to be merged with > + * the current one, i.e. the one fetched to FETCH_HEAD. > + * > + * When there are several such branches, consider the request ambiguous and > + * err on the safe side by doing nothing and just emit a warning. > + */ The comment lines cross the 80 column boundary. The usual convention in this project is to try to keep lines below 80 columns. For strings IMO an exception can be allowed because breaking them up makes it harder to grep for them. But comments are the easiest to format. Are you using a tab size of 4? That might explain why your line breaks are just after the 80 col boundary. The coding guidelines say you should make your tab characters 8 columns wide. > + for (rm = ref_map; rm; rm = rm->next) { > + if (!rm->peer_ref) { > + if (source_ref) { > + warning(_("multiple branch detected, incompatible with --set-upstream")); > + goto skip; > + } else { > + source_ref = rm; > + } > + } > + } > + if (source_ref) { > + if (!strcmp(source_ref->name, "HEAD") || This line has a trailing space. > + starts_with(source_ref->name, "refs/heads/")) { > + install_branch_config(0, branch->name, > + transport->remote->name, > + source_ref->name); In other places around this code, multi line function calls are aligned with the opening parenthesis. It is a good idea to follow that convention. So this should change to something like: install_branch_config(0, branch->name, transport->remote->name, source_ref->name); Maybe this discrepancy is because you are using the wrong tab size? > + } else if (starts_with(source_ref->name, "refs/remotes/")) { > + warning(_("not setting upstream for a remote remote-tracking branch")); > + } else if (starts_with(source_ref->name, "refs/tags/")) { > + warning(_("not setting upstream for a remote tag")); > + } else { > + warning(_("unknown branch type")); > + } No need to wrap single line if statements in braces. > + } else { > + warning(_("no source branch found.\n" > + "you need to specify exactly one branch with the --set-upstream option.")); > + } > + } > + skip: > free_refs(ref_map); > > /* if neither --no-tags nor --tags was specified, do automated tag > diff --git a/builtin/pull.c b/builtin/pull.c > index f1eaf6e6ed..d25ff13a60 100644 > --- a/builtin/pull.c > +++ b/builtin/pull.c > @@ -129,6 +129,7 @@ static char *opt_refmap; > static char *opt_ipv4; > static char *opt_ipv6; > static int opt_show_forced_updates = -1; > +static char *set_upstream; > > static struct option pull_options[] = { > /* Shared options */ > @@ -243,6 +244,9 @@ static struct option pull_options[] = { > PARSE_OPT_NOARG), > OPT_BOOL(0, "show-forced-updates", &opt_show_forced_updates, > N_("check for forced-updates on all updated branches")), > + OPT_PASSTHRU(0, "set-upstream", &set_upstream, NULL, > + N_("set upstream for git pull/fetch"), > + PARSE_OPT_NOARG), > > OPT_END() > }; > @@ -556,6 +560,8 @@ static int run_fetch(const char *repo, const char **refspecs) > argv_array_push(&args, "--show-forced-updates"); > else if (opt_show_forced_updates == 0) > argv_array_push(&args, "--no-show-forced-updates"); > + if (set_upstream) > + argv_array_push(&args, set_upstream); > > if (repo) { > argv_array_push(&args, repo); > diff --git a/t/t5553-set-upstream.sh b/t/t5553-set-upstream.sh > new file mode 100755 > index 0000000000..bd1a94f494 > --- /dev/null > +++ b/t/t5553-set-upstream.sh > @@ -0,0 +1,178 @@ > +#!/bin/sh > + > +test_description='"git fetch/pull --set-upstream" basic tests.' > +. ./test-lib.sh > + > +check_config () { > + printf "%s\n" "$2" "$3" >"expect.$1" && > + { > + git config "branch.$1.remote" && git config "branch.$1.merge" > + } >"actual.$1" && > + test_cmp "expect.$1" "actual.$1" > +} > + > +check_config_missing () { > + test_expect_code 1 git config "branch.$1.remote" && > + test_expect_code 1 git config "branch.$1.merge" > +} > + > +clear_config () { > + for branch in "$@"; do > + test_might_fail git config --unset-all "branch.$branch.remote" > + test_might_fail git config --unset-all "branch.$branch.merge" > + done > +} > + > +ensure_fresh_upstream () { > + rm -rf parent && git init --bare parent > +} > + > +test_expect_success 'setup bare parent fetch' ' > + ensure_fresh_upstream && > + git remote add upstream parent > +' > + > +test_expect_success 'setup commit on master and other fetch' ' > + test_commit one && > + git push upstream master && > + git checkout -b other && > + test_commit two && > + git push upstream other > +' > + > +#tests for fetch --set-upstream Add a space after the '#'. Same in other comments below. > + > +test_expect_success 'fetch --set-upstream does not set upstream w/o branch' ' > + clear_config master other && > + git checkout master && > + git fetch --set-upstream upstream && > + check_config_missing master && > + check_config_missing other > +' > + > +test_expect_success 'fetch --set-upstream upstream master sets branch master but not other' ' > + clear_config master other && > + git fetch --set-upstream upstream master && > + check_config master upstream refs/heads/master && > + check_config_missing other > +' > + > +test_expect_success 'fetch --set-upstream upstream other sets branch other' ' > + clear_config master other && > + git fetch --set-upstream upstream other && > + check_config master upstream refs/heads/other && > + check_config_missing other > +' > + > +test_expect_success 'fetch --set-upstream master:other does not set the branch other2' ' > + clear_config other2 && > + git fetch --set-upstream upstream master:other2 && > + check_config_missing other2 > +' > + > +test_expect_success 'fetch --set-upstream http://nosuchdomain.example.com fails with invalid url' ' > + # master explicitly not cleared, we check that it is not touched from previous value > + clear_config other other2 && > + test_must_fail git fetch --set-upstream http://nosuchdomain.example.com && > + check_config master upstream refs/heads/other && > + check_config_missing other && > + check_config_missing other2 > +' > + > +test_expect_success 'fetch --set-upstream with valid URL sets upstream to URL' ' > + clear_config other other2 && > + url="file://'"$PWD"'" && > + git fetch --set-upstream "$url" && > + check_config master "$url" HEAD && > + check_config_missing other && > + check_config_missing other2 > +' > + > +#tests for pull --set-upstream > + > +test_expect_success 'setup bare parent pull' ' > + git remote rm upstream && > + ensure_fresh_upstream && > + git remote add upstream parent > +' > + > +test_expect_success 'setup commit on master and other pull' ' > + test_commit three && > + git push --tags upstream master && > + test_commit four && > + git push upstream other > +' > + > +test_expect_success 'pull --set-upstream upstream master sets branch master but not other' ' > + clear_config master other && > + git pull --set-upstream upstream master && > + check_config master upstream refs/heads/master && > + check_config_missing other > +' > + > +test_expect_success 'pull --set-upstream master:other2 does not set the branch other2' ' > + clear_config other2 && > + git pull --set-upstream upstream master:other2 && > + check_config_missing other2 > +' > + > +test_expect_success 'pull --set-upstream upstream other sets branch master' ' > + clear_config master other && > + git pull --set-upstream upstream other && > + check_config master upstream refs/heads/other && > + check_config_missing other > +' > + > +test_expect_success 'pull --set-upstream upstream tag does not set the tag' ' > + clear_config three && > + git pull --tags --set-upstream upstream three && > + check_config_missing three > +' > + > +test_expect_success 'pull --set-upstream http://nosuchdomain.example.com fails with invalid url' ' > + # master explicitly not cleared, we check that it is not touched from previous value > + clear_config other other2 three && > + test_must_fail git pull --set-upstream http://nosuchdomain.example.com && > + check_config master upstream refs/heads/other && > + check_config_missing other && > + check_config_missing other2 && > + check_config_missing three > +' > + > +test_expect_success 'pull --set-upstream upstream HEAD sets branch HEAD' ' > + clear_config master other && > + git pull --set-upstream upstream HEAD && > + check_config master upstream HEAD && > + git checkout other && > + git pull --set-upstream upstream HEAD && > + check_config other upstream HEAD > +' > + > +test_expect_success 'pull --set-upstream upstream with more than one branch does nothing' ' > + clear_config master three && > + git pull --set-upstream upstream master three && > + check_config_missing master && > + check_config_missing three > +' > + > +test_expect_success 'pull --set-upstream with valid URL sets upstream to URL' ' > + clear_config master other other2 && > + git checkout master && > + url="file://'"$PWD"'" && > + git pull --set-upstream "$url" && > + check_config master "$url" HEAD && > + check_config_missing other && > + check_config_missing other2 > +' > + > +test_expect_success 'pull --set-upstream with valid URL and branch sets branch' ' > + clear_config master other other2 && > + git checkout master && > + url="file://'"$PWD"'" && > + git pull --set-upstream "$url" master && > + check_config master "$url" refs/heads/master && > + check_config_missing other && > + check_config_missing other2 > +' > + > +test_done > -- > 2.20.1.98.gecbdaf0 >
Matthieu Moy <git@matthieu-moy.fr> writes: > From: Corentin BOMPARD <corentin.bompard@etu.univ-lyon1.fr> > > Add the --set-upstream option to git pull/fetch > which lets the user set the upstream configuration > (branch.<current-branch-name>.merge and > branch.<current-branch-name>.remote) for the current branch. > > A typical use-case is: > > git clone http://example.com/my-public-fork > git remote add main http://example.com/project-main-repo > git pull --set-upstream main master > > or, instead of the last line: > > git fetch --set-upstream main master > git merge # or git rebase > > This functionality is analog to push --set-upstream. I was writing a one-paragraph summary for this topic, for the "What's cooking" report, and here is what I have: "git fetch" learned "--set-upstream" option to help those who first clone from a forked repository they intend to push to, add the true upstream via "git remote add" and then "git fetch" from it. After describing it like so, I cannot shake the feeling that the workflow this intends to support feels somewhat backwards and suboptimal. - Unless you rely on server-side "fork" like GitHub does, you would first clone from the upstream, and then push to your "fork". The flow whose first step is to clone from your "fork", not from the true upstream, feels backwards (cloning from upstream then adding your fork as a secondary may be more natural, without need for the complexity of --set-upstream to pull/fetch/push, no?). - The second step adds the true upstream using "git remote", and at that point, in your mind you are quite clear that you want to pull from there (and push to your own fork). Not having the "I am adding this new remote; from now on, it is my upstream" feature at this step, and instead having to say that with your first "git pull", feels backwards. If this feature were instead added to "git remote", then the last step in your example does not even have to say "main" (and no need for this new option), does it?
Junio C Hamano <gitster@pobox.com> writes: > Matthieu Moy <git@matthieu-moy.fr> writes: > >> From: Corentin BOMPARD <corentin.bompard@etu.univ-lyon1.fr> >> >> Add the --set-upstream option to git pull/fetch >> which lets the user set the upstream configuration >> (branch.<current-branch-name>.merge and >> branch.<current-branch-name>.remote) for the current branch. >> >> A typical use-case is: >> >> git clone http://example.com/my-public-fork >> git remote add main http://example.com/project-main-repo >> git pull --set-upstream main master >> >> or, instead of the last line: >> >> git fetch --set-upstream main master >> git merge # or git rebase >> >> This functionality is analog to push --set-upstream. > > I was writing a one-paragraph summary for this topic, for the > "What's cooking" report, and here is what I have: > > "git fetch" learned "--set-upstream" option to help those who first > clone from a forked repository they intend to push to, add the true > upstream via "git remote add" and then "git fetch" from it. > > After describing it like so, I cannot shake the feeling that the > workflow this intends to support feels somewhat backwards and > suboptimal. > > - Unless you rely on server-side "fork" like GitHub does, Note that these days, this is not a very restrictive statement ;-). And when you make a fork on GitHub or GitLab from the web UI, the next thing you see is the page of your fork with a button "clone or download" pointing to your local copy's URL. So even though there are arguments to clone upstream first, it's also quite natural from the UI point of view to clone the local copy, and add upstream when needed. > you would first clone from the upstream, and then push to your > "fork". The flow whose first step is to clone from your "fork", not > from the true upstream, feels backwards (cloning from upstream then > adding your fork as a secondary may be more natural, without need for > the complexity of --set-upstream to pull/fetch/push, no?). To me, it depends on the involvement in the project. If I plan to send several contributions to a project, I'd usually clone the upstream and add my fork. But I also often do: - Find a project on GitHub/GitLab/... - Think about a minor contribution I can make - Fork from the web UI - clone my fork - code, commit, push - make a PR Only if my PR takes time to get accepted, I'll add upstream as a remote and pull from there to rebase my PR. > - The second step adds the true upstream using "git remote", and at > that point, in your mind you are quite clear that you want to > pull from there (and push to your own fork). Not having the "I > am adding this new remote; from now on, it is my upstream" Note that you can also group "remote add" and "pull" by saying just git pull --set-upstream http://example.com/project-main-repo master (I still tend to prefer the "remote add" + "pull" flow to name the remote, though). > feature at this step, and instead having to say that with your > first "git pull", feels backwards. If this feature were instead > added to "git remote", then the last step in your example does > not even have to say "main" (and no need for this new option), > does it? There's already "git remote add --track <branch> <remote> <url>", but it does something different: it does not set the upstream information but only sets the glob refspec to fetch only one branch from the remote. We could add a new option like git remote --set-upstream <branch> <remote> <url> That would do git remote add <remote> <url> git branch --set-upstream-to=<branch> That wouldn't make the commands really easier to type IMHO, as you would still have to pull at some point, so it's: git remote add main http://example.com/project-main-repo git pull --set-upstream main master Vs git remote add --set-upstream master main http://example.com/project-main-repo git pull The second is a bit shorter (saves the second instance of "master"), but I tend to prefer the first to avoid the overly long "git remote add" command. Also, if one has several local branches, one may run just one "git remote add" and several "git pull --set-upstream". Note that there are other possible use-cases, like "upstream was using a flow where 'master' was the main branch, but now commits to 'develop' branch and only merges to 'master' for releases", where you can just git pull --set-upstream origin develop Actually, since "--set-upstream" means "next time, *pull* from this branch", it felt weird to have it in "git *push*" and not in "git pull". Certainly, not having "git pull --set-upstream" it "git pull" wasn't that much bothering (otherwise, someone would have implemented it long ago), but I still find it a nice-to-have shortcut. Cheers,
Pratyush Yadav <me@yadavpratyush.com> writes: > This is not really a review. Just some minor nitpicks I spotted while > reading through. Thanks for the comments. >> -static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity, deepen_relative; >> +static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity, deepen_relative, set_upstream; > > This line is getting pretty long. I think it is a good idea to split it > into two. Indeed, and it was already >80 characters, I've split it. >> + if (set_upstream) { >> + struct branch *branch = branch_get("HEAD"); >> + struct ref *rm; >> + struct ref *source_ref = NULL; >> + >> + /* >> + * We're setting the upstream configuration for the current branch. The >> + * relevent upstream is the fetched branch that is meant to be merged with >> + * the current one, i.e. the one fetched to FETCH_HEAD. >> + * >> + * When there are several such branches, consider the request ambiguous and >> + * err on the safe side by doing nothing and just emit a warning. >> + */ > > The comment lines cross the 80 column boundary. The usual convention in > this project is to try to keep lines below 80 columns. For strings IMO > an exception can be allowed because breaking them up makes it harder to > grep for them. But comments are the easiest to format. > > Are you using a tab size of 4? I'm not, but it's possible that the original authors had. Anyway, I've wrapped it. >> + for (rm = ref_map; rm; rm = rm->next) { >> + if (!rm->peer_ref) { >> + if (source_ref) { >> + warning(_("multiple branch detected, incompatible with --set-upstream")); >> + goto skip; >> + } else { >> + source_ref = rm; >> + } >> + } >> + } >> + if (source_ref) { >> + if (!strcmp(source_ref->name, "HEAD") || > > This line has a trailing space. Fixed. > So this should change to something like: > > install_branch_config(0, branch->name, > transport->remote->name, > source_ref->name); I've added a newline after the comma, I don't like mixing "several arguments on the same line" and "one argument per line". > > Maybe this discrepancy is because you are using the wrong tab size? > >> + } else if (starts_with(source_ref->name, "refs/remotes/")) { >> + warning(_("not setting upstream for a remote remote-tracking branch")); >> + } else if (starts_with(source_ref->name, "refs/tags/")) { >> + warning(_("not setting upstream for a remote tag")); >> + } else { >> + warning(_("unknown branch type")); >> + } > > No need to wrap single line if statements in braces. Fixed. >> +#tests for fetch --set-upstream > > Add a space after the '#'. Same in other comments below. Fixed. Thanks. Version 2 fixing all these follows.
Matthieu Moy <Matthieu.Moy@matthieu-moy.fr> writes: > To me, it depends on the involvement in the project. If I plan to send > several contributions to a project, I'd usually clone the upstream and > add my fork. But I also often do: > > - Find a project on GitHub/GitLab/... > - Think about a minor contribution I can make > - Fork from the web UI > - clone my fork > - code, commit, push > - make a PR > > Only if my PR takes time to get accepted, I'll add upstream as a remote > and pull from there to rebase my PR. OK. >> - The second step adds the true upstream using "git remote", and at >> that point, in your mind you are quite clear that you want to >> pull from there (and push to your own fork). Not having the "I >> am adding this new remote; from now on, it is my upstream" > > Note that you can also group "remote add" and "pull" by saying just > > git pull --set-upstream http://example.com/project-main-repo master > > (I still tend to prefer the "remote add" + "pull" flow to name the > remote, though). I do too, and that's where my "shouldn't this feature be part of 'remote add' comes from. > That wouldn't make the commands really easier to type IMHO, as you would > still have to pull at some point, so it's: > > git remote add main http://example.com/project-main-repo > git pull --set-upstream main master > > Vs > > git remote add --set-upstream master main http://example.com/project-main-repo > git pull > > The second is a bit shorter (saves the second instance of "master"), but > I tend to prefer the first to avoid the overly long "git remote add" > command. I do not particularly care about five extra keystrokes. The reason I prefer the latter more is conceptual clarity of it saying "I use 'remote' to set things up, and then use 'pull' to get updated" (as opposed to "I use 'remote' to set things half-way up, and then use the first 'pull' to finish setting things up and getting updated. I should remember that I do not need to give --set-upstream to later 'pull' I used to get further updates"). > Actually, since "--set-upstream" means "next time, *pull* from this > branch", it felt weird to have it in "git *push*" and not in "git pull". > Certainly, not having "git pull --set-upstream" it "git pull" wasn't > that much bothering (otherwise, someone would have implemented it long > ago), but I still find it a nice-to-have shortcut. Yeah, I do not think 'push --set-upstream' is a great feature, either, but since we have it already, I do not mind too much to have another on the 'pull' side. It just feels that we are piling band aid for the lack of the right feature in the right command by adding it to wrong command(s).
Junio C Hamano <gitster@pobox.com> writes: >> That wouldn't make the commands really easier to type IMHO, as you would >> still have to pull at some point, so it's: >> >> git remote add main http://example.com/project-main-repo >> git pull --set-upstream main master >> >> Vs >> >> git remote add --set-upstream master main http://example.com/project-main-repo >> git pull >> >> The second is a bit shorter (saves the second instance of "master"), but >> I tend to prefer the first to avoid the overly long "git remote add" >> command. > > I do not particularly care about five extra keystrokes. The reason > I prefer the latter more is conceptual clarity of it saying "I use > 'remote' to set things up, and then use 'pull' to get updated" (as > opposed to "I use 'remote' to set things half-way up, and then use > the first 'pull' to finish setting things up and getting updated. > I should remember that I do not need to give --set-upstream to later > 'pull' I used to get further updates"). That's a good argument to add a similar feature to "git remote", and it's a good idea for a microproject in the future actually. I admit I didn't consider this possibility before this discussion, thanks. I think I'll still appreciate having the possibility to "pull --set-upstream" too: * "git remote add" is ran once for a remote, "git pull --set-upstream" can be run several times for several branches. * In practice, even when "remote add" supports "--set-upstream", I'll very likely forget it, and by the time I run "git pull", it'll be too late to add --set-upstream to my "remote add" command.
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index 3c9b4f9e09..99df1f3d4e 100644 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@ -169,6 +169,13 @@ ifndef::git-pull[] Disable recursive fetching of submodules (this has the same effect as using the `--recurse-submodules=no` option). +--set-upstream:: + If the remote is fetched successfully, pull and add upstream + (tracking) reference, used by argument-less + linkgit:git-pull[1] and other commands. For more information, + see `branch.<name>.merge` and `branch.<name>.remote` in + linkgit:git-config[1]. + --submodule-prefix=<path>:: Prepend <path> to paths printed in informative messages such as "Fetching submodule foo". This option is used diff --git a/builtin/fetch.c b/builtin/fetch.c index 717dd14e89..5557ae1c04 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -23,6 +23,7 @@ #include "packfile.h" #include "list-objects-filter-options.h" #include "commit-reach.h" +#include "branch.h" #define FORCED_UPDATES_DELAY_WARNING_IN_MS (10 * 1000) @@ -50,7 +51,7 @@ static int fetch_prune_tags_config = -1; /* unspecified */ static int prune_tags = -1; /* unspecified */ #define PRUNE_TAGS_BY_DEFAULT 0 /* do we prune tags by default? */ -static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity, deepen_relative; +static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity, deepen_relative, set_upstream; static int progress = -1; static int enable_auto_gc = 1; static int tags = TAGS_DEFAULT, unshallow, update_shallow, deepen; @@ -123,6 +124,8 @@ static struct option builtin_fetch_options[] = { OPT__VERBOSITY(&verbosity), OPT_BOOL(0, "all", &all, N_("fetch from all remotes")), + OPT_BOOL(0, "set-upstream", &set_upstream, + N_("set upstream for git pull/fetch")), OPT_BOOL('a', "append", &append, N_("append to .git/FETCH_HEAD instead of overwriting")), OPT_STRING(0, "upload-pack", &upload_pack, N_("path"), @@ -1367,6 +1370,49 @@ static int do_fetch(struct transport *transport, retcode = 1; goto cleanup; } + + if (set_upstream) { + struct branch *branch = branch_get("HEAD"); + struct ref *rm; + struct ref *source_ref = NULL; + + /* + * We're setting the upstream configuration for the current branch. The + * relevent upstream is the fetched branch that is meant to be merged with + * the current one, i.e. the one fetched to FETCH_HEAD. + * + * When there are several such branches, consider the request ambiguous and + * err on the safe side by doing nothing and just emit a warning. + */ + for (rm = ref_map; rm; rm = rm->next) { + if (!rm->peer_ref) { + if (source_ref) { + warning(_("multiple branch detected, incompatible with --set-upstream")); + goto skip; + } else { + source_ref = rm; + } + } + } + if (source_ref) { + if (!strcmp(source_ref->name, "HEAD") || + starts_with(source_ref->name, "refs/heads/")) { + install_branch_config(0, branch->name, + transport->remote->name, + source_ref->name); + } else if (starts_with(source_ref->name, "refs/remotes/")) { + warning(_("not setting upstream for a remote remote-tracking branch")); + } else if (starts_with(source_ref->name, "refs/tags/")) { + warning(_("not setting upstream for a remote tag")); + } else { + warning(_("unknown branch type")); + } + } else { + warning(_("no source branch found.\n" + "you need to specify exactly one branch with the --set-upstream option.")); + } + } + skip: free_refs(ref_map); /* if neither --no-tags nor --tags was specified, do automated tag diff --git a/builtin/pull.c b/builtin/pull.c index f1eaf6e6ed..d25ff13a60 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -129,6 +129,7 @@ static char *opt_refmap; static char *opt_ipv4; static char *opt_ipv6; static int opt_show_forced_updates = -1; +static char *set_upstream; static struct option pull_options[] = { /* Shared options */ @@ -243,6 +244,9 @@ static struct option pull_options[] = { PARSE_OPT_NOARG), OPT_BOOL(0, "show-forced-updates", &opt_show_forced_updates, N_("check for forced-updates on all updated branches")), + OPT_PASSTHRU(0, "set-upstream", &set_upstream, NULL, + N_("set upstream for git pull/fetch"), + PARSE_OPT_NOARG), OPT_END() }; @@ -556,6 +560,8 @@ static int run_fetch(const char *repo, const char **refspecs) argv_array_push(&args, "--show-forced-updates"); else if (opt_show_forced_updates == 0) argv_array_push(&args, "--no-show-forced-updates"); + if (set_upstream) + argv_array_push(&args, set_upstream); if (repo) { argv_array_push(&args, repo); diff --git a/t/t5553-set-upstream.sh b/t/t5553-set-upstream.sh new file mode 100755 index 0000000000..bd1a94f494 --- /dev/null +++ b/t/t5553-set-upstream.sh @@ -0,0 +1,178 @@ +#!/bin/sh + +test_description='"git fetch/pull --set-upstream" basic tests.' +. ./test-lib.sh + +check_config () { + printf "%s\n" "$2" "$3" >"expect.$1" && + { + git config "branch.$1.remote" && git config "branch.$1.merge" + } >"actual.$1" && + test_cmp "expect.$1" "actual.$1" +} + +check_config_missing () { + test_expect_code 1 git config "branch.$1.remote" && + test_expect_code 1 git config "branch.$1.merge" +} + +clear_config () { + for branch in "$@"; do + test_might_fail git config --unset-all "branch.$branch.remote" + test_might_fail git config --unset-all "branch.$branch.merge" + done +} + +ensure_fresh_upstream () { + rm -rf parent && git init --bare parent +} + +test_expect_success 'setup bare parent fetch' ' + ensure_fresh_upstream && + git remote add upstream parent +' + +test_expect_success 'setup commit on master and other fetch' ' + test_commit one && + git push upstream master && + git checkout -b other && + test_commit two && + git push upstream other +' + +#tests for fetch --set-upstream + +test_expect_success 'fetch --set-upstream does not set upstream w/o branch' ' + clear_config master other && + git checkout master && + git fetch --set-upstream upstream && + check_config_missing master && + check_config_missing other +' + +test_expect_success 'fetch --set-upstream upstream master sets branch master but not other' ' + clear_config master other && + git fetch --set-upstream upstream master && + check_config master upstream refs/heads/master && + check_config_missing other +' + +test_expect_success 'fetch --set-upstream upstream other sets branch other' ' + clear_config master other && + git fetch --set-upstream upstream other && + check_config master upstream refs/heads/other && + check_config_missing other +' + +test_expect_success 'fetch --set-upstream master:other does not set the branch other2' ' + clear_config other2 && + git fetch --set-upstream upstream master:other2 && + check_config_missing other2 +' + +test_expect_success 'fetch --set-upstream http://nosuchdomain.example.com fails with invalid url' ' + # master explicitly not cleared, we check that it is not touched from previous value + clear_config other other2 && + test_must_fail git fetch --set-upstream http://nosuchdomain.example.com && + check_config master upstream refs/heads/other && + check_config_missing other && + check_config_missing other2 +' + +test_expect_success 'fetch --set-upstream with valid URL sets upstream to URL' ' + clear_config other other2 && + url="file://'"$PWD"'" && + git fetch --set-upstream "$url" && + check_config master "$url" HEAD && + check_config_missing other && + check_config_missing other2 +' + +#tests for pull --set-upstream + +test_expect_success 'setup bare parent pull' ' + git remote rm upstream && + ensure_fresh_upstream && + git remote add upstream parent +' + +test_expect_success 'setup commit on master and other pull' ' + test_commit three && + git push --tags upstream master && + test_commit four && + git push upstream other +' + +test_expect_success 'pull --set-upstream upstream master sets branch master but not other' ' + clear_config master other && + git pull --set-upstream upstream master && + check_config master upstream refs/heads/master && + check_config_missing other +' + +test_expect_success 'pull --set-upstream master:other2 does not set the branch other2' ' + clear_config other2 && + git pull --set-upstream upstream master:other2 && + check_config_missing other2 +' + +test_expect_success 'pull --set-upstream upstream other sets branch master' ' + clear_config master other && + git pull --set-upstream upstream other && + check_config master upstream refs/heads/other && + check_config_missing other +' + +test_expect_success 'pull --set-upstream upstream tag does not set the tag' ' + clear_config three && + git pull --tags --set-upstream upstream three && + check_config_missing three +' + +test_expect_success 'pull --set-upstream http://nosuchdomain.example.com fails with invalid url' ' + # master explicitly not cleared, we check that it is not touched from previous value + clear_config other other2 three && + test_must_fail git pull --set-upstream http://nosuchdomain.example.com && + check_config master upstream refs/heads/other && + check_config_missing other && + check_config_missing other2 && + check_config_missing three +' + +test_expect_success 'pull --set-upstream upstream HEAD sets branch HEAD' ' + clear_config master other && + git pull --set-upstream upstream HEAD && + check_config master upstream HEAD && + git checkout other && + git pull --set-upstream upstream HEAD && + check_config other upstream HEAD +' + +test_expect_success 'pull --set-upstream upstream with more than one branch does nothing' ' + clear_config master three && + git pull --set-upstream upstream master three && + check_config_missing master && + check_config_missing three +' + +test_expect_success 'pull --set-upstream with valid URL sets upstream to URL' ' + clear_config master other other2 && + git checkout master && + url="file://'"$PWD"'" && + git pull --set-upstream "$url" && + check_config master "$url" HEAD && + check_config_missing other && + check_config_missing other2 +' + +test_expect_success 'pull --set-upstream with valid URL and branch sets branch' ' + clear_config master other other2 && + git checkout master && + url="file://'"$PWD"'" && + git pull --set-upstream "$url" master && + check_config master "$url" refs/heads/master && + check_config_missing other && + check_config_missing other2 +' + +test_done