* Re: [PATCH 1/2] diffcore-rename: support rename cache
From: Nguyen Thai Ngoc Duy @ 2008-11-08 4:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Yann Dirson, git
In-Reply-To: <7vy6zvdt25.fsf@gitster.siamese.dyndns.org>
On 11/8/08, Junio C Hamano <gitster@pobox.com> wrote:
> Yann Dirson <ydirson@altern.org> writes:
>
> > On Fri, Nov 07, 2008 at 09:35:32PM +0700, Nguy???n Thái Ng???c Duy wrote:
> >> 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.
> >
> > This is something I have thought about in the past, good to see that
> > implemented :)
> >
> >> Rename caching is per commit. I don't think abitrary tree-tree caching
> >> is worth it.
> >
> > That could be a nice complement to my directory-rename patch.
>
>
> Has anybody thought about interaction between that caching and pathspec
> limited operation?
>
I didn't. But I think all out-of-pathspec diff pairs are removed
before it reaches diffcore_rename() so the cache has nothing to do
with it (except it still loads full cache for a commit).
--
Duy
^ permalink raw reply
* Re: [PATCH 1/2] Introduce rename factorization in diffcore.
From: Nguyen Thai Ngoc Duy @ 2008-11-08 4:30 UTC (permalink / raw)
To: Yann Dirson; +Cc: git
In-Reply-To: <20081030221645.3325.78288.stgit@gandelf.nowhere.earth>
On 10/31/08, Yann Dirson <ydirson@altern.org> wrote:
> - 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);
> + }
> }
> }
Hmm.. how about turn on a special flag for these rename_dst items?
Otherwise --group-renames and --find-copies-harder combination would
become hell: you have to compare all src with all dst. It could exceed
rename_limit, therefore no rename detection will be done.
--
Duy
^ permalink raw reply
* Re: [PATCH 1/2] Introduce rename factorization in diffcore.
From: Nguyen Thai Ngoc Duy @ 2008-11-08 4:32 UTC (permalink / raw)
To: Yann Dirson; +Cc: git
In-Reply-To: <fcaeb9bf0811072030u272a6732g2450e1854b96ca18@mail.gmail.com>
On 11/8/08, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> Hmm.. how about turn on a special flag for these rename_dst items?
> Otherwise --group-renames and --find-copies-harder combination would
> become hell: you have to compare all src with all dst. It could exceed
> rename_limit, therefore no rename detection will be done.
Nevermind. I read an old version.
--
Duy
^ permalink raw reply
* Re: absurdly slow git-diff
From: Abhijit Menon-Sen @ 2008-11-08 4:57 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Davide Libenzi, Junio C Hamano, Pierre Habouzit, git
In-Reply-To: <alpine.LFD.2.00.0811071554590.3468@nehalem.linux-foundation.org>
At 2008-11-07 15:57:23 -0800, torvalds@linux-foundation.org wrote:
>
> > Yeah, similar. Mine is below. There's one less branch in the for
> > loops.
>
> ..and has a comment and made the magic constant be named.
It works fine for me (the time went from 5m17s to 1.8s).
(By the way, my test case is certainly very odd, but it is a real file
from my test suite, albeit with the content x'ed away; and the change
was to adjust the expected output for all the items. I wasn't looking
for bugs. :-)
Thanks for the explanation and the patch.
-- ams
^ permalink raw reply
* [PATCH 1/2] Added support for purged files and also optimised memory usage.
From: John Chapman @ 2008-11-08 3:22 UTC (permalink / raw)
To: git; +Cc: John Chapman
Purged files are handled as if they are merely deleted, which is not
entirely optimal, but I don't know of any other way to handle them.
File data is deleted from memory as early as they can, and they are more
efficiently handled, at (significant) cost to CPU usage.
Still need to handle p4 branches with spaces in their names.
Still need to make git-p4 clone more reliable.
- Perhaps with a --continue option. (Sometimes the p4 server kills
the connection)
Signed-off-by: John Chapman <thestar@fussycoder.id.au>
---
contrib/fast-import/git-p4 | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 2216cac..38d1a17 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -946,7 +946,7 @@ class P4Sync(Command):
if includeFile:
filesForCommit.append(f)
- if f['action'] != 'delete':
+ if f['action'] not in ('delete', 'purge'):
filesToRead.append(f)
filedata = []
@@ -965,11 +965,11 @@ class P4Sync(Command):
while j < len(filedata):
stat = filedata[j]
j += 1
- text = [];
+ text = ''
while j < len(filedata) and filedata[j]['code'] in ('text', 'unicode', 'binary'):
- text.append(filedata[j]['data'])
+ text += filedata[j]['data']
+ del filedata[j]['data']
j += 1
- text = ''.join(text)
if not stat.has_key('depotFile'):
sys.stderr.write("p4 print fails with: %s\n" % repr(stat))
@@ -1038,7 +1038,7 @@ class P4Sync(Command):
continue
relPath = self.stripRepoPath(file['path'], branchPrefixes)
- if file["action"] == "delete":
+ if file["action"] in ("delete", "purge"):
self.gitStream.write("D %s\n" % relPath)
else:
data = file['data']
@@ -1077,7 +1077,7 @@ class P4Sync(Command):
cleanedFiles = {}
for info in files:
- if info["action"] == "delete":
+ if info["action"] in ("delete", "purge"):
continue
cleanedFiles[info["depotFile"]] = info["rev"]
@@ -1400,7 +1400,7 @@ class P4Sync(Command):
if change > newestRevision:
newestRevision = change
- if info["action"] == "delete":
+ if info["action"] in ("delete", "purge"):
# don't increase the file cnt, otherwise details["depotFile123"] will have gaps!
#fileCnt = fileCnt + 1
continue
--
1.6.0.3.643.g233db
^ permalink raw reply related
* [PATCH 2/2] Cached the git configuration, which is now noticibly faster on windows.
From: John Chapman @ 2008-11-08 3:22 UTC (permalink / raw)
To: git; +Cc: John Chapman
In-Reply-To: <1226114569-8506-1-git-send-email-thestar@fussycoder.id.au>
Signed-off-by: John Chapman <thestar@fussycoder.id.au>
---
contrib/fast-import/git-p4 | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 38d1a17..9f0a5f9 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -316,8 +316,11 @@ def gitBranchExists(branch):
stderr=subprocess.PIPE, stdout=subprocess.PIPE);
return proc.wait() == 0;
+_gitConfig = {}
def gitConfig(key):
- return read_pipe("git config %s" % key, ignore_error=True).strip()
+ if not _gitConfig.has_key(key):
+ _gitConfig[key] = read_pipe("git config %s" % key, ignore_error=True).strip()
+ return _gitConfig[key]
def p4BranchesInGit(branchesAreInRemotes = True):
branches = {}
--
1.6.0.3.643.g233db
^ permalink raw reply related
* Re: How it was at GitTogether'08 ?
From: David Symonds @ 2008-11-08 5:08 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <200811080254.53202.jnareb@gmail.com>
On Fri, Nov 7, 2008 at 5:54 PM, Jakub Narebski <jnareb@gmail.com> wrote:
Here's my thoughts on some of these talks.
> Mon, Oct 27, 2008
> -----------------
> * Dscho: Contributing with Git
> http://www.youtube.com/watch?v=j45cs5_nY2k
It was a good intro, but I was expecting a few more non-GitTogether
people. We had quite a large room, but there was only about a dozen
other people who came along. I don't know whether that was the fault
of the timing, lack of advertising, or a lack of interest.
> * Junio: Git Chronicle
>
> blog: Junio went though a sort of statistical history of the Git project
> that was fascinating (turns out there are still about 220 lines of code
> still around from Linus original first commit).
This was really interesting. It would be great to put this on a
general web page instead of in a PDF.
> * Petr: Renames Again and Again and Again
>
> IRC: detection of wholesame renames of directories (WIP) and '--follow'
> limitation were mentioned, but outcome is unclear; pasky plans to hack
> together some patch implementing explicit renames hinting
One thing I didn't get around to bringing up: one of the benefits of
diff-time rename detection that is often touted is that algorithms can
improve over time. Do folk here know whether that has actually
happened recently, in a general way? Do people actually expect major
improvements in the future?
> * Tom: GitHub
>
> IRC: a tour, some history, and insight into how it works; some nice
> gimmicks, such as "gist" (a git backed pastebin), or like network graphs
> (look graphically at forks of a repository).
>
> blog: Tom got to demonstrate GitHub and Gist to the group, most of whom
> are very command line oriented and had not used either before.
The demo of iGitHub (an iPhone app that can act as a clone/push
target) looked really cool, if it can get further development. It
could potentially be really handy for travellers who could push to
their iPhone, and then push from there to an internet server.
> * Scott: Linkable Library
>
> blog: got to talk about the need for a linkable git library
>
> http://thread.gmane.org/gmane.comp.version-control.git/99608
It's good to see this starting to get wider traction. I think we
discussed that there could be benefits to git itself, beyond just
helping other programs access git repositories faster than fork/exec.
> * Sam: perl.git
>
> blog: Sam demonstrated the work he went through to import 20 years of Perl
> history into the git repository that the Perl team is just now finishing
> transitioning to from Perforce.
>
> http://utsl.gen.nz/talks/perl-history/slides/
It was very cool to see old-school email addresses like <isis!aburt>
in git, handled just fine.
> * Tim: Git as a Media Repository
> http://www.thousandparsec.net/~tim/media+git.pdf
This has kicked off some mailing list discussion; I think this can be
a major weak point for git, since checking out only a subtree (and
only the latest revision) is the common SVN way, which copes with
media repositories and the like just fine.
> * JH: $$$ discussion
>
> IRC: It was about the Google Summer of Code 2008 mentor money. It was
> decided (given most of the mentors were there) to use it to help a few
> folks cover their travel expenses to the GitTogether. Most folks actually
> had their costs covered by their employer (yay for some big companies!)
> but a few paid out of pocket, so we are helping them defray it a bit. and
> the rest is being used to order and distribute some git t-shirts.
Yes, how are the t-shirts going? I seem to remember JH had volunteered
to do the logistics there.
Dave.
^ permalink raw reply
* Re: [PATCH 2/2] Cached the git configuration, which is now noticibly faster on windows.
From: David Symonds @ 2008-11-08 5:19 UTC (permalink / raw)
To: John Chapman; +Cc: git
In-Reply-To: <1226114569-8506-2-git-send-email-thestar@fussycoder.id.au>
On Fri, Nov 7, 2008 at 7:22 PM, John Chapman <thestar@fussycoder.id.au> wrote:
> +_gitConfig = {}
> def gitConfig(key):
> - return read_pipe("git config %s" % key, ignore_error=True).strip()
> + if not _gitConfig.has_key(key):
> + _gitConfig[key] = read_pipe("git config %s" % key, ignore_error=True).strip()
> + return _gitConfig[key]
If this is truly a noticeable bottleneck on Windows, something like
the following might be even better: (completely untested!)
_gitConfig = None
def gitConfig(key):
if _gitConfig is None:
lines = read_pipe("git config -l", ignore_error=True).readlines():
_gitConfig = dict([l.strip().split('=', 1) for l in lines])
return _gitConfig.get(key, None)
Dave.
^ permalink raw reply
* Re: absurdly slow git-diff
From: Junio C Hamano @ 2008-11-08 5:30 UTC (permalink / raw)
To: Davide Libenzi
Cc: Linus Torvalds, Abhijit Menon-Sen, Pierre Habouzit,
Git Mailing List
In-Reply-To: <alpine.DEB.1.10.0811071547080.8736@alien.or.mcafeemobile.com>
Davide Libenzi <davidel@xmailserver.org> writes:
> Yeah, similar. Mine is below. There's one less branch in the for loops.
Thanks, will apply like this, but I am not sure if you meant windowN or
just window...
-- >8 --
From: Davide Libenzi <davidel@xmailserver.org>
Date: Fri, 7 Nov 2008 21:24:33 -0800
Subject: [PATCH] xdiff: give up scanning similar lines early
In a corner case of large files whose lines do not match uniquely, the
loop to eliminate a line that matches multiple locations adjacent to a run
of lines that do not uniquely match wasted too much cycles. Fix this by
giving up early after scanning 100 lines in both direction.
---
xdiff/xprepare.c | 15 +++++++++++++--
1 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
index e87ab57..6a70cdf 100644
--- a/xdiff/xprepare.c
+++ b/xdiff/xprepare.c
@@ -23,10 +23,9 @@
#include "xinclude.h"
-
#define XDL_KPDIS_RUN 4
#define XDL_MAX_EQLIMIT 1024
-
+#define XDL_SIMSCAN_WINDOWN 100
typedef struct s_xdlclass {
@@ -313,6 +312,18 @@ static int xdl_clean_mmatch(char const *dis, long i, long s, long e) {
long r, rdis0, rpdis0, rdis1, rpdis1;
/*
+ * Limits the window the is examined during the similar-lines
+ * scan. The loops below stops when dis[i - r] == 1 (line that
+ * has no match), but there are corner cases where the loop
+ * proceed all the way to the extremities by causing huge
+ * performance penalties in case of big files.
+ */
+ if (i - s > XDL_SIMSCAN_WINDOWN)
+ s = i - XDL_SIMSCAN_WINDOWN;
+ if (e - i > XDL_SIMSCAN_WINDOWN)
+ e = i + XDL_SIMSCAN_WINDOWN;
+
+ /*
* Scans the lines before 'i' to find a run of lines that either
* have no match (dis[j] == 0) or have multiple matches (dis[j] > 1).
* Note that we always call this function with dis[i] > 1, so the
--
1.6.0.3.674.gdf99f
^ permalink raw reply related
* [PATCH 1/7 v3] bisect: add "git bisect replace" subcommand
From: Christian Couder @ 2008-11-08 7:48 UTC (permalink / raw)
To: Junio C Hamano, Johannes Schindelin; +Cc: git
This subcommand should be used when you have a branch or a part of a
branch that isn't easily bisectable because of a bug that has been
fixed latter.
We suppose that a bug as been introduced at some point, say A, and
that it has been fixed latter at another point, say B, but that
between these points the code is not easily testable because of the
bug, so it's not easy to bisect between these points.
In this case you can create a branch starting at the parent of A, say
O, that has a fixed history. In this fixed history for example, there
could be first a commit C that is the result of squashing A and B
together and then all the commits between A and B that have been
cherry picked.
For example, let's say the commits between A and B are X1, X2, ... Xn
and they have been cherry picked after C as Y1, Y2, ... Yn:
C--Y1--Y2--...--Yn
/
...--O--A--X1--X2--...--Xn--B--...
By design, the last cherry picked commit (Yn) should point to the same
tree as commit B.
So in this case you can say:
$ git bisect replace B Yn
and a branch will be created that points to commit Yn and that has a
special name like: "bisect-replace-B"
When bisecting, the branch names will be scanned and each branch named
"bisect-replace-X" and pointing to commit Y will be grafted so that
X will only have Y as parent.
In the example above, that means that instead of the above graph, the
following graph will be bisected:
C--Y1--Y2--...--Yn
/ \
...--O B--...
This means that the bisections on this branch will be much easier
because the bug introduced by commit A and fixed by commit B will not
annoy you anymore.
As the branches created by "git bisect replace" can be shared between
developers, this feature might be especially usefull on big projects
where many people often bisect the same code base.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin-rev-list.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++-
git-bisect.sh | 37 +++++++++++++++++++++++++-
2 files changed, 105 insertions(+), 3 deletions(-)
Changes since the previous series are the following:
- don't create a tag anymore and use the "bisect-replace-*"
branch to replace to detect grafts that should be created,
- tried to move "replace" subcommand before "run" subcommand,
- added patches from 4 to 7
Perhaps a few more tests are needed for latter patches
especially patch 7.
I wonder about the following:
- perhaps "git bisect replace" should have a "NAME" argument
so that the branch names could be like "bisect-rename-X-NAME"
which could be more descriptive,
- maybe a way to check that the replace branches all point to
existing commits would be useful,
- perhaps bisect log/replay could be improved to handle
replace branches
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 06cdeb7..c0717f6 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -574,6 +574,72 @@ static struct commit_list *find_bisection(struct commit_list *list,
return best;
}
+static void replace_parents(struct commit *commit,
+ const char *refname,
+ const unsigned char *sha1)
+{
+ struct commit *new_parent = lookup_commit(sha1);
+ if (!new_parent) {
+ warning("branch '%s' points to unknown commit '%s'",
+ refname, sha1_to_hex(sha1));
+ return;
+ }
+
+ free_commit_list(commit->parents);
+ commit->parents = NULL;
+ commit_list_insert(new_parent, &commit->parents);
+}
+
+static int bisect_replace(const char *refname, const unsigned char *sha1,
+ int flag, void *cb_data)
+{
+ unsigned char child[20];
+ struct object *obj;
+ struct commit_graft *graft;
+
+ if (prefixcmp(refname, "bisect-replace-"))
+ return 0;
+
+ if (get_sha1_hex(refname + 15, child)) {
+ warning("bad sha1 in branch name '%s'", refname);
+ return 0;
+ }
+
+ /* Check if child commit exist and is already parsed */
+
+ obj = lookup_object(child);
+ if (obj) {
+ struct commit *commit;
+ if (obj->type != OBJ_COMMIT) {
+ warning("branch name '%s' refers to non commit '%s'",
+ refname, refname + 15);
+ return 0;
+ }
+ commit = (struct commit *) obj;
+ if (commit->object.parsed) {
+ replace_parents(commit, refname, sha1);
+ return 0;
+ }
+ }
+
+ /* Create a graft to replace child commit parents */
+
+ graft = xmalloc(sizeof(*graft) + 20);
+
+ hashcpy(graft->sha1, child);
+ graft->nr_parent = 1;
+ hashcpy(graft->parent[0], sha1);
+
+ register_commit_graft(graft, 1);
+
+ return 0;
+}
+
+static void bisect_replace_all(void)
+{
+ for_each_branch_ref(bisect_replace, NULL);
+}
+
int cmd_rev_list(int argc, const char **argv, const char *prefix)
{
struct commit_list *list;
@@ -646,8 +712,11 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
save_commit_buffer = revs.verbose_header ||
revs.grep_filter.pattern_list;
- if (bisect_list)
+
+ if (bisect_list) {
+ bisect_replace_all();
revs.limited = 1;
+ }
if (prepare_revision_walk(&revs))
die("revision walk setup failed");
diff --git a/git-bisect.sh b/git-bisect.sh
index 79de701..1daa81c 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -1,6 +1,6 @@
#!/bin/sh
-USAGE='[help|start|bad|good|skip|next|reset|visualize|replay|log|run]'
+USAGE='[help|start|bad|good|skip|next|reset|visualize|replay|log|replace|run]'
LONG_USAGE='git bisect help
print this long help message.
git bisect start [<bad> [<good>...]] [--] [<pathspec>...]
@@ -21,6 +21,8 @@ git bisect replay <logfile>
replay bisection log.
git bisect log
show bisect log.
+git bisect replace <rev> [<rev>]
+ use another branch for bisection.
git bisect run <cmd>...
use <cmd>... to automatically bisect.
@@ -566,6 +568,36 @@ bisect_replay () {
bisect_auto_next
}
+bisect_replace() {
+ test "$#" -ge 1 -a "$#" -le 2 ||
+ die "'git bisect replace' accept one or two arguments"
+
+ source="$1"
+ target="${2:-HEAD}"
+
+ # Check arguments
+ src_commit=$(git rev-parse --verify "$source^{commit}") ||
+ die "Bad rev input: $source"
+ tgt_commit=$(git rev-parse --verify "$target^{commit}") ||
+ die "Bad rev input: $target"
+
+ test "$src_commit" != "tgt_commit" ||
+ die "source and target should be different commits"
+
+ # Check that trees from source and target are identical
+ src_tree=$(git rev-parse --verify "$src_commit^{tree}") ||
+ die "Could not get tree for source: $source"
+ tgt_tree=$(git rev-parse --verify "$tgt_commit^{tree}") ||
+ die "Could not get tree for target: $target"
+
+ test "$src_tree" = "$tgt_tree" ||
+ die "source and target should point to the same tree"
+
+ # Create branch for the target commit
+ tgt_branch="bisect-replace-$src_commit"
+ git branch "$tgt_branch" "$tgt_commit"
+}
+
bisect_run () {
bisect_next_check fail
@@ -618,7 +650,6 @@ bisect_run () {
done
}
-
case "$#" in
0)
usage ;;
@@ -645,6 +676,8 @@ case "$#" in
cat "$GIT_DIR/BISECT_LOG" ;;
run)
bisect_run "$@" ;;
+ replace)
+ bisect_replace "$@" ;;
*)
usage ;;
esac
--
1.6.0.3.619.g9a6a
^ permalink raw reply related
* [PATCH 2/7 v3] bisect: add test cases for "git bisect replace"
From: Christian Couder @ 2008-11-08 7:48 UTC (permalink / raw)
To: Junio C Hamano, Johannes Schindelin; +Cc: git
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
t/t6035-bisect-replace.sh | 130 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 130 insertions(+), 0 deletions(-)
create mode 100755 t/t6035-bisect-replace.sh
diff --git a/t/t6035-bisect-replace.sh b/t/t6035-bisect-replace.sh
new file mode 100755
index 0000000..6ab3667
--- /dev/null
+++ b/t/t6035-bisect-replace.sh
@@ -0,0 +1,130 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Christian Couder
+#
+test_description='Test git bisect replace functionality'
+
+exec </dev/null
+
+. ./test-lib.sh
+
+add_and_commit_file()
+{
+ _file="$1"
+ _msg="$2"
+
+ git add $_file || return $?
+ test_tick || return $?
+ git commit --quiet -m "$_file: $_msg"
+}
+
+HASH1=
+HASH2=
+HASH3=
+HASH4=
+HASH5=
+HASH6=
+HASH7=
+
+test_expect_success 'set up buggy branch' '
+ echo "line 1" >> hello &&
+ echo "line 2" >> hello &&
+ echo "line 3" >> hello &&
+ echo "line 4" >> hello &&
+ add_and_commit_file hello "4 lines" &&
+ HASH1=$(git rev-parse --verify HEAD) &&
+ echo "line BUG" >> hello &&
+ echo "line 6" >> hello &&
+ echo "line 7" >> hello &&
+ echo "line 8" >> hello &&
+ add_and_commit_file hello "4 more lines with a BUG" &&
+ HASH2=$(git rev-parse --verify HEAD) &&
+ echo "line 9" >> hello &&
+ echo "line 10" >> hello &&
+ add_and_commit_file hello "2 more lines" &&
+ HASH3=$(git rev-parse --verify HEAD) &&
+ echo "line 11" >> hello &&
+ add_and_commit_file hello "1 more line" &&
+ HASH4=$(git rev-parse --verify HEAD) &&
+ sed -e "s/BUG/5/" hello > hello.new &&
+ mv hello.new hello &&
+ add_and_commit_file hello "BUG fixed" &&
+ HASH5=$(git rev-parse --verify HEAD) &&
+ echo "line 12" >> hello &&
+ echo "line 13" >> hello &&
+ add_and_commit_file hello "2 more lines" &&
+ HASH6=$(git rev-parse --verify HEAD)
+ echo "line 14" >> hello &&
+ echo "line 15" >> hello &&
+ echo "line 16" >> hello &&
+ add_and_commit_file hello "again 3 more lines" &&
+ HASH7=$(git rev-parse --verify HEAD)
+'
+
+HASHFIX2=
+HASHFIX3=
+HASHFIX4=
+
+test_expect_success 'set up fixed branch' '
+ git checkout $HASH1 &&
+ echo "line 5" >> hello &&
+ echo "line 6" >> hello &&
+ echo "line 7" >> hello &&
+ echo "line 8" >> hello &&
+ add_and_commit_file hello "4 more lines with no BUG" &&
+ HASHFIX2=$(git rev-parse --verify HEAD) &&
+ git cherry-pick $HASH3 &&
+ HASHFIX3=$(git rev-parse --verify HEAD) &&
+ git cherry-pick $HASH4 &&
+ HASHFIX4=$(git rev-parse --verify HEAD)
+'
+
+test_expect_success '"git bisect replace" buggy branch with fixed one' '
+ git bisect replace $HASH5 HEAD
+'
+
+test_expect_success 'replace works when bisecting with a later bad commit' '
+ git rev-list --bisect-all $HASH7 > rev_list.txt &&
+ grep $HASHFIX2 rev_list.txt &&
+ grep $HASHFIX3 rev_list.txt &&
+ grep $HASHFIX4 rev_list.txt &&
+ test_must_fail grep $HASH2 rev_list.txt &&
+ test_must_fail grep $HASH3 rev_list.txt &&
+ test_must_fail grep $HASH4 rev_list.txt
+'
+
+test_expect_success 'replace works starting just after replaced commit' '
+ git rev-list --bisect-all $HASH6 > rev_list.txt &&
+ grep $HASHFIX2 rev_list.txt &&
+ grep $HASHFIX3 rev_list.txt &&
+ grep $HASHFIX4 rev_list.txt &&
+ test_must_fail grep $HASH2 rev_list.txt &&
+ test_must_fail grep $HASH3 rev_list.txt &&
+ test_must_fail grep $HASH4 rev_list.txt
+'
+
+test_expect_success 'replace works starting from replaced commit' '
+ git rev-list --bisect-all $HASH5 > rev_list.txt &&
+ grep $HASHFIX2 rev_list.txt &&
+ grep $HASHFIX3 rev_list.txt &&
+ grep $HASHFIX4 rev_list.txt &&
+ test_must_fail grep $HASH2 rev_list.txt &&
+ test_must_fail grep $HASH3 rev_list.txt &&
+ test_must_fail grep $HASH4 rev_list.txt
+'
+
+test_expect_success 'standard bisect works' '
+ git bisect start $HASH6 $HASH1 &&
+ test "$(git rev-parse --verify HEAD)" = "$HASHFIX3" &&
+ git bisect good &&
+ test "$(git rev-parse --verify HEAD)" = "$HASH5" &&
+ git bisect bad &&
+ test "$(git rev-parse --verify HEAD)" = "$HASHFIX4" &&
+ git bisect bad > my_bisect_log.txt &&
+ grep "$HASHFIX4 is first bad commit" my_bisect_log.txt &&
+ git bisect reset
+'
+
+#
+#
+test_done
--
1.6.0.3.619.g9a6a
^ permalink raw reply related
* [PATCH 3/7 v3] Documentation: add "git bisect replace" documentation
From: Christian Couder @ 2008-11-08 7:49 UTC (permalink / raw)
To: Junio C Hamano, Johannes Schindelin; +Cc: git
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-bisect.txt | 61 ++++++++++++++++++++++++++++++++++++++++++
1 files changed, 61 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index c7981ef..5d5bdf7 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -24,6 +24,7 @@ on the subcommand:
git bisect visualize
git bisect replay <logfile>
git bisect log
+ git bisect replace <rev> [<rev>]
git bisect run <cmd>...
This command uses 'git-rev-list --bisect' to help drive the
@@ -186,6 +187,66 @@ $ git bisect start v2.6.20-rc6 v2.6.20-rc4 v2.6.20-rc1 --
# v2.6.20-rc4 and v2.6.20-rc1 are good
------------
+Bisect replace
+~~~~~~~~~~~~~~
+
+This subcommand should be used when you have a branch or a part of a
+branch that isn't easily bisectable because of a bug that has been
+fixed latter.
+
+We suppose that a bug as been introduced at some point, say A, and
+that it has been fixed latter at another point, say B, but that
+between these points the code is not easily testable because of the
+bug, so it's not easy to bisect between these points.
+
+In this case you can create a branch starting at the parent of A, say
+O, that has a fixed history. In this fixed history for example, there
+could be first a commit C that is the result of squashing A and B
+together and then all the commits between A and B that have been cherry
+picked.
+
+For example, let's say the commits between A and B are X1, X2, ... Xn
+and they have been cherry picked after C as Y1, Y2, ... Yn:
+
+------------
+ C--Y1--Y2--...--Yn
+ /
+...--O--A--X1--X2--...--Xn--B--...
+------------
+
+By design, the last cherry picked commit (Yn) should point to the same
+tree as commit B.
+
+So in this case you can say:
+
+------------
+$ git bisect replace B Yn
+------------
+
+and a branch will be created that points to commit Yn and that has a
+special name like: "bisect-replace-B"
+
+When bisecting, the branch names will be scanned and each branch named
+"bisect-replace-X" and pointing to commit Y will be grafted so that X
+will only have Y as parent.
+
+In the example above, that means that instead of the above graph, the
+following graph will be bisected:
+
+------------
+ C--Y1--Y2--...--Yn
+ / \
+...--O B--...
+------------
+
+This means that the bisections on this branch may be much easier
+because the bug introduced by commit A and fixed by commit B will not
+annoy you anymore.
+
+As the branches created by "git bisect replace" can be shared between
+developers, this feature might be especially usefull on big projects
+where many people often bisect the same code base.
+
Bisect run
~~~~~~~~~~
--
1.6.0.3.619.g9a6a
^ permalink raw reply related
* [PATCH 4/7 v3] rev-list: add "--bisect-replace" to list revisions with fixed up history
From: Christian Couder @ 2008-11-08 7:49 UTC (permalink / raw)
To: Junio C Hamano, Johannes Schindelin; +Cc: git
This should help both human and scripts deal better with
"bisect-replace-*" branches.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-rev-list.txt | 1 +
Documentation/rev-list-options.txt | 15 +++++++++++++++
builtin-rev-list.c | 12 +++++++++---
t/t6035-bisect-replace.sh | 12 ++++++++++++
4 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index 1c9cc28..4cc8abf 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -41,6 +41,7 @@ SYNOPSIS
[ \--bisect ]
[ \--bisect-vars ]
[ \--bisect-all ]
+ [ \--bisect-replace ]
[ \--merge ]
[ \--reverse ]
[ \--walk-reflogs ]
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 966276b..85ee03e 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -507,6 +507,21 @@ may not compile for example).
This option can be used along with `--bisect-vars`, in this case,
after all the sorted commit objects, there will be the same text as if
`--bisect-vars` had been used alone.
+
+--bisect-replace::
+
+This option will make use of the "bisect-replace-*" branches if any,
+but will not perform other bisection calculation.
+
+The purpose of the "bisect-replace-*" branches is to be grafted into
+other branches when bisecting, so that bisection can be performed on
+a fixed up history.
+
+The other `--bisect*` options use the "bisect-replace-*" branches by
+default when they perform their bisection calculations. With the
+"--bisect-replace" option, you can see what is the result of using the
+"bisect-replace-*" branches without the effects of other bisection
+calculations.
endif::git-rev-list[]
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index c0717f6..75cc1fc 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -47,12 +47,14 @@ static const char rev_list_usage[] =
" special purpose:\n"
" --bisect\n"
" --bisect-vars\n"
-" --bisect-all"
+" --bisect-all\n"
+" --bisect-replace"
;
static struct rev_info revs;
static int bisect_list;
+static int bisect_replace_only;
static int show_timestamp;
static int hdr_termination;
static const char *header_prefix;
@@ -681,6 +683,10 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
bisect_show_vars = 1;
continue;
}
+ if (!strcmp(arg, "--bisect-replace")) {
+ bisect_replace_only = 1;
+ continue;
+ }
if (!strcmp(arg, "--stdin")) {
if (read_from_stdin++)
die("--stdin given twice?");
@@ -713,10 +719,10 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
save_commit_buffer = revs.verbose_header ||
revs.grep_filter.pattern_list;
- if (bisect_list) {
+ if (bisect_list || bisect_replace_only)
bisect_replace_all();
+ if (bisect_list)
revs.limited = 1;
- }
if (prepare_revision_walk(&revs))
die("revision walk setup failed");
diff --git a/t/t6035-bisect-replace.sh b/t/t6035-bisect-replace.sh
index 6ab3667..bc07206 100755
--- a/t/t6035-bisect-replace.sh
+++ b/t/t6035-bisect-replace.sh
@@ -125,6 +125,18 @@ test_expect_success 'standard bisect works' '
git bisect reset
'
+test_expect_success '"git rev-list --bisect-replace" works' '
+ echo "$HASH7" >> rev_list.expect &&
+ echo "$HASH6" >> rev_list.expect &&
+ echo "$HASH5" >> rev_list.expect &&
+ echo "$HASHFIX4" >> rev_list.expect &&
+ echo "$HASHFIX3" >> rev_list.expect &&
+ echo "$HASHFIX2" >> rev_list.expect &&
+ echo "$HASH1" >> rev_list.expect &&
+ git rev-list --bisect-replace $HASH7 > rev_list.output &&
+ test_cmp rev_list.expect rev_list.output
+'
+
#
#
test_done
--
1.6.0.3.619.g9a6a
^ permalink raw reply related
* [PATCH 5/7 v3] commit: add "bisect_replace_all" prototype to "commit.h"
From: Christian Couder @ 2008-11-08 7:50 UTC (permalink / raw)
To: Junio C Hamano, Johannes Schindelin; +Cc: git
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin-rev-list.c | 2 +-
commit.h | 2 ++
2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 75cc1fc..7cf5f9e 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -637,7 +637,7 @@ static int bisect_replace(const char *refname, const unsigned char *sha1,
return 0;
}
-static void bisect_replace_all(void)
+void bisect_replace_all(void)
{
for_each_branch_ref(bisect_replace, NULL);
}
diff --git a/commit.h b/commit.h
index 3a7b06a..a0ec2a0 100644
--- a/commit.h
+++ b/commit.h
@@ -144,4 +144,6 @@ static inline int single_parent(struct commit *commit)
struct commit_list *reduce_heads(struct commit_list *heads);
+void bisect_replace_all(void);
+
#endif /* COMMIT_H */
--
1.6.0.3.619.g9a6a
^ permalink raw reply related
* [PATCH 6/7 v3] merge-base: add "--bisect-replace" option to use fixed up revs
From: Christian Couder @ 2008-11-08 7:51 UTC (permalink / raw)
To: Junio C Hamano, Johannes Schindelin; +Cc: git
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/git-merge-base.txt | 6 +++++-
builtin-merge-base.c | 8 +++++++-
t/t6035-bisect-replace.sh | 7 +++++++
3 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-merge-base.txt b/Documentation/git-merge-base.txt
index 2f0c525..af174f7 100644
--- a/Documentation/git-merge-base.txt
+++ b/Documentation/git-merge-base.txt
@@ -8,7 +8,7 @@ git-merge-base - Find as good common ancestors as possible for a merge
SYNOPSIS
--------
-'git merge-base' [--all] <commit> <commit>...
+'git merge-base' [--all] [--bisect-replace] <commit> <commit>...
DESCRIPTION
-----------
@@ -32,6 +32,10 @@ OPTIONS
--all::
Output all merge bases for the commits, instead of just one.
+--bisect-replace::
+ Use fixed up history from "bisect-replace-*" branches when
+ computing merge bases.
+
DISCUSSION
----------
diff --git a/builtin-merge-base.c b/builtin-merge-base.c
index 03fc1c2..31c0015 100644
--- a/builtin-merge-base.c
+++ b/builtin-merge-base.c
@@ -46,9 +46,13 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)
struct commit **rev;
int rev_nr = 0;
int show_all = 0;
+ int bisect_replace = 0;
struct option options[] = {
- OPT_BOOLEAN('a', "all", &show_all, "outputs all common ancestors"),
+ OPT_BOOLEAN('a', "all", &show_all,
+ "outputs all common ancestors"),
+ OPT_BOOLEAN(0, "bisect-replace", &bisect_replace,
+ "use revs from 'bisect-replace-*' branches"),
OPT_END()
};
@@ -56,6 +60,8 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, options, merge_base_usage, 0);
if (argc < 2)
usage_with_options(merge_base_usage, options);
+ if (bisect_replace)
+ bisect_replace_all();
rev = xmalloc(argc * sizeof(*rev));
while (argc-- > 0)
rev[rev_nr++] = get_commit_reference(*argv++);
diff --git a/t/t6035-bisect-replace.sh b/t/t6035-bisect-replace.sh
index bc07206..8fe7cc5 100755
--- a/t/t6035-bisect-replace.sh
+++ b/t/t6035-bisect-replace.sh
@@ -137,6 +137,13 @@ test_expect_success '"git rev-list --bisect-replace" works' '
test_cmp rev_list.expect rev_list.output
'
+test_expect_success '"git merge-base --bisect-replace" works' '
+ hash=$(git merge-base --all --bisect-replace $HASH7 $HASHFIX3) &&
+ test "$hash" = "$HASHFIX3" &&
+ hash=$(git merge-base --all --bisect-replace $HASH7 $HASH3) &&
+ test "$hash" = "$HASH1"
+'
+
#
#
test_done
--
1.6.0.3.619.g9a6a
^ permalink raw reply related
* [PATCH 7/7 v3] bisect: use "--bisect-replace" options when checking merge bases
From: Christian Couder @ 2008-11-08 7:52 UTC (permalink / raw)
To: Junio C Hamano, Johannes Schindelin; +Cc: git
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
git-bisect.sh | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-bisect.sh b/git-bisect.sh
index 1daa81c..87e186f 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -401,7 +401,7 @@ check_merge_bases() {
_bad="$1"
_good="$2"
_skip="$3"
- for _mb in $(git merge-base --all $_bad $_good)
+ for _mb in $(git merge-base --all --bisect-replace $_bad $_good)
do
if is_among "$_mb" "$_good"; then
continue
@@ -436,7 +436,7 @@ check_good_are_ancestors_of_bad() {
# Bisecting with no good rev is ok
test -z "$_good" && return
- _side=$(git rev-list $_good ^$_bad)
+ _side=$(git rev-list --bisect-replace $_good ^$_bad)
if test -n "$_side"; then
# Return if a checkout was done
check_merge_bases "$_bad" "$_good" "$_skip" || return
--
1.6.0.3.619.g9a6a
^ permalink raw reply related
* Re: [PATCH 2/2] Cached the git configuration, which is now noticibly faster on windows.
From: Arafangion @ 2008-11-08 6:52 UTC (permalink / raw)
To: David Symonds; +Cc: git
In-Reply-To: <ee77f5c20811072119y65738f54o7e6792fb405c142c@mail.gmail.com>
On Fri, 2008-11-07 at 21:19 -0800, David Symonds wrote:
<snip>
> _gitConfig = None
> def gitConfig(key):
> if _gitConfig is None:
> lines = read_pipe("git config -l", ignore_error=True).readlines():
> _gitConfig = dict([l.strip().split('=', 1) for l in lines])
> return _gitConfig.get(key, None)
That certainly is better, if one can assume that git's configuration is
small. (And relative to the memory usage of the script, it will
definetly be small).
I shall give that a go, although the change won't make it even faster -
I suspect that much of the performance penalty in windows is the
pathetic fork() performance, particularly as the memory usage of the
script increases. (If subprocess does fork() and exec() in order to open
another process, in cygwin).
Thankyou.
^ permalink raw reply
* Re: [PATCH 1/2] diffcore-rename: support rename cache
From: Yann Dirson @ 2008-11-08 9:24 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, git
In-Reply-To: <fcaeb9bf0811072001o6df7ae00k1b1bffaadf75d3a1@mail.gmail.com>
On Sat, Nov 08, 2008 at 11:01:20AM +0700, Nguyen Thai Ngoc Duy wrote:
> On 11/8/08, Junio C Hamano <gitster@pobox.com> wrote:
> > Yann Dirson <ydirson@altern.org> writes:
> >
> > > On Fri, Nov 07, 2008 at 09:35:32PM +0700, Nguy???n Thái Ng???c Duy wrote:
> > >> 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.
> > >
> > > This is something I have thought about in the past, good to see that
> > > implemented :)
> > >
> > >> Rename caching is per commit. I don't think abitrary tree-tree caching
> > >> is worth it.
> > >
> > > That could be a nice complement to my directory-rename patch.
> >
> >
> > Has anybody thought about interaction between that caching and pathspec
> > limited operation?
> >
>
> I didn't. But I think all out-of-pathspec diff pairs are removed
> before it reaches diffcore_rename() so the cache has nothing to do
> with it (except it still loads full cache for a commit).
Well, it could be that an out-of-pathspec pair would have a better
score than an in-pathspec one. Maybe cache recording should be turned
off when doing pathspec limitation ?
^ permalink raw reply
* Re: [PATCH 1/2] diffcore-rename: support rename cache
From: Nguyen Thai Ngoc Duy @ 2008-11-08 9:29 UTC (permalink / raw)
To: Yann Dirson; +Cc: Junio C Hamano, git
In-Reply-To: <20081108092409.GD4030@nan92-1-81-57-214-146.fbx.proxad.net>
On 11/8/08, Yann Dirson <ydirson@altern.org> wrote:
> On Sat, Nov 08, 2008 at 11:01:20AM +0700, Nguyen Thai Ngoc Duy wrote:
> > On 11/8/08, Junio C Hamano <gitster@pobox.com> wrote:
> > > Yann Dirson <ydirson@altern.org> writes:
> > >
> > > > On Fri, Nov 07, 2008 at 09:35:32PM +0700, Nguy???n Thái Ng???c Duy wrote:
> > > >> 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.
> > > >
> > > > This is something I have thought about in the past, good to see that
> > > > implemented :)
> > > >
> > > >> Rename caching is per commit. I don't think abitrary tree-tree caching
> > > >> is worth it.
> > > >
> > > > That could be a nice complement to my directory-rename patch.
> > >
> > >
> > > Has anybody thought about interaction between that caching and pathspec
> > > limited operation?
> > >
> >
> > I didn't. But I think all out-of-pathspec diff pairs are removed
> > before it reaches diffcore_rename() so the cache has nothing to do
> > with it (except it still loads full cache for a commit).
>
>
> Well, it could be that an out-of-pathspec pair would have a better
> score than an in-pathspec one. Maybe cache recording should be turned
> off when doing pathspec limitation ?
Right, recording should be turned off or something. Let me see..
--
Duy
^ permalink raw reply
* Re: [PATCH] checkout: Don't crash when switching away from an invalid branch.
From: Alexandre Julliard @ 2008-11-08 10:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vbpwrf85x.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> For that matter, dying without removing the trace of that funny state
> might be even preferrable if you need to do postmortem to figure out why
> you got into such a funny state to begin with, but not everybody uses git
> to debug git.
It turns out to be user error, that was a tree I hadn't used in a long
time and I didn't realize it was using alternates, so HEAD was pointing
to a commit that had been rebased and garbage-collected in the source
repository.
Most other commands die with a "bad object HEAD" in that situation, and
checkout could certainly do that too, but I think it's nicer to provide
an easy way of getting out of that broken state. I'll resend an updated
patch.
--
Alexandre Julliard
julliard@winehq.org
^ permalink raw reply
* Re: [PATCH 2/2] Cached the git configuration, which is now noticibly faster on windows.
From: Jakub Narebski @ 2008-11-08 10:13 UTC (permalink / raw)
To: David Symonds; +Cc: John Chapman, git
In-Reply-To: <ee77f5c20811072119y65738f54o7e6792fb405c142c@mail.gmail.com>
"David Symonds" <dsymonds@gmail.com> writes:
> On Fri, Nov 7, 2008 at 7:22 PM, John Chapman <thestar@fussycoder.id.au> wrote:
>
> > +_gitConfig = {}
> > def gitConfig(key):
> > - return read_pipe("git config %s" % key, ignore_error=True).strip()
> > + if not _gitConfig.has_key(key):
> > + _gitConfig[key] = read_pipe("git config %s" % key, ignore_error=True).strip()
> > + return _gitConfig[key]
>
> If this is truly a noticeable bottleneck on Windows, something like
> the following might be even better: (completely untested!)
>
> _gitConfig = None
> def gitConfig(key):
> if _gitConfig is None:
> lines = read_pipe("git config -l", ignore_error=True).readlines():
> _gitConfig = dict([l.strip().split('=', 1) for l in lines])
> return _gitConfig.get(key, None)
Wouldn't it be better to use "git config -l -z", split lines at "\0"
(NUL), and split key from value at first "\N" (CR)? This format was
meant for scripts.
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* [PATCH v2 1/2] diffcore-rename: support rename cache
From: Nguyễn Thái Ngọc Duy @ 2008-11-08 11:27 UTC (permalink / raw)
To: git, Junio C Hamano, Yann Dirson; +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>
---
> > > Has anybody thought about interaction between that caching and pathspec
> > > limited operation?
> > >
> >
> > I didn't. But I think all out-of-pathspec diff pairs are removed
> > before it reaches diffcore_rename() so the cache has nothing to do
> > with it (except it still loads full cache for a commit).
>
>
> Well, it could be that an out-of-pathspec pair would have a better
> score than an in-pathspec one. Maybe cache recording should be turned
> off when doing pathspec limitation ?
Changes from v1:
- rebased to next to avoid conflict
- no longer generate cache if pathspec is used
diff.h | 2 +
diffcore-rename.c | 142 ++++++++++++++++++++++++++++++++++++++++++++++-
log-tree.c | 2 +
t/t4031-rename-cache.sh | 56 ++++++++++++++++++
4 files changed, 200 insertions(+), 2 deletions(-)
create mode 100755 t/t4031-rename-cache.sh
diff --git a/diff.h b/diff.h
index 42582ed..64a1edd 100644
--- a/diff.h
+++ b/diff.h
@@ -111,6 +111,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 5444f08..040e095 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -522,6 +522,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) {
@@ -531,5 +532,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/t4031-rename-cache.sh b/t/t4031-rename-cache.sh
new file mode 100755
index 0000000..f7c53fd
--- /dev/null
+++ b/t/t4031-rename-cache.sh
@@ -0,0 +1,56 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Nguyen Thai Ngoc Duy
+#
+
+test_description='Test diff rename cache'
+. ./test-lib.sh
+
+cat >expected <<EOF
+ create mode 100644 sub/c
+ copy a => sub/d (100%)
+EOF
+test_expect_success 'setup' '
+ echo a > a
+ echo b > b
+ mkdir sub
+ echo c > sub/c
+ cp a sub/d
+ A_SHA1=$(git hash-object a)
+ B_SHA1=$(git hash-object b)
+ C_SHA1=$(git hash-object sub/c)
+ D_SHA1=$(git hash-object sub/d)
+ git add a b
+ test_tick
+ git commit -m first
+ git add sub
+ 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 => sub/c (100%)
+ copy a => sub/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 => sub/c (100%)
+ create mode 100644 sub/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.892.g83538
^ permalink raw reply related
* [PATCH v2 2/2] diffcore-rename: add config option to allow to cache renames
From: Nguyễn Thái Ngọc Duy @ 2008-11-08 11:27 UTC (permalink / raw)
To: git, Junio C Hamano, Yann Dirson; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1226143653-3758-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>
---
Documentation/config.txt | 5 ++++
Documentation/diff-options.txt | 5 ++++
diff.c | 12 +++++++++
diff.h | 2 +
diffcore-rename.c | 49 ++++++++++++++++++++++++++++++++++++++++
t/t4031-rename-cache.sh | 36 +++++++++++++++++++++++++++++
6 files changed, 109 insertions(+), 0 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 965ed74..8a7f00e 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 f644947..604cb12 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 */
@@ -103,6 +104,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;
+ }
+
switch (userdiff_config(var, value)) {
case 0: break;
case -1: return -1;
@@ -2272,6 +2278,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 && diff_cache_renames)
+ DIFF_OPT_SET(options, CACHE_RENAMES);
if (options->setup & DIFF_SETUP_USE_CACHE) {
if (!active_cache)
/* read-cache does not die even when it fails
@@ -2439,6 +2447,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")) {
+ DIFF_OPT_SET(options, CACHE_RENAMES);
+ DIFF_OPT_SET(options, REFRESH_RENAME_CACHE);
+ }
/* xdiff options */
else if (!strcmp(arg, "-w") || !strcmp(arg, "--ignore-all-space"))
diff --git a/diff.h b/diff.h
index 64a1edd..0503b57 100644
--- a/diff.h
+++ b/diff.h
@@ -66,6 +66,8 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
#define DIFF_OPT_DIRSTAT_CUMULATIVE (1 << 19)
#define DIFF_OPT_DIRSTAT_BY_FILE (1 << 20)
#define DIFF_OPT_ALLOW_TEXTCONV (1 << 21)
+#define DIFF_OPT_CACHE_RENAMES (1 << 22)
+#define DIFF_OPT_REFRESH_RENAME_CACHE (1 << 23)
#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)
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 598cc8d..49651ea 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) && !DIFF_OPT_TST(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,17 @@ void diffcore_rename(struct diff_options *options)
}
diff_debug_queue("done copying original", &outq);
+ /*
+ * Only cache if:
+ * - Have a commit hint
+ * - diff.cacherenames is on
+ * - no pathspec limits
+ */
+ if (options->commit &&
+ DIFF_OPT_TST(options, CACHE_RENAMES) &&
+ !options->nr_paths)
+ save_rename_cache(&outq, options);
+
free(q->queue);
if (cacheq.queue)
free(cacheq.queue);
diff --git a/t/t4031-rename-cache.sh b/t/t4031-rename-cache.sh
index f7c53fd..2d3f993 100755
--- a/t/t4031-rename-cache.sh
+++ b/t/t4031-rename-cache.sh
@@ -53,4 +53,40 @@ 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 'do not save rename cache with limited pathspec' '
+ P=.git/rename-cache/$(git rev-parse HEAD|sed "s,\(..\)\(.*\),\1/\2,")
+ echo $P
+ rm $P
+ git config diff.cacherenames true
+ git log --summary -C -M --find-copies-harder HEAD -- sub
+ ! test -f $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.892.g83538
^ permalink raw reply related
* Re: [PATCH 1/2] diffcore-rename: support rename cache
From: Jeff King @ 2008-11-08 11:47 UTC (permalink / raw)
To: Yann Dirson; +Cc: Nguyen Thai Ngoc Duy, Junio C Hamano, git
In-Reply-To: <20081108092409.GD4030@nan92-1-81-57-214-146.fbx.proxad.net>
On Sat, Nov 08, 2008 at 10:24:10AM +0100, Yann Dirson wrote:
> Well, it could be that an out-of-pathspec pair would have a better
> score than an in-pathspec one. Maybe cache recording should be turned
> off when doing pathspec limitation ?
One thing I notice is that the cache works at the level of "here is the
best rename for this commit." Maybe it could go down a level and say
"here is the inexact rename score between these blobs". Then you would
still find the best score between two blobs each time, but save the
really computationally intensive part (which is comparing the actual
_content_ of the blobs).
That should work in the face of path limiting or any other option,
because it is caching something immutable: this is the similarity score
between two pieces of content. And then you get arbitrary tree-to-tree
speedups for free, since such a cache would be valid for every commit.
The downsides are:
- your cache is potentially bigger, since you are caching the score of
every pair you look at, instead of just "good" pairs (OTOH, you are
not doing a per-commit cache, which helps reduce the size)
- you can still "lie" about a score to pre-seed imported SVN renames,
but such lying will actually apply to all commits.
-Peff
^ permalink raw reply
* Re: [minor usability suggestion] git rebase <upstream> --onto <newbase> ?
From: Ingo Molnar @ 2008-11-08 11:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Daniel Barkalow, git
In-Reply-To: <7vy6zvfdp5.fsf@gitster.siamese.dyndns.org>
* Junio C Hamano <gitster@pobox.com> wrote:
> Daniel Barkalow <barkalow@iabervon.org> writes:
>
> > "git log origin/master -p" works, though.
>
> That's an accident. Do not rely on it.
hm, i frequently rely on tacking-on options after the fact, especially
the -- ones.
Just like the best workflow is append-mostly, command parameters are
often added append-only as well, without jumping back and forth in the
command line to edit the command.
Path or brach names starting with '--' are weird anyway, so isnt this
a reasonable expectation? I hope i'm not misunderstanding something
here.
Ingo
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox