Donation
A Donation Of 1 Million British Pounds To You In Good Faith
Mr Neil Trotter
A donation of 1 million British Pounds to you in good faith
mr neil
A Donation Of 1 Million Pounds To You In Good Faith
Options to avoid docs generation/installation
Hi, The INSTALL file says that docs are not built by default, but that's not my experience. "make all" results in the generation of several Perl man pages, e.g. "Git.3pm". Is it the case that the behaviour documented is not propagated to the perl subdir? Also, setting --mandir=/dev/null still results in the pages being installed in their usual location (at least in combination with setting --prefix to avoid disturbing my production git installation). Regards, Neil
Mr Neil Trotter
Eine Spende von 1 Million Britische Pfund zu Ihnen in gutem Glauben
git bisect replay produces wrong result
Hi, the following git-bisect log - applied to a recent linux-kernel tree produced different end results when I use git bisect replay and when I just run it as a shell script. $ git bisect replay /tmp/log We are not bisecting. Bisecting: a merge base must be tested [2decb2682f80759f631c8332f9a2a34a02150a03] Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net $ bash /tmp/log Bisecting: 2 revisions left to test after this (roughly 1 step) [57127645d79d2e83e801f141f7d03f64accf28aa] s390/zcrypt: Introduce new SHA-512 based Pseudo Random Generator. Is git bisect replay doing the wrong thing, or am I confused? I tested on 2.5.0, and the current HEAD. Thanks, NeilBrown git bisect start # bad: [5ebe6afaf0057ac3eaeb98defd5456894b446d22] Linux 4.1-rc2 git bisect bad 5ebe6afaf0057ac3eaeb98defd5456894b446d22 # good: [39a8804455fb23f09157341d3ba7db6d7ae6ee76] Linux 4.0 git bisect good 39a8804455fb23f09157341d3ba7db6d7ae6ee76 # good: [6c373ca89399c5a3f7ef210ad8f63dc3437da345] Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next git bisect good 6c373ca89399c5a3f7ef210ad8f63dc3437da345 # good: [2c33ce009ca2389dbf0535d0672214d09738e35e] Merge Linus master into drm-next git bisect good 2c33ce009ca2389dbf0535d0672214d09738e35e # good: [7d2b6ef19cf0f98cef17aa5185de3631a618710a] Merge tag 'armsoc-drivers' of git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc git bisect good 7d2b6ef19cf0f98cef17aa5185de3631a618710a # good: [836ee4874e201a5907f9658fb2bf3527dd952d30] Merge tag 'arm64-upstream' of git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux git bisect good 836ee4874e201a5907f9658fb2bf3527dd952d30 # good: [78d425677217b655ed36c492a070b5002832fc73] Merge tag 'platform-drivers-x86-v4.1-1' of git://git.infradead.org/users/dvhart/linux-platform-drivers-x86 git bisect good 78d425677217b655ed36c492a070b5002832fc73 # good: [39376ccb1968ba9f83e2a880a8bf02ad5dea44e1] Merge git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf git bisect good 39376ccb1968ba9f83e2a880a8bf02ad5dea44e1 # bad: [64887b6882de36069c18ef2d9623484d6db7cd3a] Merge branch 'for-linus-4.1' of git://git.kernel.org/pub/scm/linux/kernel/git/mason/linux-btrfs git bisect bad 64887b6882de36069c18ef2d9623484d6db7cd3a # bad: [9dbbe3cfc3c208643cf0e81c8f660f43e1b4b2e8] Merge tag 'for-linus' of git://git.kernel.org/pub/scm/virt/kvm/kvm git bisect bad 9dbbe3cfc3c208643cf0e81c8f660f43e1b4b2e8 # bad: [dcca8de0aa597f14e31a1b38690626c9f6745fd5] Merge tag 'usb-4.1-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb git bisect bad dcca8de0aa597f14e31a1b38690626c9f6745fd5 # bad: [3d99e3fe13d473ac4578c37f477a59b829530764] Merge branch 'stable' of git://git.kernel.org/pub/scm/linux/kernel/git/cmetcalf/linux-tile git bisect bad 3d99e3fe13d473ac4578c37f477a59b829530764 # good: [b7d14f3a92223c3f5e52e9f20c74cb96dc130e87] s390/mm: correct transfer of dirty young bits in __pmd_to_pte git bisect good b7d14f3a92223c3f5e52e9f20c74cb96dc130e87 signature.asc Description: PGP signature
Re: [PATCH] rebase --keep-empty -i: add test
On Sun, May 18, 2014 at 11:28:39PM +0300, Michael S. Tsirkin wrote: There's some special code in rebase -i to deal with --keep-empty. Add test for this combination. Signed-off-by: Michael S. Tsirkin m...@redhat.com Acked-by: Neil Horman nhor...@tuxdriver.com --- t/t3404-rebase-interactive.sh | 9 + 1 file changed, 9 insertions(+) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index c0023a5..3b1b863 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -67,6 +67,14 @@ test_expect_success 'setup' ' SHELL= export SHELL +test_expect_success 'rebase --keep-empty' ' + git checkout -b emptybranch master + git commit --allow-empty -m empty + git rebase --keep-empty -i HEAD~2 + git log --oneline actual + test_line_count = 6 actual +' + test_expect_success 'rebase -i with the exec command' ' git checkout master ( -- MST -- 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 1/2] sequencer: trivial fix
On Tue, May 28, 2013 at 09:32:59PM -0500, Felipe Contreras wrote:
Junio C Hamano wrote:
Neil Horman nhor...@tuxdriver.com writes:
On Mon, May 27, 2013 at 11:52:18AM -0500, Felipe Contreras wrote:
We should free objects before leaving.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
sequencer.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index ab6f8a7..7eeae2f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -626,12 +626,15 @@ static int do_pick_commit(struct commit *commit,
struct replay_opts *opts)
rerere(opts-allow_rerere_auto);
} else {
int allow = allow_empty(opts, commit);
-if (allow 0)
-return allow;
+if (allow 0) {
+res = allow;
+goto leave;
+}
if (!opts-no_commit)
res = run_git_commit(defmsg, opts, allow);
}
+leave:
free_message(msg);
free(defmsg);
--
1.8.3.rc3.312.g47657de
Acked-by: Neil Horman nhor...@tuxdriver.com
This is better done without goto in general.
The other patch 2/2/ adds one more we need to exit from the middle
of the flow and makes it look handier to add an exit label here,
but it would be even better to express the logic of that patch as a
normal cascade of if/else if/..., which is small enough and we do
not need the leave: label.
Linux kernel developers would disagree. In C 'goto' is quite of then the only
sane option, and you can see 'goto' used in the Linux kernel all over the
place
for that reason.
In this particular case it also makes perfect sense.
I agree with Felipe here. Setting asside coding practice in other projects,
while its nice to follow coding convention in a project, a jump label just makes
more sense here. To not use it either requires you to duplicate the free
statements (undesireable), or to change the sense of theif clause here and nest
your if statements (makes for ugly reading).
It probably is better to fold this patch into the other one when it
is rerolled to correct the option name gotcha on the tin.
Why? This patch is standalone and fixes an issue that is independent of the
other patch. Why squash two patches that do *two* different things?
I agree here as well. This fixes a bug that has nothing to do with the other
patch, save for it being in the same C file. Fix them separately.
Anyway, I'll happily drop this patch if you want this memory leak to remain.
But then I'll do the same in the other patch.
This mantra of avodiing 'goto' is not helping anybody.
--
Felipe Contreras
--
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 1/2] sequencer: trivial fix
On Mon, May 27, 2013 at 11:52:18AM -0500, Felipe Contreras wrote:
We should free objects before leaving.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
sequencer.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index ab6f8a7..7eeae2f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -626,12 +626,15 @@ static int do_pick_commit(struct commit *commit, struct
replay_opts *opts)
rerere(opts-allow_rerere_auto);
} else {
int allow = allow_empty(opts, commit);
- if (allow 0)
- return allow;
+ if (allow 0) {
+ res = allow;
+ goto leave;
+ }
if (!opts-no_commit)
res = run_git_commit(defmsg, opts, allow);
}
+leave:
free_message(msg);
free(defmsg);
--
1.8.3.rc3.312.g47657de
Acked-by: Neil Horman nhor...@tuxdriver.com
--
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 2/2] cherry-pick: add --skip-commits option
On Mon, May 27, 2013 at 11:52:19AM -0500, Felipe Contreras wrote:
Pretty much what it says on the tin.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
Documentation/git-cherry-pick.txt | 3 +++
builtin/revert.c| 2 ++
sequencer.c | 5 -
sequencer.h | 1 +
t/t3508-cherry-pick-many-commits.sh | 13 +
5 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-cherry-pick.txt
b/Documentation/git-cherry-pick.txt
index c205d23..fccd936 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -129,6 +129,9 @@ effect to your index in a row.
redundant commits are ignored. This option overrides that behavior and
creates an empty commit object. Implies `--allow-empty`.
+--skip-empty::
+ Instead of failing, skip commits that are or become empty.
+
--strategy=strategy::
Use the given merge strategy. Should only be used once.
See the MERGE STRATEGIES section in linkgit:git-merge[1]
diff --git a/builtin/revert.c b/builtin/revert.c
index 0401fdb..0e5ce71 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -118,6 +118,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) {
@@ -127,6 +128,7 @@ static void parse_args(int argc, const char **argv,
struct replay_opts *opts)
OPT_BOOLEAN(0, allow-empty, opts-allow_empty,
N_(preserve initially empty commits)),
OPT_BOOLEAN(0, allow-empty-message,
opts-allow_empty_message, N_(allow commits with empty messages)),
OPT_BOOLEAN(0, keep-redundant-commits,
opts-keep_redundant_commits, N_(keep redundant, empty commits)),
+ OPT_BOOLEAN(0, skip-empty, opts-skip_empty,
N_(skip empty commits)),
OPT_END(),
};
I like the idea, but this option seems a bit awkward to me. At the very least
here, don't you now need to check for conflicts if --keep-redundant-commits and
skip-empty are both specified (as iirc git doens't see the difference between
empty commits and commits made empty by prior commits in the current history).
what if we merged the two options to an OPT_STRING, something like
--empty-commits=[keep|skip|ask]. The default currently is an impiled, since the
sequencer stops on an empty commit.
Neil
--
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: allowing multiple parallel sequencers
On Tue, Apr 02, 2013 at 03:06:51PM -0400, Neil Horman wrote: On Tue, Apr 02, 2013 at 11:06:28AM -0700, Junio C Hamano wrote: Neil Horman nhor...@tuxdriver.com writes: I've recently started looking into the possibility of having git support multiple in-progress sequencers, and wanted to solicit opinions for how best to do it The thoughts I had were: 1) A per branch sequence directory... 2) Augment the git-stash command... 3) A per branch working tree. That is how I would do this myself, anyway ;-) Not sure I completely follow. Are you suggesting that all untracked files, indexes and meta data in .git be saved during a branch switch? Thanks Neil -- Scratch that, after some digging I located the git-new-workdir script in the contrib directory, which does what we're talking about here. Thanks! Neil 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 -- 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
RFC: allowing multiple parallel sequencers
Hey- I've recently started looking into the possibility of having git support multiple in-progress sequencers, and wanted to solicit opinions for how best to do it. The use case is primarily for cherry-pick - i.e. I often have need to cherry pick a large set of commits to an older kernel, and I may do this for several work efforts in parallel. As such, it would be great if I could be able to have multiple sequencer states in progress that could be swapped out with one another. I know this could be done manually by saving the sequencer directory to another name and moving it back, but it would be really nice if there was something a bit more polished and integrated. The thoughts I had were: 1) A per branch sequence directory - when creating the sequence directory, prepend the name of the branch that you are on to the sequencer directory name, and lookup the sequencer using that prefix. This would fit quite well I think. It would allow us to maintain a sequencer per branch, and would be relatively easy to implement (we would need to have a generic function to return the current branch name, and some extra check to delete sequencers when branches are deleted, but nothing too difficult). It would be problematic however, in that working in detached head state would preclude the use of the mechanism (we could work around that by using a global sequencer in detached head mode, or we could add an option to specify a sequencer prefix). 2) Augment the git-stash command to save sequencer state optionally. This would be somewhat more difficult to implement I think (we would need to add .git/sequencer/* to the untracked file list when creating the stash commit). It would however allow arbitrary sequencers to be used on arbitrary branches (including detached head mode, if thats useful). So, before I went implementing, I wanted to solicit opinions here. Does anyone have any thoughts (including completely different directions to move in for this feature)? Thanks! Neil -- 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: allowing multiple parallel sequencers
On Tue, Apr 02, 2013 at 11:06:28AM -0700, Junio C Hamano wrote: Neil Horman nhor...@tuxdriver.com writes: I've recently started looking into the possibility of having git support multiple in-progress sequencers, and wanted to solicit opinions for how best to do it The thoughts I had were: 1) A per branch sequence directory... 2) Augment the git-stash command... 3) A per branch working tree. That is how I would do this myself, anyway ;-) Not sure I completely follow. Are you suggesting that all untracked files, indexes and meta data in .git be saved during a branch switch? Thanks Neil -- 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 --preserve-merges keeps empty merge commits
On Sat, Jan 12, 2013 at 03:46:01PM -0500, Phil Hord wrote:
Since 90e1818f9a (git-rebase: add keep_empty flag, 2012-04-20)
'git rebase --preserve-merges' fails to preserve empty merge commits
unless --keep-empty is also specified. Merge commits should be
preserved in order to preserve the structure of the rebased graph,
even if the merge commit does not introduce changes to the parent.
Teach rebase not to drop merge commits only because they are empty.
A special case which is not handled by this change is for a merge commit
whose parents are now the same commit because all the previous different
parents have been dropped as a result of this rebase or some previous
operation.
---
git-rebase--interactive.sh | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 44901d5..8ed7fcc 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -190,6 +190,11 @@ is_empty_commit() {
test $tree = $ptree
}
+is_merge_commit()
+{
+ git rev-parse --verify --quiet $1^2 /dev/null 21
+}
+
# Run command with GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
# GIT_AUTHOR_DATE exported from the current environment.
do_with_author () {
@@ -874,7 +879,7 @@ git rev-list $merges_option --pretty=oneline
--abbrev-commit \
while read -r shortsha1 rest
do
- if test -z $keep_empty is_empty_commit $shortsha1
+ if test -z $keep_empty is_empty_commit $shortsha1 !
is_merge_commit $shortsha1
then
comment_out=#
else
--
1.8.1.dirty
Seems reasonable
Acked-by: Neil Horman nhor...@tuxdriver.com
--
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
Bug/Enhancement: contrib/subtree should ship with manpage
(Not sure if you guys are using Google Code's Issue Tracker; if so, I originally filed this here: https://code.google.com/p/git-core/issues/detail?id=18 ) What steps will reproduce the problem? 1. Build git from source. 2. Download git-manpages-*. 3. Install both. 4. Attempt to build and install contrib/subtree. What is the expected output? What do you see instead? Expected: git-subtree goes into $PREFIX/libexec/, and git-subtree.1 goes into $PREFIX/share/man/man1/. Actual: git-subtree.1 fails to be generated because my system doesn't ship asciidoc and xmlto. What version of the product are you using? On what operating system? v1.7.12.2 and v1.8.0.rc0.18.gf84667d, on OS X 10.8.2. Please provide any additional information below. Prototype patch attached. subtree_manpage.patch Description: Binary data
Re: [PATCH v2] add tests for 'git rebase --keep-empty'
On Thu, Aug 09, 2012 at 08:39:51AM -0700, Martin von Zweigbergk wrote: Add test cases for 'git rebase --keep-empty' with and without an empty commit already in upstream. The empty commit that is about to be rebased should be kept in both cases. Signed-off-by: Martin von Zweigbergk martin.von.zweigbe...@gmail.com Acked-by: Neil Horman nhor...@tuxdriver.com --- Added another test for when the upstream already has an empty commit. The test case protects the current behavior; I just assume the current behavior is what we want. While writing the test case, I also noticed that an interrupted 'git rebase --keep-empty' can not be continued 'git rebase --continue', but instead needs 'git cherry-pick --continue'. I guess this shouldn't really be surprising given that it's implemented in terms of cherry-pick. This should be fixed once all the different kinds of rebase use the same way of finding the commits to rebase, so I wouldn't worry about fixing this specific problem right now. t/t3401-rebase-partial.sh | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh index 7f8693b..58f4823 100755 --- a/t/t3401-rebase-partial.sh +++ b/t/t3401-rebase-partial.sh @@ -47,7 +47,23 @@ test_expect_success 'rebase ignores empty commit' ' git commit --allow-empty -m empty test_commit D git rebase C - test $(git log --format=%s C..) = D + test $(git log --format=%s C..) = D +' + +test_expect_success 'rebase --keep-empty' ' + git reset --hard D + git rebase --keep-empty C + test $(git log --format=%s C..) = D +empty +' + +test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' ' + git reset --hard A + git commit --allow-empty -m also-empty + git rebase --keep-empty D + test $(git log --format=%s A..) = also-empty +D +empty ' test_done -- 1.7.11.1.104.ge7b44f1 -- 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] add test for 'git rebase --keep-empty'
On Wed, Aug 08, 2012 at 09:48:18AM -0700, Martin von Zweigbergk wrote: Signed-off-by: Martin von Zweigbergk martin.von.zweigbe...@gmail.com --- While trying to use patch-id instead of --ignore-if-in-upstream/--cherry-pick/cherry/etc, I noticed that patch-id ignores empty patches and I was surprised that tests still pass. This test case would be useful to protect --keep-empty. t/t3401-rebase-partial.sh | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh index 7f8693b..b89b512 100755 --- a/t/t3401-rebase-partial.sh +++ b/t/t3401-rebase-partial.sh @@ -47,7 +47,14 @@ test_expect_success 'rebase ignores empty commit' ' git commit --allow-empty -m empty test_commit D git rebase C - test $(git log --format=%s C..) = D + test $(git log --format=%s C..) = D +' + +test_expect_success 'rebase --keep-empty' ' + git reset --hard D + git rebase --keep-empty C + test $(git log --format=%s C..) = D +empty ' test_done -- 1.7.11.1.104.ge7b44f1 Acked-by: Neil Horman nhor...@tuxdriver.com -- 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
On Thu, Aug 02, 2012 at 11:38:51AM +0100, Chris Webb wrote: 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 Sorry for the late response, but I just pulled back into town. 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? Neil -- 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
On Mon, Aug 06, 2012 at 12:00:16PM +0100, Chris Webb wrote: 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. Ok, then Acked-by: Neil Horman nhor...@tuxdriver.com -- 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
On Wed, Aug 01, 2012 at 10:52:34AM -0700, Junio C Hamano wrote:
Chris Webb ch...@arachsys.com writes:
[summary: this, when 59a8fde does not have any commit log message,
refuses to commit]
Thanks for CC'ing me on this. I'm on vacation currently, but will look at this
in detail as soon as I'm back next week
Neil
$ 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?
A few random thoughts.
- Any Porcelain commands that implement the sequencing workflow, if
they know what message to use when they internally run commit
without allowing the user to edit the message, share the same
issue.
- We generally try to encourage users to describe commits, and
commits with empty log messages are strongly frowned upon.
In that sense, one could argue that cherry-pick did the right
thing when it gave control back to you upon seeing an empty
message. The user is given a chance to fix the commit by running
git commit at that point to give it a descriptive message.
- These Porcelain programs, however, work from existing commits,
and the reason why git commit invoked by them may be stopped
due to empty log message is because the original commits had
empty log message to begin with. The user must have done so on
purpose (e.g. by using commit --allow-empty-message).
In that sense, it is likely that the user will simply choose to
run git commit --allow-empty-message, even if given a chance by
cherry-pick to correct the empty log message. This is a
counter-point to the give the user a chance to fix above.
We _might_ not be adding much value to the system by giving the
control back to the user.
- We had a similar discussion on what should happen when one step
in cherry-pick results in the same tree as the commit the
'pick' builds on (i.e. an empty change). The situation is a bit
different from yours, because unlike the log message, an empty
change can result by either (1) the original was an empty change,
or (2) the change picked was already present in the updated base.
We added --keep-redundant-commits and --allow-empty options
to underlying cherry-pick to support this distinction.
We may want to follow suit by triggering your change above only
when cherry-pick --allow-empty-message was given. This is
siding with the give the user a chance to fix viewpoint to
choose the default, and giving the users a way to overriding it.
- Regarding the choice of default between --allow-empty-message
vs --no-allow-empty-message, one could argue that the best
choice of the default depends on the Porcelain command.
- A non-range cherry-pick (e.g. cherry-pick A B C) is a strong
hint from the user that the user wants to replay the specific
commits that are named on the command line. This fact may
favor the user must have done so on purpose viewpoint over
give the user a chance to fix viewpoint; defaulting to
--allow-empty-message (and --allow-empty, and perhaps
--keep-redundant-commits) might be more convenient for a
non-range cherry-pick.
- A range cherry-pick (e.g. cherry-pick A..B) and rebase -i,
on the other hand, are primarily used to rebuild (and reorder
in the case of rebase -i) the history to clean it up, which
may favor give the user a chance to fix, i.e. defaulting not
to enable --allow-empty-anything might be more convenient for
a sequencing operation over a range in general.
But from the bigger UI consistency point of view, it would be
chaotic to change the default of some options for a single
command depending on the nature of the operand, so I would
recommend against going this route, and pick one view between
give the user a chance to fix or the user must have done so on
purpose and apply it consistently.
My
Re: [PATCH v8 4/4] git-rebase: add keep_empty flag
On Wed, Jul 18, 2012 at 09:10:06AM +0200, Johannes Sixt wrote:
Am 7/18/2012 8:20, schrieb Martin von Zweigbergk:
On Fri, Apr 20, 2012 at 7:36 AM, Neil Horman nhor...@tuxdriver.com wrote:
pick_one () {
ff=--ff
+
case $1 in -n) sha1=$2; ff= ;; *) sha1=$1 ;; esac
case $force_rebase in '') ;; ?*) ff= ;; esac
output git rev-parse --verify $sha1 || die Invalid commit name:
$sha1
+
+ if is_empty_commit $sha1
+ then
+ empty_args=--allow-empty
+ fi
+
test -d $rewritten
pick_one_preserving_merges $@ return
- output git cherry-pick $ff $@
+ output git cherry-pick $empty_args $ff $@
The is_empty_commit check seems to mean that if $sha1 is an empty
commit, we pass the --allow-empty option to cherry-pick. If it's not
empty, we don't. The word allow in allow-empty suggests that even
if the commit is not empty, cherry-pick would not mind. So, can we
always pass allow-empty to cherry-pick (i.e. even if the commit to
pick is not empty)?
I don't think so. If the commit is not empty, but all its changes are
already in HEAD, then it will become empty when cherry-picked to HEAD.
In such a case, we usually do not want to record an empty commit, but stop
rebase to give to user a chance to deal with the situation.
-- Hannes
Yes, this is the meaning. Allow was used in the sense of a filter, in that we
are allowing an empty commit to make it into the history, whereas a rebase or
cherry-pick would normally exclude it.
Neil
--
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 2/7] git-rebase--interactive.sh: extract function for adding pick line
On Wed, Jul 18, 2012 at 12:27:30AM -0700, Martin von Zweigbergk wrote:
Extract the code that adds a possibly commented-out pick line to the
todo file. This lets us reuse it more easily later.
---
git-rebase--interactive.sh | 21 -
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index bef7bc0..fa722b6 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -828,23 +828,26 @@ else
revisions=$onto...$orig_head
shortrevisions=$shorthead
fi
-git rev-list $merges_option --pretty=oneline --abbrev-commit \
- --abbrev=7 --reverse --left-right --topo-order \
- $revisions | \
- sed -n s/^//p |
-while read -r shortsha1 rest
-do
- if test -z $keep_empty is_empty_commit $shortsha1
+add_pick_line () {
+ if test -z $keep_empty is_empty_commit $1
then
comment_out=#
else
comment_out=
fi
+ printf '%s\n' ${comment_out}pick $1 $2 $todo
+}
+git rev-list $merges_option --pretty=oneline --abbrev-commit \
+ --abbrev=7 --reverse --left-right --topo-order \
+ $revisions | \
+ sed -n s/^//p |
+while read -r shortsha1 rest
+do
if test t != $preserve_merges
then
- printf '%s\n' ${comment_out}pick $shortsha1 $rest $todo
+ add_pick_line $shortsha1 $rest
else
sha1=$(git rev-parse $shortsha1)
if test -z $rebase_root
@@ -863,7 +866,7 @@ do
if test f = $preserve
then
touch $rewritten/$sha1
- printf '%s\n' ${comment_out}pick $shortsha1 $rest
$todo
+ add_pick_line $shortsha1 $rest
fi
fi
done
--
1.7.11.1.104.ge7b44f1
Thanks!
Acked-by: Neil Horman nhor...@tuxdriver.com
--
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
