From 29dc13319883f97618de6f03e8ffc5dc810d8786 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 17 Mar 2008 16:47:18 -0700 Subject: [PATCH 01/10] git-merge-one-file: fix longstanding stupid thinko When a merge result creates a new file, and when our side already has a file in the path, taking the merge result may clobber the untracked file. However, the logic to detect this situation was totally the wrong way. We should complain when the file exists, not when the file does not exist. Signed-off-by: Junio C Hamano --- git-merge-one-file.sh | 5 ++-- t/t1004-read-tree-m-u-wf.sh | 46 +++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh index 9ee3f80452..e1eb963266 100755 --- a/git-merge-one-file.sh +++ b/git-merge-one-file.sh @@ -48,10 +48,11 @@ case "${1:-.}${2:-.}${3:-.}" in ;; "..$3") echo "Adding $4" - test -f "$4" || { + if test -f "$4" + then echo "ERROR: untracked $4 is overwritten by the merge." exit 1 - } + fi git update-index --add --cacheinfo "$7" "$3" "$4" && exec git checkout-index -u -f -- "$4" ;; diff --git a/t/t1004-read-tree-m-u-wf.sh b/t/t1004-read-tree-m-u-wf.sh index 9d1371c2c6..283e77cc51 100755 --- a/t/t1004-read-tree-m-u-wf.sh +++ b/t/t1004-read-tree-m-u-wf.sh @@ -157,4 +157,50 @@ test_expect_success '3-way not overwriting local changes (their side)' ' ' +test_expect_success 'D/F setup' ' + + git reset --hard && + + git checkout side-a && + rm -f subdir/file2 && + mkdir subdir/file2 && + echo qfwfq >subdir/file2/another && + git add subdir/file2/another && + test_tick && + git commit -m "side-a changes file2 to directory" + +' + +test_expect_success 'D/F' ' + + git checkout side-b && + git read-tree -m -u branch-point side-b side-a && + git ls-files -u >actual && + ( + a=$(git rev-parse branch-point:subdir/file2) + b=$(git rev-parse side-a:subdir/file2/another) + echo "100644 $a 1 subdir/file2" + echo "100644 $a 2 subdir/file2" + echo "100644 $b 3 subdir/file2/another" + ) >expect && + test_cmp actual expect + +' + +test_expect_success 'D/F resolve' ' + + git reset --hard && + git checkout side-b && + git merge-resolve branch-point -- side-b side-a + +' + +test_expect_success 'D/F recursive' ' + + git reset --hard && + git checkout side-b && + git merge-recursive branch-point -- side-b side-a + +' + test_done From 8d14ac945403d6d4b1de9f9fd680247e831c0bfc Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 18 Mar 2008 21:58:01 -0700 Subject: [PATCH 02/10] Test: catch if trash cannot be removed When your test creates an unwritable directory that test framework cannot clean out by "rm -fr trash", later tests cannot start in a fresh state they expect to. Detect this and error out early. Signed-off-by: Junio C Hamano --- t/test-lib.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index 268b26c959..870b255f13 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -396,7 +396,12 @@ fi # Test repository test=trash -rm -fr "$test" +rm -fr "$test" || { + trap - exit + echo >&5 "FATAL: Cannot prepare test area" + exit 1 +} + test_create_repo $test cd "$test" From 8a785dc921386dac628a2a04ebe5f48deb7cc1a8 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 18 Mar 2008 21:59:39 -0700 Subject: [PATCH 03/10] Add tests to catch problems with un-unlinkable symlinks This currently fails not because we refuse to check out, but because we detect error but incorrectly discard it in the callchain. Signed-off-by: Junio C Hamano --- t/t1004-read-tree-m-u-wf.sh | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/t/t1004-read-tree-m-u-wf.sh b/t/t1004-read-tree-m-u-wf.sh index 283e77cc51..1356148904 100755 --- a/t/t1004-read-tree-m-u-wf.sh +++ b/t/t1004-read-tree-m-u-wf.sh @@ -157,6 +157,41 @@ test_expect_success '3-way not overwriting local changes (their side)' ' ' +test_expect_success 'funny symlink in work tree' ' + + git reset --hard && + git checkout -b sym-b side-b && + mkdir -p a && + >a/b && + git add a/b && + git commit -m "side adds a/b" && + + rm -fr a && + git checkout -b sym-a side-a && + mkdir -p a && + ln -s ../b a/b && + git add a/b && + git commit -m "we add a/b" && + + git read-tree -m -u sym-a sym-a sym-b + +' + +test_expect_failure 'funny symlink in work tree, un-unlink-able' ' + + rm -fr a b && + git reset --hard && + + git checkout sym-a && + chmod a-w a && + test_must_fail git read-tree -m -u sym-a sym-a sym-b + +' + +# clean-up from the above test +chmod a+w a +rm -fr a b + test_expect_success 'D/F setup' ' git reset --hard && From c4758d3c9342ea2245ca51f30f1cbf27ecc16ced Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 18 Mar 2008 22:01:28 -0700 Subject: [PATCH 04/10] Fix read-tree not to discard errors This fixes the issue identified with recently added tests to t1004 Signed-off-by: Junio C Hamano --- t/t1004-read-tree-m-u-wf.sh | 2 +- unpack-trees.c | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/t/t1004-read-tree-m-u-wf.sh b/t/t1004-read-tree-m-u-wf.sh index 1356148904..570d3729bd 100755 --- a/t/t1004-read-tree-m-u-wf.sh +++ b/t/t1004-read-tree-m-u-wf.sh @@ -177,7 +177,7 @@ test_expect_success 'funny symlink in work tree' ' ' -test_expect_failure 'funny symlink in work tree, un-unlink-able' ' +test_expect_success 'funny symlink in work tree, un-unlink-able' ' rm -fr a b && git reset --hard && diff --git a/unpack-trees.c b/unpack-trees.c index 4b359e0832..a59f47557a 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -54,13 +54,14 @@ static void unlink_entry(char *name, char *last_symlink) } static struct checkout state; -static void check_updates(struct unpack_trees_options *o) +static int check_updates(struct unpack_trees_options *o) { unsigned cnt = 0, total = 0; struct progress *progress = NULL; char last_symlink[PATH_MAX]; struct index_state *index = &o->result; int i; + int errs = 0; if (o->update && o->verbose_update) { for (total = cnt = 0; cnt < index->cache_nr; cnt++) { @@ -90,12 +91,13 @@ static void check_updates(struct unpack_trees_options *o) if (ce->ce_flags & CE_UPDATE) { ce->ce_flags &= ~CE_UPDATE; if (o->update) { - checkout_entry(ce, &state, NULL); + errs |= checkout_entry(ce, &state, NULL); *last_symlink = '\0'; } } } stop_progress(&progress); + return errs != 0; } static inline int call_unpack_fn(struct cache_entry **src, struct unpack_trees_options *o) @@ -369,7 +371,8 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options return unpack_failed(o, "Merge requires file-level merging"); o->src_index = NULL; - check_updates(o); + if (check_updates(o)) + return -1; if (o->dst_index) *o->dst_index = o->result; return 0; From 971f229c50aeace83d6fd30de1de755f419d4cb8 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Mon, 17 Mar 2008 08:56:27 -0700 Subject: [PATCH 05/10] Fix possible Solaris problem in 'checkout_entry()' Currently when checking out an entry "path", we try to unlink(2) it first (because there could be stale file), and if there is a directory there, try to deal with it (typically we run recursive rmdir). We ignore the error return from this unlink because there may not even be any file there. However if you are root on Solaris, you can unlink(2) a directory successfully and corrupt your filesystem. This moves the code around and check the directory first, and then unlink(2). Also we check the error code from it. Signed-off-by: Linus Torvalds Signed-off-by: Junio C Hamano --- entry.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/entry.c b/entry.c index 44f4b897d4..222aaa374b 100644 --- a/entry.c +++ b/entry.c @@ -218,7 +218,6 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *t * to emulate by hand - much easier to let the system * just do the right thing) */ - unlink(path); if (S_ISDIR(st.st_mode)) { /* If it is a gitlink, leave it alone! */ if (S_ISGITLINK(ce->ce_mode)) @@ -226,7 +225,8 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *t if (!state->force) return error("%s is a directory", path); remove_subtree(path); - } + } else if (unlink(path)) + return error("unable to unlink old '%s' (%s)", path, strerror(errno)); } else if (state->not_new) return 0; create_directories(path, state); From ef00d150e4f9959bf083adf92419b5053ba11584 Mon Sep 17 00:00:00 2001 From: Daniel Barkalow Date: Mon, 17 Mar 2008 22:05:23 -0400 Subject: [PATCH 06/10] Tighten refspec processing This changes the pattern matching code to not store the required final / before the *, and then to require each side to be a valid ref (or empty). In particular, any refspec that looks like it should be a pattern but doesn't quite meet the requirements will be found to be invalid as a fallback non-pattern. Signed-off-by: Daniel Barkalow Signed-off-by: Junio C Hamano --- remote.c | 47 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/remote.c b/remote.c index f3f7375eea..9700a33c57 100644 --- a/remote.c +++ b/remote.c @@ -396,6 +396,7 @@ static void read_config(void) struct refspec *parse_ref_spec(int nr_refspec, const char **refspec) { int i; + int st; struct refspec *rs = xcalloc(sizeof(*rs), nr_refspec); for (i = 0; i < nr_refspec; i++) { const char *sp, *ep, *gp; @@ -404,13 +405,15 @@ struct refspec *parse_ref_spec(int nr_refspec, const char **refspec) rs[i].force = 1; sp++; } - gp = strchr(sp, '*'); + gp = strstr(sp, "/*"); ep = strchr(sp, ':'); if (gp && ep && gp > ep) gp = NULL; if (ep) { if (ep[1]) { - const char *glob = strchr(ep + 1, '*'); + const char *glob = strstr(ep + 1, "/*"); + if (glob && glob[2]) + glob = NULL; if (!glob) gp = NULL; if (gp) @@ -422,11 +425,24 @@ struct refspec *parse_ref_spec(int nr_refspec, const char **refspec) } else { ep = sp + strlen(sp); } + if (gp && gp + 2 != ep) + gp = NULL; if (gp) { rs[i].pattern = 1; ep = gp; } rs[i].src = xstrndup(sp, ep - sp); + + if (*rs[i].src) { + st = check_ref_format(rs[i].src); + if (st && st != CHECK_REF_FORMAT_ONELEVEL) + die("Invalid refspec '%s'", refspec[i]); + } + if (rs[i].dst && *rs[i].dst) { + st = check_ref_format(rs[i].dst); + if (st && st != CHECK_REF_FORMAT_ONELEVEL) + die("Invalid refspec '%s'", refspec[i]); + } } return rs; } @@ -543,7 +559,8 @@ int remote_find_tracking(struct remote *remote, struct refspec *refspec) if (!fetch->dst) continue; if (fetch->pattern) { - if (!prefixcmp(needle, key)) { + if (!prefixcmp(needle, key) && + needle[strlen(key)] == '/') { *result = xmalloc(strlen(value) + strlen(needle) - strlen(key) + 1); @@ -790,7 +807,9 @@ static const struct refspec *check_pattern_match(const struct refspec *rs, { int i; for (i = 0; i < rs_nr; i++) { - if (rs[i].pattern && !prefixcmp(src->name, rs[i].src)) + if (rs[i].pattern && + !prefixcmp(src->name, rs[i].src) && + src->name[strlen(rs[i].src)] == '/') return rs + i; } return NULL; @@ -989,7 +1008,7 @@ int get_fetch_map(const struct ref *remote_refs, struct ref ***tail, int missing_ok) { - struct ref *ref_map, *rm; + struct ref *ref_map, **rmp; if (refspec->pattern) { ref_map = get_expanded_map(remote_refs, refspec); @@ -1006,10 +1025,20 @@ int get_fetch_map(const struct ref *remote_refs, } } - for (rm = ref_map; rm; rm = rm->next) { - if (rm->peer_ref && check_ref_format(rm->peer_ref->name + 5)) - die("* refusing to create funny ref '%s' locally", - rm->peer_ref->name); + for (rmp = &ref_map; *rmp; ) { + if ((*rmp)->peer_ref) { + int st = check_ref_format((*rmp)->peer_ref->name + 5); + if (st && st != CHECK_REF_FORMAT_ONELEVEL) { + struct ref *ignore = *rmp; + error("* Ignoring funny ref '%s' locally", + (*rmp)->peer_ref->name); + *rmp = (*rmp)->next; + free(ignore->peer_ref); + free(ignore); + continue; + } + } + rmp = &((*rmp)->next); } if (ref_map) From 1d0a694b8ace7b54256c8c40ca23939c8c9b49fd Mon Sep 17 00:00:00 2001 From: Daniel Barkalow Date: Mon, 17 Mar 2008 22:04:40 -0400 Subject: [PATCH 07/10] Fix t3200 config "git-config name = value" doesn't do anything most of the time. The test meant "git-config name value", but that leaves the configuration such that later tests will be confused, so move it to the end. Signed-off-by: Daniel Barkalow Signed-off-by: Junio C Hamano --- t/t3200-branch.sh | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index 38a90adad6..cb5f7a4441 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -153,16 +153,6 @@ test_expect_success 'test tracking setup via config' \ test $(git config branch.my3.remote) = local && test $(git config branch.my3.merge) = refs/heads/master' -test_expect_success 'avoid ambiguous track' ' - git config branch.autosetupmerge true && - git config remote.ambi1.url = lalala && - git config remote.ambi1.fetch = refs/heads/lalala:refs/heads/master && - git config remote.ambi2.url = lilili && - git config remote.ambi2.fetch = refs/heads/lilili:refs/heads/master && - git branch all1 master && - test -z "$(git config branch.all1.merge)" -' - test_expect_success 'test overriding tracking setup via --no-track' \ 'git config branch.autosetupmerge true && git config remote.local.url . && @@ -224,4 +214,14 @@ test_expect_success \ test -f .git/logs/refs/heads/g/h/i && diff expect .git/logs/refs/heads/g/h/i' +test_expect_success 'avoid ambiguous track' ' + git config branch.autosetupmerge true && + git config remote.ambi1.url lalala && + git config remote.ambi1.fetch refs/heads/lalala:refs/heads/master && + git config remote.ambi2.url lilili && + git config remote.ambi2.fetch refs/heads/lilili:refs/heads/master && + git branch all1 master && + test -z "$(git config branch.all1.merge)" +' + test_done From 7d004199d134c9d465e013064f72dbc04507f6c0 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Mon, 17 Mar 2008 18:56:33 -0700 Subject: [PATCH 08/10] Make revision limiting more robust against occasional bad commit dates The revision limiter uses the commit date to decide when it has seen enough commits to finalize the revision list, but that can get confused if there are incorrect dates far in the past on some commits. This makes the logic a bit more robust by - we always walk an extra SLOP commits from the source list even if we decide that the source list is probably all done (unless the source is entirely empty, of course, because then we really can't do anything at all) - we keep track of the date of the last commit we added to the destination list (this will *generally* be the oldest entry we've seen so far) - we compare that with the youngest entry (the first one) of the source list, and if the destination is older than the source, we know we want to look at the source. which causes occasional date mishaps to be handled cleanly. Signed-off-by: Linus Torvalds Signed-off-by: Junio C Hamano --- revision.c | 46 ++++++++++++++++++++++++++++++-------- t/t6009-rev-list-parent.sh | 2 +- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/revision.c b/revision.c index 63bf2c5c2d..196fedc9d1 100644 --- a/revision.c +++ b/revision.c @@ -564,14 +564,39 @@ static void cherry_pick_list(struct commit_list *list, struct rev_info *revs) free_patch_ids(&ids); } -static void add_to_list(struct commit_list **p, struct commit *commit, struct commit_list *n) +/* How many extra uninteresting commits we want to see.. */ +#define SLOP 5 + +static int still_interesting(struct commit_list *src, unsigned long date, int slop) { - p = &commit_list_insert(commit, p)->next; - *p = n; + /* + * No source list at all? We're definitely done.. + */ + if (!src) + return 0; + + /* + * Does the destination list contain entries with a date + * before the source list? Definitely _not_ done. + */ + if (date < src->item->date) + return SLOP; + + /* + * Does the source list still have interesting commits in + * it? Definitely not done.. + */ + if (!everybody_uninteresting(src)) + return SLOP; + + /* Ok, we're closing in.. */ + return slop-1; } static int limit_list(struct rev_info *revs) { + int slop = SLOP; + unsigned long date = ~0ul; struct commit_list *list = revs->commits; struct commit_list *newlist = NULL; struct commit_list **p = &newlist; @@ -591,16 +616,19 @@ static int limit_list(struct rev_info *revs) return -1; if (obj->flags & UNINTERESTING) { mark_parents_uninteresting(commit); - if (everybody_uninteresting(list)) { - if (revs->show_all) - add_to_list(p, commit, list); - break; - } - if (!revs->show_all) + if (revs->show_all) + p = &commit_list_insert(commit, p)->next; + slop = still_interesting(list, date, slop); + if (slop) continue; + /* If showing all, add the whole pending list to the end */ + if (revs->show_all) + *p = list; + break; } if (revs->min_age != -1 && (commit->date > revs->min_age)) continue; + date = commit->date; p = &commit_list_insert(commit, p)->next; show = show_early_output; diff --git a/t/t6009-rev-list-parent.sh b/t/t6009-rev-list-parent.sh index f248a3293c..c8a96a9a99 100755 --- a/t/t6009-rev-list-parent.sh +++ b/t/t6009-rev-list-parent.sh @@ -27,7 +27,7 @@ test_expect_success setup ' git log --pretty=oneline --abbrev-commit ' -test_expect_failure 'one is ancestor of others and should not be shown' ' +test_expect_success 'one is ancestor of others and should not be shown' ' git rev-list one --not four >result && >expect && From 420e9af498848f9a3994ecb471dc51d5203f51cd Mon Sep 17 00:00:00 2001 From: Daniel Barkalow Date: Mon, 17 Mar 2008 22:15:02 -0400 Subject: [PATCH 09/10] Fix tag following Before the second fetch-pack connection in the same process, unmark all of the objects marked in the first connection, in order that we'll list them as things we have instead of thinking we've already mentioned them. Signed-off-by: Daniel Barkalow Signed-off-by: Junio C Hamano --- builtin-fetch-pack.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c index 7b28024224..65350ca522 100644 --- a/builtin-fetch-pack.c +++ b/builtin-fetch-pack.c @@ -26,6 +26,8 @@ static const char fetch_pack_usage[] = #define SEEN (1U << 3) #define POPPED (1U << 4) +static int marked; + /* * After sending this many "have"s if we do not get any new ACK , we * give up traversing our history. @@ -61,6 +63,16 @@ static int rev_list_insert_ref(const char *path, const unsigned char *sha1, int return 0; } +static int clear_marks(const char *path, const unsigned char *sha1, int flag, void *cb_data) +{ + struct object *o = deref_tag(parse_object(sha1), path, 0); + + if (o && o->type == OBJ_COMMIT) + clear_commit_marks((struct commit *)o, + COMMON | COMMON_REF | SEEN | POPPED); + return 0; +} + /* This function marks a rev and its ancestors as common. In some cases, it is desirable to mark only the ancestors (for example @@ -153,6 +165,10 @@ static int find_common(int fd[2], unsigned char *result_sha1, unsigned in_vain = 0; int got_continue = 0; + if (marked) + for_each_ref(clear_marks, NULL); + marked = 1; + for_each_ref(rev_list_insert_ref, NULL); fetching = 0; From 02b00e16bb372fd13e1ac440e9fbafd223d43af0 Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Tue, 18 Mar 2008 13:26:43 +0100 Subject: [PATCH 10/10] Documentation/git-merge: document subtree strategy. There was already some documentation about subtree under Documentation/howto but it was missing from git-merge manpage. Signed-off-by: Miklos Vajna Signed-off-by: Junio C Hamano --- Documentation/merge-strategies.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt index 7df0266ba8..1276f858ad 100644 --- a/Documentation/merge-strategies.txt +++ b/Documentation/merge-strategies.txt @@ -33,3 +33,10 @@ ours:: merge is always the current branch head. It is meant to be used to supersede old development history of side branches. + +subtree:: + This is a modified recursive strategy. When merging trees A and + B, if B corresponds to a subtree of A, B is first adjusted to + match the tree structure of A, instead of reading the trees at + the same level. This adjustment is also done to the common + ancestor tree.