"git revert" (Re: pci_update_resource() getting called on sparc64)

"git revert" (Re: pci_update_resource() getting called on sparc64)

[Linus Torvalds]

Comments?

		Linus

On Mon, 8 Aug 2005, Linus Torvalds wrote:
> 
> 
> On Mon, 8 Aug 2005, Greg KH wrote:
> > 
> > Hm, how do you revert a git patch?
> 
> Something like this?
> 
> 	#!/bin/sh
> 	. git-sh-setup-script || die "Not a git archive"
> 	rev=$(git-rev-parse --verify --revs-only "$@") || exit
> 	git-diff-tree -R -p $rev | git-apply --index &&
> 		echo "Revert $rev" | git commit
> 
> Just name it "git-revert-script" and it might do what you want to do.
> 
> It may not have the nicest error messages: if you try to revert a merge
> (which won't have a diff), git-apply will say something like
> 
> 	fatal: No changes
> 
> which isn't exactly being helpful. And the revert message could be made 
> more interesting (like putting the first line of the description of what 
> we reverted into the message instead of just the revision number).
> 
> 		Linus
> 

Re: "git revert" (Re: pci_update_resource() getting called on sparc64)

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

>> It may not have the nicest error messages: if you try to revert a merge
>> (which won't have a diff), git-apply will say something like
>> 
>> 	fatal: No changes
>> 
>> which isn't exactly being helpful. And the revert message could be made 
>> more interesting (like putting the first line of the description of what 
>> we reverted into the message instead of just the revision number).

> Comments?
>
> 		Linus

I like the general idea, and if we had a commit pretty format
"oneline", then something like this would make it look nicer.

Totally untested.  I acquired your habit of coding in my e-mail
client ;-).

        #!/bin/sh
        . git-sh-setup-script || die "Not a git archive"

        rev=$(git-rev-parse --verify --revs-only "$@") &&
        commit=$(git-rev-parse --verify --revs-only "$commit^0") || exit
        if git-diff-tree -R -p $commit | git-apply --index &&
           msg=$(git-rev-list --pretty=oneline --max-count=1 $commit)
        then
                {
                        echo "Revert $msg"
                        echo
                        echo "This reverts $commit commit."
                        test "$rev" = "$commit" ||
                        echo "(original 'git revert' arguments: $@)"
                } | git commit
        else
                # Now why did it fail?
                parents=`git-cat-file commit "$commit" 2>/dev/null |
                        sed -ne '/^$/q;/^parent /p' |
                        wc -l`
                case $parents in
                0) die "Cannot revert the root commit nor non commit-ish" ;;
                1) die "The patch does not apply" ;;
                *) die "Cannot revert a merge commit" ;;
                esac
        fi

Re: "git revert" (Re: pci_update_resource() getting called on sparc64)

[Linus Torvalds]

On Mon, 8 Aug 2005, Junio C Hamano wrote:
> 
> Totally untested.  I acquired your habit of coding in my e-mail
> client ;-).

Looks good. Although I also have this advanced testing habit of just 
reading the email and if it looks sane it tested out ok ;)

But it strikes me that we could use the "-M" flag to git-diff-tree, which
makes it a lot more likely that we can revert renames, even if the file
has been slightly changed afterwards.

			Linus

Re: "git revert" (Re: pci_update_resource() getting called on sparc64)

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> On Mon, 8 Aug 2005, Junio C Hamano wrote:
>> 
>> Totally untested.  I acquired your habit of coding in my e-mail
>> client ;-).
>
> Looks good. Although I also have this advanced testing habit of just 
> reading the email and if it looks sane it tested out ok ;)

Heh, I immediately spotted two bugs after I sent it out, one
mine, another yours.

 * dereferencing potential tag to get to the underlying commit
   was a good idea of mine, poorly executed.  It should
   rev-parse $rev^0, not $commit^0.

 * git commit does not take commit message from stdin.  I think
   we should do something like this:

diff --git a/git-commit-script b/git-commit-script
--- a/git-commit-script
+++ b/git-commit-script
@@ -92,11 +92,13 @@ then
 	rm .editmsg
 	exit 1
 fi
-case "$use_commit" in
-'')
+if test -t 0 && "$use_commit" = ""
+then
 	${VISUAL:-${EDITOR:-vi}} .editmsg
-	;;
-esac
+else
+	echo >&2 "(reading commit message from standard input)"
+	cat >.editmsg
+fi
 grep -v '^#' < .editmsg | git-stripspace > .cmitmsg
 [ -s .cmitmsg ] && 
 	tree=$(git-write-tree) &&

git-commit-script, was Re: "git revert" (Re: pci_update_resource() getting called on sparc64)

[Johannes Schindelin]

Hi,

On Mon, 8 Aug 2005, Junio C Hamano wrote:

>  * git commit does not take commit message from stdin.  I think
>    we should do something like this:
>
> [...]

And could we rename the *-m* flag at the same time? Because I often catch 
myself typing

	git commit -m "Some_commit_message"

Ciao,
Dscho

Re: git-commit-script, was Re: "git revert" (Re: pci_update_resource() getting called on sparc64)

[Junio C Hamano]

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

> And could we rename the *-m* flag at the same time? Because I often catch 
> myself typing
>
> 	git commit -m "Some_commit_message"

Sure, what's the alternative spelling for the current -m,
though?  Would -c be OK for commit?

Re: "git revert" (Re: pci_update_resource() getting called on sparc64)

[Linus Torvalds]

On Mon, 8 Aug 2005, Junio C Hamano wrote:
> 
>  * git commit does not take commit message from stdin.  I think
>    we should do something like this:

Yes. Alteratively (or additionally), we should add a command line option 
to set the commit message.

CVS uses "-m", but you already took that. I forget what BK used, but I 
think it was "-Y" (with lowercase "-y" being "take message from file". Or 
maybe the other way around).

Or if you want to be more CVS-like, use "-F", and accept "-" for stdin?

But I like "test -t 0" regardless. Very few editors are ok with non-tty
stdin, although I could imagine some disturbed individual having "ed" as
their editor and expecting to be able to pipe commands into it or
something.

		Linus

Re: git-commit-script, was Re: "git revert" (Re: pci_update_resource() getting called on sparc64)

[Linus Torvalds]

On Mon, 8 Aug 2005, Junio C Hamano wrote:
> 
> Sure, what's the alternative spelling for the current -m,
> though?  Would -c be OK for commit?

How about using long names?

	git commit --reuse-message <sha1>

looks a lot more intuitively understandable to me than something like "-c"

Most of us don't use 300bps terminals any more, so typing a few extra 
characters is probably ok.

Now, to make things be backwards-compatible, it should be easy enough to 
notice

   "Oh, the argument to '-m' is just a single SHA1, let's see if it's a
    commit, and turn '-m' into '--reuse-message' automatically"

If anybody cares. Does cogito already use "-m"?

		Linus

Re: "git revert" (Re: pci_update_resource() getting called on sparc64)

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> Or if you want to be more CVS-like, use "-F", and accept "-" for stdin?

Yes my vote goes:

	-m "message"
        -c "from this commit, literally."
        -C "from this commit, but let me edit the log further."
        -F "from this file."
        -F - <stdin

> But I like "test -t 0" regardless. Very few editors are ok with non-tty
> stdin, although I could imagine some disturbed individual having "ed" as
> their editor and expecting to be able to pipe commands into it or
> something.

I am a disturbed individual who always works in Emacs and run
"git commit" in the compilation buffer, setting EDITOR to
emacsclient.  Admittably the compilation buffer uses pty, so
"test -t 0" would succeed, though.

Re: git-commit-script, was Re: "git revert" (Re: pci_update_resource() getting called on sparc64)

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> Most of us don't use 300bps terminals any more, so typing a few extra 
> characters is probably ok.

Again you are right.

Re: "git revert" (Re: pci_update_resource() getting called on sparc64)

[Johannes Schindelin]

Hi,

On Mon, 8 Aug 2005, Junio C Hamano wrote:

> Linus Torvalds <torvalds@osdl.org> writes:
> 
> > Or if you want to be more CVS-like, use "-F", and accept "-" for stdin?
> 
> Yes my vote goes:
> 
> 	-m "message"
>         -c "from this commit, literally."
>         -C "from this commit, but let me edit the log further."
>         -F "from this file."
>         -F - <stdin

The royal penguin speaketh, and the little blue penguin gets a fish:

[PATCH] git commit usage is more like cvs's

Teach git-commit-script about different ways to provide a commit message,
including the famous -m flag from CVS.

---

 git-commit-script |   48 ++++++++++++++++++++++++++++++++++--------------
 1 files changed, 34 insertions(+), 14 deletions(-)

457d879496cb5f742ed6bccc9a435f414b0fd2a7
diff --git a/git-commit-script b/git-commit-script
--- a/git-commit-script
+++ b/git-commit-script
@@ -6,20 +6,41 @@
 . git-sh-setup-script || die "Not a git archive"
 
 usage () {
-	die 'git commit [--all] [-m existing-commit] [<path>...]'
+	die 'git commit [options] [<path>...]
+Options:
+	--all		automatically do a git-update-cache on all modified
+			files
+	-m <message>	use this message in the commit
+	-c <commit>	use the same message as in the given commit
+	-C <commit>	as -c, but let me edit it
+	-F <file>	take the message from file
+	-F -		take the message from stdin'
 }
 
 files=()
+call_editor=true
+rm -f .editmsg || die "Could not write edit message"
 while case "$#" in 0) break ;; esac
 do
     case "$1" in
+    -m|-c|-C|-F)
+	[ "$#" -ne 1 ] || usage;;
+    esac
+
+    case "$1" in
     -m) shift
-        case "$#" in
-	0) usage ;;
-	*) use_commit=`git-rev-parse --verify "$1"` ||
-	   exit ;;
-	esac
-	;;
+	call_editor=""
+	echo "$1" >> .editmsg;;
+    -c) shift
+	call_editor=""
+	use_commit=`git-rev-parse --verify "$1"` ||
+	exit ;;
+    -C) shift
+	use_commit=`git-rev-parse --verify "$1"` ||
+	exit ;;
+    -F) shift
+	call_editor=""
+	cat "$1" >> .editmsg;;
     --all)
 	files=($(git-diff-files --name-only))\
 	;;
@@ -42,7 +63,7 @@ if [ ! -r "$GIT_DIR/HEAD" ]; then
 		echo "#"
 		git-ls-files | sed 's/^/# New file: /'
 		echo "#"
-	) > .editmsg
+	) >> .editmsg
 	PARENTS=""
 else
 	if [ -f "$GIT_DIR/MERGE_HEAD" ]; then
@@ -83,7 +104,7 @@ else
 		export GIT_AUTHOR_DATE
 		git-cat-file commit "$use_commit" |
 		sed -e '1,/^$/d'
-	fi >.editmsg
+	fi >> .editmsg
 	git-status-script >>.editmsg
 fi
 if [ "$?" != "0" -a ! -f $GIT_DIR/MERGE_HEAD ]
@@ -92,11 +113,10 @@ then
 	rm .editmsg
 	exit 1
 fi
-case "$use_commit" in
-'')
-	${VISUAL:-${EDITOR:-vi}} .editmsg
-	;;
-esac
+
+[ -z "$use_commit" -o "$call_editor" = true ] &&
+${VISUAL:-${EDITOR:-vi}} .editmsg
+
 grep -v '^#' < .editmsg | git-stripspace > .cmitmsg
 [ -s .cmitmsg ] && 
 	tree=$(git-write-tree) &&

Re: "git revert" (Re: pci_update_resource() getting called on sparc64)

[Johannes Schindelin]

[PATCH] Adapt git-cherry and git-rebase-script to latest changes of git commit

Teach git-cherry and git-rebase-script to use the "-c" option, since
"-m" means something different to git-commit-script now.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

 git-cherry        |    2 +-
 git-rebase-script |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

34d6840882e65109a17fa02bf2cefec269fd7637
diff --git a/git-cherry b/git-cherry
--- a/git-cherry
+++ b/git-cherry
@@ -23,7 +23,7 @@ The output is intended to be used as:
     while read commit
     do
         GIT_EXTERNAL_DIFF=git-apply-patch-script git-diff-tree -p "$commit" &&
-	git-commit-script -m "$commit"
+	git-commit-script -c "$commit"
     done
 '
 
diff --git a/git-rebase-script b/git-rebase-script
--- a/git-rebase-script
+++ b/git-rebase-script
@@ -37,7 +37,7 @@ do
 	esac
 	S=`cat "$GIT_DIR/HEAD"` &&
         GIT_EXTERNAL_DIFF=git-apply-patch-script git-diff-tree -p $commit &&
-	git-commit-script -m "$commit" || {
+	git-commit-script -c "$commit" || {
 		echo $commit >>$fail
 		git-read-tree --reset -u $S
 	}