git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Merging limitations after directory renames -- interesting test repo

From
Linus Torvalds <torvalds@linux-foundation.org>
Date
Feb 19, 2011, 01:03 UTC
Message-ID
<AANLkTimK0K=khbPk22Fpo99i02y1gdsSgMqWcRgXWe4w@mail.gmail.com>
In-Reply-To
<7vsjvkn8vh.fsf@alter.siamese.dyndns.org>
On Fri, Feb 18, 2011 at 4:52 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 5 quoted lines
>
> Yeah, I think it is probably a good idea to really limit the rename
> detection only to renames at least inside merge-recursive.  The static
> variable does look ugly but it shouldn't be a rocket surgery to pass it
> down if we want to.

Here's a slight update. It still has the "too ugly to live" if-statment without proper indentation, but it now changes "for_each_hash()" to pass in a flag value and uses that.

It also fixes one of the tests that depended on the "find copies" behavior for its result to actually ask for copies now that we don't give them unless you do.

                                 Linus
 builtin/describe.c            |    4 ++--
 diffcore-rename.c             |   17 ++++++++++-------
 hash.c                        |    4 ++--
 hash.h                        |    2 +-
 t/t4008-diff-break-rewrite.sh |    4 ++--
 5 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/builtin/describe.c b/builtin/describe.c
index 342129f..4f4fe3e 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -63,7 +63,7 @@ static inline struct commit_name *find_commit_name(const unsigned char *peeled)
 	return n;
 }
 
-static int set_util(void *chain)
+static int set_util(void *chain, int flag)
 {
 	struct commit_name *n;
 	for (n = chain; n; n = n->next) {
@@ -289,7 +289,7 @@ static void describe(const char *arg, int last_one)
 		fprintf(stderr, "searching to describe %s\n", arg);
 
 	if (!have_util) {
-		for_each_hash(&names, set_util);
+		for_each_hash(&names, set_util, 0);
 		have_util = 1;
 	}
 
diff --git a/diffcore-rename.c b/diffcore-rename.c
index df41be5..3a937ac 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -170,7 +170,7 @@ static int estimate_similarity(struct diff_filespec *src,
 	 * and the final score computation below would not have a
 	 * divide-by-zero issue.
 	 */
-	if (base_size * (MAX_SCORE-minimum_score) < delta_size * MAX_SCORE)
+	if (max_size * (MAX_SCORE-minimum_score) < delta_size * MAX_SCORE)
 		return 0;
 
 	if (!src->cnt_data && diff_populate_filespec(src, 0))
@@ -247,7 +247,7 @@ struct file_similarity {
 };
 
 static int find_identical_files(struct file_similarity *src,
-				struct file_similarity *dst)
+				struct file_similarity *dst, int copies)
 {
 	int renames = 0;
 
@@ -277,6 +277,8 @@ static int find_identical_files(struct file_similarity *src,
 			}
 			/* Give higher scores to sources that haven't been used already */
 			score = !source->rename_used;
+			if (source->rename_used && !copies)
+				continue;
 			score += basename_same(source, target);
 			if (score > best_score) {
 				best = p;
@@ -306,7 +308,7 @@ static void free_similarity_list(struct file_similarity *p)
 	}
 }
 
-static int find_same_files(void *ptr)
+static int find_same_files(void *ptr, int copies)
 {
 	int ret;
 	struct file_similarity *p = ptr;
@@ -329,7 +331,7 @@ static int find_same_files(void *ptr)
 	 * If we have both sources *and* destinations, see if
 	 * we can match them up
 	 */
-	ret = (src && dst) ? find_identical_files(src, dst) : 0;
+	ret = (src && dst) ? find_identical_files(src, dst, copies) : 0;
 
 	/* Free the hashes and return the number of renames found */
 	free_similarity_list(src);
@@ -377,7 +379,7 @@ static void insert_file_table(struct hash_table *table, int src_dst, int index,
  * and then during the second round we try to match
  * cache-dirty entries as well.
  */
-static int find_exact_renames(void)
+static int find_exact_renames(int copies)
 {
 	int i;
 	struct hash_table file_table;
@@ -390,7 +392,7 @@ static int find_exact_renames(void)
 		insert_file_table(&file_table, 1, i, rename_dst[i].two);
 
 	/* Find the renames */
-	i = for_each_hash(&file_table, find_same_files);
+	i = for_each_hash(&file_table, find_same_files, copies);
 
 	/* .. and free the hash data structure */
 	free_hash(&file_table);
@@ -467,7 +469,7 @@ void diffcore_rename(struct diff_options *options)
 	 * We really want to cull the candidates list early
 	 * with cheap tests in order to avoid doing deltas.
 	 */
-	rename_count = find_exact_renames();
+	rename_count = find_exact_renames(detect_rename == DIFF_DETECT_COPY);
 
 	/* Did we only want exact renames? */
 	if (minimum_score == MAX_SCORE)
@@ -551,6 +553,7 @@ void diffcore_rename(struct diff_options *options)
 		rename_count++;
 	}
 
+	if (detect_rename == DIFF_DETECT_COPY)
 	for (i = 0; i < dst_cnt * NUM_CANDIDATE_PER_DST; i++) {
 		struct diff_rename_dst *dst;
 
diff --git a/hash.c b/hash.c
index 1cd4c9d..c5dd002 100644
--- a/hash.c
+++ b/hash.c
@@ -81,7 +81,7 @@ void **insert_hash(unsigned int hash, void *ptr, struct hash_table *table)
 	return insert_hash_entry(hash, ptr, table);
 }
 
-int for_each_hash(const struct hash_table *table, int (*fn)(void *))
+int for_each_hash(const struct hash_table *table, int (*fn)(void *, int), int flag)
 {
 	int sum = 0;
 	unsigned int i;
@@ -92,7 +92,7 @@ int for_each_hash(const struct hash_table *table, int (*fn)(void *))
 		void *ptr = array->ptr;
 		array++;
 		if (ptr) {
-			int val = fn(ptr);
+			int val = fn(ptr, flag);
 			if (val < 0)
 				return val;
 			sum += val;
diff --git a/hash.h b/hash.h
index 69e33a4..1e2de55 100644
--- a/hash.h
+++ b/hash.h
@@ -30,7 +30,7 @@ struct hash_table {
 
 extern void *lookup_hash(unsigned int hash, const struct hash_table *table);
 extern void **insert_hash(unsigned int hash, void *ptr, struct hash_table *table);
-extern int for_each_hash(const struct hash_table *table, int (*fn)(void *));
+extern int for_each_hash(const struct hash_table *table, int (*fn)(void *, int), int flag);
 extern void free_hash(struct hash_table *table);
 
 static inline void init_hash(struct hash_table *table)
diff --git a/t/t4008-diff-break-rewrite.sh b/t/t4008-diff-break-rewrite.sh
index d79d9e1e..73b4a24 100755
--- a/t/t4008-diff-break-rewrite.sh
+++ b/t/t4008-diff-break-rewrite.sh
@@ -173,8 +173,8 @@ test_expect_success \
     'compare_diff_raw expected current'
 
 test_expect_success \
-    'run diff with -B -M' \
-    'git diff-index -B -M "$tree" >current'
+    'run diff with -B -C' \
+    'git diff-index -B -C "$tree" >current'
 
 cat >expected <<\EOF
 :100644 100644 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 08bb2fb671deff4c03a4d4a0a1315dff98d5732c C095	file0	file1
Previous: Junio C HamanoNext: Linus Torvalds
Message 6 of 34 in “Merging limitations after directory renames -- interesting test repo”
  1. Martin LanghoffFeb 18, 2011
  2. Jeff KingFeb 18, 2011
  3. Linus TorvaldsFeb 18, 2011
  4. Linus TorvaldsFeb 19, 2011
  5. Junio C HamanoFeb 19, 2011
  6. Linus TorvaldsFeb 19, 2011
  7. Linus TorvaldsFeb 19, 2011
  8. Linus TorvaldsFeb 19, 2011
  9. Junio C HamanoFeb 19, 2011
  10. Jeff KingFeb 19, 2011
  11. Jeff KingFeb 19, 2011
  12. 1/4 merge: improve inexact rename limit warningJeff King, Feb 19, 2011
  13. Junio C HamanoFeb 21, 2011
  14. Jeff KingFeb 22, 2011
  15. Junio C HamanoFeb 22, 2011
  16. Jeff KingFeb 23, 2011
  17. 2/4 bump rename limit defaults (again)Jeff King, Feb 19, 2011
  18. Piotr KrukowieckiFeb 19, 2011
  19. Jeff KingFeb 20, 2011
  20. Ævar Arnfjörð BjarmasonFeb 19, 2011
  21. Jeff KingFeb 20, 2011
  22. 3/4 commit: stop setting rename limitJeff King, Feb 19, 2011
  23. 4/4 inexact rename detection eye candyJeff King, Feb 19, 2011
  24. Linus TorvaldsFeb 19, 2011
  25. Jeff KingFeb 20, 2011
  26. 1/3 add inexact rename detection progress infrastructureJeff King, Feb 20, 2011
  27. 2/3 merge: enable progress reporting for rename detectionJeff King, Feb 20, 2011
  28. 3/3 pull: propagate --progress to mergeJeff King, Feb 20, 2011
  29. Jeff KingFeb 20, 2011
  30. Martin LanghoffFeb 19, 2011
  31. Jeff KingFeb 20, 2011
  32. Martin LanghoffFeb 20, 2011
  33. Martin LanghoffFeb 19, 2011
  34. Martin LanghoffFeb 19, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.