All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ben Peart <Ben.Peart@microsoft.com>
To: "git@vger.kernel.org" <git@vger.kernel.org>
Cc: Ben Peart <Ben.Peart@microsoft.com>,
	"newren@gmail.com" <newren@gmail.com>,
	"gitster@pobox.com" <gitster@pobox.com>,
	"pclouds@gmail.com" <pclouds@gmail.com>,
	"vmiklos@frugalware.org" <vmiklos@frugalware.org>,
	Alejandro Pauly <alpauly@microsoft.com>,
	"Johannes.Schindelin@gmx.de" <Johannes.Schindelin@gmx.de>,
	"eckhard.s.maass@googlemail.com" <eckhard.s.maass@googlemail.com>,
	Ben Peart <Ben.Peart@microsoft.com>
Subject: [PATCH v3] add status config and command line options for rename detection
Date: Fri, 11 May 2018 12:56:39 +0000	[thread overview]
Message-ID: <20180511125623.6068-1-benpeart@microsoft.com> (raw)
In-Reply-To: <20180509144213.18032-1-benpeart@microsoft.com>

After performing a merge that has conflicts git status will, by default,
attempt to detect renames which causes many objects to be examined.  In a
virtualized repo, those objects do not exist locally so the rename logic
triggers them to be fetched from the server. This results in the status call
taking hours to complete on very large repos vs seconds with this patch.

Add a new config status.renames setting to enable turning off rename detection
during status and commit.  This setting will default to the value of
diff.renames.

Add a new config status.renamelimit setting to to enable bounding the time
spent finding out inexact renames during status and commit.  This setting will
default to the value of diff.renamelimit.

Add --no-renames command line option to status that enables overriding the
config setting from the command line. Add --find-renames[=<n>] command line
option to status that enables detecting renames and optionally setting the
similarity index.

This patch depends on em/status-rename-config

Original-Patch-by: Alejandro Pauly <alpauly@microsoft.com>
Signed-off-by: Ben Peart <Ben.Peart@microsoft.com>
---

Notes:
    Base Ref: commit dc6b1d92ca9c0c538daa244e3910bb8b2a50d959 (em/status-rename-config)
    Web-Diff: https://github.com/benpeart/git/commit/5bac43610b
    Checkout: git fetch https://github.com/benpeart/git status-renames-v3 && git checkout 5bac43610b
    
    ### Interdiff (v2..v3):
    
    diff --git a/Documentation/config.txt b/Documentation/config.txt
    index 9c8eca05b1..88884f1ead 100644
    --- a/Documentation/config.txt
    +++ b/Documentation/config.txt
    @@ -3120,14 +3120,16 @@ status.displayCommentPrefix::
     	Defaults to false.
    
     status.renameLimit::
    -	The number of files to consider when performing rename detection;
    -	if not specified, defaults to the value of diff.renameLimit.
    +	The number of files to consider when performing rename detection
    +	in linkgit:git-status[1] and linkgit:git-commit[1]. Defaults to
    +	the value of diff.renameLimit.
    
     status.renames::
    -	Whether and how Git detects renames.  If set to "false",
    -	rename detection is disabled. If set to "true", basic rename
    -	detection is enabled.  If set to "copies" or "copy", Git will
    -	detect copies, as well.  Defaults to the value of diff.renames.
    +	Whether and how Git detects renames in linkgit:git-status[1] and
    +	linkgit:git-commit[1] .  If set to "false", rename detection is
    +	disabled. If set to "true", basic rename detection is enabled.
    +	If set to "copies" or "copy", Git will detect copies, as well.
    +	Defaults to the value of diff.renames.
    
     status.showStash::
     	If set to true, linkgit:git-status[1] will display the number of
    diff --git a/builtin/commit.c b/builtin/commit.c
    index db886277f4..b50e33ef48 100644
    --- a/builtin/commit.c
    +++ b/builtin/commit.c
    @@ -1373,6 +1373,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
     	if (no_renames != -1)
     		s.detect_rename = !no_renames;
     	if ((intptr_t)rename_score_arg != -1) {
    +		if (s.detect_rename < DIFF_DETECT_RENAME)
     			s.detect_rename = DIFF_DETECT_RENAME;
     		if (rename_score_arg)
     			s.rename_score = parse_rename_score(&rename_score_arg);
    diff --git a/t/t7525-status-rename.sh b/t/t7525-status-rename.sh
    old mode 100644
    new mode 100755
    index 311df8038a..ef8b1b3078
    --- a/t/t7525-status-rename.sh
    +++ b/t/t7525-status-rename.sh
    @@ -10,14 +10,13 @@ test_expect_success 'setup' '
     	git commit -m"Adding original file." &&
     	mv original renamed &&
     	echo 2 >> renamed &&
    -	git add .
    -'
    -
    -cat >.gitignore <<\EOF
    +	git add . &&
    +	cat >.gitignore <<-\EOF
     	.gitignore
     	expect*
     	actual*
     	EOF
    +'
    
     test_expect_success 'status no-options' '
     	git status >actual &&
    @@ -63,7 +62,18 @@ test_expect_success 'status status.renames=true' '
     	test_i18ngrep "renamed:" actual
     '
    
    -test_expect_success 'status config overriden' '
    +test_expect_success 'commit honors status.renames=false' '
    +	git -c status.renames=false commit --dry-run >actual &&
    +	test_i18ngrep "deleted:" actual &&
    +	test_i18ngrep "new file:" actual
    +'
    +
    +test_expect_success 'commit honors status.renames=true' '
    +	git -c status.renames=true commit --dry-run >actual &&
    +	test_i18ngrep "renamed:" actual
    +'
    +
    +test_expect_success 'status config overridden' '
     	git -c status.renames=true status --no-renames >actual &&
     	test_i18ngrep "deleted:" actual &&
     	test_i18ngrep "new file:" actual
    @@ -87,4 +97,17 @@ test_expect_success 'status score=01%' '
     	test_i18ngrep "renamed:" actual
     '
    
    +test_expect_success 'copies not overridden by find-rename' '
    +	cp renamed copy &&
    +	git add copy &&
    +
    +	git -c status.renames=copies status -M=01% >actual &&
    +	test_i18ngrep "copied:" actual &&
    +	test_i18ngrep "renamed:" actual &&
    +
    +	git -c status.renames=copies status --find-rename=01% >actual &&
    +	test_i18ngrep "copied:" actual &&
    +	test_i18ngrep "renamed:" actual
    +'
    +
     test_done
    
    ### Patches

 Documentation/config.txt     |  12 ++++
 Documentation/git-status.txt |  10 ++++
 builtin/commit.c             |  42 +++++++++++++
 diff.c                       |   2 +-
 diff.h                       |   1 +
 t/t7525-status-rename.sh     | 113 +++++++++++++++++++++++++++++++++++
 wt-status.c                  |  12 ++++
 wt-status.h                  |   4 +-
 8 files changed, 194 insertions(+), 2 deletions(-)
 create mode 100755 t/t7525-status-rename.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2659153cb3..88884f1ead 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -3119,6 +3119,18 @@ status.displayCommentPrefix::
 	behavior of linkgit:git-status[1] in Git 1.8.4 and previous.
 	Defaults to false.
 
+status.renameLimit::
+	The number of files to consider when performing rename detection
+	in linkgit:git-status[1] and linkgit:git-commit[1]. Defaults to
+	the value of diff.renameLimit.
+
+status.renames::
+	Whether and how Git detects renames in linkgit:git-status[1] and
+	linkgit:git-commit[1] .  If set to "false", rename detection is
+	disabled. If set to "true", basic rename detection is enabled.
+	If set to "copies" or "copy", Git will detect copies, as well.
+	Defaults to the value of diff.renames.
+
 status.showStash::
 	If set to true, linkgit:git-status[1] will display the number of
 	entries currently stashed away.
diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index c16e27e63d..c4467ffb98 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -135,6 +135,16 @@ ignored, then the directory is not shown, but all contents are shown.
 	Display or do not display detailed ahead/behind counts for the
 	branch relative to its upstream branch.  Defaults to true.
 
+--renames::
+--no-renames::
+	Turn on/off rename detection regardless of user configuration.
+	See also linkgit:git-diff[1] `--no-renames`.
+
+--find-renames[=<n>]::
+	Turn on rename detection, optionally setting the similarity
+	threshold.
+	See also linkgit:git-diff[1] `--find-renames`.
+
 <pathspec>...::
 	See the 'pathspec' entry in linkgit:gitglossary[7].
 
diff --git a/builtin/commit.c b/builtin/commit.c
index 5240f11225..b50e33ef48 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -143,6 +143,16 @@ static int opt_parse_m(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
+static int opt_parse_rename_score(const struct option *opt, const char *arg, int unset)
+{
+	const char **value = opt->value;
+	if (arg != NULL && *arg == '=')
+		arg = arg + 1;
+
+	*value = arg;
+	return 0;
+}
+
 static void determine_whence(struct wt_status *s)
 {
 	if (file_exists(git_path_merge_head()))
@@ -1259,11 +1269,31 @@ static int git_status_config(const char *k, const char *v, void *cb)
 			return error(_("Invalid untracked files mode '%s'"), v);
 		return 0;
 	}
+	if (!strcmp(k, "diff.renamelimit")) {
+		if (s->rename_limit == -1)
+			s->rename_limit = git_config_int(k, v);
+		return 0;
+	}
+	if (!strcmp(k, "status.renamelimit")) {
+		s->rename_limit = git_config_int(k, v);
+		return 0;
+	}
+	if (!strcmp(k, "diff.renames")) {
+		if (s->detect_rename == -1)
+			s->detect_rename = git_config_rename(k, v);
+		return 0;
+	}
+	if (!strcmp(k, "status.renames")) {
+		s->detect_rename = git_config_rename(k, v);
+		return 0;
+	}
 	return git_diff_ui_config(k, v, NULL);
 }
 
 int cmd_status(int argc, const char **argv, const char *prefix)
 {
+	static int no_renames = -1;
+	static const char *rename_score_arg = (const char *)-1;
 	static struct wt_status s;
 	int fd;
 	struct object_id oid;
@@ -1297,6 +1327,10 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 		  N_("ignore changes to submodules, optional when: all, dirty, untracked. (Default: all)"),
 		  PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
 		OPT_COLUMN(0, "column", &s.colopts, N_("list untracked files in columns")),
+		OPT_BOOL(0, "no-renames", &no_renames, N_("do not detect renames")),
+		{ OPTION_CALLBACK, 'M', "find-renames", &rename_score_arg,
+		  N_("n"), N_("detect renames, optionally set similarity index"),
+		  PARSE_OPT_OPTARG, opt_parse_rename_score },
 		OPT_END(),
 	};
 
@@ -1336,6 +1370,14 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	s.ignore_submodule_arg = ignore_submodule_arg;
 	s.status_format = status_format;
 	s.verbose = verbose;
+	if (no_renames != -1)
+		s.detect_rename = !no_renames;
+	if ((intptr_t)rename_score_arg != -1) {
+		if (s.detect_rename < DIFF_DETECT_RENAME)
+			s.detect_rename = DIFF_DETECT_RENAME;
+		if (rename_score_arg)
+			s.rename_score = parse_rename_score(&rename_score_arg);
+	}
 
 	wt_status_collect(&s);
 
diff --git a/diff.c b/diff.c
index 1289df4b1f..5dfc24aa6d 100644
--- a/diff.c
+++ b/diff.c
@@ -177,7 +177,7 @@ static int parse_submodule_params(struct diff_options *options, const char *valu
 	return 0;
 }
 
-static int git_config_rename(const char *var, const char *value)
+int git_config_rename(const char *var, const char *value)
 {
 	if (!value)
 		return DIFF_DETECT_RENAME;
diff --git a/diff.h b/diff.h
index d29560f822..dedac472ca 100644
--- a/diff.h
+++ b/diff.h
@@ -324,6 +324,7 @@ extern int git_diff_ui_config(const char *var, const char *value, void *cb);
 extern void diff_setup(struct diff_options *);
 extern int diff_opt_parse(struct diff_options *, const char **, int, const char *);
 extern void diff_setup_done(struct diff_options *);
+extern int git_config_rename(const char *var, const char *value);
 
 #define DIFF_DETECT_RENAME	1
 #define DIFF_DETECT_COPY	2
diff --git a/t/t7525-status-rename.sh b/t/t7525-status-rename.sh
new file mode 100755
index 0000000000..ef8b1b3078
--- /dev/null
+++ b/t/t7525-status-rename.sh
@@ -0,0 +1,113 @@
+#!/bin/sh
+
+test_description='git status rename detection options'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo 1 >original &&
+	git add . &&
+	git commit -m"Adding original file." &&
+	mv original renamed &&
+	echo 2 >> renamed &&
+	git add . &&
+	cat >.gitignore <<-\EOF
+	.gitignore
+	expect*
+	actual*
+	EOF
+'
+
+test_expect_success 'status no-options' '
+	git status >actual &&
+	test_i18ngrep "renamed:" actual
+'
+
+test_expect_success 'status --no-renames' '
+	git status --no-renames >actual &&
+	test_i18ngrep "deleted:" actual &&
+	test_i18ngrep "new file:" actual
+'
+
+test_expect_success 'status.renames inherits from diff.renames false' '
+	git -c diff.renames=false status >actual &&
+	test_i18ngrep "deleted:" actual &&
+	test_i18ngrep "new file:" actual
+'
+
+test_expect_success 'status.renames inherits from diff.renames true' '
+	git -c diff.renames=true status >actual &&
+	test_i18ngrep "renamed:" actual
+'
+
+test_expect_success 'status.renames overrides diff.renames false' '
+	git -c diff.renames=true -c status.renames=false status >actual &&
+	test_i18ngrep "deleted:" actual &&
+	test_i18ngrep "new file:" actual
+'
+
+test_expect_success 'status.renames overrides from diff.renames true' '
+	git -c diff.renames=false -c status.renames=true status >actual &&
+	test_i18ngrep "renamed:" actual
+'
+
+test_expect_success 'status status.renames=false' '
+	git -c status.renames=false status >actual &&
+	test_i18ngrep "deleted:" actual &&
+	test_i18ngrep "new file:" actual
+'
+
+test_expect_success 'status status.renames=true' '
+	git -c status.renames=true status >actual &&
+	test_i18ngrep "renamed:" actual
+'
+
+test_expect_success 'commit honors status.renames=false' '
+	git -c status.renames=false commit --dry-run >actual &&
+	test_i18ngrep "deleted:" actual &&
+	test_i18ngrep "new file:" actual
+'
+
+test_expect_success 'commit honors status.renames=true' '
+	git -c status.renames=true commit --dry-run >actual &&
+	test_i18ngrep "renamed:" actual
+'
+
+test_expect_success 'status config overridden' '
+	git -c status.renames=true status --no-renames >actual &&
+	test_i18ngrep "deleted:" actual &&
+	test_i18ngrep "new file:" actual
+'
+
+test_expect_success 'status score=100%' '
+	git status -M=100% >actual &&
+	test_i18ngrep "deleted:" actual &&
+	test_i18ngrep "new file:" actual &&
+
+	git status --find-rename=100% >actual &&
+	test_i18ngrep "deleted:" actual &&
+	test_i18ngrep "new file:" actual
+'
+
+test_expect_success 'status score=01%' '
+	git status -M=01% >actual &&
+	test_i18ngrep "renamed:" actual &&
+
+	git status --find-rename=01% >actual &&
+	test_i18ngrep "renamed:" actual
+'
+
+test_expect_success 'copies not overridden by find-rename' '
+	cp renamed copy &&
+	git add copy &&
+
+	git -c status.renames=copies status -M=01% >actual &&
+	test_i18ngrep "copied:" actual &&
+	test_i18ngrep "renamed:" actual &&
+
+	git -c status.renames=copies status --find-rename=01% >actual &&
+	test_i18ngrep "copied:" actual &&
+	test_i18ngrep "renamed:" actual
+'
+
+test_done
diff --git a/wt-status.c b/wt-status.c
index 32f3bcaebd..172f07cbb0 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -138,6 +138,9 @@ void wt_status_prepare(struct wt_status *s)
 	s->show_stash = 0;
 	s->ahead_behind_flags = AHEAD_BEHIND_UNSPECIFIED;
 	s->display_comment_prefix = 0;
+	s->detect_rename = -1;
+	s->rename_score = -1;
+	s->rename_limit = -1;
 }
 
 static void wt_longstatus_print_unmerged_header(struct wt_status *s)
@@ -592,6 +595,9 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)
 	}
 	rev.diffopt.format_callback = wt_status_collect_changed_cb;
 	rev.diffopt.format_callback_data = s;
+	rev.diffopt.detect_rename = s->detect_rename >= 0 ? s->detect_rename : rev.diffopt.detect_rename;
+	rev.diffopt.rename_limit = s->rename_limit >= 0 ? s->rename_limit : rev.diffopt.rename_limit;
+	rev.diffopt.rename_score = s->rename_score >= 0 ? s->rename_score : rev.diffopt.rename_score;
 	copy_pathspec(&rev.prune_data, &s->pathspec);
 	run_diff_files(&rev, 0);
 }
@@ -625,6 +631,9 @@ static void wt_status_collect_changes_index(struct wt_status *s)
 	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = wt_status_collect_updated_cb;
 	rev.diffopt.format_callback_data = s;
+	rev.diffopt.detect_rename = s->detect_rename >= 0 ? s->detect_rename : rev.diffopt.detect_rename;
+	rev.diffopt.rename_limit = s->rename_limit >= 0 ? s->rename_limit : rev.diffopt.rename_limit;
+	rev.diffopt.rename_score = s->rename_score >= 0 ? s->rename_score : rev.diffopt.rename_score;
 	copy_pathspec(&rev.prune_data, &s->pathspec);
 	run_diff_index(&rev, 1);
 }
@@ -982,6 +991,9 @@ static void wt_longstatus_print_verbose(struct wt_status *s)
 	setup_revisions(0, NULL, &rev, &opt);
 
 	rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
+	rev.diffopt.detect_rename = s->detect_rename >= 0 ? s->detect_rename : rev.diffopt.detect_rename;
+	rev.diffopt.rename_limit = s->rename_limit >= 0 ? s->rename_limit : rev.diffopt.rename_limit;
+	rev.diffopt.rename_score = s->rename_score >= 0 ? s->rename_score : rev.diffopt.rename_score;
 	rev.diffopt.file = s->fp;
 	rev.diffopt.close_file = 0;
 	/*
diff --git a/wt-status.h b/wt-status.h
index 430770b854..1673d146fa 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -89,7 +89,9 @@ struct wt_status {
 	int show_stash;
 	int hints;
 	enum ahead_behind_flags ahead_behind_flags;
-
+	int detect_rename;
+	int rename_score;
+	int rename_limit;
 	enum wt_status_format status_format;
 	unsigned char sha1_commit[GIT_MAX_RAWSZ]; /* when not Initial */
 

base-commit: dc6b1d92ca9c0c538daa244e3910bb8b2a50d959
-- 
2.17.0.windows.1


  parent reply	other threads:[~2018-05-11 12:56 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-09 14:42 [PATCH v1] add status config and command line options for rename detection Ben Peart
2018-05-09 15:59 ` Duy Nguyen
2018-05-09 17:04   ` Ben Peart
2018-05-09 16:56 ` Elijah Newren
2018-05-09 19:54   ` Ben Peart
2018-05-10 14:16 ` [PATCH v2] " Ben Peart
2018-05-10 16:19   ` Elijah Newren
2018-05-10 19:09     ` Ben Peart
2018-05-10 22:31       ` Elijah Newren
2018-05-11 12:50         ` Ben Peart
2018-05-11  1:57     ` Junio C Hamano
2018-05-11  6:39   ` Junio C Hamano
2018-05-11 12:56 ` Ben Peart [this message]
2018-05-11 14:33   ` [PATCH v3] " Elijah Newren
2018-05-12  8:04   ` Eckhard Maaß
2018-05-14 12:57     ` Ben Peart
2018-05-11 15:38 ` [PATCH v4] " Ben Peart

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180511125623.6068-1-benpeart@microsoft.com \
    --to=ben.peart@microsoft.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=alpauly@microsoft.com \
    --cc=eckhard.s.maass@googlemail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=newren@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=vmiklos@frugalware.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.