git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: git@vger.kernel.org, Jens Lehmann <Jens.Lehmann@web.de>
Subject: Re: [PATCH 1/3] fetch: add --allow-local option
Date: Sat, 18 May 2013 22:51:30 -0700	[thread overview]
Message-ID: <7vtxlz1pr1.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <CAMP44s3xdWzVviPvrN7D1fTG6Lwgg-dEzju--VuiwZA-8bV+MQ@mail.gmail.com> (Felipe Contreras's message of "Sat, 18 May 2013 07:25:37 -0500")

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Fri, May 17, 2013 at 1:30 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> This is irrelevant, it's an implementation detail of 'git pull'. *THE
>>> USER* is not running 'git fetch .'
>>
>> To those who fear running "git pull", the following has worked as a
>> quick way to "preview" what they would be getting.
>>
>>         git fetch
>>         git log ..FETCH_HEAD
>>
>> and then they can "git merge FETCH_HEAD" to conclude it, or run a
>> "git pull" for real.  We teach the more explicit form to end users
>> in our tutorial,
>
> That "tutorial" is mostly irrelevant; it has not been properly updated
> in years, and it doesn't do it's job properly.
>
> Nowadays most people use the Pro Git book, which doesn't mention
> FETCH_HEAD even once. And why would it?

Because the book was written by those who did not know about its
use, and I do not necessarily think it is their fault.  Our own
documentation can use updates.

Patches to the tutorial to explain (you can refer to postings by
people who discuss on public "users help other users" site like
stackoverflow) the equivalence of the "git pull" == "git fetch"
followed by "git merge FETCH_HEAD" better than it currently does is
welcome.  IIUC, ProGit is maintained in public and you can send
patches to it, too.

Users of stackoverflow seem to know about and how to use FETCH_HEAD:

    http://stackoverflow.com/questions/9237348/what-does-fetch-head-in-git-mean
    http://stackoverflow.com/questions/11478558/fetch-head-not-updating-after-git-fetch

and they expect exactly what I described in the earlier message.  If
you ask your search engine about "FETCH_HEAD", you would find other
real-world use of it as well (one of them I found was about somebody
requesting to teach TortoiseGit to offer FETCH_HEAD as a candate to
be merged, IIRC).

Incidentally, this discussion on our own list

    http://thread.gmane.org/gmane.comp.version-control.git/42788/focus=42798

shows that I was originally not very keen to defend "fetch, then
inspect with log FETCH_HEAD, and then finally merge" workflow to
keep working myself [*1*], which is somewhat funny.

So, no, it is not my whim, but people do depend on "git fetch" that
updates FETCH_HEAD to what is to be merged with "git pull".

>> So when "the user" is running "git fetch" on "mywork" branch that
>> happens to be forked from a local "master",...
>> we still need to have FETCH_HEAD updated to point at what we would
>> be merging if she did a "git pull".
>
> No, we don't need that. That is only needed by 'git pull', and in
> fact, it should be possible to reimplement 'git pull' so that it skips
> FETCH_HEAD when the remote is local.
>
> These are mere implementation details.

You seem to be incapable to understand what backward compatibility
is.  It is not about "keep only what I use myself, I think others
should use, and/or I understand, working, and destroy everything
else".

Also your "special-case local repository and fetch from 'origin'"
breaks down once your users need to work on a project with subsystem
maintainers.

Imagine one clones from me (i.e. git.git is her upstream), and forks
her fh-octopus branch out of her master branch ("git clone" arranges
the latter to be a fork of origin/master taken from me).  She helps
git-svn and has Eric's repository as her "git-svn" remote (because
my tree lags behind Eric's tree with respect to git-svn).  Her local
git-svn branch is a fork of Eric's master, and she has her svn-ext
branch that is a fork of it [*2*].

    git clone git://git.kernel.org/pub/scm/git/git.git git
    cd git

    git checkout -t -b fh-octopus master
    git remote add git-svn git://git.bogomips.org/git-svn.git/
    git checkout -t -b git-svn git-svn/master
    git checkout -t -b svn-ext git-svn

She has four local branches, master, fh-octopus, git-svn, and
svn-ext.  Is this a too-contrived example?  I do not think so.

On any of these branches, she can say

    git pull [--rebase]

without having to be constantly aware of what branch (either remote
or local) the work on her current branch builds on.  Like some
people in the thread $gmane/42788 discussion, she may prefer to do
the same integration with "first fetch, inspect and then integrate"
workflow, relying on the equivalence "git pull" == "git fetch" +
"git merge/rebase":

    git fetch
    git log ..FETCH_HEAD
    git merge FETCH_HEAD

But when she does this:

    git checkout fh-octopus
    git fetch

the "git fetch" does not fetch from me (i.e. her 'origin').

Would she be unhappy?

I agree that it could be argued so.

After all, she is working on a topic that is eventually based on my
'master' and the only reason she forked from her 'master' is because
she may have other changes of her own on her 'master' that are not
necessarily related to her fh-octopus topic.  She wants her "git log
@{u}.."  not to show those unrelated changes on her 'master', but
wants to rebase against her 'master'.

Because her @{u} does not refer to 'origin/master' taken from me (it
refers to her local 'master'), however, even if we change "git
fetch" to fetch from 'origin', what "git log ..@{u}" shows does not
change, so the only "improvement" is that it does not trigger "I
said 'git fetch' but it did not fetch anything?  Did I make some
mistake?" confusion.

Is that lack of confusion a big enough improvement, or is it easy
enough for her to realize that what she wanted to inspect was what
I've been doing in the meantime and run "git fetch origin"?

I know you would say it is an improvement; I am on the fence in the
sense that no confusion is better than confusion, but that is a
qualified "on the fence".  I do not mind it fixed as long as the
confusion avoidance does not regress other aspects of the operation
we have kept working for people.

But more importantly, what should happen when she is working on her
svn-ext branch?

    git checkout svn-ext
    git fetch

This does not fetch from me (i.e. her 'origin'), either.  If this
fetched from me, would she be happier?

It could be argued that it might be better if it fetched from Eric;
after all she is working on svn-ext that eventually reaches Eric's
tree.  But fetching from me while she is working on svn-ext does not
make any sense at all in her set-up, does it?

As I already said, I am OK with an enhancement that does (an
equivalent of) the following:

 - Teach "git fetch --no-fetch-head <any args>" an option that
   causes it to do everything "git fetch <any args>" would do,
   except that in that mode, it does not touch FETCH_HEAD at all
   (the "--append" option that changes the behaviour of "git fetch"
   to only change what happens to FETCH_HEAD can be thought of a
   precedent).

 - Only when 'git fetch' (with no parameters, i.e. relies on
   configured defaults) is to fetch from '.':

   - First internally fork and run "git fetch --no-fetch-head
     <remote>" from the <remote> found by the following loop:

    - Set a variable B to the name of the current branch 
    - while branch.B.remote is the local repository:
      - assign the branch the branch B forked from,
        i.e. branch.B.merge, to variable B
    - branch.B.remote is the first remote repository in the fork
      ancestry chain.  That is the <remote> we are looking for.

   - Then do the usual 'git fetch' against the local repository,
     leaving what is to be merged in FETCH_HEAD as usual.

 - (optional) make the second "special case" opt-in, as people who
   have known Git may be upset when their "git pull" that they know
   would go to local repository touches network.  I do not think it
   is a huge deal, though, and that is why I marked it as optional.

I think that would alleviate the puzzlement some people may feel
when they run 'git fetch' on a branch that integrates with their
local branch, and sees no objects are transferred.  It would do so
without breaking anything.

Anything that breaks the "pull=fetch+merge" equivalence is not
acceptable and no loud repeating from you will change that, with or
without patches.

We owe at least that much to the people who asked us to keep it
working over time and those who responded to them by working on
maintaining it over time.


[Footnotes]

*1* Back then, FETCH_HEAD did not have the commit to be merged
    necessarily on the first line, so it was not very easy to work
    with it.  But that was fixed by 96890f4c428e (write first
    for-merge ref to FETCH_HEAD first, 2011-12-26), has been with us
    since v1.7.9, and the fix was not made by me, but Joey Hess.


*2* The invariant "git pull <fetch params>" == "git fetch <the same
    fetch params>" + "git merge FETCH_HEAD" (for any <fetch params>,
    including empty) has been true forever, and over time we made
    sure it hold true when we introduced new features because people
    depended on it, with commmits like these:

     - 85295a52e683 (git-merge: Put FETCH_HEAD data in merge commit
       message, 2007-03-22)

     - 96890f4c428e (write first for-merge ref to FETCH_HEAD first,
       2011-12-26)

    A remaining known bug is when the pull is to create an octopus,
    because "git merge FETCH_HEAD" currently can resolve FETCH_HEAD
    as only one commit.  The fh-octopus topic would extend the itch
    these two fixes started to scratch by teaching "git merge" to
    expand FETCH_HEAD into all commits that are not marked as
    not-for-merge to finish scratching that itch.

    svn-ext may teach git-svn to express a svn external as a git
    submodule.

  reply	other threads:[~2013-05-19  5:51 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-16  7:31 [PATCH 0/3] fetch: fix '.' fetching Felipe Contreras
2013-05-16  7:31 ` [PATCH 1/3] fetch: add --allow-local option Felipe Contreras
2013-05-16  8:25   ` Ramkumar Ramachandra
2013-05-16  8:53     ` Felipe Contreras
2013-05-16  9:27       ` Ramkumar Ramachandra
2013-05-16  9:34         ` Felipe Contreras
2013-05-16  9:58           ` Ramkumar Ramachandra
2013-05-16 10:27             ` Felipe Contreras
2013-05-16 10:32               ` Ramkumar Ramachandra
2013-05-16 12:54                 ` Felipe Contreras
2013-05-16 15:58   ` Junio C Hamano
2013-05-16 16:26     ` Felipe Contreras
2013-05-16 16:38       ` Junio C Hamano
2013-05-16 16:52         ` Felipe Contreras
2013-05-16 18:04           ` Junio C Hamano
2013-05-16 23:07             ` Felipe Contreras
2013-05-16 23:24               ` Junio C Hamano
2013-05-17  0:04                 ` Felipe Contreras
2013-05-17 18:30                   ` Junio C Hamano
2013-05-18 12:25                     ` Felipe Contreras
2013-05-19  5:51                       ` Junio C Hamano [this message]
2013-05-19  6:10                         ` Felipe Contreras
2013-05-19  7:56                         ` About overzealous compatibility Felipe Contreras
2013-05-19 14:27                           ` Felipe Contreras
2013-05-18 13:12                     ` [PATCH 1/3] fetch: add --allow-local option Philip Oakley
2013-05-18 14:23                       ` Felipe Contreras
2013-05-18 20:53                         ` Philip Oakley
2013-05-18 22:26                           ` Felipe Contreras
2013-05-19  6:10                       ` Junio C Hamano
2013-05-16  7:31 ` [PATCH 2/3] fetch: switch allow-local off by default Felipe Contreras
2013-05-16  7:31 ` [PATCH 3/3] remote: disable allow-local for pushes Felipe Contreras

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vtxlz1pr1.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=Jens.Lehmann@web.de \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).