* [PATCH v3 0/3] Detection of directory renames
From: Yann Dirson @ 2008-11-01 22:03 UTC (permalink / raw)
To: git
This new version fixes handling of moves to and from tree toplevel,
adds supports for detecting bulk moves of files into a subdirectory of
their original dir, and adds a couple of other testcases showing what
still has to be done to properly handle moves of directories with
subdirs.
--
Yann Dirson <ydirson@altern.org> |
Debian-related: <dirson@debian.org> | Support Debian GNU/Linux:
| Freedom, Power, Stability, Gratis
http://ydirson.free.fr/ | Check <http://www.debian.org/>
^ permalink raw reply
* [PATCH 1/3] Fix error in diff_filepair::status documentation.
From: Yann Dirson @ 2008-11-01 22:03 UTC (permalink / raw)
To: git
In-Reply-To: <20081101215739.1116.59319.stgit@gandelf.nowhere.earth>
Signed-off-by: Yann Dirson <ydirson@altern.org>
---
diffcore.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/diffcore.h b/diffcore.h
index 713cca7..05d0898 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -62,7 +62,7 @@ struct diff_filepair {
struct diff_filespec *one;
struct diff_filespec *two;
unsigned short int score;
- char status; /* M C R N D U (see Documentation/diff-format.txt) */
+ char status; /* M C R A D U etc. (see DIFF_STATUS_* in diff.h) */
unsigned broken_pair : 1;
unsigned renamed_pair : 1;
unsigned is_unmerged : 1;
^ permalink raw reply related
* [PATCH 2/3] Introduce rename factorization in diffcore.
From: Yann Dirson @ 2008-11-01 22:03 UTC (permalink / raw)
To: git
In-Reply-To: <20081101215739.1116.59319.stgit@gandelf.nowhere.earth>
Rename factorization tries to group together files moving from and to
identical directories - the most common case being directory renames.
We do that by first identifying groups of bulk-moved files, and then
hiding those of the individual renames which carry no other
information (further name change, or content changes).
This feature is activated by the new --factorize-renames diffcore
flag.
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 | 301 +++++++++++++++++++++++++++++++++++++++++++++++++++--
diffcore.h | 1
tree-diff.c | 4 +
6 files changed, 307 insertions(+), 13 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 168a95b..ab6149e 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -11,6 +11,7 @@
static struct diff_rename_dst {
struct diff_filespec *two;
struct diff_filepair *pair;
+ int i_am_not_single:1; // does not look for a match, only here to be looked at
} *rename_dst;
static int rename_dst_nr, rename_dst_alloc;
@@ -49,9 +50,36 @@ static struct diff_rename_dst *locate_rename_dst(struct diff_filespec *two,
rename_dst[first].two = alloc_filespec(two->path);
fill_filespec(rename_dst[first].two, two->sha1, two->mode);
rename_dst[first].pair = NULL;
+ rename_dst[first].i_am_not_single = 0;
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 prefixlength = 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, prefixlength);
+ 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;
@@ -262,7 +290,7 @@ static int find_identical_files(struct file_similarity *src,
int score;
struct diff_filespec *source = p->filespec;
- /* False hash collission? */
+ /* False hash collision? */
if (hashcmp(source->sha1, target->sha1))
continue;
/* Non-regular files? If so, the modes must match! */
@@ -381,8 +409,11 @@ static int find_exact_renames(void)
for (i = 0; i < rename_src_nr; i++)
insert_file_table(&file_table, -1, i, rename_src[i].one);
- for (i = 0; i < rename_dst_nr; i++)
+ for (i = 0; i < rename_dst_nr; i++) {
+ if (rename_dst[i].i_am_not_single)
+ continue;
insert_file_table(&file_table, 1, i, rename_dst[i].two);
+ }
/* Find the renames */
i = for_each_hash(&file_table, find_same_files);
@@ -409,6 +440,223 @@ 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, suitable to append a filename without
+ * an additional "/".
+ * 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) {
+ *dst = '\0';
+ return 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++) {
+ struct diff_dir_rename* seen;
+
+ // 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);
+ copy_dirname(two_parent_path, rename_dst[i].pair->two->path);
+
+ struct diff_filespec* one_parent = alloc_filespec(one_parent_path);
+ fill_filespec(one_parent, null_sha1 /*FIXME*/, S_IFDIR);
+
+ struct diff_rename_dst* one_leftover =
+ locate_rename_dst_dir(one_parent);
+ if (one_leftover) { // FIXME: should only be run if !seen
+ /* this might be a dir split, or files added
+ * after the bulk rename, or just an isolated
+ * rename */
+ int two_idx, j, onep_len, maybe_dir_rename;
+
+ // try to see if it is a file added after the bulk rename
+ two_idx = one_leftover - rename_dst;
+ onep_len = strlen(one_parent_path);
+ maybe_dir_rename = 1;
+
+ // check no leftover file was already here before
+ for (j = two_idx; j < rename_dst_nr; j++) {
+ if (strncmp(rename_dst[j].two->path,
+ one_parent_path, onep_len))
+ break; // exhausted directory in this direction
+ fprintf (stderr, "[DBG] leftover file %s in %s\n",
+ rename_dst[j].two->path, one_parent_path);
+ if (rename_dst[j].i_am_not_single || // those were already here
+ (rename_dst[j].pair &&
+ !strncmp(rename_dst[j].pair->one->path,
+ one_parent_path, onep_len) && // renamed from here
+ strncmp(rename_dst[j].two->path,
+ one_parent_path, onep_len))) { // not to a subdir
+ maybe_dir_rename = 0;
+ fprintf (stderr, "[DBG] ... tells not a bulk rename\n");
+ break;
+ }
+ fprintf (stderr, "[DBG] ... not believed to prevent bulk rename\n");
+ }
+ if (!maybe_dir_rename) continue;
+ for (j = two_idx-1; j >= 0; j--) {
+ if (strncmp(rename_dst[j].two->path,
+ one_parent_path, onep_len))
+ break; // exhausted directory in this direction
+ fprintf (stderr, "[DBG] leftover file %s in %s\n",
+ rename_dst[j].two->path, one_parent_path);
+ if (rename_dst[j].i_am_not_single || // those were already here
+ (rename_dst[j].pair &&
+ !strncmp(rename_dst[j].pair->one->path,
+ one_parent_path, onep_len) && // renamed from here
+ strncmp(rename_dst[j].two->path,
+ one_parent_path, onep_len))) { // not to a subdir
+ maybe_dir_rename = 0;
+ fprintf (stderr, "[DBG] ... tells not a bulk rename\n");
+ break;
+ }
+ fprintf (stderr, "[DBG] ... not believed to prevent bulk rename\n");
+ }
+ if (!maybe_dir_rename) continue;
+
+ // here we are in the case where a directory
+ // content was completely moved, but files
+ // were added to it afterwards. Proceed as
+ // for a simple bulk move.
+ }
+
+ // already considered ?
+ for (seen=factorization_candidates; seen; seen = seen->next)
+ if (!strcmp(seen->one->path, one_parent_path)) break;
+ if (!seen) {
+ // record potential dir rename
+ 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);
+ continue;
+ }
+ if (seen->discarded)
+ continue;
+ // check that seen entry matches this rename
+ 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;
+
+ // bisect to an entry within candidate dst dir
+ struct diff_rename_dst* two_sample =
+ locate_rename_dst_dir(candidate->two);
+ if (!two_sample) {
+ die ("PANIC: %s candidate of rename not in target tree (from %s)\n",
+ candidate->two->path, candidate->one->path);
+ }
+ two_idx = two_sample - 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 +694,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, but we don't
+ * want them to be matched against single sources.
+ */
+ locate_rename_dst(p->two, 1)->i_am_not_single = 1;
+ }
}
}
if (rename_dst_nr == 0 || rename_src_nr == 0)
@@ -504,6 +761,8 @@ void diffcore_rename(struct diff_options *options)
if (rename_dst[i].pair)
continue; /* dealt with exact match already. */
+ if (rename_dst[i].i_am_not_single)
+ continue; /* not looking for a match. */
m = &mx[dst_cnt * NUM_CANDIDATE_PER_DST];
for (j = 0; j < NUM_CANDIDATE_PER_DST; j++)
@@ -561,8 +820,29 @@ 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;
+
+ // Now turn non-discarded 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;
+ // visualize toplevel dir if needed - FIXME: wrong place for this ?
+ if (!*candidate->one->path)
+ candidate->one->path = "./";
+ if (!*candidate->two->path)
+ candidate->two->path = "./";
+ 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 +857,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 05d0898..ce781ae 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;
/*
^ permalink raw reply related
* [PATCH 3/3] Add testcases for the --factorize-renames diffcore flag.
From: Yann Dirson @ 2008-11-01 22:03 UTC (permalink / raw)
To: git
In-Reply-To: <20081101215739.1116.59319.stgit@gandelf.nowhere.earth>
This notably includes a couple of tests for cases known not to be
working correctly yet.
---
t/t4030-diff-rename-factorize.sh | 259 ++++++++++++++++++++++++++++++++++++++
1 files changed, 259 insertions(+), 0 deletions(-)
create mode 100755 t/t4030-diff-rename-factorize.sh
diff --git a/t/t4030-diff-rename-factorize.sh b/t/t4030-diff-rename-factorize.sh
new file mode 100755
index 0000000..302a7ab
--- /dev/null
+++ b/t/t4030-diff-rename-factorize.sh
@@ -0,0 +1,259 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Yann Dirson
+# Copyright (c) 2005 Junio C Hamano
+#
+
+test_description='Test rename factorization in diff engine.
+
+'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/diff-lib.sh
+
+test_expect_success \
+ 'commit the index.' \
+ 'git update-ref HEAD $(echo "original empty commit" | git commit-tree $(git write-tree))'
+
+mkdir a
+echo >a/path0 'Line 1
+Line 2
+Line 3
+Line 4
+Line 5
+Line 6
+Line 7
+Line 8
+Line 9
+Line 10
+line 11
+Line 12
+Line 13
+Line 14
+Line 15
+'
+sed <a/path0 >a/path1 s/Line/Record/
+sed <a/path0 >a/path2 s/Line/Stuff/
+sed <a/path0 >a/path3 s/Line/Blurb/
+
+test_expect_success \
+ 'update-index --add file inside a directory.' \
+ 'git update-index --add a/path*'
+
+test_expect_success \
+ 'commit the index.' \
+ 'git update-ref HEAD $(echo "original set of files" | git commit-tree $(git write-tree))'
+
+mv a b
+test_expect_success \
+ 'renamed the directory.' \
+ 'git update-index --add --remove a/path0 a/path1 a/path2 a/path3 b/path*'
+
+test_expect_success \
+ 'git diff-index --factorize-renames after directory move.' \
+ 'git diff-index --factorize-renames HEAD >current'
+grep -v "^\[DBG\] " <current >current.filtered
+cat >expected <<\EOF
+:040000 040000 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 R100 a/ b/
+EOF
+
+test_expect_success \
+ 'validate the output for directory move.' \
+ 'compare_diff_patch current.filtered expected'
+
+# now test non-100% renames
+
+echo 'Line 16' >> b/path0
+mv b/path2 b/2path
+rm b/path3
+echo anything > b/path100
+test_expect_success \
+ 'edited dir contents.' \
+ 'git update-index --add --remove b/* b/path2 b/path3'
+
+test_expect_success \
+ 'git diff-index --factorize-renames after directory move and content changes.' \
+ 'git diff-index --factorize-renames HEAD >current'
+grep -v "^\[DBG\] " <current >current.filtered
+cat >expected <<\EOF
+:040000 040000 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 R100 a/ b/
+:100644 000000 c6971ab9f08a6cd9c89a0f87d94ae347aad6144a 0000000000000000000000000000000000000000 D a/path3
+:100644 100644 dbde7141d737c8aa0003672c1bc21ded48c6c3b9 dbde7141d737c8aa0003672c1bc21ded48c6c3b9 R100 a/path2 b/2path
+:100644 100644 fdbec444a77953b1bcc899d9fabfa202e5e68f08 4db595d12886f90e36765fc1732c17bccb836663 R093 a/path0 b/path0
+:000000 100644 0000000000000000000000000000000000000000 1ba4650885513e62386fd3e23aeb45beeb67d3bb A b/path100
+EOF
+
+test_expect_success \
+ 'validate the output for directory move and content changes.' \
+ 'compare_diff_patch current.filtered expected'
+
+git reset --hard
+
+# now test bulk moves that are not directory moves (get consensus before going further ?)
+
+mkdir c
+for i in 0 1 2; do cp a/path$i c/apath$i; done
+test_expect_success \
+ 'add files into a new directory.' \
+ 'git update-index --add c/apath*'
+
+test_expect_success \
+ 'commit all this.' \
+ 'git commit -m "first set of changes"'
+
+mv c/* a/
+test_expect_success \
+ 'move all of the new dir contents into a preexisting dir.' \
+ 'git update-index --add --remove a/* c/apath0 c/apath1 c/apath2'
+
+test_expect_success \
+ 'git diff-index --factorize-renames without full-dir rename.' \
+ 'git diff-index --factorize-renames HEAD >current'
+grep -v "^\[DBG\] " <current >current.filtered
+cat >expected <<\EOF
+:040000 040000 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 R100 c/* a/
+EOF
+
+test_expect_failure \
+ 'validate the output for bulk rename without full-dir rename.' \
+ 'compare_diff_patch current.filtered expected'
+
+git reset --hard
+
+# now test moves to toplevel
+
+mv c/* .
+test_expect_success \
+ 'move all of the new dir contents into toplevel.' \
+ 'git update-index --add --remove apath* c/apath0 c/apath1 c/apath2'
+
+test_expect_success \
+ 'git diff-index --factorize-renames files bulk-moved to toplevel.' \
+ 'git diff-index --factorize-renames HEAD >current'
+grep -v "^\[DBG\] " <current >current.filtered
+cat >expected <<\EOF
+:040000 040000 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 R100 c/* ./
+EOF
+
+test_expect_failure \
+ 'validate the output for files bulk-moved to toplevel.' \
+ 'compare_diff_patch current.filtered expected'
+
+git reset --hard
+
+# now test renaming with subdirs (does not take subdirs into account)
+
+mv c a/
+test_expect_success \
+ 'move the new dir as subdir of another.' \
+ 'git update-index --add --remove a/c/* c/apath0 c/apath1 c/apath2'
+
+test_expect_success \
+ 'commit all this.' \
+ 'git commit -m "move as subdir"'
+
+mv a b
+echo foo >> b/c/apath0
+test_expect_success \
+ 'rename the directory with some changes.' \
+ 'git update-index --add --remove a/path0 a/path1 a/path2 a/path3 a/c/apath0 a/c/apath1 a/c/apath2 b/path* b/c/apath*'
+
+test_expect_success \
+ 'git diff-index --factorize-renames on a move including a subdir.' \
+ 'git diff-index --factorize-renames HEAD >current'
+grep -v "^\[DBG\] " <current >current.filtered
+cat >expected <<\EOF
+:040000 040000 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 R100 a/ b/
+:100644 100644 fdbec444a77953b1bcc899d9fabfa202e5e68f08 00084e5ea68b5ae339b7c4b429e4a70fe25d069b R096 a/c/apath0 b/c/apath0
+EOF
+
+test_expect_failure \
+ 'validate the output for a move including a subdir.' \
+ 'compare_diff_patch current.filtered expected'
+
+git reset --hard
+
+# now check that moving all files but not subdirs is not mistaken for dir move
+
+mkdir b
+mv a/path* b/
+test_expect_success \
+ 'rename files in the directory but not subdir.' \
+ 'git update-index --add --remove a/path0 a/path1 a/path2 a/path3 b/path*'
+
+test_expect_success \
+ 'git diff-index --factorize-renames on a move without a subdir.' \
+ 'git diff-index --factorize-renames HEAD >current'
+grep -v "^\[DBG\] " <current >current.filtered
+cat >expected <<\EOF
+:100644 100644 fdbec444a77953b1bcc899d9fabfa202e5e68f08 fdbec444a77953b1bcc899d9fabfa202e5e68f08 R100 a/path0 b/path0
+:100644 100644 2f1f8d70c0fdad990819dfe37a31deb010805161 2f1f8d70c0fdad990819dfe37a31deb010805161 R100 a/path1 b/path1
+:100644 100644 dbde7141d737c8aa0003672c1bc21ded48c6c3b9 dbde7141d737c8aa0003672c1bc21ded48c6c3b9 R100 a/path2 b/path2
+:100644 100644 c6971ab9f08a6cd9c89a0f87d94ae347aad6144a c6971ab9f08a6cd9c89a0f87d94ae347aad6144a R100 a/path3 b/path3
+EOF
+
+test_expect_success \
+ 'validate the output for a move without a subdir.' \
+ 'compare_diff_patch current.filtered expected'
+
+git reset --hard
+
+# now check that moving subdirs into one dir and files into another is not mistaken for dir move
+# (well, clearly it is ...)
+
+mv a/c b
+mv a d
+test_expect_success \
+ 'rename subdir and files into different places.' \
+ 'git update-index --add --remove a/path0 a/path1 a/path2 a/path3 a/c/apath0 a/c/apath1 a/c/apath2 d/path* b/apath*'
+
+test_expect_success \
+ 'git diff-index --factorize-renames on a split of subdir and files into different places.' \
+ 'git diff-index --factorize-renames HEAD >current'
+grep -v "^\[DBG\] " <current >current.filtered
+cat >expected <<\EOF
+:100644 100644 fdbec444a77953b1bcc899d9fabfa202e5e68f08 fdbec444a77953b1bcc899d9fabfa202e5e68f08 R100 a/path0 d/path0
+:100644 100644 2f1f8d70c0fdad990819dfe37a31deb010805161 2f1f8d70c0fdad990819dfe37a31deb010805161 R100 a/path1 d/path1
+:100644 100644 dbde7141d737c8aa0003672c1bc21ded48c6c3b9 dbde7141d737c8aa0003672c1bc21ded48c6c3b9 R100 a/path2 d/path2
+:100644 100644 c6971ab9f08a6cd9c89a0f87d94ae347aad6144a c6971ab9f08a6cd9c89a0f87d94ae347aad6144a R100 a/path3 d/path3
+:040000 040000 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 R100 a/c/ b/
+EOF
+
+test_expect_failure \
+ 'validate the output for a split of subdir and files into different places.' \
+ 'compare_diff_patch current.filtered expected'
+
+# now test moving all files from toplevel into subdir (does not hides file moves) (needs consensus on syntax)
+# Note: this is as special case of move of a dir into one of its own subdirs, which in
+# turn is a special case of new files/dirs being added into a dir after all its contents
+# are moved away
+
+git reset --hard HEAD~2
+
+mv a/* .
+test_expect_success \
+ 'rename the directory with some changes.' \
+ 'git update-index --add --remove a/path0 a/path1 a/path2 a/path3 path*'
+
+test_expect_success \
+ 'commit all this.' \
+ 'git commit -m "move all files to toplevel"'
+
+mkdir z
+mv path* z/
+test_expect_success \
+ 'rename the directory with some changes.' \
+ 'git update-index --add --remove path0 path1 path2 path3 z/path*'
+
+test_expect_success \
+ 'git diff-index --factorize-renames everything from toplevel.' \
+ 'git diff-index --factorize-renames HEAD >current'
+grep -v "^\[DBG\] " <current >current.filtered
+cat >expected <<\EOF
+:040000 040000 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 R100 ./* z/
+EOF
+
+test_expect_failure \
+ 'validate the output for a move of everything from toplevel.' \
+ 'compare_diff_patch current.filtered expected'
+
+test_done
^ permalink raw reply related
* Re: git/lib and git/git-gui/lib merge mis-hap?
From: Junio C Hamano @ 2008-11-01 22:46 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Andreas Ericsson, Git Mailing List
In-Reply-To: <20081101202201.GA15463@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Andreas Ericsson <ae@op5.se> wrote:
>> Settling down to get some libgit2 hacking done (adding build-rules
>> to git.git), I noticed that there's a file in git.git called
>> lib/remote_add.tcl, which looks as if it belongs in git-gui/lib.
>>
>> I don't know how this happened, but since I assume it's subtree
>> merged (thus requiring more work to correct than a simple patch),
>> it would be nifty if it could get corrected so as to make space
>> for the up-and-coming git library :-)
>
> That was a bad merge of git-gui on my part. I thought it was fixed.
> It only happened in next, and was there only for a day before
> someone pointed it out to me, and I fixed it in the tree.
You seem to have further changes to the master branch of git-gui. Would
it be a good time to pull for me? The same question goes to maint as
well.
^ permalink raw reply
* Re: [PATCH] connect.c: add a way for git-daemon to pass an error back to client
From: Junio C Hamano @ 2008-11-01 22:48 UTC (permalink / raw)
To: Tom Preston-Werner; +Cc: git
In-Reply-To: <b97024a40810312329o53e37fd5td82aa69634ff1e6b@mail.gmail.com>
"Tom Preston-Werner" <tom@github.com> writes:
> Explicitly testing for "ERR " (including the space) does seem like the
> more correct thing to do. Would you like me to resubmit a modified
> patch that uses prefixcmp()?
No need for resubmission to change something minor like that. Will queue
with a fixup.
Thanks.
^ permalink raw reply
* Re: [PATCH] asciidoc: add minor workaround to add an empty line after code blocks
From: Junio C Hamano @ 2008-11-01 22:48 UTC (permalink / raw)
To: Jonas Fonseca; +Cc: Teemu Likonen, git
In-Reply-To: <20081030104503.GA17131@diku.dk>
Thanks; I do not have an environment with docbook-xsl-172 handy, so I'll
just take your word and apply it to 'maint'.
^ permalink raw reply
* Re: libgit2 - a true git library
From: Shawn O. Pearce @ 2008-11-01 22:57 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Pierre Habouzit, david, git
In-Reply-To: <alpine.LFD.2.00.0810312150200.13034@xanadu.home>
Nicolas Pitre <nico@cam.org> wrote:
> On Fri, 31 Oct 2008, Shawn O. Pearce wrote:
>
> > My take on the consensus for the license part of the discussion is
> > that libgit2 should be under the "GPL gcc library" license.
> >
> > BTW, I can't actually find a copy of that license; the only thing
> > I can locate in the GCC SVN tree is a copy of the LGPL.
>
> The exception is usually found at the top of files constituting
> libgcc.a. One example is gcc/config/arm/ieee754-df.S. ;-)
Headers updated. Its now GPL+gcc library exception.
Not that the 5 lines of useful code there really needs copyright,
but hey, whatever.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] prepare deprecation of git-revert
From: Matthieu Moy @ 2008-11-01 23:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Alex Riesen, Pierre Habouzit, git
In-Reply-To: <7vej1w73nr.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> The current state of affairs is that there is no remedy if teachers find
> "git checkout -- path" or "git revert HEAD~24" is confusing to new people.
> By introducing "git unstage path" or "git cherry-pick -r HEAD~24",
> teachers can choose to teach what they feel less confusing, and they do
> not have to teach "git checkout -- path" or "git revert HEAD~24". We
> should stop there.
I think you should go half a step further: officially deprecate
"revert", but keep it supported forever. The reason is just to help
documentation to be homogeneous (I foresee questions of users having
read here about "cherry-pick -R" and there about "revert" and asking
"should I use one or the other"). At least, the man page for "revert"
should state explicitly "this is a convenience alias for ...".
But I agree that removing support for "revert" is probably useless and
somehow harmfull.
(my 2 cents ...)
--
Matthieu
^ permalink raw reply
* Re: git/lib and git/git-gui/lib merge mis-hap?
From: Shawn O. Pearce @ 2008-11-01 23:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Andreas Ericsson, Git Mailing List
In-Reply-To: <7vljw33vz5.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
>
> You seem to have further changes to the master branch of git-gui. Would
> it be a good time to pull for me? The same question goes to maint as
> well.
Yup. I was getting ready to ask you to pull.
Can you pull maint and master?
:-)
--
Shawn.
^ permalink raw reply
* Re: [PATCH] asciidoc: add minor workaround to add an empty line after code blocks
From: Thomas Adam @ 2008-11-01 23:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonas Fonseca, Teemu Likonen, git
In-Reply-To: <7v7i7n3vwe.fsf@gitster.siamese.dyndns.org>
Hello --
2008/11/1 Junio C Hamano <gitster@pobox.com>:
> Thanks; I do not have an environment with docbook-xsl-172 handy, so I'll
> just take your word and apply it to 'maint'.
Just out of interest, how much progression on the asciidoc Git
documentation is there with respect to the latest version of asciidoc
which might give new features, if that makes sense? Something the
ELinks project does is distribute a fixed version of the asciidoc
script to avoid annoying asciidoc errors each time there's a new
asciidoc release.
Is this something worth considering for Git as well?
-- Thomas Adam
^ permalink raw reply
* Re: libgit2 - a true git library
From: Scott Chacon @ 2008-11-02 0:26 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Nicolas Pitre, Pierre Habouzit, david, git
In-Reply-To: <20081101225714.GD15463@spearce.org>
I'm sorry - why is that better than LGPL? Wouldn't it be better to
use a license that people have heard of rather than one that can't be
looked up or it's implications easily researched? What is this
affording the library that offsets the headaches of everyone trying to
figure out if they can use it or not?
Scott
On Sat, Nov 1, 2008 at 3:57 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Nicolas Pitre <nico@cam.org> wrote:
>> On Fri, 31 Oct 2008, Shawn O. Pearce wrote:
>>
>> > My take on the consensus for the license part of the discussion is
>> > that libgit2 should be under the "GPL gcc library" license.
>> >
>> > BTW, I can't actually find a copy of that license; the only thing
>> > I can locate in the GCC SVN tree is a copy of the LGPL.
>>
>> The exception is usually found at the top of files constituting
>> libgcc.a. One example is gcc/config/arm/ieee754-df.S. ;-)
>
> Headers updated. Its now GPL+gcc library exception.
>
> Not that the 5 lines of useful code there really needs copyright,
> but hey, whatever.
>
> --
> Shawn.
> --
> 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: [PATHv2 8/8] gitweb: make the supported snapshot formats array global
From: Jakub Narebski @ 2008-11-02 1:54 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Petr Baudis, Junio C Hamano
In-Reply-To: <1224426270-27755-3-git-send-email-giuseppe.bilotta@gmail.com>
On Sun, 19 Oct 2008, Giuseppe Bilotta wrote:
> The @snapshot_fmts array, containing the list of the supported snapshot
> formats, was recreated locally in three different routines (with the
> different name @supported_fmts in one of them).
>
> Its local generation is particularly expensive because two of the
> callers, href() and format_snapshot_links(), are often called many times
> in a single page.
>
> Simplify code and speed up page generation by making the array global.
>
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Very good idea, although I'd prefer for this patch to come first;
it is not controversial contrary to other patches in this (sub)series
which IMHO needs some thought first.
Acked-by: Jakub Narebski <jnareb@gmail.com>
> ---
> gitweb/gitweb.perl | 21 ++++++++-------------
> 1 files changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 5fd5a1f..d1475b7 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -748,6 +748,10 @@ if (defined $searchtext) {
> our $git_dir;
> $git_dir = "$projectroot/$project" if $project;
>
> +# list of supported snapshot formats
> +our @snapshot_fmts = gitweb_check_feature('snapshot');
> +@snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts);
> +
> # dispatch
> if (!defined $action) {
> if (defined $hash) {
> @@ -858,11 +862,7 @@ sub href (%) {
> # snapshot_format should always be defined when href()
> # is called, but just in case some code forgets, we
> # fall back to the default
> - if (!$fmt) {
> - my @snapshot_fmts = gitweb_check_feature('snapshot');
> - @snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts);
> - $fmt = $snapshot_fmts[0];
> - }
> + $fmt ||= $snapshot_fmts[0];
> $href .= $known_snapshot_formats{$fmt}{'suffix'};
> delete $params{'snapshot_format'};
> }
> @@ -1695,8 +1695,6 @@ sub format_diff_line {
> # linked. Pass the hash of the tree/commit to snapshot.
> sub format_snapshot_links {
> my ($hash) = @_;
> - my @snapshot_fmts = gitweb_check_feature('snapshot');
> - @snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts);
> my $num_fmts = @snapshot_fmts;
> if ($num_fmts > 1) {
> # A parenthesized list of links bearing format names.
> @@ -4898,20 +4896,17 @@ sub git_tree {
> }
>
> sub git_snapshot {
> - my @supported_fmts = gitweb_check_feature('snapshot');
> - @supported_fmts = filter_snapshot_fmts(@supported_fmts);
> -
> my $format = $input_params{'snapshot_format'};
> - if (!@supported_fmts) {
> + if (!@snapshot_fmts) {
> die_error(403, "Snapshots not allowed");
> }
> # default to first supported snapshot format
> - $format ||= $supported_fmts[0];
> + $format ||= $snapshot_fmts[0];
> if ($format !~ m/^[a-z0-9]+$/) {
> die_error(400, "Invalid snapshot format parameter");
> } elsif (!exists($known_snapshot_formats{$format})) {
> die_error(400, "Unknown snapshot format");
> - } elsif (!grep($_ eq $format, @supported_fmts)) {
> + } elsif (!grep($_ eq $format, @snapshot_fmts)) {
> die_error(403, "Unsupported snapshot format");
> }
>
> --
> 1.5.6.5
>
>
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH] Documentation: add a planning document for the next CLI revamp
From: Theodore Tso @ 2008-11-02 1:06 UTC (permalink / raw)
To: Elijah Newren; +Cc: Sam Vilain, git, Sam Vilain
In-Reply-To: <51419b2c0811011327j492b520dq2388fc8972b48cab@mail.gmail.com>
On Sat, Nov 01, 2008 at 02:27:03PM -0600, Elijah Newren wrote:
>
> There is another option, though it has its own problems too. There
> are basically two kinds of reverting here -- reverting all the changes
> *in* a given revision (which I'll called 'revert-in') and reverting
> all the changes *since* a given revision (typically HEAD; I'll call
> this 'revert-since'). These two operations can be supported from the
> same command, though their use cases are different enough that it may
> seem slightly weird:
In my opinion, that is a Really Bad Idea from a usability and UI
design point of view. Each command should do one and only one thing,
and not do different things depending on what options you give it.
Git violates this rules in a number of places already, What you call
"revert-since" and "revert-in" are so different that using the same
subcommand is just going to horribly confuse users.
Better to have "git revert" print a message explining that it is
deprecated, and to tell users that they probably want either "git
cherry-pick --revert" or "git revert-file", depending on whether they
are an experienced git user (in which case they probably want git
cherry-pick --revert"), or if that person who is familiar svn or hg's
"svn revert" or "hg revert", they probably want "git revert-file".
- Ted
^ permalink raw reply
* Re: libgit2 - a true git library
From: Scott Chacon @ 2008-11-02 1:07 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Nicolas Pitre, Pierre Habouzit, david, git
In-Reply-To: <d411cc4a0811011726h1fb1ad0ct5c37af753940f4a4@mail.gmail.com>
> On Sat, Nov 1, 2008 at 3:57 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
>> Nicolas Pitre <nico@cam.org> wrote:
>>> On Fri, 31 Oct 2008, Shawn O. Pearce wrote:
>>>
>>> > My take on the consensus for the license part of the discussion is
>>> > that libgit2 should be under the "GPL gcc library" license.
>>> >
>>> > BTW, I can't actually find a copy of that license; the only thing
>>> > I can locate in the GCC SVN tree is a copy of the LGPL.
>>>
>>> The exception is usually found at the top of files constituting
>>> libgcc.a. One example is gcc/config/arm/ieee754-df.S. ;-)
>>
>> Headers updated. Its now GPL+gcc library exception.
>>
>> Not that the 5 lines of useful code there really needs copyright,
>> but hey, whatever.
I guess my main concern is that if a company wanted to direct
resources at supporting Git in something (say, an editor or GUI or
whatnot), and that company is of _any_ size, they are going to have to
get their legal department to review this strange and almost totally
unused license - only knowing that it's barely different than GPL and
they know GPL will not fly. LGPL will likely be known to them and a
policy may already be in place.
Think about trying to incorporate this into something proprietary,
Shawn - how much of a pain is it going to be to get that license
reviewed in Google? However, LGPL I'm sure there is already a
reviewed policy. Now, since that may be a pain, time that Shawn could
have been spending being paid to work on the library is lost because
they can't use it, or it takes weeks/months to review it. That's my
concern.
I personally would rather see it BSD or something more permissive so
that no human has to waste even a second of their valuable time
figuring out if they can work with it or not, but I understand that
many people here are much more protective of their code. I simply
think that LGPL is a much more widely used and understood compromise
that affords nearly the same protectionism.
Scott
>>
>> --
>> Shawn.
>> --
>> 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: Git/Mercurial interoperability (and what about bzr?)
From: Theodore Tso @ 2008-11-02 1:13 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Florian Weimer, Jakub Narebski, git
In-Reply-To: <alpine.LFD.2.00.0811011047050.3483@nehalem.linux-foundation.org>
On Sat, Nov 01, 2008 at 10:51:51AM -0700, Linus Torvalds wrote:
>
>
> On Sat, 1 Nov 2008, Theodore Tso wrote:
> >
> > .hgtags is stored as a versioned file in Mercurial. That's one of the
> > problems, and it leads to no shortage of headaches.
>
> I told people this was insane long long ago, and I thought the hg people
> had learnt to use local tags. They act sanely, as far as I know (ie they
> act the same way git tags do).
>
> Of course, the problem with hg local tags is that hg apparently has no
> sane way to _propagate_ such local tag-space information from one
> repository to another. But that's purely a problem with hg itself. I don't
> know why that hasn't gotten fixed.
Yeah, well, hg calls them _local_ tags, and so people consider that by
design, they aren't supposed to be propagated outside of the local
repository. As I recall, hg doesn't support GPG signing local tags,
for the same reason.
- Ted
^ permalink raw reply
* [RFC/PATCH 1/2] bisect: add "git bisect replace" subcommand
From: Christian Couder @ 2008-11-02 1:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
This new subcommand should be used when you have a branch or a part of
a branch that isn't easily bisectable because at some point, say A, a
bug as been introduced. The bug has been fixed latter at another point,
say B, but 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 commit 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 commit B will be tagged with a special name like:
"bisect-replace-with-Yn"
When bisecting, commit with those tags will be grafted so that their
parents will be replaced by the commit specified in the tag.
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.
---
builtin-rev-list.c | 29 ++++++++++++++++++++++++++++-
git-bisect.sh | 40 +++++++++++++++++++++++++++++++++++++++-
2 files changed, 67 insertions(+), 2 deletions(-)
Thank you Junio for your help in designing this during the GitTogether,
especially for the trick to change the parents of the commit fixing the
bug.
I know this is lacking documentation but I plan to work on it soon and
send a documentation patch. The tests (in the following patch) are also
a bit lacking, so I will improve them too.
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 06cdeb7..5569bb2 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -529,6 +529,30 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
return best_bisection_sorted(list, nr);
}
+static int bisect_replace(const char *refname, const unsigned char *sha1,
+ int flag, void *cb_data)
+{
+ struct commit_graft *graft;
+
+ if (prefixcmp(refname, "bisect-replace-with-"))
+ return 0;
+
+ /* Create a graft to replace current commit */
+
+ graft = xmalloc(sizeof(*graft) + 20);
+
+ hashcpy(graft->sha1, sha1);
+ graft->nr_parent = 1;
+ if (get_sha1_hex(refname + 20, graft->parent[0])) {
+ free(graft);
+ return 0;
+ }
+
+ register_commit_graft(graft, 1);
+
+ return 0;
+}
+
static struct commit_list *find_bisection(struct commit_list *list,
int *reaches, int *all,
int find_all)
@@ -646,8 +670,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) {
+ for_each_tag_ref(bisect_replace, NULL);
revs.limited = 1;
+ }
if (prepare_revision_walk(&revs))
die("revision walk setup failed");
diff --git a/git-bisect.sh b/git-bisect.sh
index 0d0e278..245435c 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|run|replace]'
LONG_USAGE='git bisect help
print this long help message.
git bisect start [<bad> [<good>...]] [--] [<pathspec>...]
@@ -23,6 +23,8 @@ git bisect log
show bisect log.
git bisect run <cmd>...
use <cmd>... to automatically bisect.
+git bisect replace <rev> [<rev>]
+ use another branch for bisection.
Please use "git help bisect" to get the full man page.'
@@ -618,6 +620,40 @@ bisect_run () {
done
}
+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"
+
+ # Tag the source commit
+ src_tag="bisect-replace-with-$tgt_commit"
+ git tag "$src_tag" "$src_commit" || exit
+
+ # Create branch for the target commit
+ tgt_branch="bisect-replace-$src_commit"
+ git branch "$tgt_branch" "$tgt_commit" || exit
+}
+
case "$#" in
0)
@@ -645,6 +681,8 @@ case "$#" in
cat "$GIT_DIR/BISECT_LOG" ;;
run)
bisect_run "$@" ;;
+ replace)
+ bisect_replace "$@" ;;
*)
usage ;;
esac
--
1.6.0.3.531.gd12eb.dirty
^ permalink raw reply related
* [RFC/PATCH 2/2] bisect: add test case for "git bisect replace"
From: Christian Couder @ 2008-11-02 1:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
---
t/t6035-bisect-replace.sh | 89 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 89 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..ed2061e
--- /dev/null
+++ b/t/t6035-bisect-replace.sh
@@ -0,0 +1,89 @@
+#!/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=
+
+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)
+'
+
+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 &&
+ 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_done
--
1.6.0.3.531.gd12eb.dirty
^ permalink raw reply related
* Re: libgit2 - a true git library
From: Shawn O. Pearce @ 2008-11-02 1:36 UTC (permalink / raw)
To: Scott Chacon; +Cc: Nicolas Pitre, Pierre Habouzit, david, git
In-Reply-To: <d411cc4a0811011807g229f8becs9f411d6e19fb6c12@mail.gmail.com>
Scott Chacon <schacon@gmail.com> wrote:
> > On Sat, Nov 1, 2008 at 3:57 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> >>
> >> Headers updated. Its now GPL+gcc library exception.
>
> I personally would rather see it BSD or something more permissive so
> that no human has to waste even a second of their valuable time
> figuring out if they can work with it or not, but I understand that
> many people here are much more protective of their code. I simply
> think that LGPL is a much more widely used and understood compromise
> that affords nearly the same protectionism.
Apparently BSD won't fly, as you have already seen on the list.
If we did put the library under a BSD license we'd lose some core
contributors. Or they at least wouldn't improve the library code,
even if git.git linked to it in the future. I don't want to lose
these folks.
IANAL, but from what I can tell the main difference between LGPL
and GPL+"gcc library exception" is that the LGPL requires that
the end-user must be able to relink the derived executable with
their own replacement library. The GPL+"gcc library exception"
makes no such requirement.
If you read the exception clause it practically makes the library
even easier to use commerically than the BSD license does, however
modifications to the library sources must still be distributed.
Isn't that actually somewhat close to the Mozilla Public License?
--
Shawn.
^ permalink raw reply
* Re: libgit2 - a true git library
From: Shawn O. Pearce @ 2008-11-02 1:50 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Pierre Habouzit, git, Scott Chacon
In-Reply-To: <490CD101.1030604@op5.se>
Andreas Ericsson <ae@op5.se> wrote:
> Shawn O. Pearce wrote:
>>
>> Eh, I disagree here. In git.git today "struct commit" exposes its
>> buffer with the canonical commit encoding. Having that visible
>> wrecks what Nico and I were thinking about doing with pack v4 and
>> encoding commits in a non-canonical format when stored in packs.
>> Ditto with trees.
>
> Err... isn't that backwards?
No.
> Surely you want to store stuff in the
> canonical format so you're forced to do as few translations as
> possible?
No. We suspect that canonical format is harder to decompress and
parse during revision traversal. Other encodings in the pack file
may produce much faster runtime performance, and reduce page faults
(due to smaller pack sizes).
We hardly ever use the canonical format for actual output; most
output rips the canonical format apart and then formats the data
the way it was requested. If we have the data *already* parsed in
the pack its much faster to output.
> Or are you trying to speed up packing by skipping the
> canonicalization part?
Wrong; we're trying to speed up reading. Packing may go slower,
especially during the first conversion of v2->v4 for any given
repository, but packing is infrequent so the minor (if any) drop
in performance here is probably worth the reading performance gains.
> Well, if macro usage is adhered to one wouldn't have to worry,
> since the macro can just be rewritten with a function later (if,
> for example, translation or some such happens to be required).
> Older code linking to a newer library would work (assuming the
> size of the commit object doesn't change anyway),
You are assuming too much magic. If the older ABI used a macro
and the newer one (which supports pack v4) organized struct commit
differently and the user upgrades libgit2.so the older applications
just broke, horribly.
We know we want to do pack v4 in the near future. Or at least
experiment with it and see if it works. If it does, we don't
want to have to cause a major ABI breakage across all those newly
installed libgit2s... yikes.
I'm really in favor of accessor functions for the first version of
the library. They can always be converted to macros once someone
shows that their git visualizer program saves 10 ms on a 8,000 ms
render operation by avoiding accessor functions. I'd rather spend
our brain cycles optimizing the runtime and the in-core data so
we spend less time in our tight revision traversal loops.
Seriously. We make at least 10 or 11 function calls *per commit*
that comes out of get_revision(). If the formatting application is
really suffering from its 4 or 5 accessor function calls in order
to get that returned data, we probably should also be looking at
how we can avoid function cals in the library.
Oh, and even with 4 or 5 accessor functions per commit in the
application that is *still* better than the 10 or so calls the
application probably makes today scraping "git log --format=raw"
off a pipe and segment it into the different fields it needs.
Unless pipes in Linux somehow allow negative time warping with
CPU counters. Though on dual-core systems they might, since the
two processes can run on different cores. But oh, you didn't want
to worry about threading support too much in libgit2, so I guess
you also don't want to use multi-core systems.
--
Shawn.
^ permalink raw reply
* Re: libgit2 - a true git library
From: Shawn O. Pearce @ 2008-11-02 1:56 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git, Scott Chacon
In-Reply-To: <20081101173042.GE26229@artemis.corp>
Pierre Habouzit <madcoder@debian.org> wrote:
> On Fri, Oct 31, 2008 at 06:41:54PM +0000, Shawn O. Pearce wrote:
> > How about this?
> >
> > http://www.spearce.org/projects/scm/libgit2/apidocs/CONVENTIONS
>
> FWIW I've read what you say about types, while this is good design to
> make things abstract, accessors are slower _and_ disallow many
> optimizations as it's a function call and that it may clobber all your
> pointers values.
Yea, optimizing C is a bitch.
I'm in favor of accessors *IN THE APPLICATION*.
Within the library's own C code, I think we should expose the struct,
and use its members where it makes sense to. Especially in the
really tight loops where we don't want to introduce more overhead.
My rationale here is that we can change the struct at any time,
and yet not change the ABI.
> For types that _will_ be in the tight loops, we must make the types
> explicit or it'll bite us hard performance-wise. I'm thinking what is
> "struct object" or "struct commit" in git.git. It's likely that we will
> loose a *lot* of those types are opaque.
Yes, but I'm arguing they should be opaque to the application, and
visible to the library. Today the application is suffering from
massive fork+exec overhead. I really don't give a damn if the
application's compiler has to deal with a function call to read
from a private member of an opaque type. Its still thousands of
CPU instructions less per operation.
Come back to me a year after libgit2 has been widely deployed on
Linux distros and we have multiple applications linking to it.
Lets talk then about the harmful performance problems caused by
making these types opaque to the application. About that time
we'll also be talking about how great pack v4 is and why its a good
thing those types were opaque, as we didn't have to break the ABI
to introduce it.
> It's IMNSHO on the matter that core structures of git _will_ have to be
> made explicit. I'm thinking objects and their "subtypes" (commits,
> trees, blobs). Maybe a couple of things on the same vein.
Sure, but in the library only.
--
Shawn.
^ permalink raw reply
* Re: libgit2 - a true git library
From: Johannes Schindelin @ 2008-11-02 2:30 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Andreas Ericsson, git, Scott Chacon
In-Reply-To: <20081101204259.GC15463@spearce.org>
Hi,
On Sat, 1 Nov 2008, Shawn O. Pearce wrote:
> But I was also under the impression that the brilliant engineers who
> work for Microsoft decided that on their platform special annotations
> have to be inserted on functions that a DLL wants to export to
> applications.
Exactly. This is the "good" old __declspec(dllexport) for you. It is a
pain in the butt, but that is what you have to go for if libgit2 is
supposed to be any more portable than ligit.a.
Ciao,
Dscho
^ permalink raw reply
* Re: Git remote status
From: Jeff King @ 2008-11-02 3:30 UTC (permalink / raw)
To: Gonsolo; +Cc: git
In-Reply-To: <490CB390.9000206@gmail.com>
On Sat, Nov 01, 2008 at 08:52:48PM +0100, Gonsolo wrote:
> If I switch branches with "git checkout master" git tells me something
> like "Your branch is ahead of the tracked remote branch 'origin/master'
> by 39 commits".
> Is there a "git remote status" or git-status switch to get the same
> information without switching branches?
"git status" will do this automatically in recent versions of git (as of
1.6.0, I believe).
You can also use "git branch -v" to see a summary of how all branches
relate to their tracked counterparts.
> Sometimes it's valuable whether one should push changes (for example
> before installing a new Ubuntu version ;) ).
For that, I might want to actually _see_ the changes. So I would use:
git shortlog origin/master..
(or "log" with a variety of formatting options to get as much
information as you like). And if the relationship is more complex (i.e.,
I want to see if I need to push _or_ pull):
gitk origin/master...
-Peff
^ permalink raw reply
* Re: git reset --hard w/o touching every file
From: Jeff King @ 2008-11-02 3:33 UTC (permalink / raw)
To: Edward Z. Yang; +Cc: git
In-Reply-To: <geicn8$ss8$1@ger.gmane.org>
On Sat, Nov 01, 2008 at 04:03:50PM -0400, Edward Z. Yang wrote:
> Pierre Habouzit wrote:
> > git checkout HEAD -- <list of the files>
>
> What if I do not know a priori which files *do* need to be updated? Is
> there a command that I can get this information from? Also, I may not
Sorry, I don't quite understand. You want to check out some subset of
files, but you don't know which subset?
Try "git status" or "git diff" to look at which files have changes?
Or maybe we didn't understand your original question.
> necessarily be checking out HEAD.
Doing
git checkout v1.5 -- <list of files>
or using any other ref will work just fine.
-Peff
^ permalink raw reply
* Re: [PATCH] Documentation: add a planning document for the next CLI revamp
From: Jeff King @ 2008-11-02 3:42 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Sam Vilain, git, Scott Chacon, Tom Preston-Werner, J.H.,
Christian Couder, Kai Blin
In-Reply-To: <alpine.DEB.1.00.0810311745030.22125@pacific.mpi-cbg.de.mpi-cbg.de>
On Fri, Oct 31, 2008 at 05:46:35PM +0100, Johannes Schindelin wrote:
> > > + * 'git branch --switch' : alternative to checkout
> >
> > Blech. I think switching branches is the one thing that checkout does
> > unconfusedly. And this is much more typing. Not to mention that So I
> > would rather see "git switch" if checkout is somehow unpalatable.
>
> You know, I asked for this because a _user_ told me "Guess how long it
> took me to find out how to check out a branch!".
>
> I think if you are not confused by CVS/SVN, the name "checkout" is utterly
> unintuitive.
OK. I am not opposed to such a change as long as:
- this is not just _a_ user, but a _common_ user confusion. IOW, I
don't recall this complaint coming up a lot (or at least not nearly
as often as other ones do). But maybe you have more data.
- it is done consistently.
My initial "blech" was a little premature, as I was thinking "instead
of checkout", though it does clearly say "alternative" there.
However, (and somebody else in the thread very cleverly came up with
this analysis, not me), this is basically going from "verb the noun"
to "noun --verb". And that's reasonable, if users think in terms of
nouns. But we should be consistent in applying that transformation,
and make it available for other nouns that match that verb. In other
words, "git tag --switch". And however one might manipulate remote
tracking branches ("git branch -r --switch", I guess).
Personally I find it somewhat backwards, but I think CVS rotted my
brain long ago.
-Peff
^ 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