On Thu, 2013-04-11 at 10:37 -0700, Junio C Hamano wrote:
> Carlos Martín Nieto <cmn@xxxxxxxx> writes:
>
> > I can't quite decide whether the behaviour of 'git pull' with no
> > upstream configured but a default remote with no fetch refspecs
> > merging the remote's HEAD is a feature, a bug or something in between,
> > but it's used by t7409 so maybe someone else is using it and we
> > shouldn't break it.
>
> Isn't it the simplest "works without any configuration" from the
> original days?
I don't recall remotes not having refspecs when they're int he config,
though I guess it's equivalent to running 'git pull
git://example.org/myrepo.git'.
>
> > There's another check that could be made earlier ('git pull
> > someremote' when that's not the branch's upstream remote), but then
> > you have to start figuring out what the flags to fetch are.
>
> When the user gave us explicitly the name of the remote, it does not
> sound too bad to fetch from there. "git pull someremote thatbranch"
> can be given after seeing a failure and succeed without retransfer,
> no?
It's not too bad, though you're paying for connection and ref
advertisement twice which breaks the otherwise quick pace of git
commands.
What I find bad from a UI point of view is that after fetching (which
could even be from the wrong remote for 'git pull' w/o upstream info)
git turns around and says "I was never going to merge/rebase that" for
things that we can know before fetching because they depend solely on
the configuration.
>
> I am not sure if it is worth the added complexity and potential to
> introduce new bugs in general by trying to outsmart the for-merge
> logic that kicks in only after we learn what the other side offers
> and fetch from it, but anyway, let's see what we got here...
>
> > diff --git a/git-pull.sh b/git-pull.sh
> > index 266e682..b62f5d3 100755
> > --- a/git-pull.sh
> > +++ b/git-pull.sh
> > @@ -43,6 +43,8 @@ log_arg= verbosity= progress= recurse_submodules=
> > merge_args= edit=
> > curr_branch=$(git symbolic-ref -q HEAD)
> > curr_branch_short="${curr_branch#refs/heads/}"
> > +upstream=$(git config "branch.$curr_branch_short.merge")
> > +remote=$(git config "branch.$curr_branch_short.remote")
> > rebase=$(git config --bool branch.$curr_branch_short.rebase)
>
> Learning these upfront sounds sensible.
>
> > if test -z "$rebase"
> > then
> > @@ -138,6 +140,47 @@ do
> > esac
> > shift
> > done
> > +if test true = "$rebase"
> > +then
> > + op_type=rebase
> > + op_prep=against
> > +else
> > + op_type=merge
> > + op_prep=with
> > +fi
> > +
> > +check_args_against_config () {
> > + # If fetch gets user-provided arguments, the user is
> > + # overriding the upstream configuration, so we have to wait
> > + # for fetch to do its work to know if we can merge.
> > + if [ $# -gt 0 ]; then
> > + return
> > + fi
>
> > + # Figure out what remote we're going to be fetching from
> > + use_remote=origin
> > + if [ -n "$remote" ]; then
> > + use_remote="$remote"
> > + fi
> > +
> > + # If the remote doesn't have a fetch refspec, then we'll merge
> > + # whatever fetch marks for-merge, same as above.
>
> The "above" in this sentence refers to...?
>
> I guess "we have to wait", but it wasn't very clear.
>
Yes, it refers to having to wait for fetch to complete before we can
know if we'll be able to merge.
> > + fetch=$(git config --get-all "remote.$use_remote.fetch")
> > + if [ -z "$fetch" ]; then
> > + return
> > + fi
>
> Hmm, it is probably correct to punt on this case, but it defeats
> large part of the effect of your effort, doesn't it? We fetch what
> is covered by remote.$name.fetch _and_ what need to complete the
> merge operation (otherwise branch.$name.merge that is not covered by
> remote.$there.fetch will not work). So
>
> [remote "origin"]
> url = $over_there
> [branch "master"]
> remote = origin
> merge = refs/heads/master
>
> would still fetch refs/heads/master from there and merge it.
If you run 'git pull' in this situation, then everything's fine and the
right thing gets merged.
>
> > + # The typical 'git pull' case where it should merge from the
> > + # current branch's upstream. We can already check whether we
> > + # we can do it. If HEAD is detached or there is no upstream
> > + # branch, complain now.
>
> Drop "typical", and rephrase "merge from" to also cover "rebase" (I
> often say "integrate with").
Sounds good.
>
> To return to your original description:
>
> A 'git pull' without specifying a remote is asked to take the
> current branch's upstream as the branch to merge from. This
> cannot work without an upstream configuration nor with HEAD
> detached, but we only check for this after fetching.
>
> Wouldn't it be sufficient to add something like this before fetch
> happens:
>
> if test $# != 0 || # args explicitly specified
> test -n "$curr_branch" || # not detached
> test -n "$upstream" # what to integrate with is known
> then
> return ;# then no problem
> fi
> die "underspecified 'git pull'"
>
> without changing anything else? For that matter, $upstream is
> likely to be empty when detached, so the second test may not even be
> necessary.
>
I'm not sure if this allows us to print out the help message about
missing upstream configuration in the right case. I'll test.
cmn
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html