Marc Strapetz [off-list ref] writes:
When using "git commit --amend --only --message <message> --", I'd
expect to have just the commit message of my last commit changed,
according to the man page:
"--only Make a commit only from the paths specified on the command line,
disregarding any contents that have been staged so far. [...] If this
option is specified together with --amend, then no paths need to be
specified, which can be used to amend the last commit without committing
changes that have already been staged."
However, all staged changes are committed as well. So looks like either
the man page or Git is wrong here!?
I do not think the combination with --amend, --only and no paths
ever worked. We rejected such a combination before 6a74642c5, which
merely made us to accept the combination but I do not think the
commit did anything to re-read the tree from the HEAD being amended
to the index.
Something like this, but I haven't thought about what other things
it may break.
builtin/commit.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index f43eaaf..59ef5e1 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -184,6 +184,9 @@ static int list_paths(struct string_list *list, const char *with_tree,
int i;
char *m;
+ if (!pattern)
+ return 0;
+
for (i = 0; pattern[i]; i++)
;
m = xcalloc(1, i);
@@ -345,7 +348,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
* and create commit from the_index.
* We still need to refresh the index here.
*/
- if (!pathspec || !*pathspec) {
+ if (!(only && amend) && (!pathspec || !*pathspec)) {
fd = hold_locked_index(&index_lock, 1);
refresh_cache_or_die(refresh_flags);
if (active_cache_changed) {On Tue, Jul 10, 2012 at 01:14:32PM -0700, Junio C Hamano wrote:
I do not think the combination with --amend, --only and no paths
ever worked. We rejected such a combination before 6a74642c5, which
merely made us to accept the combination but I do not think the
commit did anything to re-read the tree from the HEAD being amended
to the index.
Something like this, but I haven't thought about what other things
it may break.
Our emails just crossed. I came to the exact same conclusion, and just
wrote almost the exact same patch.
- if (!pathspec || !*pathspec) {
+ if (!(only && amend) && (!pathspec || !*pathspec)) {
It is sufficient to check only "only" in the first part of your
conditional, as we disallow empty pathspecs with "-o". And even if we
didn't disallow it, this would do the right thing by trying to create a
partial commit with no changes (which would fail, of course, but is the
only sane thing for the prepare_index function to do; certainly doing an
as-is commit is simply wrong).
-Peff
On Tue, Jul 10, 2012 at 04:30:52PM -0400, Jeff King wrote:
On Tue, Jul 10, 2012 at 01:14:32PM -0700, Junio C Hamano wrote:
quoted
I do not think the combination with --amend, --only and no paths
ever worked. We rejected such a combination before 6a74642c5, which
merely made us to accept the combination but I do not think the
commit did anything to re-read the tree from the HEAD being amended
to the index.
Something like this, but I haven't thought about what other things
it may break.
Our emails just crossed. I came to the exact same conclusion, and just
wrote almost the exact same patch.
Here it is with a test and commit message. I believe this fix could also
make:
git commit --allow-empty --only --
work if we removed the "--only does not make sense without paths" check.
But I seriously doubt that anybody cares, given that "--only" is the
default (i.e., just omitting it already does what you want there,
whether you have pathspecs or not).
-- >8 --
Subject: [PATCH] commit: fix "--amend --only" with no pathspec
When we do not have any pathspec, we typically disallow an
explicit "--only", because it makes no sense (your commit
would, by definition, be empty). But since 6a74642
(git-commit --amend: two fixes., 2006-04-20), we have
allowed "--amend --only" with the intent that it would amend
the commit, ignoring any contents staged in the index.
However, while that commit allowed the combination, we never
actually implemented the logic to make it work. The current
code notices that we have no pathspec and assumes we want to
do an as-is commit (i.e., the "--only" is ignored).
Instead, we must make sure to follow the partial-commit
code-path. We also need to tweak the list_paths function to
handle a NULL pathspec.
Signed-off-by: Jeff King <redacted>
---
builtin/commit.c | 5 ++++-
t/t7501-commit.sh | 10 ++++++++++
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index f43eaaf..3c3385c 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -184,6 +184,9 @@ static int list_paths(struct string_list *list, const char *with_tree,
int i;
char *m;
+ if (!pattern)
+ return 0;
+
for (i = 0; pattern[i]; i++)
;
m = xcalloc(1, i);
@@ -345,7 +348,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
* and create commit from the_index.
* We still need to refresh the index here.
*/
- if (!pathspec || !*pathspec) {
+ if (!only && (!pathspec || !*pathspec)) {
fd = hold_locked_index(&index_lock, 1);
refresh_cache_or_die(refresh_flags);
if (active_cache_changed) {diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index b20ca0e..9f8d423 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -108,6 +108,16 @@ test_expect_success 'amend commit' '
EDITOR=./editor git commit --amend
'
+test_expect_success 'amend --only ignores staged contents' '
+ test_when_finished "git reset --hard" &&
+ cp file file.expect &&
+ echo changed >file &&
+ git add file &&
+ git commit --no-edit --amend --only &&
+ git cat-file blob HEAD:file >file.actual &&
+ test_cmp file.expect file.actual
+'
+
test_expect_success 'set up editor' '
cat >editor <<-\EOF &&
#!/bin/sh
--
1.7.11.35.gbaf554e.dirty