git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/2] rebase --recreate-merges --keep-empty: don't prune empty
@ 2018-03-20 10:11 Phillip Wood
  2018-03-20 10:11 ` [PATCH 1/2] add failing test for rebase --recreate-merges --keep-empty Phillip Wood
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Phillip Wood @ 2018-03-20 10:11 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Johannes Schindelin, Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

These patches apply on top of js/rebase-recreate-merge. They extend
the --keep-empty fix from maint [1] to work with --recreate-merges.

[1] https://public-inbox.org/git/20180320100315.15261-3-phillip.wood@talktalk.net/T/#u

Phillip Wood (2):
  add failing test for rebase --recreate-merges --keep-empty
  rebase --recreate-merges --keep-empty: don't prune empty commits

 sequencer.c                       | 30 ++++++++++++++++--------------
 t/t3421-rebase-topology-linear.sh |  3 ++-
 2 files changed, 18 insertions(+), 15 deletions(-)

-- 
2.16.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] add failing test for rebase --recreate-merges --keep-empty
  2018-03-20 10:11 [PATCH 0/2] rebase --recreate-merges --keep-empty: don't prune empty Phillip Wood
@ 2018-03-20 10:11 ` Phillip Wood
  2018-03-20 10:11 ` [PATCH 2/2] rebase --recreate-merges --keep-empty: don't prune empty commits Phillip Wood
  2018-03-20 15:42 ` [PATCH 0/2] rebase --recreate-merges --keep-empty: don't prune empty Johannes Schindelin
  2 siblings, 0 replies; 9+ messages in thread
From: Phillip Wood @ 2018-03-20 10:11 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Johannes Schindelin, Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

If there are empty commits on the left hand side of $upstream...HEAD
then the empty commits on the right hand side that we want to keep are
being pruned. This will be fixed in the next commit.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 t/t3421-rebase-topology-linear.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
index 68fe2003ef..cb7f176f1d 100755
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -217,6 +217,7 @@ test_run_rebase success ''
 test_run_rebase failure -m
 test_run_rebase failure -i
 test_run_rebase failure -p
+test_run_rebase failure --recreate-merges
 
 #       m
 #      /
-- 
2.16.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/2] rebase --recreate-merges --keep-empty: don't prune empty commits
  2018-03-20 10:11 [PATCH 0/2] rebase --recreate-merges --keep-empty: don't prune empty Phillip Wood
  2018-03-20 10:11 ` [PATCH 1/2] add failing test for rebase --recreate-merges --keep-empty Phillip Wood
@ 2018-03-20 10:11 ` Phillip Wood
  2018-03-20 15:38   ` Johannes Schindelin
  2018-03-20 15:42 ` [PATCH 0/2] rebase --recreate-merges --keep-empty: don't prune empty Johannes Schindelin
  2 siblings, 1 reply; 9+ messages in thread
From: Phillip Wood @ 2018-03-20 10:11 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Johannes Schindelin, Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

If there are empty commits on the left hand side of $upstream...HEAD
then the empty commits on the right hand side that we want to keep are
pruned because they are marked as cherry-picks. Fix this by keeping
the commits that are empty or are not marked as cherry-picks.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 sequencer.c                       | 30 ++++++++++++++++--------------
 t/t3421-rebase-topology-linear.sh |  4 ++--
 2 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index d8cc63dbe4..da87c390ed 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2975,14 +2975,15 @@ static int make_script_with_merges(struct pretty_print_context *pp,
 	 */
 	while ((commit = get_revision(revs))) {
 		struct commit_list *to_merge;
-		int is_octopus;
+		int is_octopus, is_empty;
 		const char *p1, *p2;
 		struct object_id *oid;
 
 		tail = &commit_list_insert(commit, tail)->next;
 		oidset_insert(&interesting, &commit->object.oid);
 
-		if ((commit->object.flags & PATCHSAME))
+		is_empty = is_original_commit_empty(commit);
+		if (!is_empty && (commit->object.flags & PATCHSAME))
 			continue;
 
 		strbuf_reset(&oneline);
@@ -2992,7 +2993,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,
 		if (!to_merge) {
 			/* non-merge commit: easy case */
 			strbuf_reset(&buf);
-			if (!keep_empty && is_original_commit_empty(commit))
+			if (!keep_empty && is_empty)
 				strbuf_addf(&buf, "%c ", comment_line_char);
 			strbuf_addf(&buf, "%s %s %s", cmd_pick,
 				    oid_to_hex(&commit->object.oid),
@@ -3172,12 +3173,9 @@ int sequencer_make_script(FILE *out, int argc, const char **argv,
 
 	init_revisions(&revs, NULL);
 	revs.verbose_header = 1;
-	if (recreate_merges)
-		revs.cherry_mark = 1;
-	else {
+	if (!recreate_merges)
 		revs.max_parents = 1;
-		revs.cherry_pick = 1;
-	}
+	revs.cherry_mark = 1;
 	revs.limited = 1;
 	revs.reverse = 1;
 	revs.right_only = 1;
@@ -3205,14 +3203,18 @@ int sequencer_make_script(FILE *out, int argc, const char **argv,
 		return make_script_with_merges(&pp, &revs, out, flags);
 
 	while ((commit = get_revision(&revs))) {
+		int is_empty  = is_original_commit_empty(commit);
+
 		strbuf_reset(&buf);
-		if (!keep_empty && is_original_commit_empty(commit))
+		if (!keep_empty && is_empty)
 			strbuf_addf(&buf, "%c ", comment_line_char);
-		strbuf_addf(&buf, "%s %s ", insn,
-			    oid_to_hex(&commit->object.oid));
-		pretty_print_commit(&pp, commit, &buf);
-		strbuf_addch(&buf, '\n');
-		fputs(buf.buf, out);
+		if (is_empty || !(commit->object.flags & PATCHSAME)) {
+			strbuf_addf(&buf, "%s %s ", insn,
+				    oid_to_hex(&commit->object.oid));
+			pretty_print_commit(&pp, commit, &buf);
+			strbuf_addch(&buf, '\n');
+			fputs(buf.buf, out);
+		}
 	}
 	strbuf_release(&buf);
 	return 0;
diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
index cb7f176f1d..7384010075 100755
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -215,9 +215,9 @@ test_run_rebase () {
 }
 test_run_rebase success ''
 test_run_rebase failure -m
-test_run_rebase failure -i
+test_run_rebase success -i
 test_run_rebase failure -p
-test_run_rebase failure --recreate-merges
+test_run_rebase success --recreate-merges
 
 #       m
 #      /
-- 
2.16.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] rebase --recreate-merges --keep-empty: don't prune empty commits
  2018-03-20 10:11 ` [PATCH 2/2] rebase --recreate-merges --keep-empty: don't prune empty commits Phillip Wood
@ 2018-03-20 15:38   ` Johannes Schindelin
  0 siblings, 0 replies; 9+ messages in thread
From: Johannes Schindelin @ 2018-03-20 15:38 UTC (permalink / raw)
  To: Phillip Wood; +Cc: Git Mailing List, Junio C Hamano

Hi Phillip,

On Tue, 20 Mar 2018, Phillip Wood wrote:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
> 
> If there are empty commits on the left hand side of $upstream...HEAD
> then the empty commits on the right hand side that we want to keep are
> pruned because they are marked as cherry-picks. Fix this by keeping
> the commits that are empty or are not marked as cherry-picks.

Thank you!

> @@ -3172,12 +3173,9 @@ int sequencer_make_script(FILE *out, int argc, const char **argv,
>  
>  	init_revisions(&revs, NULL);
>  	revs.verbose_header = 1;
> -	if (recreate_merges)
> -		revs.cherry_mark = 1;
> -	else {
> +	if (!recreate_merges)
>  		revs.max_parents = 1;
> -		revs.cherry_pick = 1;
> -	}
> +	revs.cherry_mark = 1;
>  	revs.limited = 1;
>  	revs.reverse = 1;
>  	revs.right_only = 1;

Yeah, this looks ugly. I'd rather have your patch series applied first and
then rebase my --recreate-merges patch series on top.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] rebase --recreate-merges --keep-empty: don't prune empty
  2018-03-20 10:11 [PATCH 0/2] rebase --recreate-merges --keep-empty: don't prune empty Phillip Wood
  2018-03-20 10:11 ` [PATCH 1/2] add failing test for rebase --recreate-merges --keep-empty Phillip Wood
  2018-03-20 10:11 ` [PATCH 2/2] rebase --recreate-merges --keep-empty: don't prune empty commits Phillip Wood
@ 2018-03-20 15:42 ` Johannes Schindelin
  2018-03-20 18:40   ` Phillip Wood
  2 siblings, 1 reply; 9+ messages in thread
From: Johannes Schindelin @ 2018-03-20 15:42 UTC (permalink / raw)
  To: Phillip Wood; +Cc: Git Mailing List, Junio C Hamano

Hi Phillip,

On Tue, 20 Mar 2018, Phillip Wood wrote:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
> 
> These patches apply on top of js/rebase-recreate-merge. They extend
> the --keep-empty fix from maint [1] to work with --recreate-merges.

As indicated in another reply, I'd rather rebase the --recreate-merges
patches on top of your --keep-empty patch series. This obviously means
that I would fold essentially all of your 2/2 changes into my
"rebase-helper --make-script: introduce a flag to recreate merges"

The 1/2 (with s/failure/success/g) would then be added to the
--recreate-merges patch series at the end.

Would that be okay with you?

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] rebase --recreate-merges --keep-empty: don't prune empty
  2018-03-20 15:42 ` [PATCH 0/2] rebase --recreate-merges --keep-empty: don't prune empty Johannes Schindelin
@ 2018-03-20 18:40   ` Phillip Wood
  2018-03-20 19:32     ` Junio C Hamano
  0 siblings, 1 reply; 9+ messages in thread
From: Phillip Wood @ 2018-03-20 18:40 UTC (permalink / raw)
  To: Johannes Schindelin, Phillip Wood; +Cc: Git Mailing List, Junio C Hamano

On 20/03/18 15:42, Johannes Schindelin wrote:
> Hi Phillip,
> 
> On Tue, 20 Mar 2018, Phillip Wood wrote:
> 
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> These patches apply on top of js/rebase-recreate-merge. They extend
>> the --keep-empty fix from maint [1] to work with --recreate-merges.
> 
> As indicated in another reply, I'd rather rebase the --recreate-merges
> patches on top of your --keep-empty patch series. This obviously means
> that I would fold essentially all of your 2/2 changes into my
> "rebase-helper --make-script: introduce a flag to recreate merges"
> 
> The 1/2 (with s/failure/success/g) would then be added to the
> --recreate-merges patch series at the end.
> 
> Would that be okay with you?

Yes, that's fine, it would give a clearer history

Best Wishes

Phillip

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] rebase --recreate-merges --keep-empty: don't prune empty
  2018-03-20 18:40   ` Phillip Wood
@ 2018-03-20 19:32     ` Junio C Hamano
  2018-03-22 11:03       ` Phillip Wood
  0 siblings, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2018-03-20 19:32 UTC (permalink / raw)
  To: Phillip Wood; +Cc: Johannes Schindelin, Phillip Wood, Git Mailing List

Phillip Wood <phillip.wood@talktalk.net> writes:

> On 20/03/18 15:42, Johannes Schindelin wrote:
> ...
>> As indicated in another reply, I'd rather rebase the --recreate-merges
>> patches on top of your --keep-empty patch series. This obviously means
>> that I would fold essentially all of your 2/2 changes into my
>> "rebase-helper --make-script: introduce a flag to recreate merges"
>> 
>> The 1/2 (with s/failure/success/g) would then be added to the
>> --recreate-merges patch series at the end.
>> 
>> Would that be okay with you?
>
> Yes, that's fine, it would give a clearer history

With or without the above plan, what we saw from you were a bit
messy to queue.  The --keep-empty fix series is based on 'maint',
while the --signoff series depends on changes that happened to
sequencer between 'maint' and 'master', but yet depends on the
former.

In what I'll be pushing out at the end of today's integration run,
I'll have two topics organized this way:

 - pw/rebase-keep-empty-fixes: built by applying the three
   '--keep-empty' patches on top of 'maint'.

 - pw/rebase-signoff: built by first merging the above to 0f57f731
   ("Merge branch 'pw/sequencer-in-process-commit'", 2018-02-13) and
   then applying "rebase --signoff" series.

Also, I'll revert merge of Dscho's recreate-merges topic to 'next';
doing so would probably have to invalidate a few evil merges I've
been carrying to resolve conflicts between it and bc/object-id
topic, so today's integration cycle may take a bit longer than
usual.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] rebase --recreate-merges --keep-empty: don't prune empty
  2018-03-20 19:32     ` Junio C Hamano
@ 2018-03-22 11:03       ` Phillip Wood
  2018-03-22 16:48         ` Junio C Hamano
  0 siblings, 1 reply; 9+ messages in thread
From: Phillip Wood @ 2018-03-22 11:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Phillip Wood, Git Mailing List

On 20/03/18 19:32, Junio C Hamano wrote:
> Phillip Wood <phillip.wood@talktalk.net> writes:
> 
>> On 20/03/18 15:42, Johannes Schindelin wrote:
>> ...
>>> As indicated in another reply, I'd rather rebase the --recreate-merges
>>> patches on top of your --keep-empty patch series. This obviously means
>>> that I would fold essentially all of your 2/2 changes into my
>>> "rebase-helper --make-script: introduce a flag to recreate merges"
>>>
>>> The 1/2 (with s/failure/success/g) would then be added to the
>>> --recreate-merges patch series at the end.
>>>
>>> Would that be okay with you?
>>
>> Yes, that's fine, it would give a clearer history
> 
> With or without the above plan, what we saw from you were a bit
> messy to queue.  The --keep-empty fix series is based on 'maint',
> while the --signoff series depends on changes that happened to
> sequencer between 'maint' and 'master', but yet depends on the
> former.

Yes, that is awkward and unfortunate but the idea behind splitting them
into two separate series was to have a single set of bug fixes in the
history. The feature needed to be based on master, so if I'd had the bug
fixes in the same series you'd of had to cherry-pick them to maint which
would break branch/tag --contains. I'm not sure if that is a better option.

Best Wishes

Phillip

> In what I'll be pushing out at the end of today's integration run,
> I'll have two topics organized this way:
> 
>  - pw/rebase-keep-empty-fixes: built by applying the three
>    '--keep-empty' patches on top of 'maint'.
> 
>  - pw/rebase-signoff: built by first merging the above to 0f57f731
>    ("Merge branch 'pw/sequencer-in-process-commit'", 2018-02-13) and
>    then applying "rebase --signoff" series.
> 
> Also, I'll revert merge of Dscho's recreate-merges topic to 'next';
> doing so would probably have to invalidate a few evil merges I've
> been carrying to resolve conflicts between it and bc/object-id
> topic, so today's integration cycle may take a bit longer than
> usual.
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] rebase --recreate-merges --keep-empty: don't prune empty
  2018-03-22 11:03       ` Phillip Wood
@ 2018-03-22 16:48         ` Junio C Hamano
  0 siblings, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2018-03-22 16:48 UTC (permalink / raw)
  To: Phillip Wood; +Cc: Johannes Schindelin, Phillip Wood, Git Mailing List

Phillip Wood <phillip.wood@talktalk.net> writes:

> On 20/03/18 19:32, Junio C Hamano wrote:
>
>> With or without the above plan, what we saw from you were a bit
>> messy to queue.  The --keep-empty fix series is based on 'maint',
>> while the --signoff series depends on changes that happened to
>> sequencer between 'maint' and 'master', but yet depends on the
>> former.
>
> Yes, that is awkward and unfortunate but the idea behind splitting them
> into two separate series was to have a single set of bug fixes in the
> history. The feature needed to be based on master, so if I'd had the bug
> fixes in the same series you'd of had to cherry-pick them to maint which
> would break branch/tag --contains. I'm not sure if that is a better option.

I said "a bit messy" but that was a statement of a fact, not a
complaint.  Sometimes, we cannot avoid that necessary solutions to
real-life problems must be messy.

I still think what you sent was the best organization, given the
constraints ;-).

Thanks.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-03-22 16:48 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-20 10:11 [PATCH 0/2] rebase --recreate-merges --keep-empty: don't prune empty Phillip Wood
2018-03-20 10:11 ` [PATCH 1/2] add failing test for rebase --recreate-merges --keep-empty Phillip Wood
2018-03-20 10:11 ` [PATCH 2/2] rebase --recreate-merges --keep-empty: don't prune empty commits Phillip Wood
2018-03-20 15:38   ` Johannes Schindelin
2018-03-20 15:42 ` [PATCH 0/2] rebase --recreate-merges --keep-empty: don't prune empty Johannes Schindelin
2018-03-20 18:40   ` Phillip Wood
2018-03-20 19:32     ` Junio C Hamano
2018-03-22 11:03       ` Phillip Wood
2018-03-22 16:48         ` Junio C Hamano

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.version-control.git
	nntp://ie5yzdi7fg72h7s4sdcztq5evakq23rdt33mfyfcddc5u3ndnw24ogqd.onion/inbox.comp.version-control.git
	nntp://4uok3hntl7oi7b4uf4rtfwefqeexfzil2w6kgk2jn5z2f764irre7byd.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git