[PATCH] rebase: mark the C reimplementation as an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 26, 2018, 06:10 UTC
Message-ID: <xmqq36roz7ve.fsf_-_@gitster-ct.c.googlers.com>
In-Reply-To: <xmqq36rq2cp0.fsf@gitster-ct.c.googlers.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 26 quoted lines

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> 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@evledraar.gmail.com/ ]
>
> 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@pobox.com>
---
 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