Re: [PATCH v2 3/8] cherry-pick: add --skip-empty option
On Wed, Jun 5, 2013 at 1:13 PM, Junio C Hamano wrote: > Felipe Contreras writes: > >> On Tue, Jun 4, 2013 at 1:30 PM, Junio C Hamano wrote: >>> Felipe Contreras writes: >>> You didn't answer, what happens when you run --skip-empty and --allow-empty? >>> >>> I'll answer to a slightly different question: What should happen? >>> >>> I think it should error out, because --allow-empty is about >>> "allowing empty commits to be preserved", and --skip-empty is about >>> "skipping empty commits (as it says on the package)". >> >> Exactly, so they are related after all. However, --allow-empty says this: >> >> "By default, cherry-picking an empty commit will fail," > > OK, that is a very good point. Especially because by the time > reader reaches this new description, --allow-empty has already > mentioned this, we can just be brief and it is sufficient to say > "Instead of failing," upfront. > >> In fact, it might make sense to change the default in v2.0 to >> --empty-commits=skip. If it makes sense in 'git rebase', why doesn't >> it in 'git cherry-pick'? > > I think the primary reason behind the "Why are you picking a no-op? > Let me stop to make sure you didn't make a mistake." is because > cherry-pick and revert have long been operations for a single commit > explicitly given by the user, not bunch of commits in a range. Yeah, but this has changed already. > These two operating modes are conceptually very different, even when > we consider scripted use. A script that feeds one commit at a time > has a chance to do patch equivalence or user-interactive filtering > on its own, and would be helped by the "are you sure you meant to > record that no-op?" check if it filtered in a wrong way (e.g. the > user specified an empty commit by mistake). One that feeds a range, > on the other hand, relies on or at least expects cherry-pick to have > such smart, and a smarter cherry-pick could select what to pick from > the given range. How would a script know that a single pick ends up as a no-op? It can't know what is the reason the cherry-pick failed. The only way to know for sure is to check the last commit, and for that we don't need the last cherry-pick to fail. > In the longer term, especially if we envision to move large part of > logic to prepare the sequencer insn list from rebase to cherry-pick, > a ranged cherry-pick should learn a way to filter the range with > patch equivalence, for example. Once that happens, the behaviour > would become inconsistent at the end-user level if we stopped at a > commit only because it was not exactly patch equivalent to another > one that is already on the branch we are cherry-picking to, but > ended up to be a no-op, while we did not stop at another commit > because patch equivalence filtered it out beforehand to skip it. > Skipping a no-op by default would remove that inconsistency. > > So in that sense, one could argue that it may be a bugfix to skip > commits that become no-op when replayed, when picking a range of > commits (but not a single one). And I do not think you would need > to wait until 2.0 for such a change. Right. But first we need to agree that failing an empty cherry-pick serves no purpose. -- 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 v2 3/8] cherry-pick: add --skip-empty option
Felipe Contreras writes: > On Tue, Jun 4, 2013 at 1:30 PM, Junio C Hamano wrote: >> Felipe Contreras writes: >> >>> You didn't answer, what happens when you run --skip-empty and --allow-empty? >> >> I'll answer to a slightly different question: What should happen? >> >> I think it should error out, because --allow-empty is about >> "allowing empty commits to be preserved", and --skip-empty is about >> "skipping empty commits (as it says on the package)". > > Exactly, so they are related after all. However, --allow-empty says this: > > "By default, cherry-picking an empty commit will fail," OK, that is a very good point. Especially because by the time reader reaches this new description, --allow-empty has already mentioned this, we can just be brief and it is sufficient to say "Instead of failing," upfront. > In fact, it might make sense to change the default in v2.0 to > --empty-commits=skip. If it makes sense in 'git rebase', why doesn't > it in 'git cherry-pick'? I think the primary reason behind the "Why are you picking a no-op? Let me stop to make sure you didn't make a mistake." is because cherry-pick and revert have long been operations for a single commit explicitly given by the user, not bunch of commits in a range. These two operating modes are conceptually very different, even when we consider scripted use. A script that feeds one commit at a time has a chance to do patch equivalence or user-interactive filtering on its own, and would be helped by the "are you sure you meant to record that no-op?" check if it filtered in a wrong way (e.g. the user specified an empty commit by mistake). One that feeds a range, on the other hand, relies on or at least expects cherry-pick to have such smart, and a smarter cherry-pick could select what to pick from the given range. In the longer term, especially if we envision to move large part of logic to prepare the sequencer insn list from rebase to cherry-pick, a ranged cherry-pick should learn a way to filter the range with patch equivalence, for example. Once that happens, the behaviour would become inconsistent at the end-user level if we stopped at a commit only because it was not exactly patch equivalent to another one that is already on the branch we are cherry-picking to, but ended up to be a no-op, while we did not stop at another commit because patch equivalence filtered it out beforehand to skip it. Skipping a no-op by default would remove that inconsistency. So in that sense, one could argue that it may be a bugfix to skip commits that become no-op when replayed, when picking a range of commits (but not a single one). And I do not think you would need to wait until 2.0 for such a change. -- 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 v2 3/8] cherry-pick: add --skip-empty option
On Tue, Jun 4, 2013 at 1:30 PM, Junio C Hamano wrote: > Felipe Contreras writes: > >> You didn't answer, what happens when you run --skip-empty and --allow-empty? > > I'll answer to a slightly different question: What should happen? > > I think it should error out, because --allow-empty is about > "allowing empty commits to be preserved", and --skip-empty is about > "skipping empty commits (as it says on the package)". Exactly, so they are related after all. However, --allow-empty says this: "By default, cherry-picking an empty commit will fail," Should we change that too if we introduce --skip-empty? I don't think so. I think it makes more sense to have a new --empty-commits=[keep|skip|fail] option, so we don't have to document how each option affects the other, and what is the default. Or rather; the option documents that. In fact, it might make sense to change the default in v2.0 to --empty-commits=skip. If it makes sense in 'git rebase', why doesn't it in 'git cherry-pick'? -- 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 v2 3/8] cherry-pick: add --skip-empty option
Felipe Contreras writes: > You didn't answer, what happens when you run --skip-empty and --allow-empty? I'll answer to a slightly different question: What should happen? I think it should error out, because --allow-empty is about "allowing empty commits to be preserved", and --skip-empty is about "skipping empty commits (as it says on the package)". -- 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 v2 3/8] cherry-pick: add --skip-empty option
On Tue, Jun 4, 2013 at 12:35 PM, Junio C Hamano wrote: > Felipe Contreras writes: > >>> I think just "Skip commits that are or become empty" without saying >>> "Instead of failing," is fine, too. >> >> I think "Instead of failing" makes perfect sense, because it's not our >> job to describe what other options do,... >> ... >> simply explain what this option >> do. > > Which means "Skip commits" and nothing else. Saying "Instead of > failing" explains what would happen if you ran the command without > any option, Which *BY FAR* the most widely use-case of cherry-pick. What? 99% of the time? > which is entirely irrelevant, It's totally and completely relevant. It couldn't be more relevant. > We share the same "the --skip-empty option does not have anything to > do with the --allow-empty option, and we do not have to say anything > about what happens when the command is run with that unrelated > option". You didn't answer, what happens when you run --skip-empty and --allow-empty? -- 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 v2 3/8] cherry-pick: add --skip-empty option
Felipe Contreras writes: >> I think just "Skip commits that are or become empty" without saying >> "Instead of failing," is fine, too. > > I think "Instead of failing" makes perfect sense, because it's not our > job to describe what other options do,... > ... > simply explain what this option > do. Which means "Skip commits" and nothing else. Saying "Instead of failing" explains what would happen if you ran the command without any option, which is entirely irrelevant, just like the case when you ran the command without an unrelated option --allow-empty. We share the same "the --skip-empty option does not have anything to do with the --allow-empty option, and we do not have to say anything about what happens when the command is run with that unrelated option". But we are viewing it from a different angle. -- 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 v2 3/8] cherry-pick: add --skip-empty option
On Mon, Jun 3, 2013 at 4:45 PM, Junio C Hamano wrote: > Felipe Contreras writes: > >> On Mon, Jun 3, 2013 at 1:40 PM, Junio C Hamano wrote: >>> Felipe Contreras writes: >>> Pretty much what it says on the tin. Signed-off-by: Felipe Contreras --- Documentation/git-cherry-pick.txt | 3 +++ builtin/revert.c| 2 ++ sequencer.c | 6 ++ sequencer.h | 1 + t/t3508-cherry-pick-many-commits.sh | 13 + 5 files changed, 25 insertions(+) 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. >>> >>> Not quite sure. Is this "instead of recording an empty commit," >>> (which may or may not fail depending on the allow-empty settings)? >> >> We are explaining --skip-empty, not --allow-empty. > > Exactly. That is why I found the "Instead of failing" questionable. > It is very easy to read the above as "commits that are or become > empty usually causes the command to fail, and this is a way to cause > it not to fail.". > > It is true that > > cherry-pick A > > when A becomes empty, dies. But > > cherry-pick --allow-empty A > > when A becomes empty, does not fail, but still does create an empty > commit. A large difference with --skip-empty, which applies to use > scenario different from --allow-empty was meant to address, is that > > cherry-pick --skip-empty A > > when A becomes empty, does not fail and does not create an empty > commit, either. We are not explaining --allow-empty. What happens when you do --skip-empty --allow-empty? Somebody suggested a new option, so we could do --foo-empty=skip,allow to clarify that. > I think just "Skip commits that are or become empty" without saying > "Instead of failing," is fine, too. I think "Instead of failing" makes perfect sense, because it's not our job to describe what other options do, simply explain what this option do. If the user is interested in other options, he can read them in the help for those. -- 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 v2 3/8] cherry-pick: add --skip-empty option
On Mon, Jun 3, 2013 at 11:10 PM, Felipe Contreras wrote: > On Mon, Jun 3, 2013 at 1:40 PM, Junio C Hamano wrote: >> Felipe Contreras writes: >> >>> Pretty much what it says on the tin. >>> >>> Signed-off-by: Felipe Contreras >>> --- >>> Documentation/git-cherry-pick.txt | 3 +++ >>> builtin/revert.c| 2 ++ >>> sequencer.c | 6 ++ >>> sequencer.h | 1 + >>> t/t3508-cherry-pick-many-commits.sh | 13 + >>> 5 files changed, 25 insertions(+) >>> >>> 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. >> >> Not quite sure. Is this "instead of recording an empty commit," >> (which may or may not fail depending on the allow-empty settings)? > > We are explaining --skip-empty, not --allow-empty. By the way, I guess both options are incompatible ? I think it would be nice to fail if both are specified. Antoine, -- 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 v2 3/8] cherry-pick: add --skip-empty option
Felipe Contreras writes: > On Mon, Jun 3, 2013 at 1:40 PM, Junio C Hamano wrote: >> Felipe Contreras writes: >> >>> Pretty much what it says on the tin. >>> >>> Signed-off-by: Felipe Contreras >>> --- >>> Documentation/git-cherry-pick.txt | 3 +++ >>> builtin/revert.c| 2 ++ >>> sequencer.c | 6 ++ >>> sequencer.h | 1 + >>> t/t3508-cherry-pick-many-commits.sh | 13 + >>> 5 files changed, 25 insertions(+) >>> >>> 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. >> >> Not quite sure. Is this "instead of recording an empty commit," >> (which may or may not fail depending on the allow-empty settings)? > > We are explaining --skip-empty, not --allow-empty. Exactly. That is why I found the "Instead of failing" questionable. It is very easy to read the above as "commits that are or become empty usually causes the command to fail, and this is a way to cause it not to fail.". It is true that cherry-pick A when A becomes empty, dies. But cherry-pick --allow-empty A when A becomes empty, does not fail, but still does create an empty commit. A large difference with --skip-empty, which applies to use scenario different from --allow-empty was meant to address, is that cherry-pick --skip-empty A when A becomes empty, does not fail and does not create an empty commit, either. I think just "Skip commits that are or become empty" without saying "Instead of failing," is fine, too. -- 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 v2 3/8] cherry-pick: add --skip-empty option
On Mon, Jun 3, 2013 at 1:40 PM, Junio C Hamano wrote: > Felipe Contreras writes: > >> Pretty much what it says on the tin. >> >> Signed-off-by: Felipe Contreras >> --- >> Documentation/git-cherry-pick.txt | 3 +++ >> builtin/revert.c| 2 ++ >> sequencer.c | 6 ++ >> sequencer.h | 1 + >> t/t3508-cherry-pick-many-commits.sh | 13 + >> 5 files changed, 25 insertions(+) >> >> 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. > > Not quite sure. Is this "instead of recording an empty commit," > (which may or may not fail depending on the allow-empty settings)? We are explaining --skip-empty, not --allow-empty. -- 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 v2 3/8] cherry-pick: add --skip-empty option
Junio C Hamano writes: > Felipe Contreras writes: > >> Pretty much what it says on the tin. >> >> Signed-off-by: Felipe Contreras >> --- >> Documentation/git-cherry-pick.txt | 3 +++ >> builtin/revert.c| 2 ++ >> sequencer.c | 6 ++ >> sequencer.h | 1 + >> t/t3508-cherry-pick-many-commits.sh | 13 + >> 5 files changed, 25 insertions(+) >> >> 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. > > Not quite sure. Is this "instead of recording an empty commit," > (which may or may not fail depending on the allow-empty settings)? > > If that is what this patch is meant to do, I think the change makes > sense. Also what I noticed while looking at 4-5/8. Wouldn't "revert --skip-empty A..B" make sense as well? -- 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 v2 3/8] cherry-pick: add --skip-empty option
Felipe Contreras writes:
> Pretty much what it says on the tin.
>
> Signed-off-by: Felipe Contreras
> ---
> Documentation/git-cherry-pick.txt | 3 +++
> builtin/revert.c| 2 ++
> sequencer.c | 6 ++
> sequencer.h | 1 +
> t/t3508-cherry-pick-many-commits.sh | 13 +
> 5 files changed, 25 insertions(+)
>
> 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.
Not quite sure. Is this "instead of recording an empty commit,"
(which may or may not fail depending on the allow-empty settings)?
If that is what this patch is meant to do, I think the change makes
sense.
> --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(),
> };
> if (parse_options_concat(options, ARRAY_SIZE(options),
> cp_extra))
> diff --git a/sequencer.c b/sequencer.c
> index f7be7d8..d3c7d72 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -627,6 +627,12 @@ static int do_pick_commit(struct commit *commit, struct
> replay_opts *opts)
> goto leave;
> }
>
> + if (opts->skip_empty && is_index_unchanged() == 1) {
> + warning(_("skipping %s... %s"),
> + find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
> + msg.subject);
> + goto leave;
> + }
> allow = allow_empty(opts, commit);
> if (allow < 0) {
> res = allow;
> diff --git a/sequencer.h b/sequencer.h
> index 1fc22dc..3b04844 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -34,6 +34,7 @@ struct replay_opts {
> int allow_empty;
> int allow_empty_message;
> int keep_redundant_commits;
> + int skip_empty;
>
> int mainline;
>
> diff --git a/t/t3508-cherry-pick-many-commits.sh
> b/t/t3508-cherry-pick-many-commits.sh
> index 19c99d7..3dc19c6 100755
> --- a/t/t3508-cherry-pick-many-commits.sh
> +++ b/t/t3508-cherry-pick-many-commits.sh
> @@ -187,4 +187,17 @@ test_expect_success 'cherry-pick --stdin works' '
> check_head_differs_from fourth
> '
>
> +test_expect_success 'cherry-pick skip empty' '
> + git clean -fxd &&
> + git checkout -b empty fourth &&
> + git commit --allow-empty -m empty &&
> + test_commit ontop &&
> + git checkout -f master &&
> + git reset --hard fourth &&
> + git cherry-pick --skip-empty fourth..empty &&
> + echo ontop > expected &&
> + git log --format=%s fourth..HEAD > actual
> + test_cmp expected actual
> +'
> +
> test_done
--
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 3/8] cherry-pick: add --skip-empty option
Pretty much what it says on the tin.
Signed-off-by: Felipe Contreras
---
Documentation/git-cherry-pick.txt | 3 +++
builtin/revert.c| 2 ++
sequencer.c | 6 ++
sequencer.h | 1 +
t/t3508-cherry-pick-many-commits.sh | 13 +
5 files changed, 25 insertions(+)
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=::
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(),
};
if (parse_options_concat(options, ARRAY_SIZE(options),
cp_extra))
diff --git a/sequencer.c b/sequencer.c
index f7be7d8..d3c7d72 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -627,6 +627,12 @@ static int do_pick_commit(struct commit *commit, struct
replay_opts *opts)
goto leave;
}
+ if (opts->skip_empty && is_index_unchanged() == 1) {
+ warning(_("skipping %s... %s"),
+ find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
+ msg.subject);
+ goto leave;
+ }
allow = allow_empty(opts, commit);
if (allow < 0) {
res = allow;
diff --git a/sequencer.h b/sequencer.h
index 1fc22dc..3b04844 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -34,6 +34,7 @@ struct replay_opts {
int allow_empty;
int allow_empty_message;
int keep_redundant_commits;
+ int skip_empty;
int mainline;
diff --git a/t/t3508-cherry-pick-many-commits.sh
b/t/t3508-cherry-pick-many-commits.sh
index 19c99d7..3dc19c6 100755
--- a/t/t3508-cherry-pick-many-commits.sh
+++ b/t/t3508-cherry-pick-many-commits.sh
@@ -187,4 +187,17 @@ test_expect_success 'cherry-pick --stdin works' '
check_head_differs_from fourth
'
+test_expect_success 'cherry-pick skip empty' '
+ git clean -fxd &&
+ git checkout -b empty fourth &&
+ git commit --allow-empty -m empty &&
+ test_commit ontop &&
+ git checkout -f master &&
+ git reset --hard fourth &&
+ git cherry-pick --skip-empty fourth..empty &&
+ echo ontop > expected &&
+ git log --format=%s fourth..HEAD > actual
+ test_cmp expected actual
+'
+
test_done
--
1.8.3.rc3.312.g47657de
--
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
