Re: rebase --root conflicts with --committer-date-is-author-date
Junio C Hamano gits...@pobox.com wrote: Chris, do you remember if there was a reason why it was a bad idea to teach the normal rebase codepath to handle --root? I think we would have needed to allow am to apply a creation patch and start a new history on an unborn branch in order to do so, but I am not sure if there was a valid reason why such a change to am would have been a bad idea. Hi. It's a long time ago, but I don't remember any reason and it feels sensible that am should be able to create an unborn branch in the same way interactive rebase can. I suspect I had done the necessary work for rebase -i but not for am, and incorrectly assumed that interactive rebase was in any case a superset of non-interactive. Best wishes, Chris. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] rebase --root: sentinel commit cloaks empty commits
Thomas Rast t...@thomasrast.ch wrote: Please take a closer look at the last two test cases that specify the expected behaviour of rebasing a branch that tracks the empty tree. At this point they expect the Nothing to do error (aborts with untouched history). This is consistent with rebasing only empty commits without `--root`, which also doesn't just delete them from the history. Furthermore, I think the two alternatives adding a note that all commits in the range were empty, and removing the empty commits (thus making the branch empty) are better discussed in a separate bug report. Makes sense to me, though I have never thought much about rebasing empty commits. Maybe Chris has a more informed opinion? I definitely agree with you both that --root should be (and isn't) consistent with normal interactive rebasing. The difference isn't deliberate on my part. On a personal note, I've always disliked the way interactive rebase stops when you pick an existing empty commit or empty log message rather than preserving it. Jumping through a few hoops is perhaps sensible when you create that kind of strange commit, but just annoying when picking an existing empty/logless commit as part of a series. But as you say, that's a separate issue than --root behaving differently to non --root. Cheers, Chris. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC v2 07/19] rebase -i: The replay of root commits is not shown with --verbose
Fabian Ruch baf...@gmail.com wrote: you're the original author of the code touched by this patch. Is the second -q option really a simple copy-and-paste of the first or am I overlooking something here? I'd like to confirm this as, in retrospect, I feel a bit uncertain about the hasty claim in the log message. It was a while ago and I don't remember the details of this patch I'm afraid, but your rationale here looks sensible to me. I tend to find git too noisy by default rather than too quiet, so it wouldn't surprise me if I never thought to try the --verbose version. Cheers, Chris. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 00/14] New remote-hg helper
Felipe Contreras felipe.contre...@gmail.com writes: Implemented now. I'm not handling the 'tip' revision, but most likely it's also the '.' revision. In this case a fake 'master' bookmark will be created to track that revision. Hi Felipe. Sorry for the slow response, I've been snowed under with work and have only just got around to testing your latest version. The new remote-hg.track-branches=false option is great and does exactly what I was hoping for. For the benefit of the list archives, one natural way to use it is git clone -c remote-hg.track-branches=false hg::foo when cloning the relevant repositories, if you don't want the setting globally for every hg-remote clone. During testing, I've seen some strange behaviour which I think is caused by using the . revision instead of tip: $ hg init h $ hg init h2 $ ( cd h touch foo hg add foo hg commit -m foo hg push ../h2 ) pushing to ../h2 searching for changes adding changesets adding manifests adding file changes added 1 changesets with 1 changes to 1 files $ git clone hg::h g Cloning into 'g'... $ git clone hg::h2 g2 Cloning into 'g2'... warning: remote HEAD refers to nonexistent ref, unable to checkout. $ The reason for this is that by default . == null (not tip) in the repo h2 which we pushed into from h. The hg equivalent of a bare repo typically has a null checkout like this. (Actually, the checkout of HEAD seems to break whenever . is different from tip, not just when it's null as in this example.) Apart from this, everything seems solid and works well. Really useful; thanks! Best wishes, Chris. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 00/14] New remote-hg helper
Hi. I routinely work with projects in both hg and git, so I'm really interested in this. Thanks for working on it! I grabbed the latest version from https://github.com/felipec/git/blob/fc-remote-hg/contrib/remote-hg/git-remote-hg and have been trying it out. For the most part, it seems to work very nicely for the hg repos I have access to and can test against. I've spotted a couple of issues along the way that I thought would be worth reporting. The first is really a symptom of a general difference between hg and git: an hg repository can have multiple heads, whereas a git repo has exactly one head. To demonstrate: $ hg init hgtest cd hgtest $ echo zero foo hg add foo hg commit -m zero $ echo one foo hg commit -m one $ hg checkout -r 0 1 files updated, 0 files merged, 0 files removed, 0 files unresolved $ echo two foo hg commit -m two created new head $ hg log --graph @ changeset: 2:ca09651009cb | tag: tip | parent: 0:9f552c53d116 | user:Chris Webb ch...@arachsys.com | date:Tue Oct 30 09:33:38 2012 + | summary: two | | o changeset: 1:58fad8998339 |/ user:Chris Webb ch...@arachsys.com |date:Tue Oct 30 09:33:25 2012 + |summary: one | o changeset: 0:9f552c53d116 user:Chris Webb ch...@arachsys.com date:Tue Oct 30 09:33:08 2012 + summary: zero $ cd .. Now if I try to convert this: $ git clone hg::$PWD/hgtest gittest Cloning into 'gittest'... WARNING: Branch 'default' has more than one head, consider merging Traceback (most recent call last): File /home/chris/bin/git-remote-hg, line 773, in module sys.exit(main(sys.argv)) File /home/chris/bin/git-remote-hg, line 759, in main do_list(parser) File /home/chris/bin/git-remote-hg, line 463, in do_list list_branch_head(repo, cur) File /home/chris/bin/git-remote-hg, line 425, in list_branch_head tip = get_branch_tip(repo, cur) File /home/chris/bin/git-remote-hg, line 418, in get_branch_tip return repo.branchtip(branch) AttributeError: 'mqrepo' object has no attribute 'branchtip' Strip the second head and it's fine: $ hg -R hgtest strip 2 1 files updated, 0 files merged, 0 files removed, 0 files unresolved saved backup bundle to /tmp/hgtest/hgtest/.hg/strip-backup/ca09651009cb-backup.hg $ git clone hg::$PWD/hgtest gittest Cloning into 'gittest'... $ Not sure what the most friendly thing to do here is. Perhaps refuse to clone/pull from a repo with multiple heads unless you name the specific head you want? The second thing I spotted is the behaviour of bookmarks on push: $ hg init hgtest cd hgtest $ echo zero foo hg add foo hg commit -m zero $ hg bookmark development $ cd .. $ git clone hg::$PWD/hgtest gittest cd gittest Cloning into 'gittest'... $ git checkout development Branch development set up to track remote branch development from origin. Switched to a new branch 'development' $ echo one foo git add foo git commit -m one [development 9f67dc4] one 1 file changed, 1 insertion(+), 1 deletion(-) $ git status # On branch development # Your branch is ahead of 'origin/development' by 1 commit. # nothing to commit $ git push warning: helper reported unexpected status of refs/hg/origin/bookmarks/development To hg::/tmp/hgtest/hgtest * [new branch] branches/default - branches/default * [new branch] development - development $ hg log -R ../hgtest changeset: 1:1c0714d93864 tag: tip user:Chris Webb ch...@arachsys.com date:Tue Oct 30 09:51:51 2012 + summary: one changeset: 0:f56c463398ea bookmark:development user:Chris Webb ch...@arachsys.com date:Tue Oct 30 09:50:53 2012 + summary: zero i.e. the development bookmark hasn't been updated by the push. This might be connected to the warning message warning: helper reported unexpected status of refs/hg/origin/bookmarks/development I'm testing with hg 2.2.2 and current git master, so I expect this could be a python api change in the more recent versions of hg if you don't see the same behaviour. Best wishes, Chris. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 00/14] New remote-hg helper
Chris Webb ch...@arachsys.com writes: The first is really a symptom of a general difference between hg and git: an hg repository can have multiple heads, whereas a git repo has exactly one head. By this I mean an hg repository without bookmarks or branches can still have multiple heads, whereas a git branch points at exactly one place. Sorry, very vague description there! Cheers, Chris. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 00/14] New remote-hg helper
Felipe Contreras felipe.contre...@gmail.com writes: Yes, it seems this is an API issue; repo.branchtip doesn't exist in python 2.2. Hi. Presumably this is a problem with old mercurial not a problem with old python as mentioned in the commit? Both issues should be fixed now :) They are indeed, and it now works nicely on all the repos I've tested it with, including http://selenic.com/hg: very impressive! I wonder whether it's worth ignoring heads with bookmarks pointing to them when it comes to considering heads of branches, or at least allowing the hg branch tracking to be easily disabled? A common idiom when working with hg bookmarks is to completely ignore the (not very useful) hg branches (i.e. all commits are on the default hg branch) and have a bookmark for each line of development used exactly as a git branch would be. On such a repository, at the moment you will always get a warning about multiple heads on branches/default, even though you actually don't care about branches/default (and would prefer it not to exist) and just want the branches coming from the bookmarks. I've also seen repositories with no hg branches, but with a single unbookmarked tip and bookmarks on some other heads to mark non-mainline development. It would be nice for branches/default to track the unbookmarked tip in this case, without warning about the other, bookmarked heads. Finally, on a simple repo with no branches and where there's no clash with a bookmark called master, I'd love to be able to a get a more idiomatic origin/master rather than origin/branches/default. Just some idle thoughts... Best wishes, Chris. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 00/14] New remote-hg helper
Chris Webb ch...@arachsys.com writes: A common idiom when working with hg bookmarks is to completely ignore the (not very useful) hg branches (i.e. all commits are on the default hg branch) and have a bookmark for each line of development used exactly as a git branch would be. On such a repository, at the moment you will always get a warning about multiple heads on branches/default, even though you actually don't care about branches/default (and would prefer it not to exist) and just want the branches coming from the bookmarks. Something which you can do with hg clone is hg clone http://my.repo/foo#master to clone just the history behind the master bookmark from foo. This works nicely with git-remote-hg too: git clone hg::http://my.repo/foo#master gives you just origin/master and origin/branches/default, not origin/otherbookmark. This is a case where it would be particularly nice to be able to kill origin/branches/default and just keep the identical origin/master. Cheers, Chris. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] add t3420-rebase-topology
Martin von Zweigbergk martinv...@gmail.com writes: For consistency, it seems like git rebase -p --root should always be a no-op, while git rebase [-i/-m] --root should be no-op if the history has no merges. Also, since git rebase -i tries to fast-forward through existing commits, it seems like git rebase -i --root should ideally not create a sentinel commit, but instead stop at the first commit marked for editing. If, OTOH, --force-rebase is given, we should rewrite history from the first commit, which in the case of --root would mean creating a sentinel commit. So, in short, I have a feeling that the sentinel commit should be created if and only if both --root and --force-rebase (but not --onto) are given. [...] So I'm getting more and more convinced that the sentinel commit should only be created if --force-rebase was given. Let me know if I'm missing something. No, that sounds fairly convincing to me. Personally, the only behaviour I want to be able to get at without --force-rebase is for rebase -i --root to allow the root commit to be dropped, edited or reworded, and commits to reordered to make a different one the root, in the same way as normal interactive rebase does for later commits. The least intrusive implementation for rebase --root was the unconditional sentinel, but as you say, you don't need it (and it's a bit surprising) if the root commit on the instruction sheet is the same as the original root: in the edit/reword case, you could just checkout the existing root and then drop out in the normal way. You only really need the sentinel to deal with the possibility of a conflict if the root is replaced with a different commit. I think I agree with you it would be better only to create the sentinel on demand when it's required or if --force is given. Cheers, Chris. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] add t3420-rebase-topology
Martin von Zweigbergk martinv...@gmail.com writes: On Tue, Sep 18, 2012 at 12:53 AM, Johannes Sixt j.s...@viscovery.net wrote: Why? Is it more like --root implies --force? It doesn't currently exactly imply --force, but the effect is the same. Also see my reply to Junio's email in this thread. Maybe Chris has some thoughts on this? Hi Martin and Johannes. Sorry for the slow follow-up here. You're right that rebase --root without --onto always creates a brand new root as a result of the implementation using a sentinel commit. Clearly this is what's wanted with --interactive, but rebase --root with neither --onto nor --interactive is a slightly odd combination for which I struggle to imagine a natural use. Perhaps you're right that for consistency it should be a no-op unless --force-rebase is given? If we did this, this combination would be a no-op unconditionally as by definition we're always descended from the root of our current commit. However, given the not-very-useful behaviour, I suspect that rebase --root is much more likely to be a mistyped version of rebase -i --root than rebase --root --force-rebase. (Unless I'm missing a reasonable use for this? History linearisation perhaps?) Best wishes, Chris. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] cherry-pick: add --allow-empty-message option
Neil Horman nhor...@tuxdriver.com writes: Having read over this thread, I think this is definately the way to go. As discussed having cherry-pick stop and give the user a chance to fix empty history messages by default, and providing a switch to override that behavior makes sense to me. That said, shouldn't there be extra code here in the rebase scripts to automate commit migration in that path as well? Yes, this patch just adds the support to the low-level git cherry-pick as you say. I'll follow up with a patch to use the new feature in rebase [-i] when I get some free time, hopefully later this week. Cheers, Chris. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Cherry-picking commits with empty messages
Junio C Hamano gits...@pobox.com writes: My recommendation, backed by the above line of thought, is to add support for the --allow-empty-message option to both rebase [-i] and cherry-pick, defaulting to false. Thanks for the very detailed analysis and advice Junio. I like your suggested --allow-empty-message option for cherry-pick because it's consistent with the same option in standard commit, and doesn't change the behaviour for existing users who might rely on cherry-pick catching blank messages. With rebase -i, the fix might be slightly more involved than just passing through --allow-empty-message (if given) to cherry-pick, especially given that sometimes we git cherry-pick -n git commit --allow-empty-message, and at other times we do standard git cherry-pick which refuses to pick a commit without a message. Given a history with empty commits, as a general principle it feels like it should be possible to edit or reword those commits to make them non-empty without giving --allow-empty-message, but that to generate new history containing empty messages, --allow-empty-message should be required, whether to commit [--amend] during rebase, or to the rebase -i command itself. Cheers, Chris. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] cherry-pick: add --allow-empty-message option
Scripts such as git rebase -i cannot currently cherry-pick commits which
have an empty commit message, as git cherry-pick calls git commit
without the --allow-empty-message option.
Add an --allow-empty-message option to git cherry-pick which is passed
through to git commit, so this behaviour can be overridden.
Signed-off-by: Chris Webb ch...@arachsys.com
---
Documentation/git-cherry-pick.txt | 5 +
builtin/revert.c | 2 ++
sequencer.c | 3 +++
sequencer.h | 1 +
t/t3505-cherry-pick-empty.sh | 5 +
5 files changed, 16 insertions(+)
diff --git a/Documentation/git-cherry-pick.txt
b/Documentation/git-cherry-pick.txt
index 0e170a5..c205d23 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -118,6 +118,11 @@ effect to your index in a row.
previous commit are dropped. To force the inclusion of those commits
use `--keep-redundant-commits`.
+--allow-empty-message::
+ By default, cherry-picking a commit with an empty message will fail.
+ This option overrides that behaviour, allowing commits with empty
+ messages to be cherry picked.
+
--keep-redundant-commits::
If a commit being cherry picked duplicates a commit already in the
current history, it will become empty. By default these
diff --git a/builtin/revert.c b/builtin/revert.c
index 82d1bf8..5652f23 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -117,6 +117,7 @@ static void parse_args(int argc, const char **argv, struct
replay_opts *opts)
OPT_END(),
OPT_END(),
OPT_END(),
+ OPT_END(),
};
if (opts-action == REPLAY_PICK) {
@@ -124,6 +125,7 @@ static void parse_args(int argc, const char **argv, struct
replay_opts *opts)
OPT_BOOLEAN('x', NULL, opts-record_origin, append
commit name),
OPT_BOOLEAN(0, ff, opts-allow_ff, allow
fast-forward),
OPT_BOOLEAN(0, allow-empty, opts-allow_empty,
preserve initially empty commits),
+ OPT_BOOLEAN(0, allow-empty-message,
opts-allow_empty_message, allow commits with empty messages),
OPT_BOOLEAN(0, keep-redundant-commits,
opts-keep_redundant_commits, keep redundant, empty commits),
OPT_END(),
};
diff --git a/sequencer.c b/sequencer.c
index bf078f2..1ea5293 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -311,6 +311,9 @@ static int run_git_commit(const char *defmsg, struct
replay_opts *opts,
if (allow_empty)
argv_array_push(array, --allow-empty);
+ if (opts-allow_empty_message)
+ argv_array_push(array, --allow-empty-message);
+
rc = run_command_v_opt(array.argv, RUN_GIT_CMD);
argv_array_clear(array);
return rc;
diff --git a/sequencer.h b/sequencer.h
index aa5f17c..d849420 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -30,6 +30,7 @@ struct replay_opts {
int allow_ff;
int allow_rerere_auto;
int allow_empty;
+ int allow_empty_message;
int keep_redundant_commits;
int mainline;
diff --git a/t/t3505-cherry-pick-empty.sh b/t/t3505-cherry-pick-empty.sh
index 5a1340c..a0c6e30 100755
--- a/t/t3505-cherry-pick-empty.sh
+++ b/t/t3505-cherry-pick-empty.sh
@@ -53,6 +53,11 @@ test_expect_success 'index lockfile was removed' '
'
+test_expect_success 'cherry-pick a commit with an empty message with
--allow-empty-message' '
+ git checkout -f master
+ git cherry-pick --allow-empty-message empty-branch
+'
+
test_expect_success 'cherry pick an empty non-ff commit without --allow-empty'
'
git checkout master
echo fourth file2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Cherry-picking commits with empty messages
Whilst doing some extra sanity checking of my git-rebase--interactive.sh
patch yesterday, I came across a behaviour which has been present for some
time, but seems surprising. You can reproduce with
$ git init -q foo cd foo
$ touch one git add one git commit -q -m one
$ touch two git add two git commit -q -m two
$ touch three git add three git commit -q -m '' --allow-empty-message
$ touch four git add four git commit -q -m '' --allow-empty-message
$ git rebase -i HEAD~3 # and swap the two commits with empty messages
Aborting commit due to empty commit message.
Could not apply 59a8fde...
This happens on my ancient laptop which is apparently running 1.7.8.3, as well
as current master, so is unconnected to recent changes.
The reason is that git cherry-pick won't pick a commit with an empty commit
message, even when that message is unmodified from the original:
$ git rebase --abort
$ git checkout -q HEAD~2
$ git cherry-pick 59a8fde
Aborting commit due to empty commit message.
I can see that this check could make sense when the message has been
modified, but it seems strange when it hasn't, and isn't ideal behaviour
when called from rebase -i. (We otherwise make sure we call git commit with
--allow-empty-message to avoid problems with reordering or editing empty
commits.)
I could just remove the check in the 'message unmodified' case with
something like
diff --git a/sequencer.c b/sequencer.c
index bf078f2..cf8bc05 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -306,6 +306,7 @@ static int run_git_commit(const char *defmsg, struct
replay_opts *opts,
if (!opts-edit) {
argv_array_push(array, -F);
argv_array_push(array, defmsg);
+ argv_array_push(array, --allow-empty-message);
}
if (allow_empty)
but perhaps there are other users of the sequencer for whom this check is
desirable? If so, would an --allow-empty-message to git cherry-pick be a
better plan, which git rebase -i can use where appropriate?
Best wishes,
Chris.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rebase -i: handle fixup of root commit correctly
Johannes Sixt j...@kdbg.org writes:
Am 24.07.2012 14:17, schrieb Chris Webb:
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index bef7bc0..0d2056f 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -493,25 +493,28 @@ do_next () {
author_script_content=$(get_author_ident_from_commit HEAD)
echo $author_script_content $author_script
eval $author_script_content
-output git reset --soft HEAD^
-pick_one -n $sha1 || die_failed_squash $sha1 $rest
+if ! pick_one -n $sha1
+then
+git rev-parse --verify HEAD $amend
+die_failed_squash $sha1 $rest
+fi
case $(peek_next_command) in
squash|s|fixup|f)
# This is an intermediate commit; its message will only
be
# used in case of trouble. So use the long version:
-do_with_author output git commit --no-verify -F
$squash_msg ||
+do_with_author output git commit --amend --no-verify -F
$squash_msg ||
die_failed_squash $sha1 $rest
This new sequence looks *VERY* suspicious. It makes a HUGE
difference in what is left behind if the cherry-pick fails. Did you
think about what happens when the cherry-pick fails in a
squash+squash+fixup+fixup sequence (or any combination thereof) and
then the rebase is continued (after a manual resolution)?
I had to deal with the case where there's a conflict while picking the
squash/fixup, and we have to ensure we commit --amend in rebase --continue.
This is why I've written
git rev-parse --verify HEAD $amend
in the above, to use the pre-existing support for amending the HEAD commit
in rebase --continue. (We test for this fixup-conflict case in various ways
in t3404 and not doing an amend there would result in double commits and
spectacular test breakage.)
Is this the issue you mean here, or is it something more subtle which I'm
not properly following?
If we have a conflict in the middle of a chain of fixup/squashes, as far as
I can see, we have a HEAD with all the previous successful fixups applied,
conflict markers for the current failed pick, and when the conflict has been
resolved, git rebase --continue will commit --amend the resolution and
continue? Isn't that the correct behaviour here?
Cheers,
Chris.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rebase -i: handle fixup of root commit correctly
Chris Webb ch...@arachsys.com writes: If we have a conflict in the middle of a chain of fixup/squashes, as far as I can see, we have a HEAD with all the previous successful fixups applied, conflict markers for the current failed pick, and when the conflict has been resolved, git rebase --continue will commit --amend the resolution and continue? Isn't that the correct behaviour here? As an explicit test, I've just tried a chain of four squashed commits, each of which deliberately resulted in a conflict to manually resolve. For each squash, I was left with conflict markers on top of what had already been squashed in the expected way, and when I continued after resolving these, the resolution was 'commit --amend'ed in the expected way, with the same behaviour and resulting commit at the end of the rebase -i as I get with a copy of git without this patch. Cheers, Chris. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rebase -i: handle fixup of root commit correctly
Johannes Sixt j...@kdbg.org writes: One subtlety to watch out for is when commit messages are edited. That is, if you edit the proposed message at 'rebase --continue' after the first squash failed, is the new text preserved until the last squash? I *think* that previously that was the case. Hi. Yes, doing this seems to work fine both in the original code, and after my patch. I've just checked to be certain using my previous test case of four conflicting squashes again, editing the message at each stage and ensuring the edits are all retained in the final commit. Best wishes, Chris. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] rebase -i: handle fixup of root commit correctly
There is a bug with git rebase -i --root when a fixup or squash line is
applied to the new root. We attempt to amend the commit onto which they
apply with git reset --soft HEAD^ followed by a normal commit. Unlike a
real commit --amend, this sequence will fail against a root commit as it
has no parent.
Fix rebase -i to use commit --amend for fixup and squash instead, and
add a test for the case of a fixup of the root commit.
Signed-off-by: Chris Webb ch...@arachsys.com
---
Sorry, I should have spotted this issue when I did the original root-rebase
series. I've checked that this patch doesn't break any of the existing
tests, as well as satisfying the newly introduced check for the root-fixup
case.
git-rebase--interactive.sh| 25 +
t/t3404-rebase-interactive.sh | 8
2 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index bef7bc0..0d2056f 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -493,25 +493,28 @@ do_next () {
author_script_content=$(get_author_ident_from_commit HEAD)
echo $author_script_content $author_script
eval $author_script_content
- output git reset --soft HEAD^
- pick_one -n $sha1 || die_failed_squash $sha1 $rest
+ if ! pick_one -n $sha1
+ then
+ git rev-parse --verify HEAD $amend
+ die_failed_squash $sha1 $rest
+ fi
case $(peek_next_command) in
squash|s|fixup|f)
# This is an intermediate commit; its message will only
be
# used in case of trouble. So use the long version:
- do_with_author output git commit --no-verify -F
$squash_msg ||
+ do_with_author output git commit --amend --no-verify -F
$squash_msg ||
die_failed_squash $sha1 $rest
;;
*)
# This is the final command of this squash/fixup group
if test -f $fixup_msg
then
- do_with_author git commit --no-verify -F
$fixup_msg ||
+ do_with_author git commit --amend --no-verify
-F $fixup_msg ||
die_failed_squash $sha1 $rest
else
cp $squash_msg $GIT_DIR/SQUASH_MSG || exit
rm -f $GIT_DIR/MERGE_MSG
- do_with_author git commit --no-verify -e ||
+ do_with_author git commit --amend --no-verify
-F $GIT_DIR/SQUASH_MSG -e ||
die_failed_squash $sha1 $rest
fi
rm -f $squash_msg $fixup_msg
@@ -748,7 +751,6 @@ In both case, once you're done, continue with:
fi
. $author_script ||
die Error trying to find the author identity to amend
commit
- current_head=
if test -f $amend
then
current_head=$(git rev-parse --verify HEAD)
@@ -756,13 +758,12 @@ In both case, once you're done, continue with:
die \
You have uncommitted changes in your working tree. Please, commit them
first and then run 'git rebase --continue' again.
- git reset --soft HEAD^ ||
- die Cannot rewind the HEAD
+ do_with_author git commit --amend --no-verify -F $msg
-e ||
+ die Could not commit staged changes.
+ else
+ do_with_author git commit --no-verify -F $msg -e ||
+ die Could not commit staged changes.
fi
- do_with_author git commit --no-verify -F $msg -e || {
- test -n $current_head git reset --soft
$current_head
- die Could not commit staged changes.
- }
fi
record_in_rewritten $(cat $state_dir/stopped-sha)
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8078db6..3f75d32 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -903,4 +903,12 @@ test_expect_success 'rebase -i --root temporary sentinel
commit' '
git rebase --abort
'
+test_expect_success 'rebase -i --root fixup root commit' '
+ git checkout B
+ FAKE_LINES=1 fixup 2 git rebase -i --root
+ test A = $(git cat-file commit HEAD | sed -ne \$p)
+ test B = $(git show HEAD:file1)
+ test 0 = $(git cat-file commit HEAD | grep -c ^parent\ )
+'
+
test_done
--
1.7.11.2.251.g6a928a6
--
To unsubscribe from this list: send
Using git commit --amend on a commit with an empty message
Github gists can be cloned as normal git repositories, but the commits made
through the web interface appear with an empty commit message. Running
git commit --amend against them exposes a slightly odd behaviour of git,
which I can also demonstrate as follows:
$ git init foo cd foo
$ touch one git add one
$ git commit -m '' --allow-empty-message
[master (root-commit) 535cb36]
0 files changed
create mode 100644 one
When I try to correct this commit message in an editor, it refuses to
proceed, objecting to the existing empty commit message:
$ git commit --amend
fatal: commit has empty message
Shouldn't this drop me into the editor and fail only if the resulting
message on exit is empty? (For comparison, git commit --amend -m 'oops' will
work fine; it's apparently only the edit case which doesn't.)
In fact, we even fail to start the editor if --allow-empty-message is
explicitly provided:
$ git commit --amend --allow-empty-message
fatal: commit has empty message
Assuming this isn't intentional for some reason I don't understand, I think
this is the correct tiny fix? make test succeeds fine both before and after.
-- 8 --
Subject: [PATCH] Allow edit of empty message with commit --amend
If git commit --amend is used on a commit with an empty message, it fails
unless -m is given, whether or not --allow-empty-message is specified.
Instead, allow it to proceed to the editor with an empty commit message.
Unless --allow-empty-message is in force, it will still abort later if an
empty message is saved from the editor. (That check was already present
and necessary to prevent a non-empty commit message being edited to an
empty one.)
Signed-off-by: Chris Webb ch...@arachsys.com
---
builtin/commit.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index f43eaaf..6515da2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -640,7 +640,7 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
hook_arg1 = message;
} else if (use_message) {
buffer = strstr(use_message_buffer, \n\n);
- if (!buffer || buffer[2] == '\0')
+ if (!use_editor (!buffer || buffer[2] == '\0'))
die(_(commit has empty message));
strbuf_add(sb, buffer + 2, strlen(buffer + 2));
hook_arg1 = commit;
--
1.7.10
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Using git commit --amend on a commit with an empty message
Junio C Hamano gits...@pobox.com writes: Yeah, it is a bug that exists only because nobody sane uses empty message commits, let alone tries to amend such commits, hence went unnoticed for a long time. Quite. I only noticed it because this is the default behaviour of Github gists and I wanted to replace the empty commit messages with more meaningful ones. The patch looks sane; if we want to keep this as a feature or a bugfix, we may want to pretect it with a new test, though. Yes, it's hardly something people will test often. Okay, I'll send a version two with a suitable test. Best wishes, Chris. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] Allow edit of empty message with commit --amend
If git commit --amend is used on a commit with an empty message, it fails
unless -m is given, whether or not --allow-empty-message is specified.
Instead, allow it to proceed to the editor with an empty commit message.
Unless --allow-empty-message is in force, it will still abort later if an
empty message is saved from the editor. (This check was already necessary
to prevent a non-empty commit message being edited to an empty one.)
Add a test for --amend --edit of an empty commit message which fails
without this fix, as it's a rare case that won't get frequently tested
otherwise.
Signed-off-by: Chris Webb ch...@arachsys.com
---
builtin/commit.c |2 +-
t/t7501-commit.sh | 15 +++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index f43eaaf..6515da2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -640,7 +640,7 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
hook_arg1 = message;
} else if (use_message) {
buffer = strstr(use_message_buffer, \n\n);
- if (!buffer || buffer[2] == '\0')
+ if (!use_editor (!buffer || buffer[2] == '\0'))
die(_(commit has empty message));
strbuf_add(sb, buffer + 2, strlen(buffer + 2));
hook_arg1 = commit;
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index b20ca0e..5ad636b 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -138,6 +138,21 @@ test_expect_success '--amend --edit' '
test_cmp expect msg
'
+test_expect_success '--amend --edit of empty message' '
+ cat replace -\EOF
+ #!/bin/sh
+ echo amended $1
+ EOF
+ chmod 755 replace
+ echo amended expect
+ git commit --allow-empty --allow-empty-message -m
+ echo more bongo file
+ git add file
+ EDITOR=./replace git commit --edit --amend
+ git diff-tree -s --format=%s HEAD msg
+ test_cmp expect msg
+'
+
test_expect_success '-m --edit' '
echo amended expect
git commit --allow-empty -m buffer
--
1.7.10
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
