lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Mon, 16 May 2016 08:43:33 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Al Viro <viro@...iv.linux.org.uk>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [git pull] vfs.git

On Sun, May 15, 2016 at 8:32 PM, Al Viro <viro@...iv.linux.org.uk> wrote:
> FWIW, I considered sending that pile in several pull requests, but for some
> reason git request-pull v4.6 vfs work.lookups spews something very odd into
> diffstat - files that have never been touched by it and, in fact, doing
> merge with mainline does *not* end up with those files anywhere in the
> diff.

This is "normal" if you have multiple merge bases (which in turn
happens various ways, but all of them involved the branch having
merges in itself, either back-merges or just merging two or more topic
branches that had different starting points).

What happens is that request-pull will just pick the first merge base
and then do a diff against that - which can happen to work, but most
of the time you'll get part of the diff being from the differences
that happened betweeen the different merge bases, rather than just the
code that is in "your" branch.

The only way to get a reliably correct diff is to actually do the
merge (which is a much more complex operation when there are multiple
merge bases - git will actually do intermediate merges between the
merge bases and then do a merge based on that - it's why the default
git merge is called a "recursive" merge).

Some maintainers actually do that merge (into a private branch) to get
that reliable end result and also to get a heads-up about any merge
conflicts I would see when pulling. But I don't actually require it,
it's ok if the pull request ends up looking messy due to just using
the plain request-pull machinery without any merging.

The shortlog is always reliable, because that is not a "diff"
operation (which is inherently about two end-points - and with
multiple merge bases there are more than two end-points so "diff" is
inevitably too weak an operation to show the differences). The
shortlog is about the set of commits, and git will do the proper set
operation to see "commits in A but not in B". So if you get surprising
results in the shortlog, then that means that something is actually
wrong in your tree and you should look out.

> Full pile doesn't produce any oddities of that sort, so...

The fact that the full pile doesn't is likely just because the merges
you added, which brought in a new merge base and it may now be unique
(because you merged with my tree as you did them) or it just happens
to be one of the merge points that works by luck for "diff".

> If you prefer that stuff to go in separate pulls, please say so.

This time I'd really prefer it, especially that parallel lookup
branch. That's such a big fundamental change that it definitely merits
being merged separately rather than as part of "here's all the vfs
changes for 4.7"..

As mentioned, you *can* get the diff right by testing a pre-merge, but
you don't need to, and I'll take the messy diff.

Even when it doesn't match the diff after-the-merge, I can trivially
verify _why_ the pull-request diff differs. That happens not just
because of the multiple merge-bases issue, but you get differences
with merge diffs in other cases too (ie any case where merging had to
do any non-trivial merging action - perhaps due to part of a patch
already being applied in my tree for other reasons etc etc).

           Linus

Powered by blists - more mailing lists