threads / patch / 7973

patchgit-commit -f <file>: take message from file, and remove file upon completion.

Subject: [PATCH] git-commit -f <file>: take message from file, and remove file upon completion.

## tl;dr

10 messages between May 4, 2007 and May 8, 2007. Diffs are folded; open one to read it.

replies: 9people: 4as markdown or json

Gerrit Pape· May 4, 2007, 14:30 UTC · lore

Change (undocumented) git commit -f option from a synonym for git-commit -F to "Take the commit message from the given file, and remove the file upon successful commit".

git-revert and git-cherry-pick should make use of this to properly cleanup the automatically created ./.msg file.

Signed-off-by: Gerrit Pape <pape@smarden.org>
---
 Documentation/git-commit.txt |    7 ++++---
 git-commit.sh                |   25 +++++++++++++++++++++++--
 2 files changed, 27 insertions(+), 5 deletions(-)
Show changes to 2 files +27 −5

Documentation/git-commit.txt, git-commit.sh

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 53a7bb0..da68c81 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git-commit' [-a | --interactive] [-s] [-v]
-	   [(-c | -C) <commit> | -F <file> | -m <msg> | --amend]
+	   [(-c | -C) <commit> | (-F | -f) <file> | -m <msg> | --amend]
 	   [--no-verify] [-e] [--author <author>]
 	   [--] [[-i | -o ]<file>...]
 
@@ -63,9 +63,10 @@ OPTIONS
 	invoked; with '-c' the user can further edit the commit
 	message.
 
--F <file>::
+-F or -f <file>::
 	Take the commit message from the given file.  Use '-' to
-	read the message from the standard input.
+	read the message from the standard input.  With '-f', the
+	file is removed upon successful commit; with '-F' not.
 
 --author <author>::
 	Override the author name used in the commit.  Use
diff --git a/git-commit.sh b/git-commit.sh
index f28fc24..e4518c4 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -74,6 +74,7 @@ also=
 interactive=
 only=
 logfile=
+logfile_rm=
 use_commit=
 amend=
 edit_flag=
@@ -90,15 +91,31 @@ untracked_files=
 while case "$#" in 0) break;; esac
 do
 	case "$1" in
-	-F|--F|-f|--f|--fi|--fil|--file)
+	-f)
 		case "$#" in 1) usage ;; esac
 		shift
 		no_edit=t
 		log_given=t$log_given
 		logfile="$1"
+		test "$logfile" = '-' || logfile_rm=1
 		shift
 		;;
-	-F*|-f*)
+	-F|--F|--f|--fi|--fil|--file)
+		case "$#" in 1) usage ;; esac
+		shift
+		no_edit=t
+		log_given=t$log_given
+		logfile="$1"
+		shift
+		;;
+	-f*)
+		no_edit=t
+		log_given=t$log_given
+		logfile=`expr "z$1" : 'z-[Ff]\(.*\)'`
+		test "$logfile" = '-' || logfile_rm=1
+		shift
+		;;
+	-F*)
 		no_edit=t
 		log_given=t$log_given
 		logfile=`expr "z$1" : 'z-[Ff]\(.*\)'`
@@ -653,6 +670,10 @@ then
 		       --summary --root HEAD --`
 		echo "Created${initial_commit:+ initial} commit $commit"
 	fi
+	if test -n "$logfile_rm"
+	then
+		rm -f "$logfile"
+	fi
 fi
 
 exit "$ret"

-- 
1.5.1.2
Gerrit Pape· May 4, 2007, 14:32 UTC · re: Gerrit Pape · lore

[PATCH] Have git-revert, git-cherry-pick cleanup ./.msg upon successful completion.

git-revert and git-cherry-pick left behind the commit message file ./.msg, have them use the -f option to git-commit to properly cleanup the automatically created file.

Signed-off-by: Gerrit Pape <pape@smarden.org>
---
 builtin-revert.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Show changes to builtin-revert.c +2 −2
diff --git a/builtin-revert.c b/builtin-revert.c
index 4ba0ee6..9acdf47 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -376,10 +376,10 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 
 	if (!no_commit) {
 		if (edit)
-			return execl_git_cmd("commit", "-n", "-F", ".msg",
+			return execl_git_cmd("commit", "-n", "-f", ".msg",
 				"-e", NULL);
 		else
-			return execl_git_cmd("commit", "-n", "-F", ".msg",
+			return execl_git_cmd("commit", "-n", "-f", ".msg",
 				NULL);
 	}
 	if (reencoded_message)

-- 
1.5.1.2
Alex Riesen· May 4, 2007, 15:28 UTC · re: Gerrit Pape · lore

Re: [PATCH] Have git-revert, git-cherry-pick cleanup ./.msg upon successful completion.

On 5/4/07, Gerrit Pape <pape@smarden.org> wrote:
> git-revert and git-cherry-pick left behind the commit message file ./.msg,
> have them use the -f option to git-commit to properly cleanup the
> automatically created file.
Could we also have the files in $GIT_DIR instead of in working directory?
Shawn O. Pearce· May 5, 2007, 03:46 UTC · re: Gerrit Pape · lore

Re: [PATCH] Have git-revert, git-cherry-pick cleanup ./.msg upon successful completion.

Gerrit Pape <pape@smarden.org> wrote:
> git-revert and git-cherry-pick left behind the commit message file ./.msg,
> have them use the -f option to git-commit to properly cleanup the
> automatically created file.

I'm actually sort of against changing the behavior of git-commit -f to mean "delete the file". We never did that before. Users might get surprised when their file goes away!

What about this change instead? We make cherry-pick/revert use the same temporary file as merge, which is under .git/ (something Alex mentioned he wanted). I think the use of ".msg" in cherry-pick/revert has always just been a bug, and not a feature, so I'm really not against changing things around like this.

Show changes to builtin-revert.c +6 −11
diff --git a/builtin-revert.c b/builtin-revert.c
index 4ba0ee6..67c13a3 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -237,6 +237,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	int i;
 	char *oneline, *reencoded_message = NULL;
 	const char *message, *encoding;
+	const char *defmsg = git_path("MERGE_MSG");
 
 	git_config(git_default_config);
 	me = action == REVERT ? "revert" : "cherry-pick";
@@ -280,7 +281,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	 * reverse of it if we are revert.
 	 */
 
-	msg_fd = hold_lock_file_for_update(&msg_file, ".msg", 1);
+	msg_fd = hold_lock_file_for_update(&msg_file, defmsg, 1);
 
 	encoding = get_encoding(message);
 	if (!encoding)
@@ -330,7 +331,6 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 				sha1_to_hex(head), "HEAD",
 				sha1_to_hex(next->object.sha1), oneline) ||
 			write_tree(head, 0, NULL)) {
-		const char *target = git_path("MERGE_MSG");
 		add_to_msg("\nConflicts:\n\n");
 		read_cache();
 		for (i = 0; i < active_nr;) {
@@ -345,10 +345,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 			}
 		}
 		if (close(msg_fd) || commit_lock_file(&msg_file) < 0)
-			die ("Error wrapping up .msg");
-		unlink(target);
-		if (rename(".msg", target))
-			die ("Could not move .msg to %s", target);
+			die ("Error wrapping up %s", defmsg);
 		fprintf(stderr, "Automatic %s failed.  "
 			"After resolving the conflicts,\n"
 			"mark the corrected paths with 'git-add <paths>'\n"
@@ -362,7 +359,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 		exit(1);
 	}
 	if (close(msg_fd) || commit_lock_file(&msg_file) < 0)
-		die ("Error wrapping up .msg");
+		die ("Error wrapping up %s", defmsg);
 	fprintf(stderr, "Finished one %s.\n", me);
 
 	/*
@@ -376,11 +373,9 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 
 	if (!no_commit) {
 		if (edit)
-			return execl_git_cmd("commit", "-n", "-F", ".msg",
-				"-e", NULL);
+			return execl_git_cmd("commit", "-n", NULL);
 		else
-			return execl_git_cmd("commit", "-n", "-F", ".msg",
-				NULL);
+			return execl_git_cmd("commit", "-n", "-F", defmsg, NULL);
 	}
 	if (reencoded_message)
 		free(reencoded_message);

-- 
Shawn.
Junio C Hamano· May 6, 2007, 06:49 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] Have git-revert, git-cherry-pick cleanup ./.msg upon successful completion.

"Shawn O. Pearce" <spearce@spearce.org> writes:
Show 5 quoted lines
> What about this change instead?  We make cherry-pick/revert
> use the same temporary file as merge, which is under .git/
> (something Alex mentioned he wanted).  I think the use of ".msg"
> in cherry-pick/revert has always just been a bug, and not a feature,
> so I'm really not against changing things around like this.

While I would not say this is not an improvement, this makes MERGE_MSG even less about merges and pushes us away from a sensible "git whatnow".

Shawn O. Pearce· May 8, 2007, 01:35 UTC · re: Junio C Hamano · lore

Re: [PATCH] Have git-revert, git-cherry-pick cleanup ./.msg upon successful completion.

Junio C Hamano <junkio@cox.net> wrote:
Show 11 quoted lines
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > What about this change instead?  We make cherry-pick/revert
> > use the same temporary file as merge, which is under .git/
> > (something Alex mentioned he wanted).  I think the use of ".msg"
> > in cherry-pick/revert has always just been a bug, and not a feature,
> > so I'm really not against changing things around like this.
> 
> While I would not say this is not an improvement, this makes
> MERGE_MSG even less about merges and pushes us away from a
> sensible "git whatnow".
 
I think that ship has already sailed.  Look at builtin-revert.c
on:
    333         const char *target = git_path("MERGE_MSG");

We're already using MERGE_MSG to prep the message for a conflicted cherry-pick or revert that the user needs to resolve by hand. I think we do the same thing in git-rebase, don't we?

Gerrit's patch to try and use COMMIT_MSG feels wrong to me, as git-commit overwrites that file with what it gets from its "input".

I agree my patch steps us further from a "git whatnow", but we're already in deep with MERGE_MSG. We might as well keep that existing convention that it can be used to prep the commit message for the next git-commit invocation, and record other data somehow for the "git whatnow" case.

-- 
Shawn.
Junio C Hamano· May 8, 2007, 01:42 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] Have git-revert, git-cherry-pick cleanup ./.msg upon successful completion.

"Shawn O. Pearce" <spearce@spearce.org> writes:
> Gerrit's patch to try and use COMMIT_MSG feels wrong to me, as
> git-commit overwrites that file with what it gets from its "input".
Yes.
Show 5 quoted lines
> I agree my patch steps us further from a "git whatnow", but we're
> already in deep with MERGE_MSG.  We might as well keep that existing
> convention that it can be used to prep the commit message for the
> next git-commit invocation, and record other data somehow for the
> "git whatnow" case.
Fair enough.
Gerrit Pape· May 7, 2007, 07:46 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] Have git-revert, git-cherry-pick cleanup ./.msg upon successful completion.

On Fri, May 04, 2007 at 11:46:15PM -0400, Shawn O. Pearce wrote:
Show 8 quoted lines
> Gerrit Pape <pape@smarden.org> wrote:
> > git-revert and git-cherry-pick left behind the commit message file ./.msg,
> > have them use the -f option to git-commit to properly cleanup the
> > automatically created file.
> 
> I'm actually sort of against changing the behavior of git-commit
> -f to mean "delete the file".  We never did that before.  Users
> might get surprised when their file goes away!

-f never was documented AFAICS, only -F, and even though the file is gone, the commit message is available through git show then. I personally would like to have such an option to git commit regardless of revert and cherry-pick.

I concur with Alex that the teporary file should be in the git directory and not working directory.

Regards, Gerrit.
Gerrit Pape· May 7, 2007, 10:53 UTC · re: Gerrit Pape · lore

[PATCH] Have git-revert, git-cherry-pick use $GIT_DIR/COMMIT_MSG instead of ./.msg.

On Fri, May 04, 2007 at 05:28:21PM +0200, Alex Riesen wrote:
Show 6 quoted lines
> On 5/4/07, Gerrit Pape <pape@smarden.org> wrote:
> >git-revert and git-cherry-pick left behind the commit message file ./.msg,
> >have them use the -f option to git-commit to properly cleanup the
> >automatically created file.
>
> Could we also have the files in $GIT_DIR instead of in working directory?

Yes, .git/COMMIT_MSG is already used by git-commit and git-svn, and can be used here too.

Signed-off-by: Gerrit Pape <pape@smarden.org>
---
 builtin-revert.c |   24 +++++++++++++-----------
 1 files changed, 13 insertions(+), 11 deletions(-)
Show changes to builtin-revert.c +13 −11
diff --git a/builtin-revert.c b/builtin-revert.c
index 9acdf47..5aff283 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -126,14 +126,15 @@ char *get_encoding(const char *message)
 	return NULL;
 }
 
-struct lock_file msg_file;
+static const char* msg_file;
+struct lock_file msg_file_lock;
 static int msg_fd;
 
 static void add_to_msg(const char *string)
 {
 	int len = strlen(string);
 	if (write_in_full(msg_fd, string, len) < 0)
-		die ("Could not write to .msg");
+		die ("Could not write to %s", msg_file);
 }
 
 static void add_message_to_msg(const char *message)
@@ -280,7 +281,8 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	 * reverse of it if we are revert.
 	 */
 
-	msg_fd = hold_lock_file_for_update(&msg_file, ".msg", 1);
+	msg_file = git_path("COMMIT_MSG");
+	msg_fd = hold_lock_file_for_update(&msg_file_lock, msg_file, 1);
 
 	encoding = get_encoding(message);
 	if (!encoding)
@@ -344,11 +346,11 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 					i++;
 			}
 		}
-		if (close(msg_fd) || commit_lock_file(&msg_file) < 0)
-			die ("Error wrapping up .msg");
+		if (close(msg_fd) || commit_lock_file(&msg_file_lock) < 0)
+			die ("Error wrapping up %s", msg_file);
 		unlink(target);
-		if (rename(".msg", target))
-			die ("Could not move .msg to %s", target);
+		if (rename(msg_file, target))
+			die ("Could not move %s to %s", msg_file, target);
 		fprintf(stderr, "Automatic %s failed.  "
 			"After resolving the conflicts,\n"
 			"mark the corrected paths with 'git-add <paths>'\n"
@@ -361,8 +363,8 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 		}
 		exit(1);
 	}
-	if (close(msg_fd) || commit_lock_file(&msg_file) < 0)
-		die ("Error wrapping up .msg");
+	if (close(msg_fd) || commit_lock_file(&msg_file_lock) < 0)
+		die ("Error wrapping up %s", msg_file);
 	fprintf(stderr, "Finished one %s.\n", me);
 
 	/*
@@ -376,10 +378,10 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 
 	if (!no_commit) {
 		if (edit)
-			return execl_git_cmd("commit", "-n", "-f", ".msg",
+			return execl_git_cmd("commit", "-n", "-f", msg_file,
 				"-e", NULL);
 		else
-			return execl_git_cmd("commit", "-n", "-f", ".msg",
+			return execl_git_cmd("commit", "-n", "-f", msg_file,
 				NULL);
 	}
 	if (reencoded_message)

-- 
1.5.1.2
Gerrit Pape· May 7, 2007, 10:54 UTC · re: Gerrit Pape · lore

[PATCH] git-commit: fix usage to show (-F|-f) <logfile>.

Signed-off-by: Gerrit Pape <pape@smarden.org>
---
 git-commit.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to git-commit.sh +1 −1
diff --git a/git-commit.sh b/git-commit.sh
index e4518c4..bbd76c6 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -3,7 +3,7 @@
 # Copyright (c) 2005 Linus Torvalds
 # Copyright (c) 2006 Junio C Hamano
 
-USAGE='[-a | --interactive] [-s] [-v] [--no-verify] [-m <message> | -F <logfile> | (-C|-c) <commit> | --amend] [-u] [-e] [--author <author>] [[-i | -o] <path>...]'
+USAGE='[-a | --interactive] [-s] [-v] [--no-verify] [-m <message> | (-F|-f) <logfile> | (-C|-c) <commit> | --amend] [-u] [-e] [--author <author>] [[-i | -o] <path>...]'
 SUBDIRECTORY_OK=Yes
 . git-sh-setup
 require_work_tree

-- 
1.5.1.2

← back to recent threads