"Imre Deak" <imre.deak@xxxxxxxxx> writes:
> On Sun, May 11, 2008 at 5:36 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Imre Deak <imre.deak@xxxxxxxxx> writes:
>>
>> > When we can only guess if we have a creation patch, we do
>> > this by treating the patch as such if there weren't any old
>> > lines. Zero length files can be patched without old lines
>> > though, so do an extra check for file size.
>>
>> You described what your patch does, but you did not explain why it is a
>> good addition. One way to do so is to illustrate in what occasion what
>> the existing code does is insufficient.
>
> The patch makes it possible to apply foreign patches (not created with
> git diff) to zero length files already existing in the index. The problem:
>
> $ git init
> Initialized empty Git repository in .git/
> $ rm -rf a
> $ touch a
> $ git add a
> $ git commit -madd
> Created initial commit 818f2b7: add
> 0 files changed, 0 insertions(+), 0 deletions(-)
> create mode 100644 a
> $ echo 123 > a.new
> $ diff -u a a.new > patch
> $ git apply patch
> error: a: already exists in working directory
>
> The error happens because git guesses that `patch` is a creation patch
> and since `a` already exists in the index it will bail out.
Ok, that is much clearer. How about this two-liner instead, then (the
first hunk is just an unreleated typofix)?
Originally when Linus wrote "git-apply" he was trying to be very cautious
and used the /dev/null cue only in a positive way (i.e. if the patch is
from /dev/null it is a creation). But the preimage being something other
than /dev/null was not used as a statement that the patch is not a
creation.
Non SCM patches of any nontrivial size would be created by comparing two
trees with "diff -ru" (with some more combination of options). We would
reliably use /dev/null cue in this case --- if the patch is from /dev/null
it is creation but more importantly if it is not from /dev/null it is not
creation but modification.
Patches from foreign SCMs also follow the /dev/null convention (e.g. SVN
and CVS --- I did not check Hg but I would be surprised if it didn't
follow suit), and we can reliably use the lack of /dev/null as a cue that
it is not a creation patch.
A single-file non SCM patches are done by comparing $a.orig and $a, $a~
and $a, etc., i.e. a pair of files the original and the modified with some
file suffixes. What would people do to represent a creation in such a
case? --- You are right. By doing "diff -u /dev/null $a" (it is more
cumbersome to do "touch $a.empty; diff -u $a.empty $a").
So I think it is reasonable to use non-/dev/null-ness of first as a cue
that it is not a creation patch.
Linus, what do you think? FYR, the original patch that led to this
conversation was:
http://thread.gmane.org/gmane.comp.version-control.git/81531
---
builtin-apply.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index 1103625..9d7cb05 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -418,7 +418,7 @@ static int guess_p_value(const char *nameline)
}
/*
- * Get the name etc info from the --/+++ lines of a traditional patch header
+ * Get the name etc info from the ---/+++ lines of a traditional patch header
*
* FIXME! The end-of-filename heuristics are kind of screwy. For existing
* files, we can happily check the index for a match, but for creating a
@@ -451,6 +451,8 @@ static void parse_traditional_patch(const char *first, const char *second, struc
name = find_name(first, NULL, p_value, TERM_SPACE | TERM_TAB);
patch->old_name = name;
} else {
+ patch->is_new = 0;
+ patch->is_delete = 0;
name = find_name(first, NULL, p_value, TERM_SPACE | TERM_TAB);
name = find_name(second, name, p_value, TERM_SPACE | TERM_TAB);
patch->old_name = patch->new_name = name;
--
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
