Re: rebase-in-C stability for 2.20
Ævar Arnfjörð Bjarmason writes: > According to Junio's calendar we're now 2 days from 2.20-rc0. We have > the js/rebase-autostash-detach-fix bug I reported sitting in "pu" still, > and then this. > > I see we still have rebase.useBuiltin in the code as an escape hatch, > but it's undocumented. > > Given that we're still finding regressions bugs in the rebase-in-C > version should we be considering reverting 5541bd5b8f ("rebase: default > to using the builtin rebase", 2018-08-08)? > > I love the feature, but fear that the current list of known regressions > serve as a canary for a larger list which we'd discover if we held off > for another major release (and would re-enable rebase.useBuiltin=true in > master right after 2.20 is out the door). > > But maybe I'm being overly paranoid. What do those more familiar with > this think? I was hoping that having it early in GfW 2.19 would have smoked out all the remaining issues, but it seems that those building from the source and testing have usage patterns different enough to find more issues. This is a normal part of the development process, and hopefully the remaining bugs are minor and can be flushed out in the -rc testing period---this kind of thing is the whole reason why we code-freeze and test rcs. It unfortunately is too late to depend on the rebase.useBuiltin as an escape hatch, as 'master', 'next', or anywhere else had the "rebase and rebase -i in C" series merged without it set to false and used in any seriousness. Quite honestly, I think we can trust the rest of the "rebase and rebase -i in C" series much more than its "use the scripted one even though we have built-in one" part. So if we have to seriously consider holding back, we may be better off ripping the whole thing out. I do not offhand know how involved such a reversion would be, though, and I am *NOT* looking forward to having do such a major surgery right before the release.
Re: rebase-in-C stability for 2.20
On Tue, Nov 13, 2018 at 1:52 PM Ævar Arnfjörð Bjarmason wrote: > According to Junio's calendar we're now 2 days from 2.20-rc0. We have > the js/rebase-autostash-detach-fix bug I reported sitting in "pu" still, > and then this. > > I see we still have rebase.useBuiltin in the code as an escape hatch, > but it's undocumented. > > Given that we're still finding regressions bugs in the rebase-in-C > version should we be considering reverting 5541bd5b8f ("rebase: default > to using the builtin rebase", 2018-08-08)? That date feels slightly misleading; it has a commit date of Oct 11, and only merged to master less than two weeks ago. Given how big the two different rebase in C series were, I'd expect a couple small things to fall out after it hit master, which is what appears to be happening. > I love the feature, but fear that the current list of known regressions > serve as a canary for a larger list which we'd discover if we held off > for another major release (and would re-enable rebase.useBuiltin=true in > master right after 2.20 is out the door). > > But maybe I'm being overly paranoid. What do those more familiar with > this think? I probably don't qualify as more familar, but giving my $0.02 anyway... I'm happy that setting rebase.useBuiltin=false by default exists an escape hatch if things don't settle down as we get closer to the release, but I'd rather wait until further into the RC cycle before going that route, personally.
Re: rebase-in-C stability for 2.20
> But maybe I'm being overly paranoid. What do those more familiar with > this think? I am not too worried, * as rebase is a main porcelain, that is even hard to use in a script. so any failures are not deep down in some automation, but when found exposed quickly (and hopefully reported). * 5541bd5b8f was merged to next a month ago; internally we distribute the next branch to Googlers (on a weekly basis) and we have not had any bug reports regarding rebase. (Maybe our environment is too strict for the wide range of bugs reported) * Johannes reported that the rebase is used in GfW https://public-inbox.org/git/nycvar.qro.7.76.6.1808241320540...@tvgsbejvaqbjf.bet/ https://github.com/git-for-windows/git/pull/1800 and from my cursory reading it is part of 2.19.windows, which has a large user base. > (and would re-enable rebase.useBuiltin=true in > master right after 2.20 is out the door). That would be fine with me as well, but I'd rather document rebase.useBuiltin instead of flip-flopping the switch around the release. Have there been any fixes that are only in the C version (has the shell version already bitrotted)? Stefan