Merge branch 'ab/retire-scripted-add-p'

Finally retire the scripted "git add -p/-i" implementation and have
everybody use the one reimplemented in C.

* ab/retire-scripted-add-p:
  docs & comments: replace mentions of "git-add--interactive.perl"
  add API: remove run_add_interactive() wrapper function
  add: remove "add.interactive.useBuiltin" & Perl "git add--interactive"

.gitignore

@@ -14,7 +14,6 @@
 /bin-wrappers/
 /git
 /git-add
-/git-add--interactive
 /git-am
 /git-annotate
 /git-apply

Documentation/config/add.txt

@@ -7,6 +7,7 @@ add.ignore-errors (deprecated)::
 	variables.
 
 add.interactive.useBuiltin::
-	Set to `false` to fall back to the original Perl implementation of
+	Unused configuration variable. Used in Git versions v2.25.0 to
-	the interactive version of linkgit:git-add[1] instead of the built-in
+	v2.36.0 to enable the built-in version of linkgit:git-add[1]'s
-	version. Is `true` by default.
+	interactive mode, which then became the default in Git
+	versions v2.37.0 to v2.39.0.

Documentation/git-add.txt

@@ -274,7 +274,7 @@ status::
 ------------
               staged     unstaged path
      1:       binary      nothing foo.png
-     2:     +403/-35        +1/-1 git-add--interactive.perl
+     2:     +403/-35        +1/-1 add-interactive.c
 ------------
 +
 It shows that foo.png has differences from HEAD (but that is
@@ -282,7 +282,7 @@ binary so line count cannot be shown) and there is no
 difference between indexed copy and the working tree
 version (if the working tree version were also different,
 'binary' would have been shown in place of 'nothing').  The
-other file, git-add{litdd}interactive.perl, has 403 lines added
+other file, add-interactive.c, has 403 lines added
 and 35 lines deleted if you commit what is in the index, but
 working tree file has further modifications (one addition and
 one deletion).
@@ -303,7 +303,7 @@ like this:
 ------------
            staged     unstaged path
   1:       binary      nothing foo.png
-* 2:     +403/-35        +1/-1 git-add--interactive.perl
+* 2:     +403/-35        +1/-1 add-interactive.c
 ------------
 +
 To remove selection, prefix the input with `-`

INSTALL

@@ -120,7 +120,7 @@ Issues of note:
 	  for everyday use (e.g. "bisect", "request-pull").
 
 	- "Perl" version 5.8 or later is needed to use some of the
-	  features (e.g. preparing a partial commit using "git add -i/-p",
+	  features (e.g. sending patches using "git send-email",
 	  interacting with svn repositories with "git svn").  If you can
 	  live without these, use NO_PERL.  Note that recent releases of
 	  Redhat/Fedora are reported to ship Perl binary package with some

Makefile

@@ -708,7 +708,6 @@ SCRIPT_LIB += git-mergetool--lib
 SCRIPT_LIB += git-sh-i18n
 SCRIPT_LIB += git-sh-setup
 
-SCRIPT_PERL += git-add--interactive.perl
 SCRIPT_PERL += git-archimport.perl
 SCRIPT_PERL += git-cvsexportcommit.perl
 SCRIPT_PERL += git-cvsimport.perl

builtin/add.c

@@ -238,58 +238,14 @@ static int refresh(int verbose, const struct pathspec *pathspec)
 	return ret;
 }
 
-int run_add_interactive(const char *revision, const char *patch_mode,
-			const struct pathspec *pathspec)
-{
-	int i;
-	struct child_process cmd = CHILD_PROCESS_INIT;
-	int use_builtin_add_i =
-		git_env_bool("GIT_TEST_ADD_I_USE_BUILTIN", -1);
-
-	if (use_builtin_add_i < 0 &&
-	    git_config_get_bool("add.interactive.usebuiltin",
-				&use_builtin_add_i))
-		use_builtin_add_i = 1;
-
-	if (use_builtin_add_i != 0) {
-		enum add_p_mode mode;
-
-		if (!patch_mode)
-			return !!run_add_i(the_repository, pathspec);
-
-		if (!strcmp(patch_mode, "--patch"))
-			mode = ADD_P_ADD;
-		else if (!strcmp(patch_mode, "--patch=stash"))
-			mode = ADD_P_STASH;
-		else if (!strcmp(patch_mode, "--patch=reset"))
-			mode = ADD_P_RESET;
-		else if (!strcmp(patch_mode, "--patch=checkout"))
-			mode = ADD_P_CHECKOUT;
-		else if (!strcmp(patch_mode, "--patch=worktree"))
-			mode = ADD_P_WORKTREE;
-		else
-			die("'%s' not supported", patch_mode);
-
-		return !!run_add_p(the_repository, mode, revision, pathspec);
-	}
-
-	strvec_push(&cmd.args, "add--interactive");
-	if (patch_mode)
-		strvec_push(&cmd.args, patch_mode);
-	if (revision)
-		strvec_push(&cmd.args, revision);
-	strvec_push(&cmd.args, "--");
-	for (i = 0; i < pathspec->nr; i++)
-		/* pass original pathspec, to be re-parsed */
-		strvec_push(&cmd.args, pathspec->items[i].original);
-
-	cmd.git_cmd = 1;
-	return run_command(&cmd);
-}
-
 int interactive_add(const char **argv, const char *prefix, int patch)
 {
 	struct pathspec pathspec;
+	int unused;
+
+	if (!git_config_get_bool("add.interactive.usebuiltin", &unused))
+		warning(_("the add.interactive.useBuiltin setting has been removed!\n"
+			  "See its entry in 'git help config' for details."));
 
 	parse_pathspec(&pathspec, 0,
 		       PATHSPEC_PREFER_FULL |
@@ -297,9 +253,10 @@ int interactive_add(const char **argv, const char *prefix, int patch)
 		       PATHSPEC_PREFIX_ORIGIN,
 		       prefix, argv);
 
-	return run_add_interactive(NULL,
+	if (patch)
-				   patch ? "--patch" : NULL,
+		return !!run_add_p(the_repository, ADD_P_ADD, NULL, &pathspec);
-				   &pathspec);
+	else
+		return !!run_add_i(the_repository, &pathspec);
 }
 
 static int edit_patch(int argc, const char **argv, const char *prefix)

builtin/checkout.c

@@ -29,6 +29,7 @@
 #include "xdiff-interface.h"
 #include "entry.h"
 #include "parallel-checkout.h"
+#include "add-interactive.h"
 
 static const char * const checkout_usage[] = {
 	N_("git checkout [<options>] <branch>"),
@@ -499,7 +500,7 @@ static int checkout_paths(const struct checkout_opts *opts,
 		    "--merge", "--conflict", "--staged");
 
 	if (opts->patch_mode) {
-		const char *patch_mode;
+		enum add_p_mode patch_mode;
 		const char *rev = new_branch_info->name;
 		char rev_oid[GIT_MAX_HEXSZ + 1];
 
@@ -517,15 +518,16 @@ static int checkout_paths(const struct checkout_opts *opts,
 			rev = oid_to_hex_r(rev_oid, &new_branch_info->commit->object.oid);
 
 		if (opts->checkout_index && opts->checkout_worktree)
-			patch_mode = "--patch=checkout";
+			patch_mode = ADD_P_CHECKOUT;
 		else if (opts->checkout_index && !opts->checkout_worktree)
-			patch_mode = "--patch=reset";
+			patch_mode = ADD_P_RESET;
 		else if (!opts->checkout_index && opts->checkout_worktree)
-			patch_mode = "--patch=worktree";
+			patch_mode = ADD_P_WORKTREE;
 		else
 			BUG("either flag must have been set, worktree=%d, index=%d",
 			    opts->checkout_worktree, opts->checkout_index);
-		return run_add_interactive(rev, patch_mode, &opts->pathspec);
+		return !!run_add_p(the_repository, patch_mode, rev,
+				   &opts->pathspec);
 	}
 
 	repo_hold_locked_index(the_repository, &lock_file, LOCK_DIE_ON_ERROR);

builtin/clean.c

@@ -560,7 +560,7 @@ static int parse_choice(struct menu_stuff *menu_stuff,
 
 /*
  * Implement a git-add-interactive compatible UI, which is borrowed
- * from git-add--interactive.perl.
+ * from add-interactive.c.
  *
  * Return value:
  *

builtin/reset.c

@@ -26,6 +26,7 @@
 #include "submodule.h"
 #include "submodule-config.h"
 #include "dir.h"
+#include "add-interactive.h"
 
 #define REFRESH_INDEX_DELAY_WARNING_IN_MS (2 * 1000)
 
@@ -390,7 +391,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		if (reset_type != NONE)
 			die(_("options '%s' and '%s' cannot be used together"), "--patch", "--{hard,mixed,soft}");
 		trace2_cmd_mode("patch-interactive");
-		return run_add_interactive(rev, "--patch=reset", &pathspec);
+		return !!run_add_p(the_repository, ADD_P_RESET, rev,
+				   &pathspec);
 	}
 
 	/* git reset tree [--] paths... can be used to

builtin/stash.c

@@ -18,6 +18,7 @@
 #include "diffcore.h"
 #include "exec-cmd.h"
 #include "reflog.h"
+#include "add-interactive.h"
 
 #define INCLUDE_ALL_FILES 2
 
@@ -1229,7 +1230,7 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps,
 	old_index_env = xstrdup_or_null(getenv(INDEX_ENVIRONMENT));
 	setenv(INDEX_ENVIRONMENT, the_repository->index_file, 1);
 
-	ret = run_add_interactive(NULL, "--patch=stash", ps);
+	ret = !!run_add_p(the_repository, ADD_P_STASH, NULL, ps);
 
 	the_repository->index_file = old_repo_index_file;
 	if (old_index_env && *old_index_env)

ci/run-build-and-tests.sh

@@ -26,7 +26,6 @@ linux-TEST-vars)
 	export GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=1
 	export GIT_TEST_MULTI_PACK_INDEX=1
 	export GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP=1
-	export GIT_TEST_ADD_I_USE_BUILTIN=0
 	export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
 	export GIT_TEST_WRITE_REV_INDEX=1
 	export GIT_TEST_CHECKOUT_WORKERS=2

commit.h

@@ -274,8 +274,6 @@ struct ref;
 int for_each_commit_graft(each_commit_graft_fn, void *);
 
 int interactive_add(const char **argv, const char *prefix, int patch);
-int run_add_interactive(const char *revision, const char *patch_mode,
-			const struct pathspec *pathspec);
 
 struct commit_extra_header {
 	struct commit_extra_header *next;

pathspec.c

@@ -545,7 +545,7 @@ static void NORETURN unsupported_magic(const char *pattern,
 	}
 	/*
 	 * We may want to substitute "this command" with a command
-	 * name. E.g. when add--interactive dies when running
+	 * name. E.g. when "git add -p" or "git add -i" dies when running
 	 * "checkout -p"
 	 */
 	die(_("%s: pathspec magic not supported by this command: %s"),

t/README

@@ -449,10 +449,6 @@ the --sparse command-line argument.
 GIT_TEST_PRELOAD_INDEX=<boolean> exercises the preload-index code path
 by overriding the minimum number of cache entries required per thread.
 
-GIT_TEST_ADD_I_USE_BUILTIN=<boolean>, when false, disables the
-built-in version of git add -i. See 'add.interactive.useBuiltin' in
-git-config(1).
-
 GIT_TEST_INDEX_THREADS=<n> enables exercising the multi-threaded loading
 of the index for the whole test suite by bypassing the default number of
 cache entries and thread minimums. Setting this to 1 will make the

t/t2016-checkout-patch.sh

@@ -4,12 +4,6 @@ test_description='git checkout --patch'
 
 . ./lib-patch-mode.sh
 
-if ! test_have_prereq ADD_I_USE_BUILTIN && ! test_have_prereq PERL
-then
-	skip_all='skipping interactive add tests, PERL not set'
-	test_done
-fi
-
 test_expect_success 'setup' '
 	mkdir dir &&
 	echo parent > dir/foo &&

t/t3701-add-interactive.sh

@@ -7,7 +7,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-terminal.sh
 
-if test_have_prereq !ADD_I_USE_BUILTIN,!PERL
+if test_have_prereq !PERL
 then
 	skip_all='skipping add -i (scripted) tests, perl not available'
 	test_done
@@ -46,6 +46,21 @@ force_color () {
 	)
 }
 
+test_expect_success 'warn about add.interactive.useBuiltin' '
+	cat >expect <<-\EOF &&
+	warning: the add.interactive.useBuiltin setting has been removed!
+	See its entry in '\''git help config'\'' for details.
+	No changes.
+	EOF
+
+	for v in = =true =false
+	do
+		git -c "add.interactive.useBuiltin$v" add -p >out 2>actual &&
+		test_must_be_empty out &&
+		test_cmp expect actual || return 1
+	done
+'
+
 test_expect_success 'setup (initial)' '
 	echo content >file &&
 	git add file &&
@@ -547,15 +562,7 @@ test_expect_success 'split hunk "add -p (edit)"' '
 	! grep "^+15" actual
 '
 
-test_expect_success 'setup ADD_I_USE_BUILTIN check' '
+test_expect_success 'split hunk "add -p (no, yes, edit)"' '
-	result=success &&
-	if ! test_have_prereq ADD_I_USE_BUILTIN
-	then
-		result=failure
-	fi
-'
-
-test_expect_$result 'split hunk "add -p (no, yes, edit)"' '
 	test_write_lines 5 10 20 21 30 31 40 50 60 >test &&
 	git reset &&
 	# test sequence is s(plit), n(o), y(es), e(dit)
@@ -579,7 +586,7 @@ test_expect_success 'split hunk with incomplete line at end' '
 	test_must_fail git grep --cached before
 '
 
-test_expect_$result 'edit, adding lines to the first hunk' '
+test_expect_success 'edit, adding lines to the first hunk' '
 	test_write_lines 10 11 20 30 40 50 51 60 >test &&
 	git reset &&
 	tr _ " " >patch <<-EOF &&

t/t6132-pathspec-exclude.sh

@@ -293,11 +293,7 @@ test_expect_success 'add with all negative' '
 	test_cmp expect actual
 '
 
-test_lazy_prereq ADD_I_USE_BUILTIN_OR_PERL '
+test_expect_success 'add -p with all negative' '
-	test_have_prereq ADD_I_USE_BUILTIN || test_have_prereq PERL
-'
-
-test_expect_success ADD_I_USE_BUILTIN_OR_PERL 'add -p with all negative' '
 	H=$(git rev-parse HEAD) &&
 	git reset --hard $H &&
 	git clean -f &&

t/test-lib.sh

@@ -1937,10 +1937,6 @@ test_lazy_prereq SHA1 '
 	esac
 '
 
-test_lazy_prereq ADD_I_USE_BUILTIN '
-	test_bool_env GIT_TEST_ADD_I_USE_BUILTIN true
-'
-
 # Ensure that no test accidentally triggers a Git command
 # that runs the actual maintenance scheduler, affecting a user's
 # system permanently.