Daniel Barkalow <barkalow@xxxxxxxxxxxx> writes:
> "git apply" should be able to notice the many clues that this patch
> doesn't go at the root: (1) it's not -r; (2) it's not a rename, but the
> filenames aren't the same; (3) there isn't an extra path element to
> remove.
I would sort-of agree with (1) and (3) but people can do
cd drivers && diff -u Makefile~ Makefile
so (2) is not a good clue and (3) is not really either.
Especially files like 'Makefile' are everywhere that dwimming
and getting it wrong is worse than being stupid but consistent.
We do -p1 by default because the BCP on the kernel list (which
is where git came from -- but I suspect many projects follow the
convention) is to use "diff a/dir/file b/dir/file". This is
also a convenient setup for people who apply git generated (or
BCP formatted) patches from the toplevel of the tree.
But when you are dealing with handcrafted patches in a
subdirectory, such as the one you would get by:
$ cd some/dir && edit file.c && diff -u file.c~ file.c >G.patch
you do not have the "a/some/dir b/some/dir" prefix, so -p1 to
strip "a/" and "b/" is a wrong thing to begin with, but looking
at G.patch you would not even have "some/dir", so no "correct"
value exists if we refuse to DWIM (and I refuse to). We support
path strip levels with -p<n> option ever since you added it with
e36f8b60, so that is Ok.
However, the current version (in v1.5.0.1) of git-apply has a
few inconsistencies.
* When --index (or --cached) is given, and when the patch is a
git generated (or BCP formatted) patch, it finds out where in
the working tree you are, and strips the right prefix
automatically.
* When there is no --index nor --cached, however, it does not
bother to find out where in the working tree your $cwd is,
and assumes you are at the toplevel (you may not even be in a
git managed tree but are using git-apply as a better "patch",
in which case there is no way to find out where the toplevel
is). You need to give -p<n> to explicitly strip the leading
path. This is not a problem but happens to be a feature to
help applying handcrafted patches, like G.patch above. Also,
it is consistent with how GNU patch would behave. However,
-p<n> is ignored when the patch is a git generated one, which
is a minor additional problem.
I was initially in favor of correcting this inconsistency by
matching the behaviour of non --index/--cached case to that of
the behaviour of --index/--cached case (in other words, making
things more convenient for people who apply git generated and/or
BCP formatted patches). But Linus talked me out of it --- and I
think he is right.
Inconsistency is fixed by making the behaviour more similar to
"GNU patch". The behaviour of --index/--cached case is changed
so that it does not automatically strip "some/dir" part when you
are in a subdirectory, which is how git-apply without these
options operates. Of course, ignoring -p<n> for git generated
patches no longer makes sense with this change, which is also
fixed, so you can use the same -p<n> as you would give to "GNU
patch".
What is cooking in 'next' should behave the way described above.
If not, patches to fix it is very much welcome ;-).
P.S. I think the list hasn't heard from you for quite some
time. Welcome back.
-
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