[PATCH 1/2] diff --quick

Subsystems: the rest

DORMANTno replies

9 messages, 2 authors, 2016-06-15 · open the first message on its own page

[PATCH 1/2] diff --quick

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:59

This adds the command line option 'quick' to tell 'git diff-*' that
we are not interested in the actual diff contents but only if there
is any change.  This option automatically turns --exit-code on, and
turns off output formatting, as it does not make much sense to show
the first hit we happened to have found.

The backends have not been taught about the option with this patch.
That is a topic for later rounds.

Signed-off-by: Junio C Hamano <redacted>
---

 * Obviously this comes on top of Alex's --exit-code.

 diff.c |   28 ++++++++++++++++++++++++++--
 diff.h |    4 ++--
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/diff.c b/diff.c
index 7d938c1..99f5802 100644
--- a/diff.c
+++ b/diff.c

@@ -1958,6 +1958,23 @@ int diff_setup_done(struct diff_options *options)
 	if (options->abbrev <= 0 || 40 < options->abbrev)
 		options->abbrev = 40; /* full */
 
+	/*
+	 * It does not make sense to show the first hit we happened
+	 * to have found.  It does not make sense not to return with
+	 * exit code in such a case either.
+	 */
+	if (options->quick) {
+		options->output_format = DIFF_FORMAT_NO_OUTPUT;
+		options->exit_with_status = 1;
+	}
+
+	/*
+	 * If we postprocess in diffcore, we cannot simply return
+	 * upon the first hit.  We need to run diff as usual.
+	 */
+	if (options->pickaxe || options->filter)
+		options->quick = 0;
+
 	return 0;
 }

@@ -2136,6 +2153,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		options->detect_rename = 0;
 	else if (!strcmp(arg, "--exit-code"))
 		options->exit_with_status = 1;
+	else if (!strcmp(arg, "--quick"))
+		options->quick = 1;
 	else
 		return 0;
 	return 1;

@@ -2900,6 +2919,11 @@ static void diffcore_apply_filter(const char *filter)
 
 void diffcore_std(struct diff_options *options)
 {
+	if (options->quick) {
+		options->has_changes = !!diff_queued_diff.nr;
+		return;
+	}
+
 	if (options->break_opt != -1)
 		diffcore_break(options->break_opt);
 	if (options->detect_rename)

@@ -2912,8 +2936,8 @@ void diffcore_std(struct diff_options *options)
 		diffcore_order(options->orderfile);
 	diff_resolve_rename_copy();
 	diffcore_apply_filter(options->filter);
-	if (options->exit_with_status)
-		options->has_changes = !!diff_queued_diff.nr;
+
+	options->has_changes = !!diff_queued_diff.nr;
 }

diff --git a/diff.h b/diff.h
index 81fa265..2555120 100644
--- a/diff.h
+++ b/diff.h

@@ -57,6 +57,8 @@ struct diff_options {
 		 find_copies_harder:1,
 		 color_diff:1,
 		 color_diff_words:1,
+		 has_changes:1,
+		 quick:1,
 		 exit_with_status:1;
 	int context;
 	int break_opt;

@@ -72,8 +74,6 @@ struct diff_options {
 	const char *msg_sep;
 	const char *stat_sep;
 	long xdl_opts;
-	/* 0 - no differences; only meaningful if exit_with_status set */
-	int has_changes;
 
 	int stat_width;
 	int stat_name_width;

-- 
1.5.0.3.1036.g6baf1

[PATCH 2/5] diff --quiet

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:59

This adds the command line option 'quiet' to tell 'git diff-*'
that we are not interested in the actual diff contents but only
want to know if there is any change.  This option automatically
turns --exit-code on, and turns off output formatting, as it
does not make much sense to show the first hit we happened to
have found.

The --quiet option is silently turned off (but --exit-code is
still in effect, so is silent output) if postprocessing filters
such as pickaxe and diff-filter are used.  For all practical
purposes I do not think of a reason to want to use these filters
and not viewing the diff output.

The backends have not been taught about the option with this patch.
That is a topic for later rounds.

Signed-off-by: Junio C Hamano <redacted>
---
 diff.c |   27 +++++++++++++++++++++++++--
 diff.h |    4 ++--
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/diff.c b/diff.c
index 7d938c1..d8f9242 100644
--- a/diff.c
+++ b/diff.c

@@ -1958,6 +1958,23 @@ int diff_setup_done(struct diff_options *options)
 	if (options->abbrev <= 0 || 40 < options->abbrev)
 		options->abbrev = 40; /* full */
 
+	/*
+	 * It does not make sense to show the first hit we happened
+	 * to have found.  It does not make sense not to return with
+	 * exit code in such a case either.
+	 */
+	if (options->quiet) {
+		options->output_format = DIFF_FORMAT_NO_OUTPUT;
+		options->exit_with_status = 1;
+	}
+
+	/*
+	 * If we postprocess in diffcore, we cannot simply return
+	 * upon the first hit.  We need to run diff as usual.
+	 */
+	if (options->pickaxe || options->filter)
+		options->quiet = 0;
+
 	return 0;
 }

@@ -2136,6 +2153,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		options->detect_rename = 0;
 	else if (!strcmp(arg, "--exit-code"))
 		options->exit_with_status = 1;
+	else if (!strcmp(arg, "--quiet"))
+		options->quiet = 1;
 	else
 		return 0;
 	return 1;

@@ -2900,6 +2919,8 @@ static void diffcore_apply_filter(const char *filter)
 
 void diffcore_std(struct diff_options *options)
 {
+	if (options->quiet)
+		return;
 	if (options->break_opt != -1)
 		diffcore_break(options->break_opt);
 	if (options->detect_rename)

@@ -2912,8 +2933,8 @@ void diffcore_std(struct diff_options *options)
 		diffcore_order(options->orderfile);
 	diff_resolve_rename_copy();
 	diffcore_apply_filter(options->filter);
-	if (options->exit_with_status)
-		options->has_changes = !!diff_queued_diff.nr;
+
+	options->has_changes = !!diff_queued_diff.nr;
 }

@@ -2952,6 +2973,7 @@ void diff_addremove(struct diff_options *options,
 		fill_filespec(two, sha1, mode);
 
 	diff_queue(&diff_queued_diff, one, two);
+	options->has_changes = 1;
 }
 
 void diff_change(struct diff_options *options,

@@ -2977,6 +2999,7 @@ void diff_change(struct diff_options *options,
 	fill_filespec(two, new_sha1, new_mode);
 
 	diff_queue(&diff_queued_diff, one, two);
+	options->has_changes = 1;
 }
 
 void diff_unmerge(struct diff_options *options,

diff --git a/diff.h b/diff.h
index 81fa265..a0d2ce1 100644
--- a/diff.h
+++ b/diff.h

@@ -57,6 +57,8 @@ struct diff_options {
 		 find_copies_harder:1,
 		 color_diff:1,
 		 color_diff_words:1,
+		 has_changes:1,
+		 quiet:1,
 		 exit_with_status:1;
 	int context;
 	int break_opt;

@@ -72,8 +74,6 @@ struct diff_options {
 	const char *msg_sep;
 	const char *stat_sep;
 	long xdl_opts;
-	/* 0 - no differences; only meaningful if exit_with_status set */
-	int has_changes;
 
 	int stat_width;
 	int stat_name_width;

-- 
1.5.0.3.1036.g6baf1

[PATCH 1/5] Remove unused diffcore_std_no_resolve

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:59

This was only used by diff-tree-helper program, whose purpose
was to translate a raw diff to a patch.

Signed-off-by: Junio C Hamano <redacted>
---
 * These are re-spin of the earlier 2 patch series, go directly
   on top of Alex's --exit-code.

 diff.c |   11 -----------
 diff.h |    2 --
 2 files changed, 0 insertions(+), 13 deletions(-)

diff --git a/diff.c b/diff.c
index cc81801..7d938c1 100644
--- a/diff.c
+++ b/diff.c

@@ -2917,17 +2917,6 @@ void diffcore_std(struct diff_options *options)
 }
 
 
-void diffcore_std_no_resolve(struct diff_options *options)
-{
-	if (options->pickaxe)
-		diffcore_pickaxe(options->pickaxe, options->pickaxe_opts);
-	if (options->orderfile)
-		diffcore_order(options->orderfile);
-	diffcore_apply_filter(options->filter);
-	if (options->exit_with_status)
-		options->has_changes = !!diff_queued_diff.nr;
-}
-
 void diff_addremove(struct diff_options *options,
 		    int addremove, unsigned mode,
 		    const unsigned char *sha1,

diff --git a/diff.h b/diff.h
index d13fc89..81fa265 100644
--- a/diff.h
+++ b/diff.h

@@ -173,8 +173,6 @@ extern int diff_setup_done(struct diff_options *);
 
 extern void diffcore_std(struct diff_options *);
 
-extern void diffcore_std_no_resolve(struct diff_options *);
-
 #define COMMON_DIFF_OPTIONS_HELP \
 "\ncommon diff options:\n" \
 "  -z            output diff-raw with lines terminated with NUL.\n" \

-- 
1.5.0.3.1036.g6baf1

[PATCH 4/5] revision.c: explain what tree_difference does

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:59

This explains how tree_difference variable is used, and updates two
places where the code knows symbolic constant REV_TREE_SAME is 0.

Signed-off-by: Junio C Hamano <redacted>
---
 revision.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/revision.c b/revision.c
index 3c2eb12..129d197 100644
--- a/revision.c
+++ b/revision.c

@@ -213,6 +213,13 @@ static int everybody_uninteresting(struct commit_list *orig)
 	return 1;
 }
 
+/*
+ * The goal is to get REV_TREE_NEW as the result only if the
+ * diff consists of all '+' (and no other changes), and
+ * REV_TREE_DIFFERENT otherwise (of course if the trees are
+ * the same we want REV_TREE_SAME).  That means that once we
+ * get to REV_TREE_DIFFERENT, we do not have to look any further.
+ */
 static int tree_difference = REV_TREE_SAME;
 
 static void file_add_remove(struct diff_options *options,

@@ -277,11 +284,11 @@ int rev_same_tree_as_empty(struct rev_info *revs, struct tree *t1)
 	empty.buf = "";
 	empty.size = 0;
 
-	tree_difference = 0;
+	tree_difference = REV_TREE_SAME;
 	retval = diff_tree(&empty, &real, "", &revs->pruning);
 	free(tree);
 
-	return retval >= 0 && !tree_difference;
+	return retval >= 0 && (tree_difference == REV_TREE_SAME);
 }
 
 static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)

-- 
1.5.0.3.1036.g6baf1

[PATCH 5/5] try-to-simplify-commit: use diff-tree --quiet machinery.

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:59

This uses diff-tree --quiet machinery to terminate the internal
diff-tree between a commit and its parents via revs.pruning (not
revs.diffopt) as soon as we find enough about the tree change.

With respect to the optionally given pathspec, we are interested
if the tree of commit is identical to the parent's, only adds
new paths to the parent's, or there are other differences.  As
soon as we find out that there is one such other kind of
difference, we do not have to compare the rest of the tree.

Because we do not call standard diff_addremove/diff_change, we
instruct the diff-tree machinery to stop early by setting
has_changes when we say we found the trees to be different.

Signed-off-by: Junio C Hamano <redacted>
---
 revision.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/revision.c b/revision.c
index 129d197..bcdb6a1 100644
--- a/revision.c
+++ b/revision.c

@@ -243,6 +243,8 @@ static void file_add_remove(struct diff_options *options,
 		diff = REV_TREE_NEW;
 	}
 	tree_difference = diff;
+	if (tree_difference == REV_TREE_DIFFERENT)
+		options->has_changes = 1;
 }
 
 static void file_change(struct diff_options *options,

@@ -252,6 +254,7 @@ static void file_change(struct diff_options *options,
 		 const char *base, const char *path)
 {
 	tree_difference = REV_TREE_DIFFERENT;
+	options->has_changes = 1;
 }
 
 int rev_compare_tree(struct rev_info *revs, struct tree *t1, struct tree *t2)

@@ -261,6 +264,7 @@ int rev_compare_tree(struct rev_info *revs, struct tree *t1, struct tree *t2)
 	if (!t2)
 		return REV_TREE_DIFFERENT;
 	tree_difference = REV_TREE_SAME;
+	revs->pruning.has_changes = 0;
 	if (diff_tree_sha1(t1->object.sha1, t2->object.sha1, "",
 			   &revs->pruning) < 0)
 		return REV_TREE_DIFFERENT;

@@ -285,6 +289,7 @@ int rev_same_tree_as_empty(struct rev_info *revs, struct tree *t1)
 	empty.size = 0;
 
 	tree_difference = REV_TREE_SAME;
+	revs->pruning.has_changes = 0;
 	retval = diff_tree(&empty, &real, "", &revs->pruning);
 	free(tree);

@@ -552,6 +557,7 @@ void init_revisions(struct rev_info *revs, const char *prefix)
 	revs->ignore_merges = 1;
 	revs->simplify_history = 1;
 	revs->pruning.recursive = 1;
+	revs->pruning.quiet = 1;
 	revs->pruning.add_remove = file_add_remove;
 	revs->pruning.change = file_change;
 	revs->lifo = 1;

-- 
1.5.0.3.1036.g6baf1

[PATCH 3/5] Teach --quiet to diff backends.

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:59

This teaches git-diff-files, git-diff-index and git-diff-tree
backends to exit early under --quiet option.

Signed-off-by: Junio C Hamano <redacted>
---
 diff-lib.c  |    6 ++++++
 tree-diff.c |    2 ++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index f9a1a10..5c5b05b 100644
--- a/diff-lib.c
+++ b/diff-lib.c

@@ -324,6 +324,9 @@ int run_diff_files(struct rev_info *revs, int silent_on_removed)
 		struct cache_entry *ce = active_cache[i];
 		int changed;
 
+		if (revs->diffopt.quiet && revs->diffopt.has_changes)
+			break;
+
 		if (!ce_path_match(ce, revs->prune_data))
 			continue;

@@ -565,6 +568,9 @@ static int diff_cache(struct rev_info *revs,
 		struct cache_entry *ce = *ac;
 		int same = (entries > 1) && ce_same_name(ce, ac[1]);
 
+		if (revs->diffopt.quiet && revs->diffopt.has_changes)
+			break;
+
 		if (!ce_path_match(ce, pathspec))
 			goto skip_entry;

diff --git a/tree-diff.c b/tree-diff.c
index c827582..44cde74 100644
--- a/tree-diff.c
+++ b/tree-diff.c

@@ -161,6 +161,8 @@ static void show_entry(struct diff_options *opt, const char *prefix, struct tree
 int diff_tree(struct tree_desc *t1, struct tree_desc *t2, const char *base, struct diff_options *opt)
 {
 	while (t1->size | t2->size) {
+		if (opt->quiet && opt->has_changes)
+			break;
 		if (opt->nr_paths && t1->size && !interesting(t1, base, opt)) {
 			update_tree_entry(t1);
 			continue;

-- 
1.5.0.3.1036.g6baf1

Re: [PATCH 2/5] diff --quiet

From: Alex Riesen <hidden>
Date: 2016-06-15 22:42:59

On 3/14/07, Junio C Hamano [off-list ref] wrote:

The --quiet option is silently turned off (but --exit-code is
still in effect, so is silent output) if postprocessing filters
such as pickaxe and diff-filter are used.  For all practical
purposes I do not think of a reason to want to use these filters
and not viewing the diff output.

pickaxe filter can be useful in hooks to enforce policy
on use of some constructs. I can well imagine (do not
even have to imagine) a stupid company policy forbidding
commits with "\<printf\>" in them. Have no idea for
diff-filter though.

Re: [PATCH 2/5] diff --quiet

From: Alex Riesen <hidden>
Date: 2016-06-15 22:42:59

On 3/14/07, Alex Riesen [off-list ref] wrote:

On 3/14/07, Junio C Hamano [off-list ref] wrote:

quoted

The --quiet option is silently turned off (but --exit-code is
still in effect, so is silent output) if postprocessing filters
such as pickaxe and diff-filter are used.  For all practical
purposes I do not think of a reason to want to use these filters
and not viewing the diff output.

pickaxe filter can be useful in hooks to enforce policy

Actually, I find it a bit strange to leave the output silent,
if --quiet is turned off. For all the user sees, it is still quiet
and exit-coded, just slow. Suggest either restore the
output format when turning the option off or not turning
it off at all (if possible. Is it?)

Re: [PATCH 2/5] diff --quiet

From: Alex Riesen <hidden>
Date: 2016-06-15 22:42:59

On 3/14/07, Junio C Hamano [off-list ref] wrote:

This adds the command line option 'quiet' to tell 'git diff-*'
that we are not interested in the actual diff contents but only
want to know if there is any change.  This option automatically
turns --exit-code on, and turns off output formatting, as it
does not make much sense to show the first hit we happened to
have found.

Now I'm happy :)

~/linux$ time git diff-tree -r -s v2.6.16 v2.6.20

real    0m0.137s
user    0m0.117s
sys     0m0.020s
~/linux$ time ~/projects/git-diff/git-diff-tree -r --quiet v2.6.16 v2.6.20

real    0m0.006s
user    0m0.000s
sys     0m0.007s

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help