Re: [PATCH 0/3] Reject non-ff pulls by default

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Sep 9, 2013, 00:29 UTC
Message-ID: <CAMP44s2seqO_0o=G2PjoL77HNSNcjTe4s6ZYj90_wsUT30pW8A@mail.gmail.com>
In-Reply-To: <20130909000153.GG5359@vauxhall.crustytoothpaste.net>

On Sun, Sep 8, 2013 at 7:01 PM, brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 19 quoted lines

> On Sun, Sep 08, 2013 at 05:38:50PM -0500, Felipe Contreras wrote:
>> Yeah, but the key question at hand in this discussion is; what happens
>> when 'git pull' stops working for them, and they don't know what to
>> do, will they choose 'git pull --rebase' by mistake?
>
> I agree, they will not choose git pull --rebase by mistake.
>
>> I say the answer is no, because:
>>
>> 1) As you say in your scenario, somebody is telling these guys what to
>> do, so when 'git pull' fails, somebody will figure out that they were
>> doing a merge, so 'git pull --merge' is what they want to type from
>> now on.
>
> Yes, that would be me.  My hesitance here is that as the one usually
> driving git updates (which so far have happened once a year), I will end
> up retraining forty developers.  I don't think the current behavior is
> broken or really problematic at all: merging has always been the
> default, and people have come to expect that.

It may not be broken for you, but it is for other people. Would you be so egocentric as to ignore everybody else because "it works for you"?

> People using workflows
> that don't want merge have always either needed to set a configuration
> option or use --rebase.  As the man page says, --rebase is unsafe, and
> that's why it's not the default.

Yes, but the problem is that people using other workflows end up avoiding 'git pull' at all, so at the end of the day we have one core command that the majority of users avoid, that's not good.

> I would be much less unhappy with your earlier change that did not
> affect uses with arguments.  That would limit the number of use cases
> affected.

I have no problem with: git pull $remote $branch

Allowing non-fast-forward merges.

And: git pull $remote git pull

Not allowing them by default.

But the problem is that it's not easy to implement.

Either way, I'll venture that you don't want 'git pull $remote' to change, so it would be a waste of the time to try to get the above to work.

Show 9 quoted lines

>> 2) Git itself would be warning them for months that a 'non
>> fast-forward was found, and a merge will be done for them', so when
>> the warning turns to an error, they'll know they want a merge, so
>> they'll do 'git pull --merge', either because the warning told them
>> that's git was doing all along, or because they figured that out by
>> googling, or reading the man page, or whatever.
>
> Again, you assume that git updates happen on a regular basis, and you
> assume that most developers really know what happens under the hood.

No. The developers don't have to know what happens under the hood, Git would be telling them "WARNING: we are doing a merge", what else is the developer to think, but that 'git pull' is doing a merge?

As for the updates, yes, I assume updates happen at least each three months. If your company updates each year, I don't see what much more we can do to you help you. Doing a single change per year is certainly going to hold the project back.

Fortunately this was only point 2), there's still point 1); you can tell them to use 'git pull --merge' from now on, and since you update once every year, you can do it while you give the training for the year.

Or there's another option:

3) Distribute Git in your company with /etc/gitconfig having pull.mode
= merge. This way nothing will change.

I think we are being very accommodating to your company's use-case which is very far from the norm. Even in the absolute worst case scenario, you would have to tell people to use 'git pull --merge' instead, is that really so horrible? Should we really halt Git's progress because you would have to tell people to type nine extra characters or run one configuration command?

Show 11 quoted lines

> I don't see a warning now; in fact, I see:
>
>   vauxhall ok % git status
>   # On branch master
>   # Your branch and 'upstream/master' have diverged,
>   # and have 1 and 128 different commits each, respectively.
>   #   (use "git pull" to merge the remote branch into yours)
>   #
>
> The current behavior of git is to explicitly encourage this behavior,
> and now you want to make it not work.

Yes, that's why it's a change.

Show 5 quoted lines

> I think this change is a bad
> idea, and I think the number of changes required to the test suite
> indicates that.  If there's going to be a change here, it should have a
> deprecation period with the above message changed and appropriate
> warnings, not a flag day; your patches don't do that.

My patches pretty much do nothing else but introduce a warning. Nothing is broken, nothing is changed in the test suite:

http://article.gmane.org/gmane.comp.version-control.git/233669

You are confusing my proposal with Junio's one.

Also, my proposal was to enable this behavior (pull.mode = merge-ff-only) only for Git v2.0, which might happen probably way later than a year from now, so you your users might actually see the warning after all. But yeah, that's _my_ proposal.

-- 
Felipe Contreras