git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* Some advanced index playing
@ 2006-12-03 17:01 Alan Chandler
  2006-12-03 18:24 ` Linus Torvalds
                   ` (2 more replies)
  0 siblings, 3 replies; 46+ messages in thread
From: Alan Chandler @ 2006-12-03 17:01 UTC (permalink / raw)
  To: git

With all the discussion about the index file in the last few days I would have 
thought that this issue would have come up.  But I don't think it has.

I have been editing a set of files to make a commit, and after editing each 
one had done a git update-index.

At this point I am just about to commit when I realise that one of the files 
has changes in it that really ought to be a separate commit. 

So effectively, I want to do one of three things

a) git-commit <that-file>

Except I can't because there is a safety valve that prevents this and there is 
no force option.

b) Revert the index entry for that file back to the previous HEAD commit 
point, whilst leaving the edits in the working tree, so that I can then 
commit without that one file.

I can't find a command to do that.  The nearest seems to be 
git-update-index --remove, but the manual says that it will not do anything 
if the file still exists.

c) Revert the entire index back to the state it was at the last commit so I 
can selectively add back in the files that I have editted.

The command to do that seems to be

git-read-tree HEAD 

I tried this, and it did indeed seem to exactly this - not quite what I 
wanted, but actually a reasonable compromise.

However, it took me a long time scanning possible commands before I found it 
so I thought I might add some text to one of the tutorials

Any ideas of where?

What happened to the text written here

http://marc.theaimsgroup.com/?l=git&m=116406699903565&w=2

I thought this might be a place to put something like this, but having just 
updated my version of git from source, it doesn't seem to have been put in to 
git anywhere yet.



-- 
Alan Chandler

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Some advanced index playing
  2006-12-03 17:01 Some advanced index playing Alan Chandler
@ 2006-12-03 18:24 ` Linus Torvalds
  2006-12-03 19:36   ` Junio C Hamano
                     ` (2 more replies)
  2006-12-03 18:31 ` Jakub Narebski
  2006-12-03 18:34 ` Linus Torvalds
  2 siblings, 3 replies; 46+ messages in thread
From: Linus Torvalds @ 2006-12-03 18:24 UTC (permalink / raw)
  To: Alan Chandler, Junio C Hamano; +Cc: Git Mailing List



On Sun, 3 Dec 2006, Alan Chandler wrote:
>
> With all the discussion about the index file in the last few days I would have 
> thought that this issue would have come up.  But I don't think it has.
> 
> I have been editing a set of files to make a commit, and after editing each 
> one had done a git update-index.
> 
> At this point I am just about to commit when I realise that one of the files 
> has changes in it that really ought to be a separate commit. 
> 
> So effectively, I want to do one of three things
> 
> a) git-commit <that-file>
> 
> Except I can't because there is a safety valve that prevents this and there is 
> no force option.

I think that is actually a misfeature. 

This _should_ just work. It's the easy and logical way to do it, and it's 
the one that matches all the other behaviours of "git commit" these days.

The reason for the safety valve is actually not really "safety" any more, 
it's purely "historical behaviour". Ie the sanity check is not there 
because you would be doing anything unsafe, but simply because the 
behaviour in this area _changed_, so the semantics are different from what 
they were originally.

But those "original" semantics are now so old and so uninteresting that 
the safety feature has gone from being a safety feature to just being 
annoying, and hindering you from doing what you want to do.

Side note: you -can- do what you want to do, but it's insanely stupid. 
Here's what you'd do:

	git ls-tree HEAD -- that-file | git update-index --index-info
	git commit that-file

but there is no way in hell I will claim that this is a _good_ thing.

[ That said, the whole

	git ls-tree <treeish> -- <file-list> | git update-index --index-info

  is a useful pattern to know. You can basically insert _any_ part of old 
  historical state into the index with this, which can be useful if you 
  want to play games without changing the _other_ parts of the index. ]

So anyway, I would suggest that we just get rid of that partial commit 
"safety check" in "git commit" for now. It still makes sense for when 
you're in the middle of a _merge_, but the "verify that index matches" is 
not worth it.

Or at _least_ there should be a flag to force it.

Junio?


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Some advanced index playing
  2006-12-03 17:01 Some advanced index playing Alan Chandler
  2006-12-03 18:24 ` Linus Torvalds
@ 2006-12-03 18:31 ` Jakub Narebski
  2006-12-03 18:34 ` Linus Torvalds
  2 siblings, 0 replies; 46+ messages in thread
From: Jakub Narebski @ 2006-12-03 18:31 UTC (permalink / raw)
  To: git

Alan Chandler wrote:

> With all the discussion about the index file in the last few days I would have 
> thought that this issue would have come up.  But I don't think it has.
> 
> I have been editing a set of files to make a commit, and after editing each 
> one had done a git update-index.
> 
> At this point I am just about to commit when I realise that one of the files 
> has changes in it that really ought to be a separate commit. 
> 
> So effectively, I want to do one of three things
> 
> a) git-commit <that-file>
> 
> Except I can't because there is a safety valve that prevents this and there is 
> no force option.

I do wonder what this safety valve is for, and why it acts also if
index version is equal to working directory version...

> b) Revert the index entry for that file back to the previous HEAD commit 
> point, whilst leaving the edits in the working tree, so that I can then 
> commit without that one file.
> 
> I can't find a command to do that.  The nearest seems to be 
> git-update-index --remove, but the manual says that it will not do anything 
> if the file still exists.

There is "git update-index --force-remove <that-file>". Currently there
is no "git reset -- <that-file>", but you can revert index entry to the
one in HEAD using
  $ git ls-tree HEAD -- <that-file> | git update-index --index-info

And you can always try "git-rm + git-add".
 
> c) Revert the entire index back to the state it was at the last commit so I 
> can selectively add back in the files that I have editted.
> 
> The command to do that seems to be
> 
> git-read-tree HEAD 

No, the command for that is just "git reset" (which means 
"git reset --mixed HEAD"), and touches HEAD and index but not working
directory


> What happened to the text written here
> 
>   http://marc.theaimsgroup.com/?l=git&m=116406699903565&w=2
    "[DRAFT 2] Branching and merging with git"

> I thought this might be a place to put something like this, but having just 
> updated my version of git from source, it doesn't seem to have been put in to 
> git anywhere yet.

linux@horizon.com, perhaps you should send this text in the form of patch
creating Documentation/tutorial-3.txt file?
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Some advanced index playing
  2006-12-03 17:01 Some advanced index playing Alan Chandler
  2006-12-03 18:24 ` Linus Torvalds
  2006-12-03 18:31 ` Jakub Narebski
@ 2006-12-03 18:34 ` Linus Torvalds
  2006-12-03 20:26   ` Junio C Hamano
  2006-12-03 20:40   ` Some advanced index playing Alan Chandler
  2 siblings, 2 replies; 46+ messages in thread
From: Linus Torvalds @ 2006-12-03 18:34 UTC (permalink / raw)
  To: Alan Chandler; +Cc: git



On Sun, 3 Dec 2006, Alan Chandler wrote:
> 
> c) Revert the entire index back to the state it was at the last commit so I 
> can selectively add back in the files that I have editted.
> 
> The command to do that seems to be
> 
> git-read-tree HEAD 

A side note on this..

It definitely works, but it's not really the right thing to do for a few 
reasons:

 - it isn't even what you wanted. You didn't want to reset _all_ the index 
   values, you only really wanted to reset a few of them. So as mentioned 
   in the previous email, the command sequence you'd wanted for that 
   operation is

	git ls-tree <tree> -- <path pattern list> |
		git update-index --index-info

But, that said, if you actually want to reset the whole index, 
"git-read-tree HEAD" works, but is not what you should do:

 - you really want to keep the index "stat()" cache valid, and 
   git-read-tree will throw that all out. So you would need to do a

	git update-index --refresh

   after you've reset the index ("git status" will do it for you, and if 
   you don't do either "git status" or the above,  

 - instead of re-reading the index 100% and then having to refresh it back 
   to mostly the same stat() into it already had, you can _merge_ the old 
   index with the information in HEAD, by using

	git read-tree -m HEAD

   which basically does a merge from the old index and the HEAD tree.

 - However, that actually fails if the old index wasn't just dirty, but 
   had unmerged paths etc, because then a "merge" would throw away that 
   unmerged information. So what you _really_ want to do is

	git read-tree --reset HEAD

   which (as the flag implies) will _reset_ the index to the tree in HEAD, 
   and this will do exactly what you were looking for: keep the "stat()" 
   information alone, but reset the actual index contents.

 - HOWEVER. This is exactly what "git reset" does.

So in short, you should just have done "git reset", and you'd have reset 
your index back to the state of your last commit.

So "git reset" is generally your friend whenever something goes wrong. If 
you also want to reset your checked-out files (which you did NOT want to 
do in this case, of course), you would have added the "--hard" flag to git 
reset.

And that (finally) concludes this particularly boring "git usage 101" 
session.


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Some advanced index playing
  2006-12-03 18:24 ` Linus Torvalds
@ 2006-12-03 19:36   ` Junio C Hamano
  2006-12-03 20:11   ` Alan Chandler
  2006-12-04 10:41   ` Junio C Hamano
  2 siblings, 0 replies; 46+ messages in thread
From: Junio C Hamano @ 2006-12-03 19:36 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git, Alan Chandler

Linus Torvalds <torvalds@osdl.org> writes:

> I think that is actually a misfeature. 
>
> This _should_ just work. It's the easy and logical way to do it, and it's 
> the one that matches all the other behaviours of "git commit" these days.
>
> The reason for the safety valve is actually not really "safety" any more, 
> it's purely "historical behaviour". Ie the sanity check is not there 
> because you would be doing anything unsafe, but simply because the 
> behaviour in this area _changed_, so the semantics are different from what 
> they were originally.
> ...
> Or at _least_ there should be a flag to force it.
>
> Junio?

I agree that if this sequence:

	$ edit foo
        $ git update-index foo
        $ edit foo
        $ git commit foo
        
is what the user actually gives from the command line, after
this commit is made, the user would want to _lose_ the state of
foo at the update-index after this commit is made, 100% of the
time (not "most likely", nor "usually", but "always").  So I am
very in favor of removing that check.

I am a bit worried if the reason behind this safety valve might
have been something else, and we describe the reason as "we
would lose data with this sequence otherwise" only to illustrate
what's happening behind the scene in technical terms.

In other words, while I think no user would ever want to keep
the state of foo at update-index after the above exact sequence
as the end-user action, I am worried if a usage sequence that
involve a group of operations encapsulated in a larger command
(a synthetic command that touches index and working files
without making the user painfully aware of the index -- likes of
git-mv, git-rm, ...) might have been the true motivation of the
safety valve.

I need to be reminded by somebody who went back to the list
discussion around the time we introduced --only, and made sure
that the "you would lose the snapshot you staged in the index if
we allowed it" literally meant only that and nothing else; not
some other common sequence that had the above command sequence
inside, and keeping the state of 'foo' at update-index time made
sense for that usage pattern, although I do not think of
anything offhand...


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Some advanced index playing
  2006-12-03 18:24 ` Linus Torvalds
  2006-12-03 19:36   ` Junio C Hamano
@ 2006-12-03 20:11   ` Alan Chandler
  2006-12-03 20:19     ` Jakub Narebski
  2006-12-03 20:40     ` Linus Torvalds
  2006-12-04 10:41   ` Junio C Hamano
  2 siblings, 2 replies; 46+ messages in thread
From: Alan Chandler @ 2006-12-03 20:11 UTC (permalink / raw)
  To: git; +Cc: Linus Torvalds, Junio C Hamano

On Sunday 03 December 2006 18:24, Linus Torvalds wrote:
> On Sun, 3 Dec 2006, Alan Chandler wrote:
> > With all the discussion about the index file in the last few days I would
> > have thought that this issue would have come up.  But I don't think it
> > has.
> >
> > I have been editing a set of files to make a commit, and after editing
> > each one had done a git update-index.
> >
> > At this point I am just about to commit when I realise that one of the
> > files has changes in it that really ought to be a separate commit.
> >
> > So effectively, I want to do one of three things
> >
> > a) git-commit <that-file>
> >
> > Except I can't because there is a safety valve that prevents this and
> > there is no force option.
...
>
> 	git ls-tree HEAD -- that-file | git update-index --index-info
> 	git commit that-file

I don't quite understand this - maybe it should be

git ls-tree HEAD -- that-file | git update-index --index-info
git commit
git commit -a

either I want to ONLY commit that file at the working tree state (and index 
before these commands), or I want to commit ALL except this file (so I can 
later come and commit just that file)

so having reset the index to the state of HEAD for "that-file" then the commit 
should make a commit with all the other changes in the index (but NOT 
that-file) and then the git commit -a picks up "that-file"

-- 
Alan Chandler

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Some advanced index playing
  2006-12-03 20:11   ` Alan Chandler
@ 2006-12-03 20:19     ` Jakub Narebski
  2006-12-03 20:29       ` Alan Chandler
  2006-12-03 20:40     ` Linus Torvalds
  1 sibling, 1 reply; 46+ messages in thread
From: Jakub Narebski @ 2006-12-03 20:19 UTC (permalink / raw)
  To: git

Alan Chandler wrote:

> On Sunday 03 December 2006 18:24, Linus Torvalds wrote:

> ...
>>
>>      git ls-tree HEAD -- that-file | git update-index --index-info
>>      git commit that-file
> 
> I don't quite understand this - maybe it should be
> 
> git ls-tree HEAD -- that-file | git update-index --index-info
> git commit
> git commit -a
> 
> either I want to ONLY commit that file at the working tree state (and index 
> before these commands), or I want to commit ALL except this file (so I can 
> later come and commit just that file)
> 
> so having reset the index to the state of HEAD for "that-file" then the commit 
> should make a commit with all the other changes in the index (but NOT 
> that-file) and then the git commit -a picks up "that-file"

$ git ls-tree HEAD -- that-file | git update-index --index-info
$ git commit that-file
$ git commit

resets index state of 'that-file' to the HEAD version, so the safety valve
is bypassed and "git commit that-file" commit _working directory_ version
only (because --only is default for "git commit <path>"). Then you commit
the rest of files.


While

$ git ls-tree HEAD -- that-file | git update-index --index-info
$ git commit
$ git commit -a

first commits rest of files, then all files to their working directory
version.

P.S. Could you wrap lines a little earlier (76 columns perhaps)? Thanks
in advance
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Some advanced index playing
  2006-12-03 18:34 ` Linus Torvalds
@ 2006-12-03 20:26   ` Junio C Hamano
  2006-12-05  3:48     ` [PATCH] git-explain Junio C Hamano
  2006-12-03 20:40   ` Some advanced index playing Alan Chandler
  1 sibling, 1 reply; 46+ messages in thread
From: Junio C Hamano @ 2006-12-03 20:26 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git, Alan Chandler

Linus Torvalds <torvalds@osdl.org> writes:

> So "git reset" is generally your friend whenever something goes wrong. If 
> you also want to reset your checked-out files (which you did NOT want to 
> do in this case, of course), you would have added the "--hard" flag to git 
> reset.
>
> And that (finally) concludes this particularly boring "git usage 101" 
> session.
>
> 			Linus

One observation about git, made in a relatively distant past,
was "git is not a usable system yet; there is no 'git undo'".  I
think it was on the kernel list (I think it was from Alan who
seems to have lost his last name from his From: line lately, but
I may be mistaken).

It left a deep psychological trauma in me, not because it was
stated in a brutal way (it wasn't) but because I fully agreed
with that statement from the end user point of view, but I did
not see a good solution to the problem (and I from the beginning
kept saying "I do not do Porcelains" and kept calling what is
shipped with core "Porcelain-ish").

"git reset" is one part of "undo".  For example, undoing a
commit can be approximated with "reset HEAD^" or "reset --hard
HEAD^"; undoing a conflicted and unfinished merge can be
approximated with "reset HEAD" or "reset --hard HEAD".

But for one thing, these are only "approximations" (the working
tree files after these two forms of reset are different from the
state you had before running "commit" or "merge").  And for
another thing, "reset" is only one part of "undo".  "reset"
would not help "undo"-ing a botched "git bisect good", for
example; you need "git bisect reset".  Similarly, "git rebase"
in the middle can be undone with its own --abort option.  But
the user has to know about them.  Another twist is that once
completed, "rebase --abort" obviously would not mean "undo the
last rebase".

I think one cause of the "problem" (if not having a general
"undo" is a problem, and I think it is to some extent) is that
git Porcelain-ish commands try to stay stateless to allow mixing
and matching of different commands to leave the door open to the
end user to be flexible, but they go too far.  Some of the
commands do leave its state (e.g. MERGE_HEAD is a sign of a
merge in progress, and git-commit notices it), and some of the
commands know about state markers from the other commands
(e.g. "git reset" removes MERGE_HEAD).  However, I think we do
not do enough of inter-command coordination.  Although I haven't
checked, I would be very surprised if we already prevented "git
bisect" from running, while a merge is in progress, for example.

While I do not think we need to supply "git undo" for a command
that already ran successfully to its completion (e.g. I think
the answer to "how would I undo a commit I just made" can be
left as "use one of the reset, or --amend, depending on what you
want"), it might be a worthwhile thing to aim for to give a
unified "git oops" command that recovers from a "unusual" state
your git working tree may be left in.

For UI-usability minded people, it might be a good thing to
enumerate the possible 'states' a working tree can be in, draw a
state transition diagram among them, with possible and forbidden
transitions.  After that, we can annotate the allowed
transitions with "use this command to make this transtion
happen".

The following list may be a starter; it is not comprehensive,
and does not list the transitions, though.

 - the base state

 - bisect in progress

 - conflicted merge in progress (or --no-commit given)

 - immediately after "merge --squash" is run but not yet committed

 - conflicted git am/git applymbox in progress.

 - conflicted git rebase in progress (this actually has two
   separate states depending on --merge was used or not).

 - conflicted git revert/cherry-pick (I list this separately
   from 'merge in progress' because this is an example of
   command leaving too little state).

 - conflicted 'git checkout -m' (although this is almost the
   same as the base state, it has higher stages in the index).

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Some advanced index playing
  2006-12-03 20:19     ` Jakub Narebski
@ 2006-12-03 20:29       ` Alan Chandler
  0 siblings, 0 replies; 46+ messages in thread
From: Alan Chandler @ 2006-12-03 20:29 UTC (permalink / raw)
  To: git; +Cc: Jakub Narebski

On Sunday 03 December 2006 20:19, Jakub Narebski wrote:

> P.S. Could you wrap lines a little earlier (76 columns perhaps)?
> Thanks in advance

Sorry wrap was set at 78 (kmail).  I've changed it to 72


-- 
Alan Chandler

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Some advanced index playing
  2006-12-03 18:34 ` Linus Torvalds
  2006-12-03 20:26   ` Junio C Hamano
@ 2006-12-03 20:40   ` Alan Chandler
  1 sibling, 0 replies; 46+ messages in thread
From: Alan Chandler @ 2006-12-03 20:40 UTC (permalink / raw)
  To: git; +Cc: Linus Torvalds

On Sunday 03 December 2006 18:34, Linus Torvalds wrote:
...
> So in short, you should just have done "git reset", and you'd have
> reset your index back to the state of your last commit.
>
> So "git reset" is generally your friend whenever something goes
> wrong. If you also want to reset your checked-out files (which you
> did NOT want to do in this case, of course), you would have added the
> "--hard" flag to git reset.

Doh [slaps head with wet blanket]

I was so worried about NOT changing my working tree - I totally 
overlooked the reset command.  I had never quite understood 
what --mixed really meant before.  But re-reading the man page now, its 
obvious.



-- 
Alan Chandler

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Some advanced index playing
  2006-12-03 20:11   ` Alan Chandler
  2006-12-03 20:19     ` Jakub Narebski
@ 2006-12-03 20:40     ` Linus Torvalds
  1 sibling, 0 replies; 46+ messages in thread
From: Linus Torvalds @ 2006-12-03 20:40 UTC (permalink / raw)
  To: Alan Chandler; +Cc: git, Junio C Hamano



On Sun, 3 Dec 2006, Alan Chandler wrote:
> >
> > 	git ls-tree HEAD -- that-file | git update-index --index-info
> > 	git commit that-file
> 
> I don't quite understand this - maybe it should be
> 
> git ls-tree HEAD -- that-file | git update-index --index-info
> git commit
> git commit -a

Sure. It depends on which file you want to commit first.

If you want to commit "that-file" first, you do my sequence.

If you want to commit everything _but_ "that-file", you do the second 
sequence (which basically removes the changes to "that-file" from the 
index, then commits the index, and then with "git commit -a" commits the 
remaining dirty state, which is obviously those changes to "that-file" 
that you still had in the working tree).

> either I want to ONLY commit that file at the working tree state (and index 
> before these commands), or I want to commit ALL except this file (so I can 
> later come and commit just that file)

Right. If you do

	git ls-tree HEAD -- that-file | git update-index --index-info
	git commit that-file

you basically ONLY commit "that-file". You first reset it (in the index) 
to the old state, but that's just so that "git commit that-file" will now 
happily commit the current state (in the working tree) of "that-file".

So "git commit that-file" will basically _ignore_ your current index. 
Because you told "git commit" (by naming "that-file") that you _only_ 
wanted to commit "that-file". So whatever state you had in your current 
index doesn't matter at all - it will only look at the HEAD tree _and_ 
that single file that you specified.


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Some advanced index playing
  2006-12-03 18:24 ` Linus Torvalds
  2006-12-03 19:36   ` Junio C Hamano
  2006-12-03 20:11   ` Alan Chandler
@ 2006-12-04 10:41   ` Junio C Hamano
  2 siblings, 0 replies; 46+ messages in thread
From: Junio C Hamano @ 2006-12-04 10:41 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Alan Chandler, git

Linus Torvalds <torvalds@osdl.org> writes:

> I think that is actually a misfeature. 
>
> This _should_ just work. It's the easy and logical way to do it, and it's 
> the one that matches all the other behaviours of "git commit" these days.
> ...
> So anyway, I would suggest that we just get rid of that partial commit 
> "safety check" in "git commit" for now. It still makes sense for when 
> you're in the middle of a _merge_, but the "verify that index matches" is 
> not worth it.

The codepath has a big "don't do this during a merge" check in
front.  I think this is a safe thing to do, so let's do this.

diff --git a/git-commit.sh b/git-commit.sh
index 81c3a0c..c829791 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -350,19 +350,9 @@ t,)
 			refuse_partial "Cannot do a partial commit during a merge."
 		fi
 		TMP_INDEX="$GIT_DIR/tmp-index$$"
-		if test -z "$initial_commit"
-		then
-			# make sure index is clean at the specified paths, or
-			# they are additions.
-			dirty_in_index=`git-diff-index --cached --name-status \
-				--diff-filter=DMTU HEAD -- "$@"`
-			test -z "$dirty_in_index" ||
-			refuse_partial "Different in index and the last commit:
-$dirty_in_index"
-		fi
 		commit_only=`git-ls-files --error-unmatch -- "$@"` || exit
 
-		# Build the temporary index and update the real index
+		# Build a temporary index and update the real index
 		# the same way.
 		if test -z "$initial_commit"
 		then

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH] git-explain
  2006-12-03 20:26   ` Junio C Hamano
@ 2006-12-05  3:48     ` Junio C Hamano
  2006-12-05  3:55       ` Nicolas Pitre
  2006-12-05 10:43       ` Jakub Narebski
  0 siblings, 2 replies; 46+ messages in thread
From: Junio C Hamano @ 2006-12-05  3:48 UTC (permalink / raw)
  To: git; +Cc: Linus Torvalds

Junio C Hamano <junkio@cox.net> writes:

> Linus Torvalds <torvalds@osdl.org> writes:
>
>> So "git reset" is generally your friend whenever something goes wrong. If 
>> you also want to reset your checked-out files (which you did NOT want to 
>> do in this case, of course), you would have added the "--hard" flag to git 
>> reset.
>>
>> And that (finally) concludes this particularly boring "git usage 101" 
>> session.
>
> One observation about git, made in a relatively distant past,
> was "git is not a usable system yet; there is no 'git undo'".  I
> think it was on the kernel list (I think it was from Alan who
> seems to have lost his last name from his From: line lately, but
> I may be mistaken).
>
> It left a deep psychological trauma in me, not because it was
> stated in a brutal way (it wasn't) but because I fully agreed
> with that statement from the end user point of view, but I did
> not see a good solution to the problem (and I from the beginning
> kept saying "I do not do Porcelains" and kept calling what is
> shipped with core "Porcelain-ish").
> ...
> I think one cause of the "problem" (if not having a general
> "undo" is a problem, and I think it is to some extent) is that
> git Porcelain-ish commands try to stay stateless to allow mixing
> and matching of different commands to leave the door open to the
> end user to be flexible, but they go too far.

They go too far by doing too little.

And here is throwing an idea to remedy it; it is far from
complete but sending it out early before spending too much time
on pursuit of a wild goose.

-- >8 --
[PATCH] git-explain

This patch adds "git-explain" script that notices various clues
other commands can leave the working tree and repository in and
intended to guide the end user out of the confused mess.

This is only a demonstration-of-concept, as many commands do not
leave enough information for us to truly figure out what state
we are in nor how we got into the mess.  As a demonstration, it
makes "git merge" to leave a new "$GIT_DIR/FAILED_MERGE" file
when it gives up before touching the working tree.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 git-explain.sh |  172 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 git-merge.sh   |    3 +-
 git-reset.sh   |    3 +-
 3 files changed, 176 insertions(+), 2 deletions(-)

diff --git a/git-explain.sh b/git-explain.sh
new file mode 100755
index 0000000..07115ae
--- /dev/null
+++ b/git-explain.sh
@@ -0,0 +1,172 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Junio C Hamano
+#
+
+SUBDIRECTORY_OK=Yes
+. git-sh-setup
+
+TOP=`git-rev-parse --show-cdup`
+test -z "$TOP" && TOP=./
+
+say_with_indent() {
+	echo "$@" | sed -e 's/^/	/'
+}
+
+explain_merge_conflict () {
+	test -f $GIT_DIR/HEAD &&
+	test -f "$GIT_DIR/MERGE_HEAD" &&
+	test -f "$GIT_DIR/MERGE_MSG" &&
+	test "z`git ls-files -u`" != z || return 1
+
+	title=$(sed -e q "$GIT_DIR/MERGE_MSG")
+
+	conflicts=$(git ls-files -u |
+		sed -e 's/^[0-7]* [0-9a-f]* [0-3]	//' |
+		uniq)
+
+	git diff-index -r --name-only HEAD >"$tmp-1"
+	git diff-tree -r --name-only HEAD MERGE_HEAD >"$tmp-2"
+	locals=`comm -23 "$tmp-1" "$tmp-2"`
+
+	cat <<EOF
+You tried a merge whose title will be
+
+	$title
+
+This may have successfully merged some paths and they are already
+staged for the next commit, but the following paths could not be
+automatically merged:
+
+EOF
+
+	say_with_indent "$conflicts"
+
+	if test "z$locals" != z
+	then
+		cat <<\EOF
+
+Also you seem to have local modifications unrelated to this
+merge in the following paths:
+
+EOF
+
+		say_with_indent "$locals"
+	fi
+
+	cat <<\EOF
+
+There are two ways to proceed from here:
+
+(1) You can decide to abort this for now and get back to the
+    state before you started the merge.
+
+	$ git reset --hard
+
+    will revert your working tree files to the latest commit
+    before this merge.  Note that you would lose the local
+    modifications with this action, so you might want to
+    preserve them with this command before doing so:
+
+	$ git diff HEAD -- <locally changed files> >preserved-local-changes
+
+    Then you can re-apply the changes after running "git reset
+    --hard".
+
+(2) To resolve the conflicts and record this merge, you first
+    need to examine the conflicted paths listed above, and edit
+    them in your working tree.  After you are done, stage the
+    final version of these paths and make a commit, like this:
+
+	$ git add <locally changed files>
+	$ git commit
+
+    Do not use "git commit -a" unless you want the local
+    modifications that are not related to this merge committed
+    along with the changes this merge brings in.
+EOF
+}
+
+explain_merge_stop () {
+	test -f $GIT_DIR/HEAD &&
+	test -f $GIT_DIR/FAILED_MERGE &&
+	test "z`git rev-parse HEAD`" = "z`git rev-parse ORIG_HEAD`" &&
+	test "z`git ls-files -u`" = z || return 1
+
+	git diff-index -r --name-only HEAD >"$tmp-1"
+	git diff-tree -r --name-only HEAD FAILED_MERGE >"$tmp-2"
+	conflicts=`comm -12 "$tmp-1" "$tmp-2"`
+	locals=`comm -23 "$tmp-1" "$tmp-2"`
+
+	cat <<\EOF
+You tried a merge but it stopped without touching your working tree,
+because you had local changes in the following paths that may also
+be involved in the merge:
+
+EOF
+	say_with_indent "$conflicts"
+
+	if test "z$locals" != z
+	then
+		cat <<\EOF
+
+Also you seem to have local modifications unrelated to this
+merge in the following paths:
+
+EOF
+
+		say_with_indent "$locals"
+
+	fi
+
+	cat <<\EOF
+
+There are two ways to proceed from here:
+
+(1) You can decide to abort this merge for now.  You do not have
+    to do anything in this case because the merge hasn't touched
+    your working tree files yet.
+
+(2) You can first make the merge and re-apply your local changes
+    on top of the merge result.  To do this, first stash away
+    your local changes in a temporary commit, like this:
+
+	$ git commit -a -m 'WIP'
+	$ git tag WIP
+	$ git reset --hard HEAD^
+
+    and run the same "git merge" (or "git pull") again.  After
+    you finished the merge, you can review the local change with:
+
+	$ git show WIP
+
+    and re-apply them to your working tree as you see fit.
+EOF
+}
+
+explain_am () {
+	test -d $TOP/.dotest || return 1
+
+	cat <<EOF
+You tried to apply a series of `cat .dotest/last` patch(es) and
+are currently handling patch #`cat .dotest/next`.
+
+EOF
+
+}
+
+explain_base () {
+	# This comes at the end.
+	cat <<EOF
+Don't worry, you are not in the middle of anything complicated.
+EOF
+
+}
+
+tmp="$GIT_DIR/tmp"
+trap 'rm -f $tmp-*' 0
+
+explain_merge_conflict ||
+explain_merge_stop ||
+explain_am ||
+explain_base
\ No newline at end of file
diff --git a/git-merge.sh b/git-merge.sh
index 272f004..674abad 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -20,7 +20,7 @@ index_merge=t
 
 dropsave() {
 	rm -f -- "$GIT_DIR/MERGE_HEAD" "$GIT_DIR/MERGE_MSG" \
-		 "$GIT_DIR/MERGE_SAVE" || exit 1
+		 "$GIT_DIR/MERGE_SAVE" "$GIT_DIR/FAILED_MERGE" || exit 1
 }
 
 savestate() {
@@ -400,6 +400,7 @@ case "$best_strategy" in
 '')
 	restorestate
 	echo >&2 "No merge strategy handled the merge."
+	echo "$@" >"$GIT_DIR/FAILED_MERGE"
 	exit 2
 	;;
 "$wt_strategy")
diff --git a/git-reset.sh b/git-reset.sh
index 3133b5b..045ee79 100755
--- a/git-reset.sh
+++ b/git-reset.sh
@@ -63,6 +63,7 @@ case "$reset_type" in
 	;;
 esac
 
-rm -f "$GIT_DIR/MERGE_HEAD" "$GIT_DIR/rr-cache/MERGE_RR" "$GIT_DIR/SQUASH_MSG"
+rm -f "$GIT_DIR/MERGE_HEAD" "$GIT_DIR/rr-cache/MERGE_RR" \
+	"$GIT_DIR/SQUASH_MSG" "$GIT_DIR/FAILED_MERGE"
 
 exit $update_ref_status
-- 
1.4.4.1.g15e3


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* Re: [PATCH] git-explain
  2006-12-05  3:48     ` [PATCH] git-explain Junio C Hamano
@ 2006-12-05  3:55       ` Nicolas Pitre
  2006-12-05  3:57         ` J. Bruce Fields
  2006-12-05 10:43       ` Jakub Narebski
  1 sibling, 1 reply; 46+ messages in thread
From: Nicolas Pitre @ 2006-12-05  3:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Linus Torvalds

On Mon, 4 Dec 2006, Junio C Hamano wrote:

> [PATCH] git-explain
> 
> This patch adds "git-explain" script that notices various clues
> other commands can leave the working tree and repository in and
> intended to guide the end user out of the confused mess.

What about calling it git-whatsup instead?



^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] git-explain
  2006-12-05  3:55       ` Nicolas Pitre
@ 2006-12-05  3:57         ` J. Bruce Fields
  2006-12-05  6:09           ` Junio C Hamano
  0 siblings, 1 reply; 46+ messages in thread
From: J. Bruce Fields @ 2006-12-05  3:57 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Junio C Hamano, git, Linus Torvalds

On Mon, Dec 04, 2006 at 10:55:49PM -0500, Nicolas Pitre wrote:
> On Mon, 4 Dec 2006, Junio C Hamano wrote:
> 
> > [PATCH] git-explain
> > 
> > This patch adds "git-explain" script that notices various clues
> > other commands can leave the working tree and repository in and
> > intended to guide the end user out of the confused mess.
> 
> What about calling it git-whatsup instead?

No, clearly it should be git-wtf.


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] git-explain
  2006-12-05  3:57         ` J. Bruce Fields
@ 2006-12-05  6:09           ` Junio C Hamano
  2006-12-05  7:26             ` Jeff King
                               ` (2 more replies)
  0 siblings, 3 replies; 46+ messages in thread
From: Junio C Hamano @ 2006-12-05  6:09 UTC (permalink / raw)
  To: J. Bruce Fields, Nicolas Pitre; +Cc: git, Linus Torvalds

"J. Bruce Fields" <bfields@fieldses.org> writes:

> On Mon, Dec 04, 2006 at 10:55:49PM -0500, Nicolas Pitre wrote:
>> ...
>> > [PATCH] git-explain
>> > ...
>> 
>> What about calling it git-whatsup instead?
>
> No, clearly it should be git-wtf.

Should I take these responses to mean that you two are negative
about the approach of spending extra cycles to commands that can
leave the working tree in a "in the middle of doing something"
state to help having a unified command to explain what the
situation is and suggest the user possible exits, or are you
saying that it might be a good idea but "git explain" is a bad
name?

An issue with this approach is that this can be the beginning of
hardwiring the official "right way of doing things" in the set
of tools.  Pursuing this approach would enhance the set of state
markers like "FAILED_MERGE" in the example, which means:

 - more commands would actively record what they were attempting
   to do, obviously;

 - over time "git explain" will learn about these state markers,
   and we would hardwire the "best current practice" exits from
   various states in the help messages;

 - also commands other than "git explain" would learn about the
   state markers of other commands, and change their behaviour.
   For example, "git am" might learn to refuse running while a
   merge in progress much earlier than with the current
   implementation.

The last point can easily become a double-edged sword.

Hardwiring the recommended workflow in the tools would reduce
chances of mistakes, but it could rob the flexibility from them
if we are not careful and forget to take into account some
useful combination of tools when adding such safety valves.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] git-explain
  2006-12-05  6:09           ` Junio C Hamano
@ 2006-12-05  7:26             ` Jeff King
  2006-12-05  9:21               ` Eric Wong
  2006-12-05 17:34               ` [PATCH] git-explain Horst H. von Brand
  2006-12-05  8:58             ` Johannes Schindelin
  2006-12-05  9:11             ` Raimund Bauer
  2 siblings, 2 replies; 46+ messages in thread
From: Jeff King @ 2006-12-05  7:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: J. Bruce Fields, Nicolas Pitre, git, Linus Torvalds

On Mon, Dec 04, 2006 at 10:09:17PM -0800, Junio C Hamano wrote:

> Should I take these responses to mean that you two are negative
> about the approach of spending extra cycles to commands that can
> leave the working tree in a "in the middle of doing something"
> state to help having a unified command to explain what the
> situation is and suggest the user possible exits, or are you
> saying that it might be a good idea but "git explain" is a bad
> name?

It seems like the point of this command is to show some state
information which would otherwise be hard to see. I think of 'git
status' as the way to look at the repository state. Perhaps we should
enhance the output of 'git status' to note things such as failed merges,
whether we're bisecting, in the middle of applying a patch series, etc.
There could be an optional verbosity switch to give "full explanations"
including recommended ways to deal with the situation.

> Hardwiring the recommended workflow in the tools would reduce
> chances of mistakes, but it could rob the flexibility from them
> if we are not careful and forget to take into account some
> useful combination of tools when adding such safety valves.

As long as the safety valves don't come up _routinely_ in certain
workflows, it seems OK to bypass them with a '-f' force switch. I
suspect the best way to figure out if such workflows are in use is to
put in the safety valves and see who complains; otherwise we're stuck
with brainstorming workflows and deciding whether they make sense.


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] git-explain
  2006-12-05  6:09           ` Junio C Hamano
  2006-12-05  7:26             ` Jeff King
@ 2006-12-05  8:58             ` Johannes Schindelin
  2006-12-05 21:00               ` J. Bruce Fields
  2006-12-05  9:11             ` Raimund Bauer
  2 siblings, 1 reply; 46+ messages in thread
From: Johannes Schindelin @ 2006-12-05  8:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: J. Bruce Fields, Nicolas Pitre, git, Linus Torvalds

Hi,

On Mon, 4 Dec 2006, Junio C Hamano wrote:

> "J. Bruce Fields" <bfields@fieldses.org> writes:
> 
> > On Mon, Dec 04, 2006 at 10:55:49PM -0500, Nicolas Pitre wrote:
> >> ...
> >> > [PATCH] git-explain
> >> > ...
> >> 
> >> What about calling it git-whatsup instead?
> >
> > No, clearly it should be git-wtf.
> 
> Should I take these responses to mean that you two are negative
> about the approach [...]

I think they just were in the mood for some slashdot style 
unimportant-aspects-in-a-funny-way discussion.

> An issue with this approach is that this can be the beginning of
> hardwiring the official "right way of doing things" in the set
> of tools.  Pursuing this approach would enhance the set of state
> markers like "FAILED_MERGE" in the example, which means:
> 
>  - more commands would actively record what they were attempting
>    to do, obviously;

... which is a good thing.

>  - over time "git explain" will learn about these state markers,
>    and we would hardwire the "best current practice" exits from
>    various states in the help messages;

... which is also a good thing.

>  - also commands other than "git explain" would learn about the
>    state markers of other commands, and change their behaviour.
>    For example, "git am" might learn to refuse running while a
>    merge in progress much earlier than with the current
>    implementation.

If the other commands are outside of git, it will be a problem.

> The last point [git-am refusing to run during a merge] can easily become 
> a double-edged sword.

This particular behaviour seems like a good thing, too!

> Hardwiring the recommended workflow in the tools would reduce chances of 
> mistakes, but it could rob the flexibility from them if we are not 
> careful and forget to take into account some useful combination of tools 
> when adding such safety valves.

As has been the case not at all long ago, a saftey valve which no longer 
made sense was just removed.

As for the inflexibility of a recommended workflow: by now, long-time 
gitsters have had enough time to fiddle around with git and to develop a 
workflow which Just Works. It is just a nice gesture of old-time users 
towards new-time users to pass that knowledge. And new-time users are 
often not in the least interested in learning the ropes the hard way.

Besides, the recommended workflow(s) can be changed/replaced by other 
porcelainish commands, because only those will contain the safety valves, 
right?

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 46+ messages in thread

* RE: [PATCH] git-explain
  2006-12-05  6:09           ` Junio C Hamano
  2006-12-05  7:26             ` Jeff King
  2006-12-05  8:58             ` Johannes Schindelin
@ 2006-12-05  9:11             ` Raimund Bauer
  2 siblings, 0 replies; 46+ messages in thread
From: Raimund Bauer @ 2006-12-05  9:11 UTC (permalink / raw)
  To: 'Junio C Hamano', 'J. Bruce Fields',
	'Nicolas Pitre'
  Cc: git, 'Linus Torvalds'

> An issue with this approach is that this can be the beginning 
> of hardwiring the official "right way of doing things" in the 
> set of tools.  Pursuing this approach would enhance the set 
> of state markers like "FAILED_MERGE" in the example, which means:

Wouldn't it be better to create some kind of action-log (that's
cleared at the end of the command if everything was all right)
instead of creating special markers for different commands?

That way there would be only 1 place to check for what happened ...

-- 
best regards

  Ray

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] git-explain
  2006-12-05  7:26             ` Jeff King
@ 2006-12-05  9:21               ` Eric Wong
  2006-12-08 10:49                 ` [RFC/PATCH 0/5] WIP status/rerere reporting Eric Wong
                                   ` (5 more replies)
  2006-12-05 17:34               ` [PATCH] git-explain Horst H. von Brand
  1 sibling, 6 replies; 46+ messages in thread
From: Eric Wong @ 2006-12-05  9:21 UTC (permalink / raw)
  To: Jeff King
  Cc: Junio C Hamano, J. Bruce Fields, Nicolas Pitre, git,
	Linus Torvalds

Jeff King <peff@peff.net> wrote:
> On Mon, Dec 04, 2006 at 10:09:17PM -0800, Junio C Hamano wrote:
> 
> > Should I take these responses to mean that you two are negative
> > about the approach of spending extra cycles to commands that can
> > leave the working tree in a "in the middle of doing something"
> > state to help having a unified command to explain what the
> > situation is and suggest the user possible exits, or are you
> > saying that it might be a good idea but "git explain" is a bad
> > name?
> 
> It seems like the point of this command is to show some state
> information which would otherwise be hard to see. I think of 'git
> status' as the way to look at the repository state. Perhaps we should
> enhance the output of 'git status' to note things such as failed merges,
> whether we're bisecting, in the middle of applying a patch series, etc.
> There could be an optional verbosity switch to give "full explanations"
> including recommended ways to deal with the situation.

I wholeheartedly agree that 'git status' should show something like
this.  I actually had some stuff that was a work-in-progress several
months ago that enhanced status with several things like this; but got
distracted and forgot about that repository.  I'll try to dig it out
sometime tomorrow.  I remember my work started from wanting to know
what 'git-rerere' would be recording.

-- 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] git-explain
  2006-12-05  3:48     ` [PATCH] git-explain Junio C Hamano
  2006-12-05  3:55       ` Nicolas Pitre
@ 2006-12-05 10:43       ` Jakub Narebski
  2006-12-05 23:00         ` Martin Langhoff
  1 sibling, 1 reply; 46+ messages in thread
From: Jakub Narebski @ 2006-12-05 10:43 UTC (permalink / raw)
  To: git

Junio C Hamano wrote:

> [PATCH] git-explain
> 
> This patch adds "git-explain" script that notices various clues
> other commands can leave the working tree and repository in and
> intended to guide the end user out of the confused mess.

I like it, although I think that it explains a bit too little
about aborted git-rebase / git-am, and for example doesn't touch
git-bisect at all. Both as an end-user script (BTW. the name could
be changed using aliases), and as a policy for git commands.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] git-explain
  2006-12-05  7:26             ` Jeff King
  2006-12-05  9:21               ` Eric Wong
@ 2006-12-05 17:34               ` Horst H. von Brand
  1 sibling, 0 replies; 46+ messages in thread
From: Horst H. von Brand @ 2006-12-05 17:34 UTC (permalink / raw)
  To: Jeff King
  Cc: Junio C Hamano, J. Bruce Fields, Nicolas Pitre, git,
	Linus Torvalds

Jeff King <peff@peff.net> wrote:

[...]

> As long as the safety valves don't come up _routinely_ in certain
> workflows, it seems OK to bypass them with a '-f' force switch. I
> suspect the best way to figure out if such workflows are in use is to
> put in the safety valves and see who complains; otherwise we're stuck
> with brainstorming workflows and deciding whether they make sense.

Problem is that nobody reads the manuals, next to nobody complains, and
when it doesn't work out via road A you try plan B. A might have been
exactly right, but it is blocked, and you'll never know if it is because of
fundamental reasons or by decree.
-- 
Dr. Horst H. von Brand                   User #22616 counter.li.org
Departamento de Informatica                    Fono: +56 32 2654431
Universidad Tecnica Federico Santa Maria             +56 32 2654239

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] git-explain
  2006-12-05  8:58             ` Johannes Schindelin
@ 2006-12-05 21:00               ` J. Bruce Fields
  0 siblings, 0 replies; 46+ messages in thread
From: J. Bruce Fields @ 2006-12-05 21:00 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Nicolas Pitre, git, Linus Torvalds

On Tue, Dec 05, 2006 at 09:58:25AM +0100, Johannes Schindelin wrote:
> On Mon, 4 Dec 2006, Junio C Hamano wrote:
> > "J. Bruce Fields" <bfields@fieldses.org> writes:
> > 
> > > On Mon, Dec 04, 2006 at 10:55:49PM -0500, Nicolas Pitre wrote:
> > >> ...
> > >> > [PATCH] git-explain
> > >> > ...
> > >> 
> > >> What about calling it git-whatsup instead?
> > >
> > > No, clearly it should be git-wtf.
> > 
> > Should I take these responses to mean that you two are negative
> > about the approach [...]
> 
> I think they just were in the mood for some slashdot style 
> unimportant-aspects-in-a-funny-way discussion.

Yeah, I was just being silly, apologies.


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] git-explain
  2006-12-05 10:43       ` Jakub Narebski
@ 2006-12-05 23:00         ` Martin Langhoff
  2006-12-05 23:07           ` Junio C Hamano
  0 siblings, 1 reply; 46+ messages in thread
From: Martin Langhoff @ 2006-12-05 23:00 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git

On 12/5/06, Jakub Narebski <jnareb@gmail.com> wrote:
> Junio C Hamano wrote:
>
> > [PATCH] git-explain
> >
> > This patch adds "git-explain" script that notices various clues
> > other commands can leave the working tree and repository in and
> > intended to guide the end user out of the confused mess.
>
> I like it, although I think that it explains a bit too little

I like what it does too... but why not as part of git-status?

cheers,



^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] git-explain
  2006-12-05 23:00         ` Martin Langhoff
@ 2006-12-05 23:07           ` Junio C Hamano
  2006-12-05 23:37             ` Johannes Schindelin
  0 siblings, 1 reply; 46+ messages in thread
From: Junio C Hamano @ 2006-12-05 23:07 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: git

"Martin Langhoff" <martin.langhoff@gmail.com> writes:

> On 12/5/06, Jakub Narebski <jnareb@gmail.com> wrote:
>> Junio C Hamano wrote:
>>
>> > [PATCH] git-explain
>> >
>> > This patch adds "git-explain" script that notices various clues
>> > other commands can leave the working tree and repository in and
>> > intended to guide the end user out of the confused mess.
>>
>> I like it, although I think that it explains a bit too little
>
> I like what it does too... but why not as part of git-status?

The biggest reason was that it is a demonstration of concepts,
not replacement of anything.  Maybe "git status" can be replaced
with something like that if people worked on it enough.

The current use of "git status" inside "git commit" needs to be
revamped if we pursue this further, though.  Because one of the
points the "what state is this repository in" check "explain"
does is to define what operations are sensible in each state,
and most likely in many state it would not make _any_ sense to
run "git commit" (say, in the middle of "bisect").





^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] git-explain
  2006-12-05 23:07           ` Junio C Hamano
@ 2006-12-05 23:37             ` Johannes Schindelin
  2006-12-05 23:57               ` Junio C Hamano
  0 siblings, 1 reply; 46+ messages in thread
From: Johannes Schindelin @ 2006-12-05 23:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Martin Langhoff, git

Hi,

On Tue, 5 Dec 2006, Junio C Hamano wrote:

> "Martin Langhoff" <martin.langhoff@gmail.com> writes:
> 
> > I like what it does too... but why not as part of git-status?
> 
> The biggest reason was that it is a demonstration of concepts,
> not replacement of anything.  Maybe "git status" can be replaced
> with something like that if people worked on it enough.

Okay, so what do people need? This is evidently a question for people who 
are not intimately familiar with the core of Git. So, where are you 
newbies when we need you? Speak up!

Surely, you hit a wall on the road, where you really wanted to ask "git 
wtf"?

> The current use of "git status" inside "git commit" needs to be revamped 
> if we pursue this further, though.  Because one of the points the "what 
> state is this repository in" check "explain" does is to define what 
> operations are sensible in each state, and most likely in many state it 
> would not make _any_ sense to run "git commit" (say, in the middle of 
> "bisect").

That is right, but it is only a matter of having a command line switch to 
suppress what you do not need for the commit message. After all, "status" 
really should tell you about the status of the working directory. The fact 
that we have the _same_ script for "commit" and "status" is no excuse!

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] git-explain
  2006-12-05 23:37             ` Johannes Schindelin
@ 2006-12-05 23:57               ` Junio C Hamano
  2006-12-06  0:07                 ` Carl Worth
  2006-12-06  0:27                 ` Johannes Schindelin
  0 siblings, 2 replies; 46+ messages in thread
From: Junio C Hamano @ 2006-12-05 23:57 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Tue, 5 Dec 2006, Junio C Hamano wrote:
>...
>> The current use of "git status" inside "git commit" needs to be revamped 
>> if we pursue this further, though.  Because one of the points the "what 
>> state is this repository in" check "explain" does is to define what 
>> operations are sensible in each state, and most likely in many state it 
>> would not make _any_ sense to run "git commit" (say, in the middle of 
>> "bisect").
>
> That is right, but it is only a matter of having a command line switch to 
> suppress what you do not need for the commit message. After all, "status" 
> really should tell you about the status of the working directory. The fact 
> that we have the _same_ script for "commit" and "status" is no excuse!

Sure.  The way I envisioned it was that we would have a moral
equivalent of current git-status output for explanation of the
base state.

I originally wanted to do "git oops" which would have been a
universal "undo" command, and "explain" was only the first
smaller step toward that goal.  But the more I think about it, I
feel that "undo" is less important.


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] git-explain
  2006-12-05 23:57               ` Junio C Hamano
@ 2006-12-06  0:07                 ` Carl Worth
  2006-12-06  0:27                 ` Johannes Schindelin
  1 sibling, 0 replies; 46+ messages in thread
From: Carl Worth @ 2006-12-06  0:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git

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

On Tue, 05 Dec 2006 15:57:00 -0800, Junio C Hamano wrote:
> I originally wanted to do "git oops" which would have been a
> universal "undo" command, and "explain" was only the first
> smaller step toward that goal.  But the more I think about it, I
> feel that "undo" is less important.

I like the idea of "git status" explaining what operations are "in
progress" and what the user might want to do next to complete the
operation.

As for the desire for "undo", it probably makes sense to just keep
focusing on making "git reset" a nice, safe way to undo any sort of
"in progress" operation. The recent cleanup patches are in line with
this.

One thing that conflicts with that goal a bit is that git-reset is
also used to move the current branch arbitrarily in the commit DAG,
and also (with --hard) to discard local commits. Those are definitely
less "safe" operations.

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] git-explain
  2006-12-05 23:57               ` Junio C Hamano
  2006-12-06  0:07                 ` Carl Worth
@ 2006-12-06  0:27                 ` Johannes Schindelin
  2006-12-06  1:50                   ` Nicolas Pitre
  1 sibling, 1 reply; 46+ messages in thread
From: Johannes Schindelin @ 2006-12-06  0:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Hi,

On Tue, 5 Dec 2006, Junio C Hamano wrote:

> But the more I think about it, I feel that "undo" is less important.

Me, too. I was really astonished that people said -- after quite some 
explanations -- "wow, you mean I did not lose any work?"

That's the nice thing about git: it is really hard to lose any objects. 
Unless you explicitely issue a "git prune", you can resurrect whatever was 
your state.

In this spirit, maybe it is time to enable reflog per default?


Ciao,
Dscho

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] git-explain
  2006-12-06  0:27                 ` Johannes Schindelin
@ 2006-12-06  1:50                   ` Nicolas Pitre
  0 siblings, 0 replies; 46+ messages in thread
From: Nicolas Pitre @ 2006-12-06  1:50 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git

On Wed, 6 Dec 2006, Johannes Schindelin wrote:

> In this spirit, maybe it is time to enable reflog per default?

Definitely.



^ permalink raw reply	[flat|nested] 46+ messages in thread

* [RFC/PATCH 0/5] WIP status/rerere reporting
  2006-12-05  9:21               ` Eric Wong
@ 2006-12-08 10:49                 ` Eric Wong
  2006-12-08 10:49                 ` [PATCH 1/5] rerere: avoid misrecording on a skipped or aborted rebase/am Eric Wong
                                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 46+ messages in thread
From: Eric Wong @ 2006-12-08 10:49 UTC (permalink / raw)
  To: git


This is the stuff I mentioned I had been working on several months
before in a reply to the git-explain/git-wtf/git-whatsup thread.

I've rebased it against the current master and everything still seems
to work (I don't have unit tests for them).

This has been forgotten and abandoned for a while.  I especially don't
expect the changes to git-commit.sh (status) to be applied as-is, as it
should go into the new runstatus (my work predates runstatus).

 git-am.sh       |   18 ++++++++++++++----
 git-commit.sh   |   45 ++++++++++++++++++++++++++++++++++++++++++++-
 git-rebase.sh   |   43 ++++++++++++++++++++++++++++++-------------
 git-rerere.perl |   25 +++++++++++++++++++++++++
 4 files changed, 113 insertions(+), 18 deletions(-)

[PATCH 1/5] rerere: avoid misrecording on a skipped or aborted rebase/am
[PATCH 2/5] status: show files that would have resolutions recorded by rerere
[PATCH 3/5] am and rebase resolve states get picked up by status/commit
[PATCH 4/5] am: run git rerere to record resolution on successful --resolved
[PATCH 5/5] rerere: add the diff command

-- 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH 1/5] rerere: avoid misrecording on a skipped or aborted rebase/am
  2006-12-05  9:21               ` Eric Wong
  2006-12-08 10:49                 ` [RFC/PATCH 0/5] WIP status/rerere reporting Eric Wong
@ 2006-12-08 10:49                 ` Eric Wong
  2006-12-08 19:33                   ` Junio C Hamano
  2006-12-08 20:43                   ` [PATCH 1/5] rerere: avoid misrecording on a skipped or aborted rebase/am Junio C Hamano
  2006-12-08 10:49                 ` [PATCH 2/5] status: show files that would have resolutions recorded by rerere Eric Wong
                                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 46+ messages in thread
From: Eric Wong @ 2006-12-08 10:49 UTC (permalink / raw)
  To: git; +Cc: Eric Wong

Data in rr-cache isn't valid after a patch application is
skipped or and aborted, so our next commit could be misrecorded
as a resolution of that skipped/failed commit, which is wrong.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-am.sh       |    4 ++++
 git-rebase.sh   |    8 ++++++++
 git-rerere.perl |   12 ++++++++++++
 3 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index afe322b..28ccae3 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -246,6 +246,10 @@ last=`cat "$dotest/last"`
 this=`cat "$dotest/next"`
 if test "$skip" = t
 then
+	if test -d "$GIT_DIR/rr-cache"
+	then
+		git-rerere clear
+	fi
 	this=`expr "$this" + 1`
 	resume=
 fi
diff --git a/git-rebase.sh b/git-rebase.sh
index 25530df..2b4f347 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -139,6 +139,10 @@ do
 	--skip)
 		if test -d "$dotest"
 		then
+			if test -d "$GIT_DIR/rr-cache"
+			then
+				git-rerere clear
+			fi
 			prev_head="`cat $dotest/prev_head`"
 			end="`cat $dotest/end`"
 			msgnum="`cat $dotest/msgnum`"
@@ -157,6 +161,10 @@ do
 		exit
 		;;
 	--abort)
+		if test -d "$GIT_DIR/rr-cache"
+		then
+			git-rerere clear
+		fi
 		if test -d "$dotest"
 		then
 			rm -r "$dotest"
diff --git a/git-rerere.perl b/git-rerere.perl
index d3664ff..dd86577 100755
--- a/git-rerere.perl
+++ b/git-rerere.perl
@@ -172,6 +172,18 @@ sub merge {
 -d "$rr_dir" || exit(0);
 
 read_rr();
+
+if (@ARGV && $ARGV[0] eq 'clear') {
+	for my $path (keys %merge_rr) {
+		my $name = $merge_rr{$path};
+		if (-d "$rr_dir/$name") {
+			rmtree(["$rr_dir/$name"]);
+		}
+	}
+	unlink $merge_rr;
+	exit 0;
+}
+
 my %conflict = map { $_ => 1 } find_conflict();
 
 # MERGE_RR records paths with conflicts immediately after merge
-- 
1.4.4.2.g860f4

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 2/5] status: show files that would have resolutions recorded by rerere
  2006-12-05  9:21               ` Eric Wong
  2006-12-08 10:49                 ` [RFC/PATCH 0/5] WIP status/rerere reporting Eric Wong
  2006-12-08 10:49                 ` [PATCH 1/5] rerere: avoid misrecording on a skipped or aborted rebase/am Eric Wong
@ 2006-12-08 10:49                 ` Eric Wong
  2006-12-08 10:49                 ` [PATCH 3/5] am and rebase resolve states get picked up by status/commit Eric Wong
                                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 46+ messages in thread
From: Eric Wong @ 2006-12-08 10:49 UTC (permalink / raw)
  To: git; +Cc: Eric Wong

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-commit.sh   |   17 ++++++++++++++++-
 git-rerere.perl |   18 ++++++++++++------
 2 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/git-commit.sh b/git-commit.sh
index 81c3a0c..9f6d1ef 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -48,7 +48,22 @@ run_status () {
 		GIT_INDEX_FILE="$NEXT_INDEX"
 		export GIT_INDEX_FILE
 	fi
-
+	if test -d "$GIT_DIR/rr-cache"
+	then
+	    rr_shown=
+	    git-rerere status | while read line; do
+		if [ -z "$rr_shown" ]; then
+		    echo '#'
+		    echo '# Resolutions to be recorded for files:'
+		    echo '#   (git-rerere will automatically record' \
+			 'conflict resolutions'
+		    echo '#    when these files are committed)'
+		    echo '#'
+		    rr_shown=1
+		fi
+		echo  "#	$line"
+	    done
+	fi
 	case "$status_only" in
 	t) color= ;;
 	*) color=--nocolor ;;
diff --git a/git-rerere.perl b/git-rerere.perl
index dd86577..b78194a 100755
--- a/git-rerere.perl
+++ b/git-rerere.perl
@@ -173,14 +173,20 @@ sub merge {
 
 read_rr();
 
-if (@ARGV && $ARGV[0] eq 'clear') {
-	for my $path (keys %merge_rr) {
-		my $name = $merge_rr{$path};
-		if (-d "$rr_dir/$name") {
-			rmtree(["$rr_dir/$name"]);
+if (my $arg = shift @ARGV) {
+	if ($arg eq 'clear') {
+		for my $path (keys %merge_rr) {
+			my $name = $merge_rr{$path};
+			if (-d "$rr_dir/$name") {
+				rmtree(["$rr_dir/$name"]);
+			}
+		}
+		unlink $merge_rr;
+	} elsif ($arg eq 'status') {
+		for my $path (keys %merge_rr) {
+			print $path, "\n";
 		}
 	}
-	unlink $merge_rr;
 	exit 0;
 }
 
-- 
1.4.4.2.g860f4

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 3/5] am and rebase resolve states get picked up by status/commit
  2006-12-05  9:21               ` Eric Wong
                                   ` (2 preceding siblings ...)
  2006-12-08 10:49                 ` [PATCH 2/5] status: show files that would have resolutions recorded by rerere Eric Wong
@ 2006-12-08 10:49                 ` Eric Wong
  2006-12-08 10:49                 ` [PATCH 4/5] am: run git rerere to record resolution on successful --resolved Eric Wong
  2006-12-08 10:49                 ` [PATCH 5/5] rerere: add the diff command Eric Wong
  5 siblings, 0 replies; 46+ messages in thread
From: Eric Wong @ 2006-12-08 10:49 UTC (permalink / raw)
  To: git; +Cc: Eric Wong

This should help warn of accidental commits in the middle of a
rebase operation.  It also saves messages in $dotest/resolvemsg
and shows it in "git status" so the user can be reminded of
how to continue the am or rebase operation.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-am.sh     |   10 ++++++----
 git-commit.sh |   28 ++++++++++++++++++++++++++++
 git-rebase.sh |   35 ++++++++++++++++++++++-------------
 3 files changed, 56 insertions(+), 17 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 28ccae3..179b967 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -16,7 +16,7 @@ stop_here () {
 
 stop_here_user_resolve () {
     if [ -n "$resolvemsg" ]; then
-	    echo "$resolvemsg"
+	    echo "$resolvemsg" | tee "$dotest/resolvemsg"
 	    stop_here $1
     fi
     cmdline=$(basename $0)
@@ -32,9 +32,11 @@ stop_here_user_resolve () {
     then
         cmdline="$cmdline -d=$dotest"
     fi
-    echo "When you have resolved this problem run \"$cmdline --resolved\"."
-    echo "If you would prefer to skip this patch, instead run \"$cmdline --skip\"."
-
+    cat > "$dotest/resolvemsg" <<EOF
+When you have resolved this problem run \"$cmdline --resolved\".
+If you would prefer to skip this patch, instead run \"$cmdline --skip\".
+EOF
+    cat "$dotest/resolvemsg"
     stop_here $1
 }
 
diff --git a/git-commit.sh b/git-commit.sh
index 9f6d1ef..4691835 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -32,6 +32,33 @@ save_index () {
 	cp -p "$THIS_INDEX" "$NEXT_INDEX"
 }
 
+check_dotest () {
+	if test -d .dotest
+	then
+		echo ''
+		if test -f .dotest/resolvemsg
+		then
+			cat .dotest/resolvemsg
+		else
+			echo 'A .dotest directory exists.'
+			echo 'Either a "git rebase" or "git am"' \
+					'operation is in progress'
+		fi
+	fi
+	if test -d "$GIT_DIR/.dotest-merge"
+	then
+		echo ''
+		if test -f "$GIT_DIR/.dotest-merge/resolvemsg"
+		then
+			cat "$GIT_DIR/.dotest-merge/resolvemsg"
+		else
+			echo "A $GIT_DIR/.dotest-merge/resolvemsg " \
+				'directory exists.'
+			echo 'A "git rebase --merge" operation is in progress'
+		fi
+	fi
+}
+
 run_status () {
 	# If TMP_INDEX is defined, that means we are doing
 	# "--only" partial commit, and that index file is used
@@ -64,6 +91,7 @@ run_status () {
 		echo  "#	$line"
 	    done
 	fi
+	check_dotest | sed -e 's/^/# /'
 	case "$status_only" in
 	t) color= ;;
 	*) color=--nocolor ;;
diff --git a/git-rebase.sh b/git-rebase.sh
index 2b4f347..53f3919 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -41,6 +41,16 @@ dotest=$GIT_DIR/.dotest-merge
 prec=4
 verbose=
 
+die_msg () {
+	> "$dotest/resolvemsg"
+	for i in "$@"
+	do
+		echo "$i" | tee -a "$dotest/resolvemsg" >&2
+	done
+	echo "$RESOLVEMSG" >> "$dotest/resolvemsg"
+	die "$RESOLVEMSG"
+}
+
 continue_merge () {
 	test -n "$prev_head" || die "prev_head must be defined"
 	test -d "$dotest" || die "$dotest directory does not exist"
@@ -48,18 +58,17 @@ continue_merge () {
 	unmerged=$(git-ls-files -u)
 	if test -n "$unmerged"
 	then
-		echo "You still have unmerged paths in your index"
-		echo "did you forget update-index?"
-		die "$RESOLVEMSG"
+		die_msg "You still have unmerged paths in your index" \
+				"did you forget update-index?"
 	fi
 
 	if test -n "`git-diff-index HEAD`"
 	then
 		if ! git-commit -C "`cat $dotest/current`"
 		then
-			echo "Commit failed, please do not call \"git commit\""
-			echo "directly, but instead do one of the following: "
-			die "$RESOLVEMSG"
+			die_msg \
+			"Commit failed, please do not call \"git commit\"" \
+			"directly, but instead do one of the following: "
 		fi
 		printf "Committed: %0${prec}d" $msgnum
 	else
@@ -73,6 +82,7 @@ continue_merge () {
 	echo "$prev_head" > "$dotest/prev_head"
 
 	# onto the next patch:
+	rm -f "$dotest/resolvemsg"
 	msgnum=$(($msgnum + 1))
 	echo "$msgnum" >"$dotest/msgnum"
 }
@@ -88,14 +98,13 @@ call_merge () {
 		;;
 	1)
 		test -d "$GIT_DIR/rr-cache" && git-rerere
-		die "$RESOLVEMSG"
+		die_msg
 		;;
 	2)
-		echo "Strategy: $rv $strategy failed, try another" 1>&2
-		die "$RESOLVEMSG"
+		die_msg "Strategy: $rv $strategy failed, try another"
 		;;
 	*)
-		die "Unknown exit code ($rv) from command:" \
+		die_msg "Unknown exit code ($rv) from command:" \
 			"git-merge-$strategy $cmt^ -- HEAD $cmt"
 		;;
 	esac
@@ -112,9 +121,8 @@ do
 	--continue)
 		diff=$(git-diff-files)
 		case "$diff" in
-		?*)	echo "You must edit all merge conflicts and then"
-			echo "mark them as resolved using git update-index"
-			exit 1
+		?*)	die_msg "You must edit all merge conflicts and then" \
+				"mark them as resolved using git update-index"
 			;;
 		esac
 		if test -d "$dotest"
@@ -143,6 +151,7 @@ do
 			then
 				git-rerere clear
 			fi
+			rm -f "$dotest/resolvemsg"
 			prev_head="`cat $dotest/prev_head`"
 			end="`cat $dotest/end`"
 			msgnum="`cat $dotest/msgnum`"
-- 
1.4.4.2.g860f4

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 4/5] am: run git rerere to record resolution on successful --resolved
  2006-12-05  9:21               ` Eric Wong
                                   ` (3 preceding siblings ...)
  2006-12-08 10:49                 ` [PATCH 3/5] am and rebase resolve states get picked up by status/commit Eric Wong
@ 2006-12-08 10:49                 ` Eric Wong
  2006-12-08 10:49                 ` [PATCH 5/5] rerere: add the diff command Eric Wong
  5 siblings, 0 replies; 46+ messages in thread
From: Eric Wong @ 2006-12-08 10:49 UTC (permalink / raw)
  To: git; +Cc: Eric Wong

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-am.sh |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 179b967..d0714c6 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -414,6 +414,10 @@ do
 			stop_here_user_resolve $this
 		fi
 		apply_status=0
+		if test -d "$GIT_DIR/rr-cache"
+		then
+			git rerere
+		fi
 		;;
 	esac
 
-- 
1.4.4.2.g860f4

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 5/5] rerere: add the diff command
  2006-12-05  9:21               ` Eric Wong
                                   ` (4 preceding siblings ...)
  2006-12-08 10:49                 ` [PATCH 4/5] am: run git rerere to record resolution on successful --resolved Eric Wong
@ 2006-12-08 10:49                 ` Eric Wong
  2006-12-08 12:07                   ` Jakub Narebski
  5 siblings, 1 reply; 46+ messages in thread
From: Eric Wong @ 2006-12-08 10:49 UTC (permalink / raw)
  To: git; +Cc: Eric Wong

Sometimes I like to see what I'm recording resolutions for and
what's changed during a resolution.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-rerere.perl |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/git-rerere.perl b/git-rerere.perl
index b78194a..7a3ae84 100755
--- a/git-rerere.perl
+++ b/git-rerere.perl
@@ -186,6 +186,13 @@ if (my $arg = shift @ARGV) {
 		for my $path (keys %merge_rr) {
 			print $path, "\n";
 		}
+	} elsif ($arg eq 'diff') {
+		for my $path (keys %merge_rr) {
+			my $name = $merge_rr{$path};
+			system(qw/diff/, @ARGV,
+				'-L', "a/$path", '-L', "b/$path",
+				"$rr_dir/$name/preimage", $path);
+		}
 	}
 	exit 0;
 }
-- 
1.4.4.2.g860f4

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* Re: [PATCH 5/5] rerere: add the diff command
  2006-12-08 10:49                 ` [PATCH 5/5] rerere: add the diff command Eric Wong
@ 2006-12-08 12:07                   ` Jakub Narebski
  0 siblings, 0 replies; 46+ messages in thread
From: Jakub Narebski @ 2006-12-08 12:07 UTC (permalink / raw)
  To: git

Eric Wong wrote:

> Sometimes I like to see what I'm recording resolutions for and
> what's changed during a resolution.
> 
> Signed-off-by: Eric Wong <normalperson@yhbt.net>
> ---
>  git-rerere.perl |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)

Documentation, please?
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 1/5] rerere: avoid misrecording on a skipped or aborted rebase/am
  2006-12-08 10:49                 ` [PATCH 1/5] rerere: avoid misrecording on a skipped or aborted rebase/am Eric Wong
@ 2006-12-08 19:33                   ` Junio C Hamano
  2006-12-08 20:04                     ` [PATCH 6/5] git-rerere: document the 'clear' and 'diff' commands Eric Wong
  2006-12-08 20:43                   ` [PATCH 1/5] rerere: avoid misrecording on a skipped or aborted rebase/am Junio C Hamano
  1 sibling, 1 reply; 46+ messages in thread
From: Junio C Hamano @ 2006-12-08 19:33 UTC (permalink / raw)
  To: Eric Wong; +Cc: git

This is good, but Documentation/git-rerere.txt should talk about
this new option you added.


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH 6/5] git-rerere: document the 'clear' and 'diff' commands
  2006-12-08 19:33                   ` Junio C Hamano
@ 2006-12-08 20:04                     ` Eric Wong
  0 siblings, 0 replies; 46+ messages in thread
From: Eric Wong @ 2006-12-08 20:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jakub Narebski

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 Documentation/git-rerere.txt |   22 ++++++++++++++++++++--
 1 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-rerere.txt b/Documentation/git-rerere.txt
index 8b6b651..de65cce 100644
--- a/Documentation/git-rerere.txt
+++ b/Documentation/git-rerere.txt
@@ -7,8 +7,7 @@ git-rerere - Reuse recorded resolve
 
 SYNOPSIS
 --------
-'git-rerere'
-
+'git-rerere' [clear|diff]
 
 DESCRIPTION
 -----------
@@ -167,6 +166,25 @@ would conflict the same way the test merge you resolved earlier.
 `git-rerere` is run by `git rebase` to help you resolve this
 conflict.
 
+COMMANDS
+--------
+
+Normally, git-rerere is run without arguments or user-intervention.
+However, it has several commands that allow it to interact with
+its working state.
+
+'clear'::
+
+This resets the metadata used by rerere if a merge resolution is to be
+is aborted.  Calling gitlink:git-am[1] --skip or gitlink:git-rebase[1]
+[--skip|--abort] will automatcally invoke this command.
+
+'diff'::
+
+This displays diffs for the current state of the resolution.  It is
+useful for tracking what has changed while the user is resolving
+conflicts.  Additional arguments are passed directly to the system
+diff(1) command installed in PATH.
 
 Author
 ------
-- 
1.4.4.2.g860f4

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* Re: [PATCH 1/5] rerere: avoid misrecording on a skipped or aborted rebase/am
  2006-12-08 10:49                 ` [PATCH 1/5] rerere: avoid misrecording on a skipped or aborted rebase/am Eric Wong
  2006-12-08 19:33                   ` Junio C Hamano
@ 2006-12-08 20:43                   ` Junio C Hamano
  2006-12-08 21:28                     ` Eric Wong
  1 sibling, 1 reply; 46+ messages in thread
From: Junio C Hamano @ 2006-12-08 20:43 UTC (permalink / raw)
  To: Eric Wong; +Cc: git

Eric Wong <normalperson@yhbt.net> writes:

> +if (@ARGV && $ARGV[0] eq 'clear') {
> +	for my $path (keys %merge_rr) {
> +		my $name = $merge_rr{$path};
> +		if (-d "$rr_dir/$name") {
> +			rmtree(["$rr_dir/$name"]);
> +		}
> +	}
> +	unlink $merge_rr;
> +	exit 0;
> +}

Come to think of it, I am not sure about this one.  Don't you
need to make sure that there is no existing resolution before
removing it?  In other words, shouldn't the removal be like this?

	if (-d "$rr_dir/$name" && ! -f "$rr_dir/$name/postimage") {
		rmtree(["$rr_dir/$name"]);
	}


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 1/5] rerere: avoid misrecording on a skipped or aborted rebase/am
  2006-12-08 20:43                   ` [PATCH 1/5] rerere: avoid misrecording on a skipped or aborted rebase/am Junio C Hamano
@ 2006-12-08 21:28                     ` Eric Wong
  2006-12-08 21:29                       ` [PATCH] rerere: add clear, diff, and status commands Eric Wong
  0 siblings, 1 reply; 46+ messages in thread
From: Eric Wong @ 2006-12-08 21:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Junio C Hamano <junkio@cox.net> wrote:
> Eric Wong <normalperson@yhbt.net> writes:
> 
> > +if (@ARGV && $ARGV[0] eq 'clear') {
> > +	for my $path (keys %merge_rr) {
> > +		my $name = $merge_rr{$path};
> > +		if (-d "$rr_dir/$name") {
> > +			rmtree(["$rr_dir/$name"]);
> > +		}
> > +	}
> > +	unlink $merge_rr;
> > +	exit 0;
> > +}
> 
> Come to think of it, I am not sure about this one.  Don't you
> need to make sure that there is no existing resolution before
> removing it?  In other words, shouldn't the removal be like this?
> 
> 	if (-d "$rr_dir/$name" && ! -f "$rr_dir/$name/postimage") {
> 		rmtree(["$rr_dir/$name"]);
> 	}

Yes.  However, it seems unlikely, that MERGE_RR would point to something
that has a postimage.  I suppose that a machine could crash or a user
could've hit ^C at the correct millisecond that would cause rerere to
not update MERGE_RR correctly.

Updated patches on the way (leaving out the status updates).

-- 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH] rerere: add clear, diff, and status commands
  2006-12-08 21:28                     ` Eric Wong
@ 2006-12-08 21:29                       ` Eric Wong
  2006-12-08 21:29                         ` [PATCH] rerere: record (or avoid misrecording) resolved, skipped or aborted rebase/am Eric Wong
  0 siblings, 1 reply; 46+ messages in thread
From: Eric Wong @ 2006-12-08 21:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Eric Wong

git-am and git-rebase will be updated to use 'clear', and
diff/status can be used to aid the user in tracking progress in
the resolution process.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 Documentation/git-rerere.txt |   27 +++++++++++++++++++++++++--
 git-rerere.perl              |   25 +++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-rerere.txt b/Documentation/git-rerere.txt
index 8b6b651..22494b2 100644
--- a/Documentation/git-rerere.txt
+++ b/Documentation/git-rerere.txt
@@ -7,8 +7,7 @@ git-rerere - Reuse recorded resolve
 
 SYNOPSIS
 --------
-'git-rerere'
-
+'git-rerere' [clear|diff|status]
 
 DESCRIPTION
 -----------
@@ -167,6 +166,30 @@ would conflict the same way the test merge you resolved earlier.
 `git-rerere` is run by `git rebase` to help you resolve this
 conflict.
 
+COMMANDS
+--------
+
+Normally, git-rerere is run without arguments or user-intervention.
+However, it has several commands that allow it to interact with
+its working state.
+
+'clear'::
+
+This resets the metadata used by rerere if a merge resolution is to be
+is aborted.  Calling gitlink:git-am[1] --skip or gitlink:git-rebase[1]
+[--skip|--abort] will automatcally invoke this command.
+
+'diff'::
+
+This displays diffs for the current state of the resolution.  It is
+useful for tracking what has changed while the user is resolving
+conflicts.  Additional arguments are passed directly to the system
+diff(1) command installed in PATH.
+
+'status'::
+
+Like diff, but this only prints the filenames that will be tracked
+for resolutions.
 
 Author
 ------
diff --git a/git-rerere.perl b/git-rerere.perl
index d3664ff..2703d01 100755
--- a/git-rerere.perl
+++ b/git-rerere.perl
@@ -172,6 +172,31 @@ sub merge {
 -d "$rr_dir" || exit(0);
 
 read_rr();
+
+if (my $arg = shift @ARGV) {
+	if ($arg eq 'clear') {
+		for my $path (keys %merge_rr) {
+			my $name = $merge_rr{$path};
+			if (-d "$rr_dir/$name" && ! -f "$rr_dir/$name/postimage") {
+				rmtree(["$rr_dir/$name"]);
+			}
+		}
+		unlink $merge_rr;
+	} elsif ($arg eq 'status') {
+		for my $path (keys %merge_rr) {
+			print $path, "\n";
+		}
+	} elsif ($arg eq 'diff') {
+		for my $path (keys %merge_rr) {
+			my $name = $merge_rr{$path};
+			system(qw/diff/, @ARGV,
+				'-L', "a/$path", '-L', "b/$path",
+				"$rr_dir/$name/preimage", $path);
+		}
+	}
+	exit 0;
+}
+
 my %conflict = map { $_ => 1 } find_conflict();
 
 # MERGE_RR records paths with conflicts immediately after merge
-- 
1.4.4.2.g860f4

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH] rerere: record (or avoid misrecording) resolved, skipped or aborted rebase/am
  2006-12-08 21:29                       ` [PATCH] rerere: add clear, diff, and status commands Eric Wong
@ 2006-12-08 21:29                         ` Eric Wong
  2006-12-08 21:44                           ` Jakub Narebski
  2006-12-09 20:08                           ` Junio C Hamano
  0 siblings, 2 replies; 46+ messages in thread
From: Eric Wong @ 2006-12-08 21:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Eric Wong

Data in rr-cache isn't valid after a patch application is
skipped or and aborted, so our next commit could be misrecorded
as a resolution of that skipped/failed commit, which is wrong.

git-am --skip, git-rebase --skip/--abort will automatically
invoke git-rerere clear to avoid this.

Also, since git-am --resolved indicates a resolution was
succesful, remember to run git-rerere to record the resolution
(and not surprise the user when the next commit is made).

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-am.sh     |    8 ++++++++
 git-rebase.sh |    8 ++++++++
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index afe322b..5df6787 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -246,6 +246,10 @@ last=`cat "$dotest/last"`
 this=`cat "$dotest/next"`
 if test "$skip" = t
 then
+	if test -d "$GIT_DIR/rr-cache"
+	then
+		git-rerere clear
+	fi
 	this=`expr "$this" + 1`
 	resume=
 fi
@@ -408,6 +412,10 @@ do
 			stop_here_user_resolve $this
 		fi
 		apply_status=0
+		if test -d "$GIT_DIR/rr-cache"
+		then
+			git rerere
+		fi
 		;;
 	esac
 
diff --git a/git-rebase.sh b/git-rebase.sh
index 25530df..2b4f347 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -139,6 +139,10 @@ do
 	--skip)
 		if test -d "$dotest"
 		then
+			if test -d "$GIT_DIR/rr-cache"
+			then
+				git-rerere clear
+			fi
 			prev_head="`cat $dotest/prev_head`"
 			end="`cat $dotest/end`"
 			msgnum="`cat $dotest/msgnum`"
@@ -157,6 +161,10 @@ do
 		exit
 		;;
 	--abort)
+		if test -d "$GIT_DIR/rr-cache"
+		then
+			git-rerere clear
+		fi
 		if test -d "$dotest"
 		then
 			rm -r "$dotest"
-- 
1.4.4.2.g860f4

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* Re: [PATCH] rerere: record (or avoid misrecording) resolved, skipped or aborted rebase/am
  2006-12-08 21:29                         ` [PATCH] rerere: record (or avoid misrecording) resolved, skipped or aborted rebase/am Eric Wong
@ 2006-12-08 21:44                           ` Jakub Narebski
  2006-12-08 21:50                             ` Eric Wong
  2006-12-09 20:08                           ` Junio C Hamano
  1 sibling, 1 reply; 46+ messages in thread
From: Jakub Narebski @ 2006-12-08 21:44 UTC (permalink / raw)
  To: git

Eric Wong wrote:

Just one comment:

> +       if test -d "$GIT_DIR/rr-cache"
> +       then
> +               git-rerere clear
> +       fi
>         this=`expr "$this" + 1`
>         resume=
>  fi
> @@ -408,6 +412,10 @@ do
>                         stop_here_user_resolve $this
>                 fi
>                 apply_status=0
> +               if test -d "$GIT_DIR/rr-cache"
> +               then
> +                       git rerere
> +               fi

Why do you use once "git-rerere", and another time "git rerere"?
Shouldn't scripts always use git-command form?
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] rerere: record (or avoid misrecording) resolved, skipped or aborted rebase/am
  2006-12-08 21:44                           ` Jakub Narebski
@ 2006-12-08 21:50                             ` Eric Wong
  0 siblings, 0 replies; 46+ messages in thread
From: Eric Wong @ 2006-12-08 21:50 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, Junio C Hamano

Jakub Narebski <jnareb@gmail.com> wrote:
> Eric Wong wrote:
> 
> Just one comment:
> 
> > +       if test -d "$GIT_DIR/rr-cache"
> > +       then
> > +               git-rerere clear
> > +       fi
> >         this=`expr "$this" + 1`
> >         resume=
> >  fi
> > @@ -408,6 +412,10 @@ do
> >                         stop_here_user_resolve $this
> >                 fi
> >                 apply_status=0
> > +               if test -d "$GIT_DIR/rr-cache"
> > +               then
> > +                       git rerere
> > +               fi
> 
> Why do you use once "git-rerere", and another time "git rerere"?
> Shouldn't scripts always use git-command form?

Yes.  Or at least I remember git-command form being preferred.

I don't think I've been entirely consistent in the things I've
written, and my shell history is just as inconsistent :x

-- 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH] rerere: record (or avoid misrecording) resolved, skipped or aborted rebase/am
  2006-12-08 21:29                         ` [PATCH] rerere: record (or avoid misrecording) resolved, skipped or aborted rebase/am Eric Wong
  2006-12-08 21:44                           ` Jakub Narebski
@ 2006-12-09 20:08                           ` Junio C Hamano
  1 sibling, 0 replies; 46+ messages in thread
From: Junio C Hamano @ 2006-12-09 20:08 UTC (permalink / raw)
  To: Eric Wong; +Cc: git

I looked at your previous 5-patch series but will replace with
this updated 2-patch series.

The rerere command started its life as a mere helper for
Porcelains, but with 'status' and 'diff', you are making it
somewhat Porcelain-ish.  I still have not decided myself, but I
think we probably would want to treat this as a Porcelain and
give a bit of usability to it, so what I've committed has two
changes from your version:

 (1) 'diff' without @ARGV defaults to '-u';
 (2) running rerere with unknown sub command die()s instead of
     silently exiting with 0.

The difference between your previous 5-patch series and this
round raises an interesting issue.

You used to have a safety valve in git-commit to prevent users
from running it while rebase/am is in progress, which I imagine
would be a common mistake.

There are three commands in git suite that a user would use to
eventually make new commits, out of something other than what
was edited and prepared in the working tree, that can stop in
the middle.

 (1) git-am is used to make new commits from e-mailed patches,
     and stops in the middle when a patch does not apply.
     "git-am --resolved/--skip" are the ways to continue.

 (2) git-rebase is used to rewrite the history of a branch, and
     stops in the middle when a patch does not apply or merge
     conflicts.  "git-rebase --continue/--skip/--abort" are
     possible exits [*1*].

 (3) git-pull/git-merge are used to make merge commits with
     another branch, and stops in the middle when it conflicts.
     Possible exits are "git commit" or "git reset".

Now, I am not going to propose to fix this inconsistency and
forbid use of "git commit" to resolve "git-merge" (instead, the
user would say "git-merge --resolved/--abort").  As the workflow
goes, these are very different operations and the user is often
well aware which workflow is in progress; having different
commands for different exit route is fine.

But in order to implement the safety valve like you did cleanly,
we should first have an enumeration of states (the above is a
subset --- there is the "base state" in which you are working
towards a commit based on HEAD, which is obviously concluded
with a "git commit", and "bisect in progress") and make it easy
for commands to communicate what state the working tree is in.
That would also help 'git-explain'.

[Footnote]

*1* Resuming rebase was much worse before Sean updated it with
commit 031321c6 to hide that it uses git-am as its backend,
which is purely its implementation detail.

^ permalink raw reply	[flat|nested] 46+ messages in thread

end of thread, other threads:[~2006-12-09 20:08 UTC | newest]

Thread overview: 46+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-12-03 17:01 Some advanced index playing Alan Chandler
2006-12-03 18:24 ` Linus Torvalds
2006-12-03 19:36   ` Junio C Hamano
2006-12-03 20:11   ` Alan Chandler
2006-12-03 20:19     ` Jakub Narebski
2006-12-03 20:29       ` Alan Chandler
2006-12-03 20:40     ` Linus Torvalds
2006-12-04 10:41   ` Junio C Hamano
2006-12-03 18:31 ` Jakub Narebski
2006-12-03 18:34 ` Linus Torvalds
2006-12-03 20:26   ` Junio C Hamano
2006-12-05  3:48     ` [PATCH] git-explain Junio C Hamano
2006-12-05  3:55       ` Nicolas Pitre
2006-12-05  3:57         ` J. Bruce Fields
2006-12-05  6:09           ` Junio C Hamano
2006-12-05  7:26             ` Jeff King
2006-12-05  9:21               ` Eric Wong
2006-12-08 10:49                 ` [RFC/PATCH 0/5] WIP status/rerere reporting Eric Wong
2006-12-08 10:49                 ` [PATCH 1/5] rerere: avoid misrecording on a skipped or aborted rebase/am Eric Wong
2006-12-08 19:33                   ` Junio C Hamano
2006-12-08 20:04                     ` [PATCH 6/5] git-rerere: document the 'clear' and 'diff' commands Eric Wong
2006-12-08 20:43                   ` [PATCH 1/5] rerere: avoid misrecording on a skipped or aborted rebase/am Junio C Hamano
2006-12-08 21:28                     ` Eric Wong
2006-12-08 21:29                       ` [PATCH] rerere: add clear, diff, and status commands Eric Wong
2006-12-08 21:29                         ` [PATCH] rerere: record (or avoid misrecording) resolved, skipped or aborted rebase/am Eric Wong
2006-12-08 21:44                           ` Jakub Narebski
2006-12-08 21:50                             ` Eric Wong
2006-12-09 20:08                           ` Junio C Hamano
2006-12-08 10:49                 ` [PATCH 2/5] status: show files that would have resolutions recorded by rerere Eric Wong
2006-12-08 10:49                 ` [PATCH 3/5] am and rebase resolve states get picked up by status/commit Eric Wong
2006-12-08 10:49                 ` [PATCH 4/5] am: run git rerere to record resolution on successful --resolved Eric Wong
2006-12-08 10:49                 ` [PATCH 5/5] rerere: add the diff command Eric Wong
2006-12-08 12:07                   ` Jakub Narebski
2006-12-05 17:34               ` [PATCH] git-explain Horst H. von Brand
2006-12-05  8:58             ` Johannes Schindelin
2006-12-05 21:00               ` J. Bruce Fields
2006-12-05  9:11             ` Raimund Bauer
2006-12-05 10:43       ` Jakub Narebski
2006-12-05 23:00         ` Martin Langhoff
2006-12-05 23:07           ` Junio C Hamano
2006-12-05 23:37             ` Johannes Schindelin
2006-12-05 23:57               ` Junio C Hamano
2006-12-06  0:07                 ` Carl Worth
2006-12-06  0:27                 ` Johannes Schindelin
2006-12-06  1:50                   ` Nicolas Pitre
2006-12-03 20:40   ` Some advanced index playing Alan Chandler

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).