* [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 related [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 related [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
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).