Junio C Hamano <gitster@xxxxxxxxx> writes: > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > >>> * "git rebase" and "git rebase -i" have been reimplemented in C. >> >> Here's another regression in the C version (and rc1),... >> I wasn't trying to stress test rebase. I was just wanting to rebase a >> history I was about to force-push after cleaning it up, hardly an >> obscure use-case. So [repeat last transmission in >> https://public-inbox.org/git/87y39w1wc2.fsf@xxxxxxxxxxxxxxxxxxx/ ] > > which, to those who are reading from sidelines: > > 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). > > I am fine with the proposed flip, but I'll have to see the extent of > damage this late in the game so that I won't miss anything. In > addition to the one-liner below, we'd need to update the quoted > release notes entry, and possibly adjust some tests (even though the > "reimplementation" ought to be bug-to-bug compatible, it may not be). So, in a more concrete form, what you want to see is something like this in -rc2 and later? -- >8 -- Subject: [PATCH] rebase: mark the C reimplementation as an experimental opt-in feature It turns out to be a bit too early to unleash the reimplementation to the general public. Let's rewrite some documentation and make it an opt-in feature. Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> --- Documentation/config/rebase.txt | 16 ++++++---------- builtin/rebase.c | 2 +- t/README | 4 ++-- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/Documentation/config/rebase.txt b/Documentation/config/rebase.txt index f079bf6b7e..af12623151 100644 --- a/Documentation/config/rebase.txt +++ b/Documentation/config/rebase.txt @@ -1,16 +1,12 @@ rebase.useBuiltin:: - Set to `false` to use the legacy shellscript implementation of - linkgit:git-rebase[1]. Is `true` by default, which means use - the built-in rewrite of it in C. + Set to `true` to use the experimental reimplementation of + linkgit:git-rebase[1] in C. Defaults to `false`. + The C rewrite is first included with Git version 2.20. This option -serves an an escape hatch to re-enable the legacy version in case any -bugs are found in the rewrite. This option and the shellscript version -of linkgit:git-rebase[1] will be removed in some future release. -+ -If you find some reason to set this option to `false` other than -one-off testing you should report the behavior difference as a bug in -git. +allows early adopters to opt into the experimental version to find +bugs in the rewritten version. This option and the shellscript version +of linkgit:git-rebase[1] will be removed in some future release once +the reimplementation becomes more stable. rebase.stat:: Whether to show a diffstat of what changed upstream since the last diff --git a/builtin/rebase.c b/builtin/rebase.c index 5b3e5baec8..19ad97b177 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -59,7 +59,7 @@ static int use_builtin_rebase(void) cp.git_cmd = 1; if (capture_command(&cp, &out, 6)) { strbuf_release(&out); - return 1; + return 0; } strbuf_trim(&out); diff --git a/t/README b/t/README index 28711cc508..7e925e5fea 100644 --- a/t/README +++ b/t/README @@ -345,8 +345,8 @@ for the index version specified. Can be set to any valid version GIT_TEST_PRELOAD_INDEX=<boolean> exercises the preload-index code path by overriding the minimum number of cache entries required per thread. -GIT_TEST_REBASE_USE_BUILTIN=<boolean>, when false, disables the -builtin version of git-rebase. See 'rebase.useBuiltin' in +GIT_TEST_REBASE_USE_BUILTIN=<boolean>, when true, forces the use of +builtin version of git-rebase in the test. See 'rebase.useBuiltin' in git-config(1). GIT_TEST_INDEX_THREADS=<n> enables exercising the multi-threaded loading -- 2.20.0-rc1