Git development
 help / color / mirror / Atom feed
* Re: [PATCH v2 3/3] pack-objects: honor '.keep' files
From: Shawn O. Pearce @ 2008-11-07 19:25 UTC (permalink / raw)
  To: Andreas Ericsson
  Cc: Brandon Casey, Junio C Hamano, Git Mailing List, Nicolas Pitre
In-Reply-To: <4913F859.8070500@op5.se>

Andreas Ericsson <ae@op5.se> wrote:
>
> Only -d should honor .keep, imo. .keep-files is nothing about "don't copy
> objects from this file" and all about "never delete this file".
>
> The only muddying comes from you, who decided that .keep-files should
> have impact on anything else than deleting the protected pack. Before that,
> .keep files had a clear semantic, and repack's documentation was correct.
>
> How do you explain .keep-files now? "protects pack-files that will forever
> be used"? Then why the hell is it called ".keep" instead of "eternal"?

The _intent_ behind .keep files has always been to avoid copying
the objects into a new pack during git-repack, because the objects
are held in a pack that cannot be deleted.

Sadly the implementation has never quite worked right, which is
what led to this patch series I think.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH/RFC v3] git add -i: Answer questions with a single keypress
From: Jeff King @ 2008-11-07 19:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk5bgiy18.fsf@gitster.siamese.dyndns.org>

On Thu, Nov 06, 2008 at 09:06:59AM -0800, Junio C Hamano wrote:

> Thanks.  And thanks, Jeff, for being a good reviewer.

You're welcome. I'm trying to lead by example. :)

-Peff

^ permalink raw reply

* Re: [PATCH/RFC v3] git add -i: Answer questions with a single keypress
From: Jeff King @ 2008-11-07 19:19 UTC (permalink / raw)
  To: Suraj N. Kurapati; +Cc: git
In-Reply-To: <200811060815.16797.sunaku@gmail.com>

On Thu, Nov 06, 2008 at 08:15:16AM -0800, Suraj N. Kurapati wrote:

> v1 and v2 make the mistake of setting raw mode, which prevent the user from 
> pressing Control-C to exit the program.  v3 fixes this by using cbreak mode.

OK, makes sense.

> The git-svn.perl script also uses Term::ReadKey.  Since it is already in the 
> git source repository, I thought it was okay to use Term::ReadKey.

We are not always consistent with such things, and git-add--interactive
is a little "more core" than git-svn, I think. Still, maybe I am being
overly cautious. We can always try it and see who complains. ;)

I think considering this improvement:

> >  - This only enhances one particular input, the patch loop. It is
> >    probably worth being consistent and allowing these behavior for other
> >    menus (though the numeric inputs are a bit trickier).
> 
> Understood.

it makes sense to factor the ReadKey procedure into a separate function
anyway. At which point it should be easy enough to substitute in the
original behavior if there is no ReadKey.

Side note: I don't know if such people exist, but I suppose it is
possible that somebody would _prefer_ the old behavior. In which case
factoring it out will also make it easier to predicate the behavior on a
config variable, if people want that.

-Peff

^ permalink raw reply

* [BUG] git-log --follow incompatibile with --max-count
From: Jakub Narebski @ 2008-11-07 18:38 UTC (permalink / raw)
  To: git; +Cc: Blucher, Guy

First, it is a bit strange that while "git log" understand '--follow'
option, "git rev-parse" (the plumbing equivalent to be used in scripts)
does not.


Second, somehow "git log --follow <filename>" is incompatible with
'-n' / '--max-count':

 $ git log --max-count=10 --follow --pretty=oneline --abbrev-commit gitweb/gitweb.perl 
 f24dce1... gitweb: Separate features with no project specific override
 61b4606... gitweb: Better processing format string in custom links in navbar

 $ git log --max-count=10 --pretty=oneline --abbrev-commit gitweb/gitweb.perl 
 f24dce1... gitweb: Separate features with no project specific override
 61b4606... gitweb: Better processing format string in custom links in navbar
 8db49a7... gitweb: generate parent..current URLs
 b0be383... gitweb: parse parent..current syntax from PATH_INFO
 3550ea7... gitweb: use_pathinfo filenames start with /
 b02bd7a... gitweb: generate project/action/hash URLs
 d8c2882... gitweb: parse project/action/hash_base:filename PATH_INFO
 1b2d297... gitweb: refactor input parameters parse/validation
 3e3d4ee... Merge branch 'pb/gitweb-tagcloud' into pb/gitweb
 0d1d154... gitweb: Support for simple project search form

 $ git log --max-count=10 --pretty=oneline --abbrev-commit gitweb/gitweb.perl | wc -l
 10

Without '--nax-count' it works correctly:

 $ git log --pretty=oneline --abbrev-commit gitweb/gitweb.perl | wc -l
 446

 $ git log --follow --pretty=oneline --abbrev-commit gitweb/gitweb.perl | wc -l
 485


Both bugs were noticed by Guy Blucher when writing 'historyfollow'
patch for gitweb:
  "[RFC] gitweb: add 'historyfollow' view that follows renames"
  Msg-ID: <054F21930D24A0428E5B4588462C7AED0149B4B8@ednex512.dsto.defence.gov.au>
  http://permalink.gmane.org/gmane.comp.version-control.git/99199
-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [minor usability suggestion] git rebase <upstream> --onto <newbase> ?
From: Junio C Hamano @ 2008-11-07 18:23 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: git
In-Reply-To: <20081107092035.GA1339@elte.hu>

Ingo Molnar <mingo@elte.hu> writes:

>  earth4:~/tip> git rebase 685aebb --onto linus
>  Current branch core/urgent is up to date.
> ...
> For example "git log" is exemplary in this area as it accepts just 
> about any permutation of parameters thrown at it, ...

Not really.

There may be oddball commands whose parameter order have been established
differently long before "git log" came, but almost all of them now use the
same convention used by the "git log" family, which is flags/options then
revs (and then paths if the operation makes sense with paths).  And rebase
is not one of the oddball ones.

Your "rebase" example is a rev then a flag/option, which is not how any
other git command take its command line parameters.

> Or at least we should print some sort of warning/error?

With that I agree with.  Patches? ;-)

^ permalink raw reply

* Re: multiple-commit cherry-pick?
From: Linus Torvalds @ 2008-11-07 18:08 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Junio C Hamano, Miles Bader, git
In-Reply-To: <20081107071231.GA4063@blimp.localdomain>



On Fri, 7 Nov 2008, Alex Riesen wrote:
> 
> Does not work if there are ranges given :-/
> It'd be very nice to have: git show #c1..$c2 $c3 $c4 $c5..$c6

Yeah, we've very fundamentally never supported that. Not for show, but 
also not for anything else (ie "gitk a..b c..d" does _not_ give you two 
ranges).

It's easy to see why once you understand what 'a..b' really means (ie it 
just expands to '^a' and 'b'), and how it's not really a "range" operation 
as much as a set operation that interacts with all the other arguments 
too. But unless you're very aware of that, it can be surprising.

		Linus

^ permalink raw reply

* Re: [PATCH] checkout: Don't crash when switching away from an invalid branch.
From: Johannes Schindelin @ 2008-11-07 18:05 UTC (permalink / raw)
  To: Alexandre Julliard; +Cc: git
In-Reply-To: <871vxnbhbh.fsf@wine.dyndns.org>

Hi,

On Fri, 7 Nov 2008, Alexandre Julliard wrote:

> I have a tree where for some reason HEAD was pointing to an invalid 
> commit. I'm not sure how this happened, but git checkout should be able 
> to recover from that situation without crashing.

Agree.

> diff --git a/builtin-checkout.c b/builtin-checkout.c
> index 57b94d2..7c1b8cd 100644
> --- a/builtin-checkout.c
> +++ b/builtin-checkout.c
> @@ -47,7 +47,8 @@ static int post_checkout_hook(struct commit *old, struct commit *new,
>  
>  	memset(&proc, 0, sizeof(proc));
>  	argv[0] = name;
> -	argv[1] = xstrdup(sha1_to_hex(old->object.sha1));
> +	argv[1] = old ? xstrdup(sha1_to_hex(old->object.sha1))
> +		      : "0000000000000000000000000000000000000000";

I guess you want to use the variable null_sha1 here.

> @@ -492,10 +493,13 @@ static void update_refs_for_switch(struct checkout_opts *opts,
>  	}
>  
>  	old_desc = old->name;
> -	if (!old_desc)
> +	if (!old_desc && old->commit)
>  		old_desc = sha1_to_hex(old->commit->object.sha1);
> -	strbuf_addf(&msg, "checkout: moving from %s to %s",
> -		    old_desc, new->name);
> +	if (old_desc)
> +		strbuf_addf(&msg, "checkout: moving from %s to %s",
> +			    old_desc, new->name);
> +	else
> +		strbuf_addf(&msg, "checkout: moving to %s", new->name);

Why not
		old_desc ? old_desc : "(invalid)"
?

> diff --git a/t/t2011-checkout-invalid-head.sh b/t/t2011-checkout-invalid-head.sh
> new file mode 100755

Nice!

Thank you,
Dscho

^ permalink raw reply

* [PATCH] checkout: Don't crash when switching away from an invalid branch.
From: Alexandre Julliard @ 2008-11-07 17:02 UTC (permalink / raw)
  To: git

I have a tree where for some reason HEAD was pointing to an invalid
commit. I'm not sure how this happened, but git checkout should be
able to recover from that situation without crashing.

Signed-off-by: Alexandre Julliard <julliard@winehq.org>
---
 builtin-checkout.c               |   14 +++++++++-----
 t/t2011-checkout-invalid-head.sh |   18 ++++++++++++++++++
 2 files changed, 27 insertions(+), 5 deletions(-)
 create mode 100755 t/t2011-checkout-invalid-head.sh

diff --git a/builtin-checkout.c b/builtin-checkout.c
index 57b94d2..7c1b8cd 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -47,7 +47,8 @@ static int post_checkout_hook(struct commit *old, struct commit *new,
 
 	memset(&proc, 0, sizeof(proc));
 	argv[0] = name;
-	argv[1] = xstrdup(sha1_to_hex(old->object.sha1));
+	argv[1] = old ? xstrdup(sha1_to_hex(old->object.sha1))
+		      : "0000000000000000000000000000000000000000";
 	argv[2] = xstrdup(sha1_to_hex(new->object.sha1));
 	argv[3] = changed ? "1" : "0";
 	argv[4] = NULL;
@@ -492,10 +493,13 @@ static void update_refs_for_switch(struct checkout_opts *opts,
 	}
 
 	old_desc = old->name;
-	if (!old_desc)
+	if (!old_desc && old->commit)
 		old_desc = sha1_to_hex(old->commit->object.sha1);
-	strbuf_addf(&msg, "checkout: moving from %s to %s",
-		    old_desc, new->name);
+	if (old_desc)
+		strbuf_addf(&msg, "checkout: moving from %s to %s",
+			    old_desc, new->name);
+	else
+		strbuf_addf(&msg, "checkout: moving to %s", new->name);
 
 	if (new->path) {
 		create_symref("HEAD", new->path, msg.buf);
@@ -551,7 +555,7 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
 	 * a new commit, we want to mention the old commit once more
 	 * to remind the user that it might be lost.
 	 */
-	if (!opts->quiet && !old.path && new->commit != old.commit)
+	if (!opts->quiet && !old.path && old.commit && new->commit != old.commit)
 		describe_detached_head("Previous HEAD position was", old.commit);
 
 	if (!old.commit) {
diff --git a/t/t2011-checkout-invalid-head.sh b/t/t2011-checkout-invalid-head.sh
new file mode 100755
index 0000000..764bb0a
--- /dev/null
+++ b/t/t2011-checkout-invalid-head.sh
@@ -0,0 +1,18 @@
+#!/bin/sh
+
+test_description='checkout switching away from an invalid branch'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo hello >world &&
+	git add world &&
+	git commit -m initial
+'
+
+test_expect_success 'checkout master from invalid HEAD' '
+	echo 0000000000000000000000000000000000000000 >.git/HEAD &&
+	git checkout master --
+'
+
+test_done
-- 
1.6.0.3.669.g76740

-- 
Alexandre Julliard
julliard@winehq.org

^ permalink raw reply related

* Re: [PATCH (GITK) v3 6/6] gitk: Explicitly position popup windows.
From: Johannes Sixt @ 2008-11-07 16:37 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Paul Mackerras, Alexander Gavrilov, git
In-Reply-To: <81b0412b0811070639q47244bc4t10791781fb0ec43b@mail.gmail.com>

Alex Riesen schrieb:
> 2008/11/7 Paul Mackerras <paulus@samba.org>:
>> Is there any reason to call tk::PlaceWindow under Linux or MacOS?
>> If not I'd rather add an if statement so we only call it on Windows.
>>
> 
> Yes, please. I rather like the smart placement in compiz.

Just out of curiosity because I don't use compiz: Did you mean
   "Yes, please call tk::PlaceWindow on Linux"
or
   "Yes, please add the if statement"
? ;)

-- Hannes

^ permalink raw reply

* Re: [PATCH (GITK) v3 6/6] gitk: Explicitly position popup windows.
From: Alex Riesen @ 2008-11-07 14:39 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: Alexander Gavrilov, git
In-Reply-To: <18708.11537.229423.296701@cargo.ozlabs.ibm.com>

2008/11/7 Paul Mackerras <paulus@samba.org>:
> Alexander Gavrilov writes:
>
>> For some reason, on Windows all transient windows are placed
>> in the upper left corner of the screen. Thus, it is necessary
>> to explicitly position the windows relative to their parent.
>> For simplicity this patch uses the function that is used
>> internally by Tk dialogs.
>
> Is there any reason to call tk::PlaceWindow under Linux or MacOS?
> If not I'd rather add an if statement so we only call it on Windows.
>

Yes, please. I rather like the smart placement in compiz.

^ permalink raw reply

* [PATCH 2/2] diffcore-rename: add config option to allow to cache renames
From: Nguyễn Thái Ngọc Duy @ 2008-11-07 14:35 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1226068533-10152-1-git-send-email-pclouds@gmail.com>

If diff.cacherenames is true, then renames will be cached to
$GIT_DIR/rename-cache. By default, it will not overwrite existing
cache. Add --refresh-cache to overwrite.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 if git-svn is going to use this, then perharps we should add a rule to prevent
 overwriting certain cache files with .keep files, so that git-svn generated cache
 does not get lost

 Documentation/config.txt       |    5 ++++
 Documentation/diff-options.txt |    5 ++++
 diff.c                         |   12 +++++++++++
 diff.h                         |    2 +
 diffcore-rename.c              |   41 ++++++++++++++++++++++++++++++++++++++++
 t/t4030-rename-cache.sh        |   27 ++++++++++++++++++++++++++
 6 files changed, 92 insertions(+), 0 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 29369d0..81160d3 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -630,6 +630,11 @@ diff.renames::
 	will enable basic rename detection.  If set to "copies" or
 	"copy", it will detect copies, as well.
 
+diff.cacherenames::
+	Tells git to automatically cache renames when detected. The
+	cache resides in $GIT_DIR/rename-cache, which is used by git
+	if exists.
+
 fetch.unpackLimit::
 	If the number of objects fetched over the git native
 	transfer is below this
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index c62b45c..d477a40 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -102,6 +102,11 @@ endif::git-format-patch[]
 	Turn off rename detection, even when the configuration
 	file gives the default to do so.
 
+--refresh-rename-cache::
+	By default, when git finds a cached version of a commit, it
+	will not overwrite the cache. This option makes git overwrite
+	old cache or create a new one.
+
 --check::
 	Warn if changes introduce trailing whitespace
 	or an indent that uses a space before a tab. Exits with
diff --git a/diff.c b/diff.c
index e368fef..1d65bd9 100644
--- a/diff.c
+++ b/diff.c
@@ -26,6 +26,7 @@ int diff_use_color_default = -1;
 static const char *external_diff_cmd_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
+static int diff_cache_renames;
 
 static char diff_colors[][COLOR_MAXLEN] = {
 	"\033[m",	/* reset */
@@ -109,6 +110,11 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 
+	if (!strcmp(var, "diff.cacherenames")) {
+		diff_cache_renames = git_config_bool(var, value);
+		return 0;
+	}
+
 	if (!prefixcmp(var, "diff.color.") || !prefixcmp(var, "color.diff.")) {
 		int slot = parse_diff_color_slot(var, 11);
 		if (!value)
@@ -2248,6 +2254,8 @@ int diff_setup_done(struct diff_options *options)
 
 	if (options->detect_rename && options->rename_limit < 0)
 		options->rename_limit = diff_rename_limit_default;
+	if (options->detect_rename)
+		options->cache_renames = diff_cache_renames;
 	if (options->setup & DIFF_SETUP_USE_CACHE) {
 		if (!active_cache)
 			/* read-cache does not die even when it fails
@@ -2415,6 +2423,10 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		DIFF_OPT_SET(options, RELATIVE_NAME);
 		options->prefix = arg + 11;
 	}
+	else if (!strcmp(arg, "--refresh-rename-cache")) {
+		options->cache_renames = 1;
+		options->refresh_rename_cache = 1;
+	}
 
 	/* xdiff options */
 	else if (!strcmp(arg, "-w") || !strcmp(arg, "--ignore-all-space"))
diff --git a/diff.h b/diff.h
index 8b68f6f..eb97955 100644
--- a/diff.h
+++ b/diff.h
@@ -85,6 +85,8 @@ struct diff_options {
 	int pickaxe_opts;
 	int rename_score;
 	int rename_limit;
+	int cache_renames;
+	int refresh_rename_cache;
 	int warn_on_too_large_rename;
 	int dirstat_percent;
 	int setup;
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 598cc8d..2b87e4e 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -527,6 +527,44 @@ static void load_rename_cache(struct diff_queue_struct *q,
 	free_hash(&filepair_table);
 }
 
+static void save_rename_cache(struct diff_queue_struct *outq,
+			      struct diff_options *options)
+{
+	int i;
+	FILE *fp = NULL;
+	struct stat st;
+
+	for (i = 0;i < outq->nr; i++) {
+		struct diff_filepair *dp = outq->queue[i];
+
+		if (!(dp->renamed_pair || /* rename pair */
+		      (!DIFF_FILE_VALID(dp->one) && DIFF_FILE_VALID(dp->two)))) /* create pair */
+			continue;
+
+		if (!fp) {
+			char *sha1 = sha1_to_hex(options->commit->object.sha1);
+			char *path = git_path("rename-cache/%c%c/%s", sha1[0], sha1[1], sha1+2);
+
+			/* Already cached. If not force refresh, move on */
+			if (!stat(path, &st) && !options->refresh_rename_cache)
+				return;
+
+			safe_create_leading_directories(path);
+			fp = fopen(path, "w");
+
+			if (!fp)
+				return;
+		}
+
+		fprintf(fp, "%s ", sha1_to_hex(dp->two->sha1));
+		fprintf(fp, "%s %d\n",
+			sha1_to_hex(DIFF_FILE_VALID(dp->one) ?  dp->one->sha1 : null_sha1),
+			dp->score);
+	}
+	if (fp)
+		fclose(fp);
+}
+
 void diffcore_rename(struct diff_options *options)
 {
 	int detect_rename = options->detect_rename;
@@ -770,6 +808,9 @@ void diffcore_rename(struct diff_options *options)
 	}
 	diff_debug_queue("done copying original", &outq);
 
+	if (options->commit && options->cache_renames)
+		save_rename_cache(&outq, options);
+
 	free(q->queue);
 	if (cacheq.queue)
 		free(cacheq.queue);
diff --git a/t/t4030-rename-cache.sh b/t/t4030-rename-cache.sh
index 0d8390c..24d3667 100755
--- a/t/t4030-rename-cache.sh
+++ b/t/t4030-rename-cache.sh
@@ -52,4 +52,31 @@ test_expect_success 'load create pair cache' '
 	test_cmp expected result
 '
 
+cat >expected <<EOF
+f2ad6c76f0115a6ba5b00456a849810e7ec0af20 0000000000000000000000000000000000000000 0
+78981922613b2afb6025042ff6bd878ac1994e85 78981922613b2afb6025042ff6bd878ac1994e85 60000
+EOF
+test_expect_success 'save rename cache' '
+	P=.git/rename-cache/$(git rev-parse HEAD|sed "s,\(..\)\(.*\),\1/\2,") &&
+	rm -r .git/rename-cache
+	git config diff.cacherenames true
+	git show --summary -C -M --find-copies-harder > /dev/null
+	test_cmp expected $P
+'
+
+test_expect_success 'subsequent command does not change cache' '
+	P=.git/rename-cache/$(git rev-parse HEAD|sed "s,\(..\)\(.*\),\1/\2,") &&
+	echo corrupted >> $P
+	! test_cmp expected $P &&
+	git show --summary -C -M --find-copies-harder HEAD > /dev/null &&
+	! test_cmp expected $P
+'
+
+test_expect_success 'overwrite cache with --refresh-rename-cache' '
+	P=.git/rename-cache/$(git rev-parse HEAD|sed "s,\(..\)\(.*\),\1/\2,") &&
+	! test_cmp expected $P &&
+	git show --summary -C -M --find-copies-harder --refresh-rename-cache HEAD > /dev/null &&
+	test_cmp expected $P
+'
+
 test_done
-- 
1.6.0.3.802.g47c38

^ permalink raw reply related

* [PATCH 1/2] diffcore-rename: support rename cache
From: Nguyễn Thái Ngọc Duy @ 2008-11-07 14:35 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

This patch teaches diffcore_rename() to look into
$GIT_DIR/rename-cache and make use of it to recreate diff_filepair.
With proper cache, there should be no available entry for estimation
after exact matching.

Rename caching is per commit. I don't think abitrary tree-tree caching
is worth it.

$GIT_DIR/rename-cache spans out like $GIT_DIR/objects. Each file
corresponds to one commit. Its content consists of lines like this

<Destination SHA-1> <SPC> <Source SHA-1> <SPC> <Score in decimal> <NL>

This can be used to:

 - Make --find-copies-harder pratically usable for moderate-size
   repositories. The first "git show" on a linux kernel commit was 5.3
   sec, it then went down to 0.13 sec.
 - Give git-svn a chance to (locally) import explicit renames from
   Subversion
 - People may correct rename results for better diff, if automatic
   rename detection is not good enough.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 diff.h                  |    2 +
 diffcore-rename.c       |  142 ++++++++++++++++++++++++++++++++++++++++++++++-
 log-tree.c              |    2 +
 t/t4030-rename-cache.sh |   55 ++++++++++++++++++
 4 files changed, 199 insertions(+), 2 deletions(-)
 create mode 100755 t/t4030-rename-cache.sh

diff --git a/diff.h b/diff.h
index a49d865..8b68f6f 100644
--- a/diff.h
+++ b/diff.h
@@ -110,6 +110,8 @@ struct diff_options {
 	add_remove_fn_t add_remove;
 	diff_format_fn_t format_callback;
 	void *format_callback_data;
+
+	struct commit *commit;
 };
 
 enum color_diff {
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 168a95b..598cc8d 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -5,6 +5,7 @@
 #include "diff.h"
 #include "diffcore.h"
 #include "hash.h"
+#include "commit.h"
 
 /* Table of rename/copy destinations */
 
@@ -409,13 +410,130 @@ static void record_if_better(struct diff_score m[], struct diff_score *o)
 		m[worst] = *o;
 }
 
+struct cached_filepair {
+	unsigned char dst[20];
+	unsigned char src[20];
+	int score;
+};
+
+static int free_cached_filepair(void *p)
+{
+	free(p);
+	return 0;
+}
+
+static void load_rename_cache(struct diff_queue_struct *q,
+			      struct diff_queue_struct *cacheq,
+			      struct diff_options *options)
+{
+	char *sha1_hex;
+	FILE *fp;
+	struct hash_table filepair_table;
+	struct hash_table src_table;
+	struct cached_filepair *pp;
+	int i, hash;
+	static int no_cache_available = -1;
+	struct stat st;
+	char *path;
+
+	if (no_cache_available == -1)
+		no_cache_available = stat(git_path("rename-cache"), &st) || !S_ISDIR(st.st_mode);
+
+	/* return soon so we don't need to waste CPU */
+	if (no_cache_available > 0)
+		return;
+
+
+	/* src_table initialization */
+	init_hash(&src_table);
+	for (i = 0; i < q->nr; i++) {
+		struct diff_filepair *p = q->queue[i];
+		if (DIFF_FILE_VALID(p->one)) {
+			unsigned int hash = hash_filespec(p->one);
+			insert_hash(hash, p, &src_table);
+		}
+	}
+
+	/* filepair_table initialization */
+	init_hash(&filepair_table);
+	sha1_hex = sha1_to_hex(options->commit->object.sha1);
+	path = git_path("rename-cache/%c%c/%s",sha1_hex[0], sha1_hex[1], sha1_hex+2);
+	if (stat(path, &st))
+		fp = NULL;
+	else
+		fp = fopen(path, "r");
+	if (fp) {
+		char src_sha1_hex[41], dst_sha1_hex[41];
+		struct cached_filepair p;
+
+		src_sha1_hex[40] = dst_sha1_hex[40] = '\0';
+		while (fscanf(fp, "%40c %40c %d\n", dst_sha1_hex, src_sha1_hex, &p.score) == 3) {
+			if (get_sha1_hex(src_sha1_hex, p.src) ||
+			    get_sha1_hex(dst_sha1_hex, p.dst))
+				break;
+
+			pp = xmalloc(sizeof(*pp));
+			memcpy(pp, &p, sizeof(*pp));
+			memcpy(&hash, p.dst, sizeof(hash));
+			insert_hash(hash, pp, &filepair_table);
+		}
+		fclose(fp);
+	}
+
+	for (i = 0; i < q->nr; i++) {
+		struct diff_filepair *p = q->queue[i];
+		struct diff_filepair *dp, *src;
+
+		/* find remote_dst */
+		if (DIFF_FILE_VALID(p->one) ||
+		    !DIFF_FILE_VALID(p->two) ||
+		    (options->single_follow && strcmp(options->single_follow, p->two->path)))
+			continue;
+
+		memcpy(&hash, p->two->sha1, sizeof(hash));
+		pp = lookup_hash(hash, &filepair_table);
+		if (!pp || memcmp(p->two->sha1, pp->dst, 20))
+			continue;
+
+		/* create pair */
+		if (is_null_sha1(pp->src)) {
+			if (DIFF_FILE_VALID(p->one))
+				continue;
+			diff_q(cacheq, p);
+			q->queue[i] = NULL;
+			continue;
+		}
+
+		memcpy(&hash, pp->src, sizeof(hash));
+		src = lookup_hash(hash, &src_table);
+		if (!src || memcmp(pp->src, src->one->sha1, 20))
+			continue;
+
+		src->one->rename_used++;
+		src->one->count++;
+		p->two->count++;
+
+		dp = diff_queue(NULL, src->one, p->two);
+		dp->renamed_pair = 1;
+		dp->score = pp->score;
+
+		diff_q(cacheq, dp);
+		q->queue[i] = NULL;
+		diff_free_filepair(p);
+	}
+
+	for_each_hash(&filepair_table, free_cached_filepair);
+	free_hash(&src_table);
+	free_hash(&filepair_table);
+}
+
 void diffcore_rename(struct diff_options *options)
 {
 	int detect_rename = options->detect_rename;
 	int minimum_score = options->rename_score;
 	int rename_limit = options->rename_limit;
 	struct diff_queue_struct *q = &diff_queued_diff;
-	struct diff_queue_struct outq;
+	struct diff_queue_struct outq, cacheq;
 	struct diff_score *mx;
 	int i, j, rename_count;
 	int num_create, num_src, dst_cnt;
@@ -423,8 +541,19 @@ void diffcore_rename(struct diff_options *options)
 	if (!minimum_score)
 		minimum_score = DEFAULT_RENAME_SCORE;
 
+	cacheq.queue = NULL;
+	cacheq.nr = cacheq.alloc = 0;
+
+	if (detect_rename && options->commit)
+		load_rename_cache(q, &cacheq, options);
+
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
+
+		/* was consumed by rename cache */
+		if (!p)
+			continue;
+
 		if (!DIFF_FILE_VALID(p->one)) {
 			if (!DIFF_FILE_VALID(p->two))
 				continue; /* unmerged */
@@ -563,10 +692,17 @@ void diffcore_rename(struct diff_options *options)
 	 */
 	outq.queue = NULL;
 	outq.nr = outq.alloc = 0;
-	for (i = 0; i < q->nr; i++) {
+	for (i = j = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
 		struct diff_filepair *pair_to_free = NULL;
 
+		if (!p) {
+			if (j >= cacheq.nr)
+				die("Internal error: running out of cacheq.");
+			diff_q(&outq, cacheq.queue[j++]);
+			continue;
+		}
+
 		if (!DIFF_FILE_VALID(p->one) && DIFF_FILE_VALID(p->two)) {
 			/*
 			 * Creation
@@ -635,6 +771,8 @@ void diffcore_rename(struct diff_options *options)
 	diff_debug_queue("done copying original", &outq);
 
 	free(q->queue);
+	if (cacheq.queue)
+		free(cacheq.queue);
 	*q = outq;
 	diff_debug_queue("done collapsing", q);
 
diff --git a/log-tree.c b/log-tree.c
index cec3c06..a67ef6d 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -518,6 +518,7 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
 	log.commit = commit;
 	log.parent = NULL;
 	opt->loginfo = &log;
+	opt->diffopt.commit = commit;
 
 	shown = log_tree_diff(opt, commit, &log);
 	if (!shown && opt->loginfo && opt->always_show_header) {
@@ -527,5 +528,6 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
 	}
 	opt->loginfo = NULL;
 	maybe_flush_or_die(stdout, "stdout");
+	opt->diffopt.commit = NULL;
 	return shown;
 }
diff --git a/t/t4030-rename-cache.sh b/t/t4030-rename-cache.sh
new file mode 100755
index 0000000..0d8390c
--- /dev/null
+++ b/t/t4030-rename-cache.sh
@@ -0,0 +1,55 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Nguyen Thai Ngoc Duy
+#
+
+test_description='Test diff rename cache'
+. ./test-lib.sh
+
+cat >expected <<EOF
+ create mode 100644 c
+ copy a => d (100%)
+EOF
+test_expect_success 'setup' '
+	for i in a b c
+	do
+		echo $i > $i
+	done
+	cp a d
+	A_SHA1=$(git hash-object a)
+	B_SHA1=$(git hash-object b)
+	C_SHA1=$(git hash-object c)
+	D_SHA1=$(git hash-object d)
+	git add a b
+	git commit -m first
+	git add c d
+	git commit -m second
+	git show --pretty=oneline --summary -C -M --find-copies-harder HEAD|sed 1d > result
+	test_cmp expected result
+'
+
+cat >expected <<EOF
+ copy a => c (100%)
+ copy a => d (100%)
+EOF
+test_expect_success 'load rename pair cache' '
+	P=.git/rename-cache/$(git rev-parse HEAD|sed "s,\(..\)\(.*\),\1/\2,") &&
+	mkdir -p $(dirname $P)
+	echo $C_SHA1 $A_SHA1 60000 >> $P
+	git show --pretty=oneline --summary -C -M --find-copies-harder HEAD|sed 1d > result
+	test_cmp expected result
+'
+
+cat >expected <<EOF
+ copy a => c (100%)
+ create mode 100644 d
+EOF
+test_expect_success 'load create pair cache' '
+	P=.git/rename-cache/$(git rev-parse HEAD|sed "s,\(..\)\(.*\),\1/\2,") &&
+	mkdir -p $(dirname $P)
+	echo $D_SHA1 0000000000000000000000000000000000000000 0 >> $P
+	git show --pretty=oneline --summary -C -M --find-copies-harder HEAD|sed 1d > result
+	test_cmp expected result
+'
+
+test_done
-- 
1.6.0.3.802.g47c38

^ permalink raw reply related

* Re: Git and Media repositories....
From: Santi Béjar @ 2008-11-07 13:19 UTC (permalink / raw)
  To: Tim Ansell; +Cc: git
In-Reply-To: <1225655428.11693.10.camel@vaio>

On Sun, Nov 2, 2008 at 8:50 PM, Tim Ansell <mithro@mithis.com> wrote:
> Hey guys,
>

[...]

>
> The general idea is that we always clone the complete meta-data (tags,
> commits and trees) and then only clone blobs when they are needed (using
> something like alternates). This allows us to support shallow, narrow
> and sparse checkouts while still being able to perform operations such
> as committing and merging.
>

A related use case could be to remove a blob from a repo but being
able to work normally with it, similar to:

http://wiki.freebsd.org/VCSFeatureObliterate

Santi

^ permalink raw reply

* [PATCH] Remove the period after the git-check-attr summary
From: Matt Kraai @ 2008-11-07 12:26 UTC (permalink / raw)
  To: git, gitster; +Cc: Matt Kraai

Signed-off-by: Matt Kraai <kraai@ftbfs.org>
---
The period at the end of the git-check-attr summary causes there to be
two periods after the summary in the git(1) manual page, so remove it.

 Documentation/git-check-attr.txt |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-check-attr.txt b/Documentation/git-check-attr.txt
index 256659a..8c2ac12 100644
--- a/Documentation/git-check-attr.txt
+++ b/Documentation/git-check-attr.txt
@@ -3,7 +3,7 @@ git-check-attr(1)
 
 NAME
 ----
-git-check-attr - Display gitattributes information.
+git-check-attr - Display gitattributes information
 
 
 SYNOPSIS
-- 
1.5.6.5

^ permalink raw reply related

* Re: Git and Media repositories....
From: Jakub Narebski @ 2008-11-07 13:00 UTC (permalink / raw)
  To: Tim Ansell; +Cc: git
In-Reply-To: <1225655428.11693.10.camel@vaio>

Tim Ansell <mithro@mithis.com> writes:

> Last week at the gittogether I lead some discussions about how we could
> make Git better support large media repositories (which is one area
> where Subversion still make sense). It was suggested that I post to this
> list to get a discussion going. 
> 
> The general idea is that we always clone the complete meta-data (tags,
> commits and trees) and then only clone blobs when they are needed (using
> something like alternates). This allows us to support shallow, narrow
> and sparse checkouts while still being able to perform operations such
> as committing and merging.
[...]

Well, the *workaround* you could currently use is to put large media
files in separate subdirectory, and make this subdirectory into
submodule.  This uses the fact that you can selectively clone
submodules, or leave them as a stubs...

...and this is also the code you might want to look at when
implementings stubs for 'remote' blob objects

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH 1/2] Introduce rename factorization in diffcore.
From: Jakub Narebski @ 2008-11-07 12:55 UTC (permalink / raw)
  To: Baz; +Cc: Yann Dirson, git
In-Reply-To: <2faad3050811070328t31babed4v1ba895a6ac36df94@mail.gmail.com>

Baz <brian.ewins@gmail.com> writes:
> 2008/10/30 Yann Dirson <ydirson@altern.org>:

> > Rename factorization tries to group together files moving from and to
> > identical directories - the most common case being directory renames.
> > This feature is activated by the new --factorize-renames diffcore
> > flag.
> 
> Sorry to bikeshed a bit here, but this isn't what 'factorize' means,
> and adding a flag with this name unnecessarily adds to the
> git-specific terms users have to learn.

Well, I think from _mathematical_ (arithmetic) point of view it makes
perfect sense.  Before you had:

  (rename-of-sub1-file1 rename-of-sub1-file2 rename-of-sub1-file3)

and after you have

  (rename-of-sub1) * (changes in files)
 
> Looking back through the archives, there's only a few people who've
> used the word 'factorize', and /mostly/ it seems to have been used as
> a synonym for 'refactor' in comments; not common usage but
> understandable. However in this case, factorize is being used in the
> opposite sense from its dictionary definition - to break down into
> factors - and instead is being used to mean to /combine/ things; I
> don't think that should be in the UI.
> 
> Why not just '--group-renames'?

That said, I think that '--group-renames' makes better sense (and is
shorted than '--detect-directory-renames')

+1 for '--group-renames'

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH (GITK) v3 6/6] gitk: Explicitly position popup windows.
From: Paul Mackerras @ 2008-11-07 11:57 UTC (permalink / raw)
  To: Alexander Gavrilov; +Cc: git
In-Reply-To: <1225652389-22082-7-git-send-email-angavrilov@gmail.com>

Alexander Gavrilov writes:

> For some reason, on Windows all transient windows are placed
> in the upper left corner of the screen. Thus, it is necessary
> to explicitly position the windows relative to their parent.
> For simplicity this patch uses the function that is used
> internally by Tk dialogs.

Is there any reason to call tk::PlaceWindow under Linux or MacOS?
If not I'd rather add an if statement so we only call it on Windows.

Paul.

^ permalink raw reply

* Re: [PATCH (GITK) v3 2/6] gitk: Make gitk dialog windows transient.
From: Paul Mackerras @ 2008-11-07 11:41 UTC (permalink / raw)
  To: Alexander Gavrilov; +Cc: git
In-Reply-To: <1225652389-22082-3-git-send-email-angavrilov@gmail.com>

Alexander Gavrilov writes:

> Transient windows are always kept above their parent,
> and don't occupy any space in the taskbar, which is useful
> for dialogs. Also, when transient is used, it is important
> to bind windows to the correct parent.
> 
> This commit adds transient annotations to all dialogs,
> ensures usage of the correct parent for error and
> confirmation popups, and, as a side job, makes gitk
> preserve the create tag dialog window in case of errors.
> 
> Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>

Thanks, applied.

Paul.

^ permalink raw reply

* Re: [PATCH (GITK) v3 4/6] gitk: Make cherry-pick call git-citool on conflicts.
From: Paul Mackerras @ 2008-11-07 11:51 UTC (permalink / raw)
  To: Alexander Gavrilov; +Cc: git
In-Reply-To: <1225652389-22082-5-git-send-email-angavrilov@gmail.com>

Alexander Gavrilov writes:

> Now that git-gui has facilities to help users resolve
> conflicts, it makes sense to launch it from other GUI
> tools when they happen.

The resolution capabilities of git citool seem to be that it detects
the conflict markers and lets you run meld on the 3 versions.  Have I
missed anything there?  Do people find that fixing things manually in
meld is sufficient, or do we really want something more powerful?

Paul.

^ permalink raw reply

* Re: [PATCH (GITK) v3 1/6] gitk: Add Return and Escape bindings to dialogs.
From: Paul Mackerras @ 2008-11-07 11:41 UTC (permalink / raw)
  To: Alexander Gavrilov; +Cc: git
In-Reply-To: <1225652389-22082-2-git-send-email-angavrilov@gmail.com>

Alexander Gavrilov writes:

> It is often more convenient to dismiss or accept a
> dialog using the keyboard, than by clicking buttons
> on the screen. This commit adds key binding to make
> it possible with gitk's dialogs.

Thanks, applied.

Paul.

^ permalink raw reply

* Re: [PATCH (GITK) v3 3/6] gitk: Add accelerators to frequently used menu commands.
From: Paul Mackerras @ 2008-11-07 11:50 UTC (permalink / raw)
  To: Alexander Gavrilov; +Cc: git
In-Reply-To: <1225652389-22082-4-git-send-email-angavrilov@gmail.com>

Alexander Gavrilov writes:

> -	eval $m add $params [lrange $i 4 end]
> +	set tail [lrange $i 4 end]
> +	regsub -all {\$M1T\y} $tail $M1T tail
> +	eval $m add $params $tail

This is solving the problem that the $M1T doesn't get expanded in the
call below because it's inside {}.  If we are going to have a magic
string that gets expanded like this, I'd rather it didn't look like a
variable reference, because that is confusing.

Alternatively, we could define a [meta] function that does this:

proc meta {x} {
    if {[tk windowingsystem] eq "aqua"} {
	return Cmd-$x
    }
    return Ctrl-$x
}

and then use -accelerator [meta F5] in the makemenu call, and not have
any magic string substitution in makemenu.  That will work since the
eval will evaluate the [].

Paul.

^ permalink raw reply

* Re: multiple-commit cherry-pick?
From: Miles Bader @ 2008-11-07 11:46 UTC (permalink / raw)
  To: Björn Steinbrink; +Cc: Deskin Miller, git
In-Reply-To: <20081107110331.GA2938@atjola.homenet>

>> >> > git reset --hard C
>> >> > git rebase --onto ORIG_HEAD A^

>> Hmm, I guess using rebase --abort isn't a very good idea in this case
>> though... :-/
>
> Why not? I mean, ok, you end up at C, and not where you have been before
> the reset --hard, but there's the reflog to help you get back to
> whatever previous state of the branch it is that you want.

I just mean it's not a trivial way to get back to the state before the
multi-cherry-pick -- you need to know the details of what's going on,
and handle the rest of the cleanup manually.

So, for instance, if you were to package up the above commands in a
shell script, the abort issue is one of those rough edges which would
prevent it from being as convenient as a real git command.  [A
hypothetical extension of the cherry-pick command to handle multiple
commits would presumably offer a "cherry-pick --abort" option that did
everything magically.]

-Miles

-- 
Do not taunt Happy Fun Ball.

^ permalink raw reply

* Re: [PATCH 1/2] Introduce rename factorization in diffcore.
From: Baz @ 2008-11-07 11:28 UTC (permalink / raw)
  To: Yann Dirson; +Cc: git
In-Reply-To: <20081030221645.3325.78288.stgit@gandelf.nowhere.earth>

2008/10/30 Yann Dirson <ydirson@altern.org>:
> Rename factorization tries to group together files moving from and to
> identical directories - the most common case being directory renames.
> This feature is activated by the new --factorize-renames diffcore
> flag.

Sorry to bikeshed a bit here, but this isn't what 'factorize' means,
and adding a flag with this name unnecessarily adds to the
git-specific terms users have to learn.

Looking back through the archives, there's only a few people who've
used the word 'factorize', and /mostly/ it seems to have been used as
a synonym for 'refactor' in comments; not common usage but
understandable. However in this case, factorize is being used in the
opposite sense from its dictionary definition - to break down into
factors - and instead is being used to mean to /combine/ things; I
don't think that should be in the UI.

Why not just '--group-renames'?

Cheers,
Baz

>
> This is only the first step, adding the basic functionnality and
> adding support to raw diff output (and it breaks unified-diff output
> which does not know how to handle that stuff yet).
>
> Even the output format may not be kept as is.  For now both the result
> of "mv a b" and "mv a/* b/" are displayed as "Rnnn a/ b/", which is
> probably not what we want.  "Rnnn a/* b/" could be a good choice for
> the latter if we want them to be distinguished, and even if we want
> them to look the same.
>
> Other future developements to be made on top of this include:
> * extension of unified-diff format to express this
> * application of such new diffs
> * teach git-svn (and others ?) to make use of that flag
> * merge correctly in case of addition into a moved dir
> * detect "directory splits" so merge can flag a conflict on file adds
> * use inexact dir renames to bump score of below-threshold renames
>  from/to same locations
> * add yours here
> ---
>
>  diff-lib.c        |    6 +
>  diff.c            |    5 +
>  diff.h            |    3 +
>  diffcore-rename.c |  227 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>  diffcore.h        |    1
>  tree-diff.c       |    4 +
>  6 files changed, 235 insertions(+), 11 deletions(-)
>
> diff --git a/diff-lib.c b/diff-lib.c
> index ae96c64..dcc4c2c 100644
> --- a/diff-lib.c
> +++ b/diff-lib.c
> @@ -179,7 +179,8 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
>                changed = ce_match_stat(ce, &st, ce_option);
>                if (!changed) {
>                        ce_mark_uptodate(ce);
> -                       if (!DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
> +                       if (!DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER) &&
> +                           !DIFF_OPT_TST(&revs->diffopt, FACTORIZE_RENAMES))
>                                continue;
>                }
>                oldmode = ce->ce_mode;
> @@ -310,7 +311,8 @@ static int show_modified(struct oneway_unpack_data *cbdata,
>
>        oldmode = old->ce_mode;
>        if (mode == oldmode && !hashcmp(sha1, old->sha1) &&
> -           !DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
> +           !DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER) &&
> +           !DIFF_OPT_TST(&revs->diffopt, FACTORIZE_RENAMES))
>                return 0;
>
>        diff_change(&revs->diffopt, oldmode, mode,
> diff --git a/diff.c b/diff.c
> index e368fef..f91fcf6 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -2437,6 +2437,11 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
>                DIFF_OPT_SET(options, REVERSE_DIFF);
>        else if (!strcmp(arg, "--find-copies-harder"))
>                DIFF_OPT_SET(options, FIND_COPIES_HARDER);
> +       else if (!strcmp(arg, "--factorize-renames")) {
> +               DIFF_OPT_SET(options, FACTORIZE_RENAMES);
> +               if (!options->detect_rename)
> +                       options->detect_rename = DIFF_DETECT_RENAME;
> +       }
>        else if (!strcmp(arg, "--follow"))
>                DIFF_OPT_SET(options, FOLLOW_RENAMES);
>        else if (!strcmp(arg, "--color"))
> diff --git a/diff.h b/diff.h
> index a49d865..db1658b 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -65,6 +65,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
>  #define DIFF_OPT_IGNORE_SUBMODULES   (1 << 18)
>  #define DIFF_OPT_DIRSTAT_CUMULATIVE  (1 << 19)
>  #define DIFF_OPT_DIRSTAT_BY_FILE     (1 << 20)
> +#define DIFF_OPT_FACTORIZE_RENAMES   (1 << 21)
>  #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
>  #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)
>  #define DIFF_OPT_CLR(opts, flag)    ((opts)->flags &= ~DIFF_OPT_##flag)
> @@ -220,6 +221,8 @@ extern void diffcore_std(struct diff_options *);
>  "  -C            detect copies.\n" \
>  "  --find-copies-harder\n" \
>  "                try unchanged files as candidate for copy detection.\n" \
> +"  --factorize-renames\n" \
> +"                factorize renames of all files of a directory.\n" \
>  "  -l<n>         limit rename attempts up to <n> paths.\n" \
>  "  -O<file>      reorder diffs according to the <file>.\n" \
>  "  -S<string>    find filepair whose only one side contains the string.\n" \
> diff --git a/diffcore-rename.c b/diffcore-rename.c
> index 1b2ebb4..fc789bc 100644
> --- a/diffcore-rename.c
> +++ b/diffcore-rename.c
> @@ -52,6 +52,32 @@ static struct diff_rename_dst *locate_rename_dst(struct diff_filespec *two,
>        return &(rename_dst[first]);
>  }
>
> +static struct diff_rename_dst *locate_rename_dst_dir(struct diff_filespec *dir)
> +{
> +       /* code mostly duplicated from locate_rename_dst - not sure we
> +        * could merge them efficiently,though
> +        */
> +       int first, last;
> +       int dirlength = strlen(dir->path);
> +
> +       first = 0;
> +       last = rename_dst_nr;
> +       while (last > first) {
> +               int next = (last + first) >> 1;
> +               struct diff_rename_dst *dst = &(rename_dst[next]);
> +               int cmp = strncmp(dir->path, dst->two->path, dirlength);
> +               if (!cmp)
> +                       return dst;
> +               if (cmp < 0) {
> +                       last = next;
> +                       continue;
> +               }
> +               first = next+1;
> +       }
> +       /* not found */
> +       return NULL;
> +}
> +
>  /* Table of rename/copy src files */
>  static struct diff_rename_src {
>        struct diff_filespec *one;
> @@ -409,6 +435,165 @@ static void record_if_better(struct diff_score m[], struct diff_score *o)
>                m[worst] = *o;
>  }
>
> +struct diff_dir_rename {
> +       struct diff_filespec *one;
> +       struct diff_filespec *two;
> +       int discarded;
> +       struct diff_dir_rename* next;
> +};
> +
> +/*
> + * Marks as such file_rename if it is made uninteresting by dir_rename.
> + * Returns -1 if the file_rename is outside of the range in which given
> + * renames concerned by dir_rename are to be found (ie. end of loop),
> + * 0 otherwise.
> + */
> +static int maybe_mark_uninteresting(struct diff_rename_dst* file_rename,
> +                                   struct diff_dir_rename* dir_rename,
> +                                   int one_len, int two_len)
> +{
> +       if (!file_rename->pair) /* file add */
> +               return 0;
> +       if (strncmp(file_rename->two->path,
> +                   dir_rename->two->path, two_len))
> +               return -1;
> +       if (strncmp(file_rename->pair->one->path,
> +                   dir_rename->one->path, one_len) ||
> +           !basename_same(file_rename->pair->one, file_rename->two) ||
> +           file_rename->pair->score != MAX_SCORE)
> +               return 0;
> +
> +       file_rename->pair->uninteresting_rename = 1;
> +       fprintf (stderr, "[DBG] %s* -> %s* makes %s -> %s uninteresting\n",
> +               dir_rename->one->path, dir_rename->two->path,
> +               file_rename->pair->one->path, file_rename->two->path);
> +       return 0;
> +}
> +
> +// FIXME: prevent possible overflow
> +/*
> + * Copy dirname of src into dst, with final "/".
> + * Only handles relative paths since there is no relative path in a git repo.
> + * Writes "./" when there is no "/" in src.
> + * May overwrite more chars than really needed, if src ends with a "/".
> + */
> +static const char* copy_dirname(char* dst, const char* src)
> +{
> +       char* lastslash = strrchr(src, '/');
> +       if (!lastslash)
> +               return strcpy(dst, "./");
> +       strncpy(dst, src, lastslash - src + 1);
> +       dst[lastslash - src + 1] = '\0';
> +
> +       // if src ends with a "/" strip the last component
> +       if (lastslash[1] == '\0') {
> +               lastslash = strrchr(dst, '/');
> +               if (!lastslash)
> +                       return strcpy(dst, ".");
> +               lastslash[1] = '\0';
> +       }
> +
> +       return dst;
> +}
> +
> +/*
> + * FIXME: we could optimize the 100%-rename case by preventing
> + * recursion to unfold what we know we would refold here.
> + * FIXME: do we want to replace linked list with sorted array ?
> + * FIXME: this prototype only handles renaming of dirs without
> + * a subdir.
> + * FIXME: leaks like hell.
> + * FIXME: ideas to evaluate a similarity score, anyone ?
> + *  10% * tree similarity + 90% * moved files similarity ?
> + */
> +static struct diff_dir_rename* factorization_candidates = NULL;
> +static void diffcore_factorize_renames(void)
> +{
> +       char one_parent_path[PATH_MAX], two_parent_path[PATH_MAX];
> +       int i;
> +
> +       for (i = 0; i < rename_dst_nr; i++) {
> +               // FIXME: what are those ?
> +               if (!rename_dst[i].pair)
> +                       continue;
> +               // dummy renames used by copy detection
> +               if (!strcmp(rename_dst[i].pair->one->path, rename_dst[i].pair->two->path))
> +                       continue;
> +
> +               copy_dirname(one_parent_path, rename_dst[i].pair->one->path);
> +
> +               struct diff_filespec* one_parent = alloc_filespec(one_parent_path);
> +               fill_filespec(one_parent, null_sha1 /*FIXME*/, S_IFDIR);
> +
> +               if (!locate_rename_dst_dir(one_parent)) {
> +                       // one_parent_path is empty in result tree
> +
> +                       // already considered ?
> +                       struct diff_dir_rename* seen;
> +                       for (seen=factorization_candidates; seen; seen = seen->next)
> +                               if (!strcmp(seen->one->path, one_parent_path)) break;
> +                       if (!seen) {
> +                               // record potential dir rename
> +                               copy_dirname(two_parent_path, rename_dst[i].pair->two->path);
> +
> +                               seen = xmalloc(sizeof(*seen));
> +                               seen->one = one_parent;
> +                               seen->two = alloc_filespec(two_parent_path);
> +                               fill_filespec(seen->two, null_sha1 /*FIXME*/, S_IFDIR);
> +                               seen->discarded = 0;
> +                               seen->next = factorization_candidates;
> +                               factorization_candidates = seen;
> +                               fprintf (stderr, "[DBG] %s -> %s suggests possible rename from %s to %s\n",
> +                                      rename_dst[i].pair->one->path,
> +                                      rename_dst[i].pair->two->path,
> +                                      one_parent_path, two_parent_path);
> +                               fflush(stdout);
> +                               continue;
> +                       }
> +                       if (seen->discarded)
> +                               continue;
> +                       // check that seen entry matches this rename
> +                       copy_dirname(two_parent_path, rename_dst[i].pair->two->path);
> +                       if (strcmp(two_parent_path, seen->two->path)) {
> +                               fprintf (stderr, "[DBG] discarding dir split of %s from renames (into %s and %s)\n",
> +                                      one_parent_path, two_parent_path, seen->two->path);
> +                               seen->discarded = 1;
> +                       }
> +
> +                       /* all checks ok, we keep that entry */
> +               }
> +       }
> +
> +       // turn candidates into real renames
> +       struct diff_dir_rename* candidate;
> +       for (candidate=factorization_candidates; candidate; candidate = candidate->next) {
> +               int two_idx, i, one_len, two_len;
> +               if (candidate->discarded)
> +                       continue;
> +
> +               if (!locate_rename_dst_dir(candidate->two)) {
> +                       fprintf (stderr, "PANIC: %s candidate of rename not in target tree (from %s)\n",
> +                               candidate->two->path, candidate->one->path);
> +               }
> +               // bisect to an entry within candidate dst dir
> +               two_idx = locate_rename_dst_dir(candidate->two) - rename_dst;
> +
> +               // now remove extraneous 100% files inside.
> +               one_len = strlen(candidate->one->path);
> +               two_len = strlen(candidate->two->path);
> +               for (i = two_idx; i < rename_dst_nr; i++)
> +                       if (maybe_mark_uninteresting (rename_dst+i, candidate,
> +                                                     one_len, two_len) < 0)
> +                               break;
> +               for (i = two_idx-1; i >= 0; i--)
> +                       if (maybe_mark_uninteresting (rename_dst+i, candidate,
> +                                                     one_len, two_len) < 0)
> +                               break;
> +       }
> +
> +       return;
> +}
> +
>  void diffcore_rename(struct diff_options *options)
>  {
>        int detect_rename = options->detect_rename;
> @@ -446,13 +631,22 @@ void diffcore_rename(struct diff_options *options)
>                                p->one->rename_used++;
>                        register_rename_src(p->one, p->score);
>                }
> -               else if (detect_rename == DIFF_DETECT_COPY) {
> -                       /*
> -                        * Increment the "rename_used" score by
> -                        * one, to indicate ourselves as a user.
> -                        */
> -                       p->one->rename_used++;
> -                       register_rename_src(p->one, p->score);
> +               else {
> +                       if (detect_rename == DIFF_DETECT_COPY) {
> +                               /*
> +                                * Increment the "rename_used" score by
> +                                * one, to indicate ourselves as a user.
> +                                */
> +                               p->one->rename_used++;
> +                               register_rename_src(p->one, p->score);
> +                       }
> +                       if (DIFF_OPT_TST(options, FACTORIZE_RENAMES)) {
> +                               /* similarly, rename factorization needs to
> +                                * see all files from second tree
> +                                */
> +                               //p->two->rename_used++; // FIXME: would we need that ?
> +                               locate_rename_dst(p->two, 1);
> +                       }
>                }
>        }
>        if (rename_dst_nr == 0 || rename_src_nr == 0)
> @@ -561,8 +755,24 @@ void diffcore_rename(struct diff_options *options)
>        /* At this point, we have found some renames and copies and they
>         * are recorded in rename_dst.  The original list is still in *q.
>         */
> +
> +       /* Now possibly factorize those renames and copies. */
> +       if (DIFF_OPT_TST(options, FACTORIZE_RENAMES))
> +               diffcore_factorize_renames();
> +
>        outq.queue = NULL;
>        outq.nr = outq.alloc = 0;
> +
> +       // first, turn factorization_candidates into real renames
> +       struct diff_dir_rename* candidate;
> +       for (candidate=factorization_candidates; candidate; candidate = candidate->next) {
> +               struct diff_filepair* pair;
> +               if (candidate->discarded) continue;
> +               pair = diff_queue(&outq, candidate->one, candidate->two);
> +               pair->score = MAX_SCORE;
> +               pair->renamed_pair = 1;
> +       }
> +
>        for (i = 0; i < q->nr; i++) {
>                struct diff_filepair *p = q->queue[i];
>                struct diff_filepair *pair_to_free = NULL;
> @@ -577,7 +787,8 @@ void diffcore_rename(struct diff_options *options)
>                        struct diff_rename_dst *dst =
>                                locate_rename_dst(p->two, 0);
>                        if (dst && dst->pair) {
> -                               diff_q(&outq, dst->pair);
> +                               if (!dst->pair->uninteresting_rename)
> +                                       diff_q(&outq, dst->pair);
>                                pair_to_free = p;
>                        }
>                        else
> diff --git a/diffcore.h b/diffcore.h
> index 713cca7..6d2e65b 100644
> --- a/diffcore.h
> +++ b/diffcore.h
> @@ -66,6 +66,7 @@ struct diff_filepair {
>        unsigned broken_pair : 1;
>        unsigned renamed_pair : 1;
>        unsigned is_unmerged : 1;
> +       unsigned uninteresting_rename : 1;
>  };
>  #define DIFF_PAIR_UNMERGED(p) ((p)->is_unmerged)
>
> diff --git a/tree-diff.c b/tree-diff.c
> index 9f67af6..872f757 100644
> --- a/tree-diff.c
> +++ b/tree-diff.c
> @@ -49,7 +49,9 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const
>                show_entry(opt, "+", t2, base, baselen);
>                return 1;
>        }
> -       if (!DIFF_OPT_TST(opt, FIND_COPIES_HARDER) && !hashcmp(sha1, sha2) && mode1 == mode2)
> +       if (!DIFF_OPT_TST(opt, FIND_COPIES_HARDER) &&
> +           !DIFF_OPT_TST(opt, FACTORIZE_RENAMES) &&
> +           !hashcmp(sha1, sha2) && mode1 == mode2)
>                return 0;
>
>        /*
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* Re: multiple-commit cherry-pick?
From: Björn Steinbrink @ 2008-11-07 11:03 UTC (permalink / raw)
  To: Miles Bader; +Cc: Deskin Miller, git
In-Reply-To: <buozlkcqg0c.fsf@dhapc248.dev.necel.com>

On 2008.11.07 14:09:07 +0900, Miles Bader wrote:
> Björn Steinbrink <B.Steinbrink@gmx.de> writes:
> >> > git reset --hard C
> >> > git rebase --onto ORIG_HEAD A^
> >> 
> >> Is that safe...?  Doesn't git-rebase also set ORIG_HEAD?
> >
> > One of the first things rebase does is validating and resolving its
> > arguments. And that's happening before any actions that would touch
> > ORIG_HEAD.
> 
> Ah, I see.
> 
> Hmm, I guess using rebase --abort isn't a very good idea in this case
> though... :-/

Why not? I mean, ok, you end up at C, and not where you have been before
the reset --hard, but there's the reflog to help you get back to
whatever previous state of the branch it is that you want.

Björn

^ permalink raw reply

* Re: multiple-commit cherry-pick?
From: Michael Radziej @ 2008-11-07 10:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, Alex Riesen, Miles Bader, git
In-Reply-To: <7vskq4gmf5.fsf@gitster.siamese.dyndns.org>

On Thu, Nov 06, Junio C Hamano wrote:
> Or "git show --pretty=email $commit1 $commit2" ... piped to "am"?

Or make git show write shell commands.

I often have commits that later need to be cherry-picked into other
branches. For these, I use a commit message that starts with the name of the
branch, like "implement-foo: make foo barfy". Later when I want to do the
cherry-picking, I use this:

git log t/whatever..master --reverse --pretty=tformat:'git cherry-pick %h #
%s' | sed 's/^\([^:]*\) \([^:]*\):/git checkout \2 \&\& \1/'

giving me output like:

git checkout implement-foo && git cherry-pick 90ce727 # make foo barfy
git checkout ...

... and I'm ready for cut'n'paste.



Michael


-- 
noris network AG - Deutschherrnstraße 15-19 - D-90429 Nürnberg -
Tel +49-911-9352-0 - Fax +49-911-9352-100
http://www.noris.de - The IT-Outsourcing Company
 
Vorstand: Ingo Kraupa (Vorsitzender), Joachim Astel, Hansjochen Klenk - 
Vorsitzender des Aufsichtsrats: Stefan Schnabel - AG Nürnberg HRB 17689

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox