From: Elijah Newren <firstname.lastname@example.org> To: Johannes Schindelin <Johannes.Schindelin@gmx.de> Cc: "Junio C Hamano" <email@example.com>, "Ævar Arnfjörð" <firstname.lastname@example.org>, email@example.com, "Git Mailing List" <firstname.lastname@example.org>, "Stefan Beller" <email@example.com> Subject: Re: A rebase regression in Git 2.18.0 Date: Fri, 31 Aug 2018 12:37:59 -0700 Message-ID: <CABPp-BEJ-JPa8g8G1ELsjqf3EDxe+aCBdwERBi5hr5ukrDjJJg@mail.gmail.com> (raw) In-Reply-To: <nycvar.QRO.firstname.lastname@example.org> Hi Dscho, On Fri, Aug 31, 2018 at 3:12 AM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote: > On Thu, 30 Aug 2018, Elijah Newren wrote: > > On Tue, Aug 28, 2018 at 9:58 AM Junio C Hamano <email@example.com> wrote: > > > Elijah Newren <firstname.lastname@example.org> writes: ... > > > I'd say this is the only practical solution, before you deprecate > > > the "pipe format-patch output to am -3" style of "git rebase" (and > > > optionally replace with something else). > > > > I posted a patch a while back to add an --am flag to "git rebase", > > make "--am" be implied by options which are still am-specific > > (--whitespace, --committer-date-is-author-date, and -C), and change > > --merge to be the default. > > Didn't you also post a patch to fold --merge into the --interactive > backend? What's your current state of thinking about this? Yes. I updated it once or twice, but it had conflicts with the GSoC projects each time, so I decided to just hold off on it a bit longer. I'm still planning to resubmit this once the GSoC projects merge down. > As to switching from --am as the default: I still think that --am has > serious speed advantages over --merge (or for that matter, --interactive). > I have no numbers to back that up, though, and I am currently really busy > with working on the CI, so I won't be able to measure these numbers, > either... Yep, we talked about this before and you mentioned that the rewrite in C should bring some performance improvements, and we agreed that merge-recursive is probably the next issue performance-wise. I think it's at least worth measuring what the approximate performance differences are with the rewrite of rebase in C, and posting an RFC with that info. If the answer comes back that we need to do more optimization before we switch the default, that's fine. > Also please note: I converted the `am` backend to pure C (it is waiting at > https://github.com/gitgitgadget/git/pull/24, to be submitted after the > v2.19.0 RC period). Switching to `--merge` as the default would force me > to convert that backend, too ;-) Not if git-rebase--merge is deleted and --merge is implemented on top of the interactive backend as an implicitly_interactive case. In fact, that's probably the simplest way to "convert" that backend to C. Anyway, since I plan to submit that change first, we should be good. > > I'll post it as an RFC again after the various rebase-rewrite series > > have settled and merged down...along with my other rebase cleanups > > that I was waiting on to avoid conflicts with GSoC stuff. > > Thanks for waiting! Please note that I am interested, yet I will be on > vacation for a couple of weeks in September. Don't let that stop you, > though! Enjoy your vacation! > > > The whole point of "am -3" is to do _better_ than just "patch" with > > > minimum amount of information available on the pre- and post- image > > > blobs, without knowing the remainder of the tree that the patch did > > > not touch. It is not surprising that the heuristics that look at > > > the unchanging part of the tree to infer renames that may or may not > > > exist guesses incorrectly, either with false positive or negative. > > > In the context of "rebase", we always have all the trees that are > > > involved. We should be able to do better than "am -3". > > Right. I think that Elijah's right, and --merge is that "do better" > solution. Cool, good to see others seem to agree on the direction I'd like to see things move.
prev parent reply other threads:[~2018-08-31 19:38 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-08-28 12:27 Nikolay Kasyanov 2018-08-28 13:17 ` Ævar Arnfjörð Bjarmason 2018-08-28 13:33 ` Johannes Schindelin 2018-08-28 13:46 ` Nikolay Kasyanov 2018-08-28 15:35 ` Elijah Newren 2018-08-28 16:58 ` Junio C Hamano 2018-08-29 7:06 ` [PATCH 0/3] Turn off directory rename detection in am -3 Elijah Newren 2018-08-29 7:06 ` [PATCH 1/3] t3401: add another directory rename testcase for rebase and am Elijah Newren 2018-08-29 22:12 ` Junio C Hamano 2018-08-29 23:47 ` Elijah Newren 2018-08-30 16:01 ` Junio C Hamano 2018-08-30 16:26 ` Elijah Newren 2018-08-29 7:06 ` [PATCH 2/3] merge-recursive: add ability to turn off directory rename detection Elijah Newren 2018-08-29 12:54 ` Johannes Schindelin 2018-08-29 23:00 ` Elijah Newren 2018-08-29 7:06 ` [PATCH 3/3] am: avoid directory rename detection when calling recursive merge machinery Elijah Newren 2018-08-29 12:51 ` Johannes Schindelin 2018-08-30 16:41 ` A rebase regression in Git 2.18.0 Elijah Newren 2018-08-31 10:11 ` Johannes Schindelin 2018-08-31 19:37 ` Elijah Newren [this message]
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CABPp-BEJ-JPa8g8G1ELsjqf3EDxe+aCBdwERBi5hr5ukrDjJJg@mail.gmail.com \ --email@example.com \ --cc=Johannes.Schindelin@gmx.de \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: A rebase regression in Git 2.18.0' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
email@example.com 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 \ firstname.lastname@example.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://ou63pmih66umazou.onion/inbox.comp.version-control.git nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git nntp://hjrcffqmbrq6wope.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