[PATCH] Documentation: describe git extended diff headers.

[PATCH] Documentation: describe git extended diff headers.

[Junio C Hamano]

The documentation failed to describe "diff --git" extended diff
headers, so add some.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 Documentation/diff-format.txt |   36 +++++++++++++++++++++++++++++++++++-
 1 files changed, 35 insertions(+), 1 deletions(-)

diff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt
--- a/Documentation/diff-format.txt
+++ b/Documentation/diff-format.txt
@@ -52,7 +52,7 @@ Generating patches with -p
 --------------------------
 
 When "git-diff-cache", "git-diff-tree", or "git-diff-files" are run
-with a '-p' option, they do not produce the output described above
+with a '-p' option, they do not produce the output described above;
 instead they produce a patch file.
 
 The patch generation can be customized at two levels.  This
@@ -98,3 +98,37 @@ temporary file --- it is removed when 'G
 
 For a path that is unmerged, 'GIT_EXTERNAL_DIFF' is called with 1
 parameter, <path>.
+
+
+Git specific extention to diff format
+-------------------------------------
+
+What -p option produces is slightly different from the
+traditional diff format.
+
+ (1) It is preceeded with a "git diff" header, that looks like
+     this:
+
+     diff --git a/file1 b/file2
+
+     The a/ and b/ filenames are the same unless rename/copy is
+     involved.  Especially, even for a creation or a deletion,
+     /dev/null is _not_ used in place of a/ or b/ filename.
+
+     When rename/copy is involved, file1 and file2 shows the
+     name of the source file of the rename/copy and the name of
+     the file that rename/copy produces, respectively.
+
+ (2) It is followed by extended header lines that are one or
+     more of:
+
+       old mode <mode>
+       new mode <mode>
+       deleted file mode <mode>
+       new file mode <mode>
+       copy from <path>
+       copy to <path>
+       rename from <path>
+       rename to <path>
+       similarity index <number>
+       dissimilarity index <number>
------------

Re: [PATCH] Documentation: describe git extended diff headers.

[Linus Torvalds]

On Sun, 5 Jun 2005, Junio C Hamano wrote:
>
> The documentation failed to describe "diff --git" extended diff
> headers, so add some.

You document the "rename" header as being "rename from/to", which is 
sensible, but doesn't match reality. diff.c has "rename old/new". I found 
that out the hard way when doing git-apply ;)

I'd almost prefer fixing diff.c (and now apply.c). Comments?

		Linus

[PATCH] Fix diff.c to match rename extended header to the document.

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> On Sun, 5 Jun 2005, Junio C Hamano wrote:
>> 
>> The documentation failed to describe "diff --git" extended diff
>> headers, so add some.

LT> You document the "rename" header as being "rename from/to", which is 
LT> sensible, but doesn't match reality. diff.c has "rename old/new". I found 
LT> that out the hard way when doing git-apply ;)

LT> I'd almost prefer fixing diff.c (and now apply.c). Comments?

Yes, sir ;-).

------------
This matches the git extended header to what is documented.
There is no need to touch git-external-diff-script since it gets
the string generated here and simply spits it out.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
cd /opt/packrat/playpen/public/in-place/git/git.junio/
jit-diff
# - linus: git-apply: fix rename header parsing
# + (working tree)
diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -786,8 +786,8 @@ static void diff_flush_patch(struct diff
 	case 'R':
 		sprintf(msg_,
 			"similarity index %d%%\n"
-			"rename old %s\n"
-			"rename new %s",
+			"rename from %s\n"
+			"rename to %s",
 			(int)(0.5 + p->score * 100.0/MAX_SCORE),
 			p->one->path, p->two->path);
 		msg = msg_;

Compilation finished at Sun Jun  5 15:23:44

[PATCH] Fix apply.c to match rename extended header to the document.

[Junio C Hamano]

This matches the git extended header git-apply expects to what
is documented.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
# - linus: git-apply: fix rename header parsing
# + (working tree)
diff --git a/apply.c b/apply.c
--- a/apply.c
+++ b/apply.c
@@ -435,8 +435,8 @@ static int parse_git_header(char *line, 
 			{ "new file mode ", gitdiff_newfile },
 			{ "copy from ", gitdiff_copysrc },
 			{ "copy to ", gitdiff_copydst },
-			{ "rename old ", gitdiff_renamesrc },
-			{ "rename new ", gitdiff_renamedst },
+			{ "rename from ", gitdiff_renamesrc },
+			{ "rename to ", gitdiff_renamedst },
 			{ "similarity index ", gitdiff_similarity },
 			{ "dissimilarity index ", gitdiff_dissimilarity },
 			{ "", gitdiff_unrecognized },

Re: [PATCH] Fix apply.c to match rename extended header to the document.

[Linus Torvalds]

On Sun, 5 Jun 2005, Junio C Hamano wrote:
>
> This matches the git extended header git-apply expects to what
> is documented.

Well you also need to fix the tests. 

I did it all, and pushed out. And I left git-apply accepting the old 
format, at least for a while.

		Linus

Last mile for 1.0

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> You document the "rename" header as being "rename from/to", which is 
LT> sensible, but doesn't match reality. diff.c has "rename old/new". I found 
LT> that out the hard way when doing git-apply ;)

The document was cut & paste from the version of apply.c before
you found it out "the hard way", so of course its wording is
sensible ;-) Anyway, I think we settled this part.

There are still some remaining issues before 1.0.  The ones I
personally would want to see merged and/or addressed are:

 - git-ssh-push documentation updates from Dan Barkalow, which
   he mentions in *1*.  I am the primarily guilty one about
   "other people seem to have missed it".

 - "-w heads/master heads/master" extension to the pull family,
   again from Dan, mentioned in the same message.

 - "--merge-order" extension to the git-rev-list, from Jon
   Seymour, last posted as *2*.  I have not tried it on
   something big (like linux-2.6 repository) and cannot vouch
   for its correctness myself, but I find what it claims to do
   quite sensible and attractive.

 - Tutorials you have been working on.  I'd appreciate it if my
   earlier "what about cvs annotate" example *3* finds its home
   somewhere in that document ;-).

 - "What happens when a merge goes wrong" helper script Jeff
   wanted to have in *4*.

 - Documentation from Jason McMullan for more commands, reminded
   by Petr Baudis in *5*.

[References]

*1* <Pine.LNX.4.21.0506031927000.30848-100000@iabervon.org>

*2* <20050605134733.3123.qmail@blackcubes.dyndns.org>

*3* <7vll5s35pd.fsf@assigned-by-dhcp.cox.net>

*4* <42A181C1.3010902@pobox.com>

*5* <20050605204739.GR17462@pasky.ji.cz>

Re: Last mile for 1.0

[Junio C Hamano]

I did not mention git-sync by Jason McMullan on my list of "what
I want to have in 1.0", but that was not because I object to the
idea of having a sync mechanism that knows and takes advantage
of how GIT works.  Quite the contrary.

I would like to see such a GIT specific smart sync mechanism
some day.  git-http-pull lets us use a dumb server and cannot
assume server-side smarts, git-local-pull operates in an
environment where latency does not matter, but git-ssh-pull and
git-sync are solutions for the real network environment where
latency matters, and having a smart sync mechanism is a big win.

I just do not feel, judging from its current protocol command
set, it offers enough improvements over what git-ssh-push/pull
pairs already give us; I'd be happy to be corrected, of course,
if this is a misconception.

Re: Last mile for 1.0

[Linus Torvalds]

On Sun, 5 Jun 2005, Junio C Hamano wrote:
> 
>  - Tutorials you have been working on.  I'd appreciate it if my
>    earlier "what about cvs annotate" example *3* finds its home
>    somewhere in that document ;-).

My main dislike about that was that I don't think -S is "better" than
annotate as you claim - I think it's different and can be better in many
circumstances, but I don't like the notion of telling a new user "you
don't want annotate, pickaxe is better".

Let's just be upfront about some things being missing, and suggest
alternatives. A lot of times, just "git-whatchanged -p filename" ends up
being sufficient, at other times -S might be.

Finally, please send it as a patch, that way the authorship etc stuff gets 
done right.

>  - "What happens when a merge goes wrong" helper script Jeff
>    wanted to have in *4*.

Does anybody have any suggestions? Preferably with a reasonable 
test-case, so that people can try it out.. Maybe just leaving the merge 
failures where they are?

		Linus

[PATCH] git-whatchanged vs "cvs annotate"

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> Let's just be upfront about some things being missing, and suggest
LT> alternatives.

I agree that being honest is good.  Something like this?
------------
This adds a section to talk about "cvs annotate".

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 Documentation/cvs-migration.txt |  105 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 105 insertions(+), 0 deletions(-)

diff --git a/Documentation/cvs-migration.txt b/Documentation/cvs-migration.txt
new file mode 100644
--- /dev/null
+++ b/Documentation/cvs-migration.txt
@@ -0,0 +1,105 @@
+CVS annotate.
+
+The core GIT itself does not have a "cvs annotate" equivalent.
+It has something that you may want to use when you would use
+"cvs annotate".
+
+Let's step back a bit and think about the reason why you would
+want to do "cvs annotate a-file.c" to begin with.
+
+You would use "cvs annotate" on a file when you have trouble
+with a function (or even a single "if" statement in a function)
+that happens to be defined in the file, which does not do what
+you want it to do.  And you would want to find out why it was
+written that way, because you are about to modify it to suit
+your needs, and at the same time you do not want to break its
+current callers.  For that, you are trying to find out why the
+original author did things that way in the original context.
+
+Many times, it may be enough to see the commit log messages of
+commits that touch the file in question, possibly along with the
+patches themselves, like this:
+
+	$ git-whatchanged -p a-file.c
+
+This will show log messages and patches for each commit that
+touches a-file.
+
+This, however, may not be very useful when this file has many
+modifications that are not related to the piece of code you are
+interested in.  You would see many log messages and patches that
+do not have anything to do with the piece of code you are
+interested in.  As an example, assuming that you have this piece
+code that you are interested in in the HEAD version:
+
+	if (frotz) {
+		nitfol();
+	}
+
+you would use git-rev-list and git-diff-tree like this:
+
+	$ git-rev-list HEAD |
+	  git-diff-tree --stdin -v -p -S'if (frotz) {
+		nitfol();
+	}'
+
+We have already talked about the "--stdin" form of git-diff-tree
+command that reads the list of commits and compares each commit
+with its parents.  The git-whatchanged command internally runs
+the equivalent of the above command, and can be used like this:
+
+	$ git-whatchanged -p -S'if (frotz) {
+		nitfol();
+	}'
+
+When the -S option is used, git-diff-tree command outputs
+differences between two commits only if one tree has the
+specified string in a file and the corresponding file in the
+other tree does not.  The above example looks for a commit that
+has the "if" statement in it in a file, but its parent commit
+does not have it in the same shape in the corresponding file (or
+the other way around, where the parent has it and the commit
+does not), and the differences between them are shown, along
+with the commit message (thanks to the -v flag).  It does not
+show anything for commits that do not touch this "if" statement.
+
+Also, in the original context, the same statement might have
+appeared at first in a different file and later the file was
+renamed to "a-file.c".  CVS annotate would not help you to go
+back across such a rename, but GIT would still help you in such
+a situation.  For that, you can give the -C flag to
+git-diff-tree, like this:
+
+	$ git-whatchanged -p -C -S'if (frotz) {
+		nitfol();
+	}'
+
+When the -C flag is used, file renames and copies are followed.
+So if the "if" statement in question happens to be in "a-file.c"
+in the current HEAD commit, even if the file was originally
+called "o-file.c" and then renamed in an earlier commit, or if
+the file was created by copying an existing "o-file.c" in an
+earlier commit, you will not lose track.  If the "if" statement
+did not change across such rename or copy, then the commit that
+does rename or copy would not show in the output, and if the
+"if" statement was modified while the file was still called
+"o-file.c", it would find the commit that changed the statement
+when it was in "o-file.c".
+
+[ BTW, the current versions of "git-diff-tree -C" is not eager
+  enough to find copies, and it will miss the fact that a-file.c
+  was created by copying o-file.c unless o-file.c was somehow
+  changed in the same commit.]
+
+You can use the --pickaxe-all flag in addition to the -S flag.
+This causes the differences from all the files contained in
+those two commits, not just the differences between the files
+that contain this changed "if" statement:
+
+	$ git-whatchanged -p -C -S'if (frotz) {
+		nitfol();
+	}' --pickaxe-all
+
+[ Side note.  This option is called "--pickaxe-all" because -S
+  option is internally called "pickaxe", a tool for software
+  archaeologists.]
------------

Re: Last mile for 1.0

[Thomas Glanzmann]

Hello,

> >  - "What happens when a merge goes wrong" helper script Jeff
> >    wanted to have in *4*.

> Does anybody have any suggestions? Preferably with a reasonable 
> test-case, so that people can try it out.. Maybe just leaving the merge 
> failures where they are?

I think the simplest and effectivies way to handle this is the
following: Add a flag to the current merge script which indicates that
on conflicts the user will be dropped to a shell per conflict to solve:

	- Checkout filename.LOCAL, filename.REMOTE, filename.GCA (with
	  sanitychecks of course (eg not overwriting existing files)
	- Run merga -A on them and maybe wiggle and write the product in
	  filename
	- If the user resolves the conflict drop him a note to
	  'update-cache' the resolved conflict and exit the subshell.

I also have at the moment a very nice perl merge script which works with
multiple heads, duplicates and stuff. Maybe I should write this down in
bash.

The above approach worked out very good at least for me. I often had to
resolve 'simple' conflicts when pulling in upstream changes from Linus
repo.

	Thomas

Re: Last mile for 1.0

[Linus Torvalds]

On Mon, 6 Jun 2005, Thomas Glanzmann wrote:
> 
> I think the simplest and effectivies way to handle this is the
> following: Add a flag to the current merge script which indicates that
> on conflicts the user will be dropped to a shell per conflict to solve:

Well, there was actually a much more fundamental problem, which is that 
the merge script depended on being able to do

	git-checkout-cache -f -u -a

which in turn obviously meant that _whatever_ it did, it would end up 
overwriting any dirty state in the working tree.

And you can't remove the git-checkout-cache, because after the merge we've 
lost the original state anyway, so there's no way to know whether whatever 
you had in the working directory was dirty or not.

So I've actually been working now to make the very low-level git-read-tree 
Do The Right Thing (tm), and I think I'm getting there. It basically 
depends on "git-read-tree" noticing when the state is dirty enough that we 
can't safely do the merge, and then for the safe cases it can actually 
update anything it merged properly. As a result, there's never any need 
for git-checkout-cache, and the only thing that needs updating in the 
working directory is the stuff that we end up merging by hand _outside_ of 
git-read-tree.

There's two cases: fast-forward and a real merge. And the thing is, to do 
even just the fast-forward safely, it actually needs to know what the base 
tree was (otherwise it can only tell that the file was up-to-date in the 
index, but it can't know whether the index actually matched the original 
HEAD..). So now the fast-forward case is actually a two-way merge:

	git-read-tree -u -m HEAD NEW_HEAD

where "-u" stands for "update", and tells read-tree that it should check 
out the files it merges up. 

And now git-read-tree also tries to be very careful: if one of the files 
that needs to be updated is already dirty, or it doesn't match the 
original HEAD, then git-read-tree will just exit with an error and not do 
anything at all.

But for a file that wasn't touched at all by the merge, we can leave it
dirty in the index, since whatever dirty index state was valid before the
merge is obviously valid after it too. So now you can have dirty state in 
your tree, and merges will complain only if it matters to them.

Same goes for the (much more complex) three-way merge, of course. There
the rules are a bit more complicated, and I'll have to double-check this
thing, but it all looks like even if the current state might be buggy, the 
basic _notion_ looks fine.

So now we have a one-way merge for "just update to this tree", a two-way 
merge for "update from this tree to that tree", and a three-way merge for 
"merge these two trees with that third tree as a base".

By now, I'd really like to have some test-cases. Things like "file dirty 
in working directory, removed by merge" would be good.

And the "git-merge-one-file-script" thing needs to be updated to keep the
tree updated as it merges things by hand, since it can't depend on the
git-checkout-cache fixing things up any more. Anybody?

			Linus

Re: Last mile for 1.0

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> And the "git-merge-one-file-script" thing needs to be updated to keep the
LT> tree updated as it merges things by hand, since it can't depend on the
LT> git-checkout-cache fixing things up any more. Anybody?

I'd love to bite.  One problem I have been having for the last
30 minutes or so is that t1000 test (merge from h*ll) does not
show that read-tree resolving even the trivial ones anymore, and
I do not know if you want make it that the responsibility of
git-merge-one-file-script, or if it is just an oversight.

I might have leave that to Europeans, though, due to timezone
differences ;-).

Re: Last mile for 1.0

[Linus Torvalds]

On Sun, 5 Jun 2005, Junio C Hamano wrote:
> >>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:
> 
> LT> And the "git-merge-one-file-script" thing needs to be updated to keep the
> LT> tree updated as it merges things by hand, since it can't depend on the
> LT> git-checkout-cache fixing things up any more. Anybody?
> 
> I'd love to bite.  One problem I have been having for the last
> 30 minutes or so is that t1000 test (merge from h*ll) does not
> show that read-tree resolving even the trivial ones anymore, and
> I do not know if you want make it that the responsibility of
> git-merge-one-file-script, or if it is just an oversight.

It was just two really stupid bugs, both due to me trying to make the 
three different merge loops look a bit more like each other, and that had 
broken the three-way case in two different ways.

Should be fixed, as soon as the mirroring pushes out the last off-by-one 
fix.

			Linus

Re: Last mile for 1.0

[Thomas Glanzmann]

Hello,

> Well, there was actually a much more fundamental problem, which is that 
> the merge script depended on being able to do

> 	git-checkout-cache -f -u -a

> which in turn obviously meant that _whatever_ it did, it would end up 
> overwriting any dirty state in the working tree.

true. But I don't see the problem. Just ensure that there are no
uncommitted data and no dirty files before proceeding with the merge by
calling:

git-diff-files -z -r (but ignore the deleted)
git-diff-cache -r --cached HEAD

> And you can't remove the git-checkout-cache, because after the merge we've 
> lost the original state anyway, so there's no way to know whether whatever 
> you had in the working directory was dirty or not.

> So I've actually been working now to make the very low-level git-read-tree 
> Do The Right Thing (tm), and I think I'm getting there. It basically 
> depends on "git-read-tree" noticing when the state is dirty enough that we 
> can't safely do the merge, and then for the safe cases it can actually 
> update anything it merged properly. As a result, there's never any need 
> for git-checkout-cache, and the only thing that needs updating in the 
> working directory is the stuff that we end up merging by hand _outside_ of 
> git-read-tree.

> There's two cases: fast-forward and a real merge. And the thing is, to do 
> even just the fast-forward safely, it actually needs to know what the base 
> tree was (otherwise it can only tell that the file was up-to-date in the 
> index, but it can't know whether the index actually matched the original 
> HEAD..). So now the fast-forward case is actually a two-way merge:

> 	git-read-tree -u -m HEAD NEW_HEAD

> where "-u" stands for "update", and tells read-tree that it should check 
> out the files it merges up. 

> And now git-read-tree also tries to be very careful: if one of the files 
> that needs to be updated is already dirty, or it doesn't match the 
> original HEAD, then git-read-tree will just exit with an error and not do 
> anything at all.

Now I see your point. And a cmp-and-xchg HEAD would be useful here, too.  What
if a user tries to shoot himself in the head by pull from two trees
simultaneous?

> But for a file that wasn't touched at all by the merge, we can leave it
> dirty in the index, since whatever dirty index state was valid before the
> merge is obviously valid after it too. So now you can have dirty state in 
> your tree, and merges will complain only if it matters to them.

Nice to have. :-)

> ...

> And the "git-merge-one-file-script" thing needs to be updated to keep the
> tree updated as it merges things by hand, since it can't depend on the
> git-checkout-cache fixing things up any more. Anybody?

I can't follow you there. AFAIK it retrieves all his files from
git-merge-cache and just calls git-update-cache to update the index.

	Thomas

Re: Last mile for 1.0

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> By now, I'd really like to have some test-cases. Things like "file dirty 
LT> in working directory, removed by merge" would be good.

Is test 44 in t1000 (the first one in t1000 after the lib-
read-tree-m-3way prepares the test trees) good enough?  Or do
you want more specific tests to make sure the logic catches
individual cases right?

Re: Last mile for 1.0

[Linus Torvalds]

On Mon, 6 Jun 2005, Thomas Glanzmann wrote:
> 
> true. But I don't see the problem. Just ensure that there are no
> uncommitted data and no dirty files before proceeding with the merge by
> calling:

The thing is, I historically _often_ have uncommitted data, and it's been 
one of the biggest bummers for me that a merge of a totally unrelated 
thing will crap all over my debugging patch..

> > And now git-read-tree also tries to be very careful: if one of the files 
> > that needs to be updated is already dirty, or it doesn't match the 
> > original HEAD, then git-read-tree will just exit with an error and not do 
> > anything at all.
> 
> Now I see your point. And a cmp-and-xchg HEAD would be useful here, too.  What
> if a user tries to shoot himself in the head by pull from two trees
> simultaneous?

If he uses the same index file, he'll be protected by the index lock. 

> > And the "git-merge-one-file-script" thing needs to be updated to keep the
> > tree updated as it merges things by hand, since it can't depend on the
> > git-checkout-cache fixing things up any more. Anybody?
> 
> I can't follow you there. AFAIK it retrieves all his files from
> git-merge-cache and just calls git-update-cache to update the index.

Not exactly. It updates the index directly, without necessarily updating 
the working directory. For example:

	"$1.." | "$1.$1" | "$1$1.")
	        echo "Removing $4"
	        exec git-update-cache --force-remove "$4" ;;

it _says_ "removing $4", but it never actually does so, so the working 
directory still has the file ;)

Same goes with added files or even updated files, where it uses
"--cacheinfo" to update the cache without even touching the working
directory.

Anyway, git-resovle-script needs to be made to use "git-merge-cache -o" 
too, methinks. And it needs a test-case or two.

		Linus

Re: Last mile for 1.0

[Thomas Glanzmann]

Hello,

> The thing is, I historically _often_ have uncommitted data, and it's been 
> one of the biggest bummers for me that a merge of a totally unrelated 
> thing will crap all over my debugging patch..

got the point. I useally work like that, too. But I never did it with a
distributed SCM just with CVS.

> If he uses the same index file, he'll be protected by the index lock. 

I see.

> Not exactly. It updates the index directly, without necessarily updating 
> the working directory. For example:

> 	"$1.." | "$1.$1" | "$1$1.")
> 	        echo "Removing $4"
> 	        exec git-update-cache --force-remove "$4" ;;

> it _says_ "removing $4", but it never actually does so, so the working 
> directory still has the file ;)

> Same goes with added files or even updated files, where it uses
> "--cacheinfo" to update the cache without even touching the working
> directory.

Now I see your point.

> Anyway, git-resovle-script needs to be made to use "git-merge-cache
> -o" too, methinks. And it needs a test-case or two.

Yes, it does. In my merge script in perl it already does that. :-)

	Thomas

Re: Last mile for 1.0

[Linus Torvalds]

On Sun, 5 Jun 2005, Junio C Hamano wrote:
> 
> Is test 44 in t1000 (the first one in t1000 after the lib-
> read-tree-m-3way prepares the test trees) good enough?  Or do
> you want more specific tests to make sure the logic catches
> individual cases right?

I was more thinking of all the individual cases, and also the 2-way merge
issue (which _should_ be a lot easier, but hey, I had tons of bugs there
too).

For example, what should happen if you do a merge and you have a file in
the index that isn't mentioned by any of the trees involved. I consider 
that a failure, since the merge will remove the file, and since it wasn't 
committed, it's now a "data lost" issue. But did I actually get it right? 
I have no idea.

			Linus

Re: Last mile for 1.0

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> Not exactly. It updates the index directly, without necessarily updating 
LT> the working directory. For example:

LT> 	"$1.." | "$1.$1" | "$1$1.")
LT> 	        echo "Removing $4"
LT> 	        exec git-update-cache --force-remove "$4" ;;

LT> it _says_ "removing $4", but it never actually does so, so the working 
LT> directory still has the file ;)

Yes, this was done from your explicit request not to touch the
working directory while it works AFAICR.  At least back then,
not touching the working tree was the _requirement_.

So is "the new merge world order" you mentioned in the log
message now require (and assume) the work tree more-or-less
matches the first head being merged?

Re: Last mile for 1.0

[McMullan, Jason]

[-- Attachment #1: Type: text/plain, Size: 989 bytes --]

On Sun, 2005-06-05 at 16:45 -0700, Junio C Hamano wrote:
> I did not mention git-sync by Jason McMullan on my list of "what
> I want to have in 1.0", but that was not because I object to the
> idea of having a sync mechanism that knows and takes advantage
> of how GIT works.  Quite the contrary.
> 
> [snip snip]
>
> I just do not feel, judging from its current protocol command
> set, it offers enough improvements over what git-ssh-push/pull
> pairs already give us; I'd be happy to be corrected, of course,
> if this is a misconception.

Oh, I definitely agree. git-sync is *not* 1.0 material. The protocol
and feature set are still being worked on, and git-ssh-* may even
be a better fix than git-sync for many scenarios.

What I *do* want to get into git 1.0 is a merge of git-sync's 
verify-before-write update semantics into pull.c. I should be
able to get that to you before tomorrow.

-- 
Jason McMullan <jason.mcmullan@timesys.com>
TimeSys Corporation


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Last mile for 1.0

[Linus Torvalds]

On Mon, 6 Jun 2005, Junio C Hamano wrote:
> 
> Yes, this was done from your explicit request not to touch the
> working directory while it works AFAICR.  At least back then,
> not touching the working tree was the _requirement_.

Yes. Now that read-tree verifies that the working directory is clean (at
least for any non-identity files), it's a non-issue.

> So is "the new merge world order" you mentioned in the log
> message now require (and assume) the work tree more-or-less
> matches the first head being merged?

Well, without "-u" you should see the old "order doesn't matter" case, but
yes, the theory is that the three trees are <base> <current> <merge> for
the three-way case, and <current> <new> for the two-way one.

You can get the old behaviour by using

	git-read-tree -m <cur>
	git-read-tree -m <base> <cur> <merge>

where the first read-tree ends up just makign sure that the index file 
matches the current head (use "-u" or not as you like).

(Side note: the actual read-tree phase should be totally agnostic about 
whether the current tree is the first, second or third of the trees, since 
it will happily say "oh, we saw this exact directory entry in _one_ of the 
trees, so we know it hasn't gotten lost".  So for now, order still is left 
to the final user, but I don't think you should depend on that).

		Linus

Re: Last mile for 1.0

[Linus Torvalds]

[ git list added back in, since I migth as well explain the thinking here ]

On Mon, 6 Jun 2005, Junio C Hamano wrote:
>
> I've sent you a reply in another thread, but I really think you
> need to make this "new merge world order" a bit more explicit.
> My understanding of your (earlier) wish was that you wanted the
> merge not to touch and look at any work tree material, but it
> appears to me that this round you actually expect the work tree
> be populated and more-or-less match the first head being merged.

Actually, no. I expect the old _index_ to be at least not _more_ populated 
than the trees I merge. That's not a working tree issue, that's a "we 
don't want to drop information from the index".

And yes, if you use "-u", it will populate the working tree too, but 
that's really totally unimportant from the algorithm itself.

But you can very much do everything in-index as before, if you want to. A 
pure index merge would be done usually in a new temporary index file, 
something like

  rm -f .git/tmp_index
  GIT_INDEX_FILE=.git/tmp_index git-read-tree -m <base> <merge1> <merge2>

and the new changes don't change that.

HOWEVER, it's all set up to be very clever indeed. My immediate goal is to 
make the current git-resolve-script be more easily usable, and that 
implies that it has to work in the current working directory and resolve 
conflicts there. I still think that the _long-term_ plan is to make sure 
that we don't do that, and the new thing actually supports that too.

For example, notice how I lifted all the "checkout" code from the 
checkout-cache thing? Including very much the code that supports 
"--prefix"? I didn't add the command line, but imagine just adding that, 
which updates "state.base_dir", and doing

	mkdir -p MERGE_DIR/.git
	cp .git/index MERGE_DIR/.git/index
	GIT_INDEX_FILE=MERGE_DIR/.git/index git-read-tree -u --prefix=MERGE_DIR/ <base> <merge1> <merge2>

and voila, you're basically now 75% of the way to where I wanted the thing
to be in a separate directory.

So I've given up on the separate directory for 1.0 - because it's clearly 
not going to happen - but I've not given up on the basic idea.

			Linus

3-way read-tree case matrix.

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> I did. It looks fine, although I'd like to point out that even the
LT> three-way merge _does_ mix in the original state for the case where the
LT> merge was trivially resolved...

Thanks.  I have another request for sanity checking.

I am not sure if salvaging a dirty tree in a 3-way merge (as
opposed to just downright refusing) would have as much practical
value as the 2-tree fast-forward case, but here is a proposed
case matrix for the 3-way merge.

"git-diff-tree -m O H M"

    O       H       M         result      index requirements
------------------------------------------------------------------
  1 missing missing missing   -           must not exist.
 -----------------------------------------------------------------
  2 missing missing exists    take M      must match M if exists.
 -----------------------------------------------------------------
  3 missing exists  missing   remove      must match H and be
					  up-to-date, if exists.
 -----------------------------------------------------------------
  4 missing exists  exists    no merge    must match H and be
					  up-to-date, if exists.
 -----------------------------------------------------------------
  5 exists  missing missing   no merge    must not exist.
 -----------------------------------------------------------------
  6 exists  missing O==M      remove      must not exist.
 -----------------------------------------------------------------
  7 exists  missing O!=M      no merge    must not exist.
 -----------------------------------------------------------------
  8 exists  O==H    missing   remove      must match H and be
					  up-to-date, if exists.
 -----------------------------------------------------------------
  9 exists  O!=H    missing   no merge    must match H and be
					  up-to-date, if exists.
 -----------------------------------------------------------------
 10 exists  O!=H    O!=M      no merge    must match H and be
					  up-to-date, if exists.
 -----------------------------------------------------------------
 11 exists  O!=H    O==M      take H      must match H, if exists.
 -----------------------------------------------------------------
 12 exists  O==H    O!=M      take M      if exists, must either (1)
    					  match H and be up-to-date,
                                          or (2) match M.
 -----------------------------------------------------------------
 13 exists  O==H    O==M      take M      must match H if exists.
------------------------------------------------------------------

In all "take H" or "take M" cases, if the original index matches
what is taken, I would reuse it, and keep it dirty if it is.

The goal is, "(1) do not clobber the current index; (2) arrive
at the same result as in the case started with an empty index;
(3) favor success over failure as long as (1) and (2) are
satisfied".

Re: 3-way read-tree case matrix.

[Junio C Hamano]

I suspect my mailer dropped your response on the floor when it
polled my ISP at around 21:27 my time.  I found its log
mentioning your address but no message in my mailbox.

Since the last message I sent you, I updated read-tree.c to
match the proposed behaviour, and I found a couple of problems
with it by running the 3-way merge test we already have.  I am
attaching a revised one.  I have not finished a new test suite
that runs on a populated index file yet, which is what I plan to
do next.

There is one thing that the proposed table changes from the
traditional 3-way merge semantics.  I think this is a sensible
change.

 - #2 and #3 (I botched #3 in the earlier one I sent you);
   traditionally we left <O,H,M>=<none,none,exists> and
   <O,H,M>=<none,exists,none> cases to the script policy; we
   could salvage a (potentially dirty) cache entry if we say we
   pick the one created in only one branch.

Earlier I botched <O,H,M>=<some,H!=O,M!=O> case in the table; we
should collapse to H/M if H==M; this is now handled differently
from H!=O,M!=O,H!=M case (case #10) as case #11 in the updated
table.

------------

"git-diff-tree -m O H M"

    O       H       M         result      index requirements
------------------------------------------------------------------
  1 missing missing missing   -           must not exist.
 -----------------------------------------------------------------
  2 missing missing exists    take M      must match M, if exists.
 -----------------------------------------------------------------
  3 missing exists  missing   take H      must match H, if exists.
 -----------------------------------------------------------------
  4 missing exists  exists    no merge    must match H and be
					  up-to-date, if exists.
 -----------------------------------------------------------------
  5 exists  missing missing   no merge    must not exist.
 -----------------------------------------------------------------
  6 exists  missing O==M      remove      must not exist.
 -----------------------------------------------------------------
  7 exists  missing O!=M      no merge    must not exist.
 -----------------------------------------------------------------
  8 exists  O==H    missing   remove      must match H and be
					  up-to-date, if exists.
 -----------------------------------------------------------------
  9 exists  O!=H    missing   no merge    must match H and be
					  up-to-date, if exists.
 -----------------------------------------------------------------
 10 exists  O!=H    O!=M      no merge    must match H and be
		    M!=H		  up-to-date, if exists.
 -----------------------------------------------------------------
 11 exists  O!=H    O!=M      take H	  must match H, if exists.
		    M==H
 -----------------------------------------------------------------
 12 exists  O!=H    O==M      take H      must match H, if exists.
 -----------------------------------------------------------------
 13 exists  O==H    O!=M      take M      if exists, must either (1)
    					  match H and be up-to-date,
                                          or (2) match M.
 -----------------------------------------------------------------
 14 exists  O==H    O==M      take M      must match H if exists.
------------------------------------------------------------------

In all "take H" or "take M" cases, if the original index matches
what is taken, I would reuse it, and keep it dirty if it is.

The goal is, "(a) do not clobber the current index; (b) arrive
at the same result as in the case started with an empty index;
(c) favor success over failure as long as (a) and (b) are
satisfied."

[PATCH] Tests: read-tree -m test updates.

[Junio C Hamano]

This updates t1000 (basic 3-way merge test) to check the merge
results for both successful cases (earlier one checked the
result for only one of them).  Also fixes typos in t1002 that
broke '&&' chain, potentially missing a test failure before the
chain got broken.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 t/t1000-read-tree-m-3way.sh   |   58 ++++++++++++++++++++---------------------
 t/t1002-read-tree-m-u-2way.sh |    4 +--
 2 files changed, 30 insertions(+), 32 deletions(-)

diff --git a/t/t1000-read-tree-m-3way.sh b/t/t1000-read-tree-m-3way.sh
--- a/t/t1000-read-tree-m-3way.sh
+++ b/t/t1000-read-tree-m-3way.sh
@@ -77,34 +77,6 @@ In addition:
 ################################################################
 # Try merging and showing the various diffs
 
-# The tree is dirty at this point.
-test_expect_failure \
-    '3-way merge with git-read-tree -m' \
-    "git-read-tree -m $tree_O $tree_A $tree_B"
-
-# This is done on an empty work directory, which is the normal
-# merge person behaviour.
-test_expect_success \
-    '3-way merge with git-read-tree -m' \
-    "rm -fr [NDMALTS][NDMALTSF] Z &&
-     rm .git/index &&
-     git-read-tree -m $tree_O $tree_A $tree_B"
-
-# This starts out with the first head, which is the normal
-# patch submitter behaviour.
-test_expect_success \
-    '3-way merge with git-read-tree -m' \
-    "git-read-tree $tree_A &&
-     git-checkout-cache -f -u -a &&
-     git-read-tree -m $tree_O $tree_A $tree_B"
-
-_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
-_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
-test_expect_success \
-    'git-ls-files --stage of the merge result' \
-    'git-ls-files --stage >current- &&
-     sed -e "s/ $_x40 / X /" <current- >current'
-
 cat >expected <<\EOF
 100644 X 2	AA
 100644 X 3	AA
@@ -154,8 +126,34 @@ cat >expected <<\EOF
 100644 X 0	Z/NN
 EOF
 
+_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
+_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
+
+# The tree is dirty at this point.
+test_expect_failure \
+    '3-way merge with git-read-tree -m, dirty cache' \
+    "git-read-tree -m $tree_O $tree_A $tree_B"
+
+# This is done on an empty work directory, which is the normal
+# merge person behaviour.
+test_expect_success \
+    '3-way merge with git-read-tree -m, empty cache' \
+    "rm -fr [NDMALTS][NDMALTSF] Z &&
+     rm .git/index &&
+     git-read-tree -m $tree_O $tree_A $tree_B &&
+     git-ls-files --stage |
+     sed -e 's/ $_x40 / X /' >current &&
+     diff -u expected current"
+
+# This starts out with the first head, which is the normal
+# patch submitter behaviour.
 test_expect_success \
-    'validate merge result' \
-    'diff current expected'
+    '3-way merge with git-read-tree -m, match H' \
+    "git-read-tree $tree_A &&
+     git-checkout-cache -f -u -a &&
+     git-read-tree -m $tree_O $tree_A $tree_B &&
+     git-ls-files --stage |
+     sed -e 's/ $_x40 / X /' >current &&
+     diff -u expected current"
 
 test_done
diff --git a/t/t1002-read-tree-m-u-2way.sh b/t/t1002-read-tree-m-u-2way.sh
--- a/t/t1002-read-tree-m-u-2way.sh
+++ b/t/t1002-read-tree-m-u-2way.sh
@@ -93,7 +93,7 @@ test_expect_success \
      compare_change 5diff.out expected &&
      check_cache_at yomin dirty &&
      sha1sum -c M.sha1 &&
-     : dirty index should have prevented -u from checking it out.
+     : dirty index should have prevented -u from checking it out. &&
      echo yomin yomin >yomin1 &&
      diff yomin yomin1 &&
      rm -f yomin1'
@@ -122,7 +122,7 @@ test_expect_success \
      diff --unified=0 M.out 7.out &&
      check_cache_at frotz dirty &&
      if sha1sum -c M.sha1; then false; else :; fi &&
-     : dirty index should have prevented -u from checking it out.
+     : dirty index should have prevented -u from checking it out. &&
      echo frotz frotz >frotz1 &&
      diff frotz frotz1 &&
      rm -f frotz1'
------------