Re: linux-next: unneeded merge in the security tree

[ Added Junio and git to the recipients, and leaving a lot of stuff quoted due to that... ]

On Mon, Mar 11, 2013 at 9:16 PM, Theodore Ts'o <tytso@mit.edu> wrote:

Now, quite frankly, I'd prefer people not merge -rc tags either, just real releases. -rc tags are certainly *much* better than merging random daily stuff, but the basic rule should be "don't back-merge AT ALL" rather than "back-merge tags".

That said, you didn't really want a merge at all, you just wanted to sync up and start development. Which is different (but should still prefer real releases, and only use rc tags if it's fixing stuff that happened in the merge window - which may be the case here).

So I think that may *technically* be the right solution, but it's a rather annoying UI issue, partly because you can't just do it in a single operation (you can't do a pull of the tag to both fetch and fast-forward it), but partly because "git reset --hard" is also an operation that can lose history, so it's something that people should be nervous about, and shouldn't use as some kind of standard "let's just fast-forward to Linus' tree" thing.

At the same time, it's absolutely true that when *I* pull a signed tag from a downstream developer, I don't want a fast-forward, because then I'd lose the signature. So when a maintainer pulls a submaintainer tree, you want the signature to come upstream, but when a submaintainer wants to just sync up with upstream, you don't want to generate the pointless signed merge commit, because the signature is already upstream because it's a public tag. So gthe behavior of "git pull" is fundamentally ambiguous.

But git doesn't know the difference between "official public upstream tag" and "signed tag used to verify the pull request".

I'm adding the git list just to get this issue out there and see if people have any ideas. I've got a couple of workarounds, but they aren't wonderful..

One is simple:

    git config alias.sync="pull --ff-only"

which works fine, but forces submaintainers to be careful when doing things like this, and using a special command to do back-merges.

And maybe that's the right thing to do? Back-merges *are* special, after all. But the above alias is particularly fragile, in that there's both "pull" and "merge" that people want to use this for, and it doesn't really handle both. And --ff-only will obviously fail if you actually have some work in your tree, and want to do a real merge, so then you have to do that differently. So I'm mentioning this as a better model than "git reset", but not really a *solution*.

That said, the fact that "--ff-only" errors out if you have local development may actually be a big bonus - because you really shouldn't do merges at all if you have local development, but syncing up to my tree if you don't have it (and are going to start it) may be something reasonable.

Now, the other approach - and perhaps preferable, but requiring actual changes to git itself - is to do the non-fast-forward merge *only* for FETCH_HEAD, which already has magic semantics in other ways. So if somebody does

    git fetch linus
    git merge v3.8

to sync with me, they would *not* get a merge commit with a signature, just a fast-forward. But if you do

    git pull linus v3.8

or a

    git fetch linus v3.8
    git merge FETCH_HEAD

it would look like a "maintainer merge" and stash the signature in the merge commit rather than fast-forward. It would probably work in practice.

The final approach might be to make it like the "merge summary" and simply make it configurable _and_ have a command line flag for it, defaulting to our current behavior or to the above suggested "default on for FETCH_HEAD, off for anything else".

Hmm?

            Linus

On Tue, Mar 12, 2013 at 6:13 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote:

In many cases, "git rebase x" does the exact same thing as "git reset --hard x", with an added safeguard: if you forgot to upstream something, it'll boil up on top of "x".

Gr{oetje,eeting}s,

                        Geert

-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Linus Torvalds <torvalds@linux-foundation.org> writes:

Heh, I just wrote that myself after reading the early part of this message ;-)

Yes.

Yes, that's the reasoning behind all the behaviours you described above.

I am not sure I follow. Are you solving the real problem, the pointeless merge in the "security tree" that started this thread?

I would imagine it was made by somebody thinking that pulling a tagged stable point from you is a good idea, like this:

	git pull linus v3.9-rc2

which under your FETCH_HEAD rule would look like a maintainer merge, no?

An alternative may be to activate the magic "mergetag" thing only when you give "--no-ff" explicitly; otherwise merge would unwrap the tag, whether it comes from FETCH_HEAD.

The following examples all assume that your HEAD is somewhere behind v3.9-rc2, perhaps done by

	git checkout -b test v3.8^0

Then under the "--no-ff activates the magic" rule:

	git merge v3.9-rc2

will fast-forward, but this

	git merge --no-ff v3.9-rc2

creates a real merge with the "mergetag" signature block. The one that caused trouble in the "security tree", i.e.

        git pull linus v3.9-rc2

or its equivalent

        git fetch linus v3.9-rc2
        git merge FETCH_HEAD

would still fast-forward under this rule. The maintainer needs to do

	git pull --no-ff git://git.kernel.org/... for-linus

if the pull could fast-forward under this rule, though.

Having thought this up to this point, I am not sure it generally is a good change. It feels that "pull --ff-only" that prevents people from creating pointless back-merges may still be a better mechanism.

I dunno.

Junio C Hamano <gitster@pobox.com> writes:

Scratch the last sentence.  It should have been

"whether the pull fast-forwards or not".  You'd always need to.

What if we added the ability to do something like this:

[remote "origin"]
	url = git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
	fetch = +refs/heads/master:refs/heads/master
	mergeoptions = --ff-only

This would be an analog to branch.<name>.mergeoptions, but it would apply to the source of the pull request, instead of the destination.

That way, people who do a "git pull" from Linus's tree would get the protection of --ff-only, while pulls from submaintainer trees would automatically get a merge commit, which is what we want.

It doesn't handle the case of a submaintainer pulling from a maintainer in a back-merge scenario, but that should be a pretty rare case, so maybe that's OK.

      	     	       	   	- Ted

On Tue, Mar 12, 2013 at 2:20 PM, Theodore Ts'o <tytso@mit.edu> wrote:

Hmm. Something like this could be interesting for other things:

 - use "--rebase" when pulling (this is common for people who maintain
a set of patches and do *not* export their git tree - I use it for
projects like git and subsurface where there is an upstream maintainer
and I usually send patches by email rather than git)

 - "--no-summary". As a maintainer, you people probably do want to
enable summaries for people they pull from, but *not* from upstream.
So this might even make sense to do by default when you clone a new
repository.

 - I do think that we might want a "--no-signatures" for the specific
case of merging signed tags without actually taking the signature
(because it's a "upstream" repo). The "--ff-only" thing is *too*
strict. Sometimes you really do want to merge in new code, disallowing
it entirely is tough.

Of course, I'm not really sure if we want to list the flags. Maybe it's better to just introduce the notion of "upstream" directly, and make that a flag, and make "origin" default to that when you clone. And then have git use different heurstics for pulling upstream (like warning by default when doing a back-merge, perhaps?)

                   Linus

Linus Torvalds <torvalds@linux-foundation.org> writes:

I agree that "--ff-only" thing is too strict and sometimes you would want to allow back-merges, but when you do allow such a back-merge, is there a reason you want it to be --no-signatures merge? When a subtree maintainer decides to merge a stable release point from you with a good reason, I do not see anything wrong in recording that the resulting commit _did_ merge what you released with a signature.

On Tue, Mar 12, 2013 at 2:47 PM, Junio C Hamano <gitster@pobox.com> wrote:

No, there's nothing really bad with adding the signature to the merge commit if you do make a merge. It's the fact that it currently makes a non-ff merge when that is pointless that hurts.

That said, adding the signature from an upstream tag doesn't really seem to be hugely useful. I'm not seeing much of an upside, in other words. I'd *expect* that people would pick up upstream tags regardless, no?

           Linus

Linus Torvalds <torvalds@linux-foundation.org> writes:

Yes, their "git fetch" will auto-follow, but mergetag embedded in the commit objects will give the history auditable trail the same way as the merges you make your downstream. You of course could match out-of-line tags against back-merges and verify your merges with mergetags, but you do not have to.

Theodore Ts'o <tytso@mit.edu> writes:

Is there an escape hatch for that rare case? IOW, how does a submaintainer who configured the above to override --ff-only?

On Tue, Mar 12, 2013 at 02:30:04PM -0700, Junio C Hamano wrote:

Hmm, maybe we would need to add a --no-ff-only? Or they could just do:

	git fetch origin
	git merge FETCH_HEAD

On Tue, Mar 12, 2013 at 02:28:39PM -0700, Linus Torvalds wrote:

What if git automaticallly set up the origin branch to have a certain set of mergeoptions by default? That would probably be right for most users, but it makes it obvious what's going on when they take a look at the .git/config file, and doesn't make the remote that happens to have the name "origin" as having certain magic properties. Using a set of mergeoptions would also be bit more general, and might have applications in the future.

     	      	       	  	 	       - Ted

Theodore Ts'o <tytso@mit.edu> writes:

Hmm, neither feel quite nice.

I haven't heard any comments on my alternative proposal, by the way. Did the message get lost?