diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index d033b25..3a2e0b7 100644
@@ -93,6 +93,13 @@ OPTIONS
has to be called afterwards to bring the work tree up to date with the
+ For each branch that is successfully pulled, add
+ upstream (tracking) reference, used by argument-less
+ linkgit:git-push and other commands. For more information,
+ see 'branch.<name>.merge' in linkgit:git-config.
Options related to merging
If this were multi-patch series and one of the other patches were
"pull: set-upstream design" or something, then it might have been
understandable, but otherwise the word "implementation" sits rather
strangely in the title of this patch.
> Implements pull [--set-upstream | -u] which sets the remote tracking
> branch to the one the user just pulled from.
> git pull [--set-upstream | -u] <remote> <branch>
> After successfully fetched from <remote>, sets branch.<name>.remote to
> <remote> and branch.<name>.merge to follow <remote>/<branch>
> Signed-off-by: Erwan Mathoniere <[hidden email]>
> Signed-off-by: Jordan De Gea <[hidden email]>
> Signed-off-by: Matthieu Moy <[hidden email]>
> `git push --set-upstream <remote> <branch>` sets the remote and merge config
> variables for <branch> after successfully pushed.
> This patch implements an equivalent for `git pull` or
> `git branch --set-upstream-to=<remote>/<branch>`.
> - I can't figure out what remote branch sould be tracked
> in that case: `git pull -u origin :master`
What does the command do without "-u"?
> - Is the result of 'resolve_ref_unsafe' should be freed ?
Check its function signature--it returns "const char *" which is a
sign that the memory does not belong to you (i.e. the caller), and
you should never free it.
> Documentation/git-pull.txt | 7 +++++
> builtin/pull.c | 61 +++++++++++++++++++++++++++++++++++++
> t/t5544-pull-upstream.sh | 75 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 143 insertions(+)
> create mode 100755 t/t5544-pull-upstream.sh
> diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
> index d033b25..3a2e0b7 100644
> --- a/Documentation/git-pull.txt
> +++ b/Documentation/git-pull.txt
> @@ -93,6 +93,13 @@ OPTIONS
> has to be called afterwards to bring the work tree up to date with the
> merge result.
> + For each branch that is successfully pulled, add
> + upstream (tracking) reference, used by argument-less
> + linkgit:git-push and other commands. For more information,
> + see 'branch.<name>.merge' in linkgit:git-config.
To me, "For each branch" hints that I could do this:
git pull --set-upstream git://git.kernel.org/r/e/p/o foo bar
and the command would do something to 'foo' and 'bar'.
But I suspect that is not what is going on and that is not what you
wanted to achieve.
A crucial piece of information that is lacking in the above is where
<name> comes from. The same issue exists in your proposed commit
I think that you meant to add a feature to add branch.<name>.remote
and branch.<name>.merge configuration variables for the current
branch whose name is <name>, and the values to be recorded for these
two configuration variables are the same as those given on the
command line. For example "git checkout -b topic master && git pull
origin that" would set "branch.topic.remote" and
"branch.topic.merge" to "origin" and "that", respectively.
Without explaining what <name> is, "For more information" that
refers to 'branch.<name>.merge' does not help the readers much.
Side note. It is an understandable mistake. After one
spent a lot of effort designing, implementing and debugging
a feature, by the time one describes what it does, some
assumptions one made earlier becomes so fundamental in one's
mind that one forgets that it is not obvious to others.
There a few design decisions you must have made that needs to be
either described fully or at least hinted here, too, such as (not
exhaustive and in random order):
* What should happen when the current branch already has these two
configuration variables defined? Should the user get a warning
when this changes the setting from what was already there?
* What should happen when the remote is specified as a Git URL, not
as a remote nickname? You want a nickname for the value to place
- Should Git find an existing remote that points at the URL and
use it? If so, and if the value we are about to place in
"branch.<name>.merge" is outside its configured fetch refspec,
should Git tweak the fetch refspec of the existing remote?
- Should Git create a new remote nickname for the URL? If so,
what should the fetch refspec be for the newly created remote?
Should it fetch only the branch we fetched just now?
* What should happen when more than one ref is given?
branch_get_upstream() can return only one ref; should Git choose
one of them at random? Should Git warn and turn this into a
* What should happen when the ref given to pull is not a branch
(e.g. a tag)? A tag, meant to be a stable anchoring point, is
not something suitable to be set as branch.<name>.merge, even
though it might be technically possible to configure it as such.
Should Git warn and turn this into a no-op?
I thought you were only setting up configurations for existing
branch. Why do you call create_branch() on the existing
local_branch, which is supposed to be the current one?
Perhaps create_branch() API is too crappy and it needs to be
properly refactored in preparation for this patch. I dunno.
What does it mean for this loop to execute more than once, flipping
the configuration for the current branch number of times? The last
one wins? Does it even make sense from the end-user's point of
... some stuff ... &&
cd elsewhere &&
... more stuff ...
... yet more stuff ...
to make sure that even after any of "... more stuff ..." fails, the
caller of this sequence will find it in an expected and stable
place. As written in your patch, if any of the "... more stuff ..."
fails, the caller will be in "parent" directory, not in the original
directory, because your "cd .." will not be exectued.
I wonder if you are completely missing the point of using a subshell
I think the next test will not work if this test, specifically the
first two steps that creates a new repository "son" and moves into
it, fails. Don't create such an interdependency between tests when
you can avoid it.
Hint. Immediately after creating "parent", in the same test, clone
it into "son". Everybody needs to depend on that "setup bare
parent" test anyway, so that is not making things worse. And then
update this test like so:
test_expect_success 'title' '
cd son &&
git checkout -b headbranch &&
making sure that outside test_expect_success block, the process will
stay at the original directory, no matter which tests fail or get