* [PATCH 0/3] merge -Xindex-only
@ 2013-07-07 18:02 Thomas Rast
2013-07-07 18:02 ` [PATCH 1/3] merge-recursive: remove dead conditional in update_stages() Thomas Rast
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Thomas Rast @ 2013-07-07 18:02 UTC (permalink / raw)
To: git; +Cc: Michael Haggerty
[Michael, sorry for the double mail -- I typoed the list address on
the first round.]
I recently looked into making merge-recursive more useful as a modular
piece in various tasks, e.g. Michael's git-imerge and the experiments
I made in showing evil merges.
This miniseries is the extremely low-hanging fruit. If it makes a
good first step for git-imerge, perhaps it can go in like this. It's
not a big speedup (about 2.2s vs 2.4s in a sample conflicting merge in
git.git), but it does feel much cleaner to avoid touching the worktree
unless actually necessary.
Otherwise it's probably not worth it just yet; for what I want to do
with it, we need some more reshuffling of things.
Thomas Rast (3):
merge-recursive: remove dead conditional in update_stages()
merge-recursive: untangle double meaning of o->call_depth
merge-recursive: -Xindex-only to leave worktree unchanged
Documentation/merge-strategies.txt | 4 ++++
merge-recursive.c | 46 +++++++++++++++++++++-----------------
merge-recursive.h | 1 +
t/t3030-merge-recursive.sh | 13 +++++++++++
4 files changed, 43 insertions(+), 21 deletions(-)
--
1.8.3.2.908.gbd0dbd0
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/3] merge-recursive: remove dead conditional in update_stages() 2013-07-07 18:02 [PATCH 0/3] merge -Xindex-only Thomas Rast @ 2013-07-07 18:02 ` Thomas Rast 2013-07-07 18:02 ` [PATCH 2/3] merge-recursive: untangle double meaning of o->call_depth Thomas Rast ` (2 subsequent siblings) 3 siblings, 0 replies; 15+ messages in thread From: Thomas Rast @ 2013-07-07 18:02 UTC (permalink / raw) To: git; +Cc: Michael Haggerty 650467c (merge-recursive: Consolidate different update_stages functions, 2011-08-11) changed the former argument 'clear' to always be true. Remove the useless conditional. Signed-off-by: Thomas Rast <trast@inf.ethz.ch> --- merge-recursive.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index ea9dbd3..0b9cafb 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -544,11 +544,9 @@ static int update_stages(const char *path, const struct diff_filespec *o, * would_lose_untracked). Instead, reverse the order of the calls * (executing update_file first and then update_stages). */ - int clear = 1; int options = ADD_CACHE_OK_TO_ADD | ADD_CACHE_SKIP_DFCHECK; - if (clear) - if (remove_file_from_cache(path)) - return -1; + if (remove_file_from_cache(path)) + return -1; if (o) if (add_cacheinfo(o->mode, o->sha1, path, 1, 0, options)) return -1; -- 1.8.3.2.908.gbd0dbd0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/3] merge-recursive: untangle double meaning of o->call_depth 2013-07-07 18:02 [PATCH 0/3] merge -Xindex-only Thomas Rast 2013-07-07 18:02 ` [PATCH 1/3] merge-recursive: remove dead conditional in update_stages() Thomas Rast @ 2013-07-07 18:02 ` Thomas Rast 2013-07-07 18:37 ` Junio C Hamano 2013-07-07 18:02 ` [PATCH 3/3] merge-recursive: -Xindex-only to leave worktree unchanged Thomas Rast 2013-07-08 15:21 ` [PATCH 0/3] merge -Xindex-only Michael Haggerty 3 siblings, 1 reply; 15+ messages in thread From: Thomas Rast @ 2013-07-07 18:02 UTC (permalink / raw) To: git; +Cc: Michael Haggerty o->call_depth has a double function: a nonzero call_depth means we want to construct virtual merge bases, but it also means we want to avoid touching the worktree. Introduce a new flag o->no_worktree for the latter. Signed-off-by: Thomas Rast <trast@inf.ethz.ch> --- merge-recursive.c | 38 +++++++++++++++++++++----------------- merge-recursive.h | 1 + 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index 0b9cafb..b93b762 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -407,10 +407,10 @@ static void record_df_conflict_files(struct merge_options *o, int i; /* - * If we're merging merge-bases, we don't want to bother with - * any working directory changes. + * If we're working in-core only (e.g., merging merge-bases), + * we don't want to bother with any working directory changes. */ - if (o->call_depth) + if (o->no_worktree) return; /* Ensure D/F conflicts are adjacent in the entries list. */ @@ -723,7 +723,7 @@ static void update_file_flags(struct merge_options *o, int update_cache, int update_wd) { - if (o->call_depth) + if (o->no_worktree) update_wd = 0; if (update_wd) { @@ -930,7 +930,7 @@ static struct merge_file_info merge_file_1(struct merge_options *o, result.clean = merge_submodule(result.sha, one->path, one->sha1, a->sha1, b->sha1, - !o->call_depth); + !o->no_worktree); } else if (S_ISLNK(a->mode)) { hashcpy(result.sha, a->sha1); @@ -1002,7 +1002,7 @@ static void handle_change_delete(struct merge_options *o, const char *change, const char *change_past) { char *renamed = NULL; - if (dir_in_way(path, !o->call_depth)) { + if (dir_in_way(path, !o->no_worktree)) { renamed = unique_path(o, path, a_sha ? o->branch1 : o->branch2); } @@ -1127,10 +1127,10 @@ static void handle_file(struct merge_options *o, char *add_name = unique_path(o, rename->path, other_branch); update_file(o, 0, add->sha1, add->mode, add_name); - remove_file(o, 0, rename->path, 0); + remove_file(o, 0, rename->path, o->no_worktree); dst_name = unique_path(o, rename->path, cur_branch); } else { - if (dir_in_way(rename->path, !o->call_depth)) { + if (dir_in_way(rename->path, !o->no_worktree)) { dst_name = unique_path(o, rename->path, cur_branch); output(o, 1, _("%s is a directory in %s adding as %s instead"), rename->path, other_branch, dst_name); @@ -1237,7 +1237,7 @@ static void conflict_rename_rename_2to1(struct merge_options *o, * merge base just undo the renames; they can be detected * again later for the non-recursive merge. */ - remove_file(o, 0, path, 0); + remove_file(o, 0, path, o->no_worktree); update_file(o, 0, mfi_c1.sha, mfi_c1.mode, a->path); update_file(o, 0, mfi_c2.sha, mfi_c2.mode, b->path); } else { @@ -1245,7 +1245,7 @@ static void conflict_rename_rename_2to1(struct merge_options *o, char *new_path2 = unique_path(o, path, ci->branch2); output(o, 1, _("Renaming %s to %s and %s to %s instead"), a->path, new_path1, b->path, new_path2); - remove_file(o, 0, path, 0); + remove_file(o, 0, path, o->no_worktree); update_file(o, 0, mfi_c1.sha, mfi_c1.mode, new_path1); update_file(o, 0, mfi_c2.sha, mfi_c2.mode, new_path2); free(new_path2); @@ -1404,7 +1404,7 @@ static int process_renames(struct merge_options *o, * add-source case). */ remove_file(o, 1, ren1_src, - renamed_stage == 2 || !was_tracked(ren1_src)); + o->no_worktree || renamed_stage == 2 || !was_tracked(ren1_src)); hashcpy(src_other.sha1, ren1->src_entry->stages[other_stage].sha); src_other.mode = ren1->src_entry->stages[other_stage].mode; @@ -1600,7 +1600,7 @@ static int merge_content(struct merge_options *o, o->branch2 == rename_conflict_info->branch1) ? pair1->two->path : pair1->one->path; - if (dir_in_way(path, !o->call_depth)) + if (dir_in_way(path, !o->no_worktree)) df_conflict_remains = 1; } mfi = merge_file_special_markers(o, &one, &a, &b, @@ -1620,7 +1620,7 @@ static int merge_content(struct merge_options *o, path_renamed_outside_HEAD = !path2 || !strcmp(path, path2); if (!path_renamed_outside_HEAD) { add_cacheinfo(mfi.mode, mfi.sha, path, - 0, (!o->call_depth), 0); + 0, (!o->no_worktree), 0); return mfi.clean; } } else @@ -1721,7 +1721,7 @@ static int process_entry(struct merge_options *o, if (a_sha) output(o, 2, _("Removing %s"), path); /* do not touch working file if it did not exist */ - remove_file(o, 1, path, !a_sha); + remove_file(o, 1, path, o->no_worktree || !a_sha); } else { /* Modify/delete; deleted side may have put a directory in the way */ clean_merge = 0; @@ -1752,7 +1752,7 @@ static int process_entry(struct merge_options *o, sha = b_sha; conf = _("directory/file"); } - if (dir_in_way(path, !o->call_depth)) { + if (dir_in_way(path, !o->no_worktree)) { char *new_path = unique_path(o, path, add_branch); clean_merge = 0; output(o, 1, _("CONFLICT (%s): There is a directory with name %s in %s. " @@ -1780,7 +1780,7 @@ static int process_entry(struct merge_options *o, * this entry was deleted altogether. a_mode == 0 means * we had that path and want to actively remove it. */ - remove_file(o, 1, path, !a_mode); + remove_file(o, 1, path, o->no_worktree || !a_mode); } else die(_("Fatal merge failure, shouldn't happen.")); @@ -1806,7 +1806,7 @@ int merge_trees(struct merge_options *o, return 1; } - code = git_merge_trees(o->call_depth, common, head, merge); + code = git_merge_trees(o->no_worktree, common, head, merge); if (code != 0) { if (show(o, 4) || o->call_depth) @@ -1915,6 +1915,7 @@ int merge_recursive(struct merge_options *o, for (iter = ca; iter; iter = iter->next) { const char *saved_b1, *saved_b2; + unsigned saved_no_worktree; o->call_depth++; /* * When the merge fails, the result contains files @@ -1924,6 +1925,8 @@ int merge_recursive(struct merge_options *o, * "conflicts" were already resolved. */ discard_cache(); + saved_no_worktree = o->no_worktree; + o->no_worktree = 1; saved_b1 = o->branch1; saved_b2 = o->branch2; o->branch1 = "Temporary merge branch 1"; @@ -1932,6 +1935,7 @@ int merge_recursive(struct merge_options *o, NULL, &merged_common_ancestors); o->branch1 = saved_b1; o->branch2 = saved_b2; + o->no_worktree = saved_no_worktree; o->call_depth--; if (!merged_common_ancestors) diff --git a/merge-recursive.h b/merge-recursive.h index 9e090a3..d8dd7a1 100644 --- a/merge-recursive.h +++ b/merge-recursive.h @@ -15,6 +15,7 @@ struct merge_options { const char *subtree_shift; unsigned buffer_output : 1; unsigned renormalize : 1; + unsigned no_worktree : 1; /* do not touch worktree */ long xdl_opts; int verbosity; int diff_rename_limit; -- 1.8.3.2.908.gbd0dbd0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] merge-recursive: untangle double meaning of o->call_depth 2013-07-07 18:02 ` [PATCH 2/3] merge-recursive: untangle double meaning of o->call_depth Thomas Rast @ 2013-07-07 18:37 ` Junio C Hamano 0 siblings, 0 replies; 15+ messages in thread From: Junio C Hamano @ 2013-07-07 18:37 UTC (permalink / raw) To: Thomas Rast; +Cc: git, Michael Haggerty Thomas Rast <trast@inf.ethz.ch> writes: > o->call_depth has a double function: a nonzero call_depth means we > want to construct virtual merge bases, but it also means we want to > avoid touching the worktree. Introduce a new flag o->no_worktree for > the latter. I do remember discussing this with you the other day, and while I do agree that we may not want to touch the working tree in the outermost merge (i.e. o->call_depth is zero) in some applications, I do not think of a situation where you _do_ want to touch working tree while performing the inner merge. I'd feel safer if the code said that "no matter what no-worktree option says, we won't touch the working tree if o->call_depth is not zero" clearly in some way, e.g. if (o->call_depth || o->in_index_merge) return; /* leave without touching working tree */ In other words, I do not like the part of the code that pretends these two are independent options, when what we really want is to have two modes for the outermost (o->call_depth == 0) case. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/3] merge-recursive: -Xindex-only to leave worktree unchanged 2013-07-07 18:02 [PATCH 0/3] merge -Xindex-only Thomas Rast 2013-07-07 18:02 ` [PATCH 1/3] merge-recursive: remove dead conditional in update_stages() Thomas Rast 2013-07-07 18:02 ` [PATCH 2/3] merge-recursive: untangle double meaning of o->call_depth Thomas Rast @ 2013-07-07 18:02 ` Thomas Rast 2013-07-08 15:21 ` [PATCH 0/3] merge -Xindex-only Michael Haggerty 3 siblings, 0 replies; 15+ messages in thread From: Thomas Rast @ 2013-07-07 18:02 UTC (permalink / raw) To: git; +Cc: Michael Haggerty Using the new no_worktree flag from the previous commit, we can teach merge-recursive to leave the worktree untouched. Expose this with a new strategy option so that scripts can use it. --- Documentation/merge-strategies.txt | 4 ++++ merge-recursive.c | 2 ++ t/t3030-merge-recursive.sh | 13 +++++++++++++ 3 files changed, 19 insertions(+) diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt index 49a9a7d..b663a2e 100644 --- a/Documentation/merge-strategies.txt +++ b/Documentation/merge-strategies.txt @@ -92,6 +92,10 @@ subtree[=<path>];; is prefixed (or stripped from the beginning) to make the shape of two trees to match. +index-only;; + Write the merge result only to the index; do not touch the + worktree. + octopus:: This resolves cases with more than two heads, but refuses to do a complex merge that needs manual resolution. It is diff --git a/merge-recursive.c b/merge-recursive.c index b93b762..a58d691 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -2094,6 +2094,8 @@ int parse_merge_opt(struct merge_options *o, const char *s) if ((o->rename_score = parse_rename_score(&score)) == -1 || *score != 0) return -1; } + else if (!strcmp(s, "index-only")) + o->no_worktree = 1; else return -1; return 0; diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh index 2f96100..2f3a16c 100755 --- a/t/t3030-merge-recursive.sh +++ b/t/t3030-merge-recursive.sh @@ -296,6 +296,19 @@ test_expect_success 'merge-recursive result' ' ' +test_expect_success 'merge-recursive --index-only' ' + + rm -fr [abcd] && + git checkout -f "$c2" && + test_expect_code 1 git merge-recursive --index-only "$c0" -- "$c2" "$c1" && + git ls-files -s >actual && + # reuses "expected" from previous test! + test_cmp expected actual && + git diff HEAD >actual-diff && + : >expected-diff && + test_cmp expected-diff actual-diff +' + test_expect_success 'fail if the index has unresolved entries' ' rm -fr [abcd] && -- 1.8.3.2.908.gbd0dbd0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] merge -Xindex-only 2013-07-07 18:02 [PATCH 0/3] merge -Xindex-only Thomas Rast ` (2 preceding siblings ...) 2013-07-07 18:02 ` [PATCH 3/3] merge-recursive: -Xindex-only to leave worktree unchanged Thomas Rast @ 2013-07-08 15:21 ` Michael Haggerty 2013-07-08 15:44 ` Thomas Rast 3 siblings, 1 reply; 15+ messages in thread From: Michael Haggerty @ 2013-07-08 15:21 UTC (permalink / raw) To: Thomas Rast; +Cc: git discussion list [Resend because of address confusion in replied-to email.] On 07/07/2013 08:00 PM, Thomas Rast wrote: > I recently looked into making merge-recursive more useful as a modular > piece in various tasks, e.g. Michael's git-imerge and the experiments > I made in showing evil merges. > > This miniseries is the extremely low-hanging fruit. If it makes a > good first step for git-imerge, perhaps it can go in like this. It's > not a big speedup (about 2.2s vs 2.4s in a sample conflicting merge in > git.git), but it does feel much cleaner to avoid touching the worktree > unless actually necessary. > > Otherwise it's probably not worth it just yet; for what I want to do > with it, we need some more reshuffling of things. Interesting. For git-imerge, it would be nice to speed up merges by skipping the working tree updates. 10% might not be so noticeable, but every little bit helps :-) But the killer benefit would be if git-imerge could do some of its automatic merge-testing and autofilling in the background while the user is resolving conflicts in the main index and working tree. Is it possible to use this option with an alternate index file (e.g., via the GIT_INDEX_FILE environment variable)? Can it be made to leave other shared state (e.g., MERGE_HEAD) alone? If so, maybe it's already there. For this feature to be useful for test merges, it would be enough to just get a retcode saying whether a given merge would succeed or conflict. For it to be used for autofilling, it would also be necessary to be able to create a commit from the merge result in the alternate index (this would only be done when there are no conflicts). I assume this part can be done in the usual way using "git commit-tree". Michael -- Michael Haggerty mhagger@alum.mit.edu http://softwareswirl.blogspot.com/ ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] merge -Xindex-only 2013-07-08 15:21 ` [PATCH 0/3] merge -Xindex-only Michael Haggerty @ 2013-07-08 15:44 ` Thomas Rast 2013-07-09 9:44 ` Michael Haggerty 0 siblings, 1 reply; 15+ messages in thread From: Thomas Rast @ 2013-07-08 15:44 UTC (permalink / raw) To: Michael Haggerty; +Cc: git discussion list Michael Haggerty <mhagger@alum.mit.edu> writes: > [Resend because of address confusion in replied-to email.] > > On 07/07/2013 08:00 PM, Thomas Rast wrote: >> I recently looked into making merge-recursive more useful as a modular >> piece in various tasks, e.g. Michael's git-imerge and the experiments >> I made in showing evil merges. >> >> This miniseries is the extremely low-hanging fruit. If it makes a >> good first step for git-imerge, perhaps it can go in like this. It's >> not a big speedup (about 2.2s vs 2.4s in a sample conflicting merge in >> git.git), but it does feel much cleaner to avoid touching the worktree >> unless actually necessary. >> >> Otherwise it's probably not worth it just yet; for what I want to do >> with it, we need some more reshuffling of things. > > Interesting. > > For git-imerge, it would be nice to speed up merges by skipping the > working tree updates. 10% might not be so noticeable, but every little > bit helps :-) > > But the killer benefit would be if git-imerge could do some of its > automatic merge-testing and autofilling in the background while the user > is resolving conflicts in the main index and working tree. > > Is it possible to use this option with an alternate index file (e.g., > via the GIT_INDEX_FILE environment variable)? Can it be made to leave > other shared state (e.g., MERGE_HEAD) alone? If so, maybe it's already > there. GIT_INDEX_FILE yes, that one works out of the box. I think for the shared state, the following is a (probably) ridiculously unsupported yet magic way of achieving this: mkdir -p unshared/.git cd unshared/.git for f in ../../.git/*; do case "$f" in *HEAD | index) cp "$f" . ;; *) ln -s "$f" . ;; esac done That gives you a repository that propagates ref changes and object writing, but does not propagate changes to index, HEAD, FETCH_HEAD or MERGE_HEAD. Which might just be what you need? Note that as far as I'm concerned, this is a live handgrenade. It could blow up in your face at any time, but it probably has its applications... -- Thomas Rast trast@{inf,student}.ethz.ch ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] merge -Xindex-only 2013-07-08 15:44 ` Thomas Rast @ 2013-07-09 9:44 ` Michael Haggerty 2013-07-09 12:08 ` Thomas Rast 0 siblings, 1 reply; 15+ messages in thread From: Michael Haggerty @ 2013-07-09 9:44 UTC (permalink / raw) To: Thomas Rast; +Cc: git discussion list On 07/08/2013 05:44 PM, Thomas Rast wrote: > Michael Haggerty <mhagger@alum.mit.edu> writes: > >> [Resend because of address confusion in replied-to email.] >> >> On 07/07/2013 08:00 PM, Thomas Rast wrote: >>> I recently looked into making merge-recursive more useful as a modular >>> piece in various tasks, e.g. Michael's git-imerge and the experiments >>> I made in showing evil merges. >>> >>> This miniseries is the extremely low-hanging fruit. If it makes a >>> good first step for git-imerge, perhaps it can go in like this. It's >>> not a big speedup (about 2.2s vs 2.4s in a sample conflicting merge in >>> git.git), but it does feel much cleaner to avoid touching the worktree >>> unless actually necessary. >>> >>> Otherwise it's probably not worth it just yet; for what I want to do >>> with it, we need some more reshuffling of things. >> >> Interesting. >> >> For git-imerge, it would be nice to speed up merges by skipping the >> working tree updates. 10% might not be so noticeable, but every little >> bit helps :-) >> >> But the killer benefit would be if git-imerge could do some of its >> automatic merge-testing and autofilling in the background while the user >> is resolving conflicts in the main index and working tree. >> >> Is it possible to use this option with an alternate index file (e.g., >> via the GIT_INDEX_FILE environment variable)? Can it be made to leave >> other shared state (e.g., MERGE_HEAD) alone? If so, maybe it's already >> there. > > GIT_INDEX_FILE yes, that one works out of the box. > > I think for the shared state, the following is a (probably) ridiculously > unsupported yet magic way of achieving this: > > mkdir -p unshared/.git > cd unshared/.git > for f in ../../.git/*; do > case "$f" in > *HEAD | index) > cp "$f" . > ;; > *) > ln -s "$f" . > ;; > esac > done > > That gives you a repository that propagates ref changes and object > writing, but does not propagate changes to index, HEAD, FETCH_HEAD or > MERGE_HEAD. Which might just be what you need? > > Note that as far as I'm concerned, this is a live handgrenade. It could > blow up in your face at any time, but it probably has its applications... I might consider such a thing for my own use, but I don't think I'll lob live hand grenades at innocent git-imerge users :-) Since you've already implemented a way to merge into the index (even an alternative index) without touching the working copy, I'll just cross my fingers and hope for the appearance of an option that makes merge leave HEAD, MERGE_HEAD, etc. untouched. It's not like I have any free time to work on git-imerge anyway :-( Michael -- Michael Haggerty mhagger@alum.mit.edu http://softwareswirl.blogspot.com/ ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] merge -Xindex-only 2013-07-09 9:44 ` Michael Haggerty @ 2013-07-09 12:08 ` Thomas Rast 2013-07-09 19:38 ` Michael Haggerty 0 siblings, 1 reply; 15+ messages in thread From: Thomas Rast @ 2013-07-09 12:08 UTC (permalink / raw) To: Michael Haggerty; +Cc: git discussion list Michael Haggerty <mhagger@alum.mit.edu> writes: > Since you've already implemented a way to merge into the index (even an > alternative index) without touching the working copy, I'll just cross my > fingers and hope for the appearance of an option that makes merge leave > HEAD, MERGE_HEAD, etc. untouched. The most annoying part is probably where to put the output, since merging is more or less defined to do one of: - update HEAD and return 0 - update MERGE_HEAD and return 1 I'm not sure how much flexibility is worth having. Would it be sufficient if you had an option, e.g. -Xresult-ref=refs/heads/foo, that changes it to: - update refs/heads/foo and return 0 - return 1, not updating any refs That would mean that it would only work for noninteractive use. In the conflicting case, the driving script would need to remember what it wanted to merge so as have the information when finally committing. -- Thomas Rast trast@{inf,student}.ethz.ch ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] merge -Xindex-only 2013-07-09 12:08 ` Thomas Rast @ 2013-07-09 19:38 ` Michael Haggerty 2013-07-09 20:01 ` Thomas Rast 0 siblings, 1 reply; 15+ messages in thread From: Michael Haggerty @ 2013-07-09 19:38 UTC (permalink / raw) To: Thomas Rast; +Cc: git discussion list On 07/09/2013 02:08 PM, Thomas Rast wrote: > Michael Haggerty <mhagger@alum.mit.edu> writes: > >> Since you've already implemented a way to merge into the index (even an >> alternative index) without touching the working copy, I'll just cross my >> fingers and hope for the appearance of an option that makes merge leave >> HEAD, MERGE_HEAD, etc. untouched. > > The most annoying part is probably where to put the output, since > merging is more or less defined to do one of: > > - update HEAD and return 0 > - update MERGE_HEAD and return 1 I don't understand what you mean here. Why does *any* reference need to be updated? Why not * load arbitrary commit-ish A into index * merge arbitrary commit-ish B into index * return error/OK depending on whether there was a conflict ? The script that started the whole process would know what A and B are and could create the commit itself using "git write-tree" and "git commit-tree -p A -p B". And if the index were an alternative index chosen via GIT_INDEX_FILE then the rest of the git repo would be none the wiser. > I'm not sure how much flexibility is worth having. Would it be > sufficient if you had an option, e.g. -Xresult-ref=refs/heads/foo, that > changes it to: > > - update refs/heads/foo and return 0 > - return 1, not updating any refs > > That would mean that it would only work for noninteractive use. In the > conflicting case, the driving script would need to remember what it > wanted to merge so as have the information when finally committing. That would be fine with me. Michael -- Michael Haggerty mhagger@alum.mit.edu http://softwareswirl.blogspot.com/ ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] merge -Xindex-only 2013-07-09 19:38 ` Michael Haggerty @ 2013-07-09 20:01 ` Thomas Rast 2013-10-26 14:40 ` Thomas Rast 0 siblings, 1 reply; 15+ messages in thread From: Thomas Rast @ 2013-07-09 20:01 UTC (permalink / raw) To: Michael Haggerty; +Cc: git discussion list Michael Haggerty <mhagger@alum.mit.edu> writes: > On 07/09/2013 02:08 PM, Thomas Rast wrote: >> Michael Haggerty <mhagger@alum.mit.edu> writes: >> >>> Since you've already implemented a way to merge into the index (even an >>> alternative index) without touching the working copy, I'll just cross my >>> fingers and hope for the appearance of an option that makes merge leave >>> HEAD, MERGE_HEAD, etc. untouched. >> >> The most annoying part is probably where to put the output, since >> merging is more or less defined to do one of: >> >> - update HEAD and return 0 >> - update MERGE_HEAD and return 1 > > I don't understand what you mean here. [...] I was simply trying to describe what the status quo is, as a basis for the next paragraph. Does that clarify it? >> I'm not sure how much flexibility is worth having. Would it be >> sufficient if you had an option, e.g. -Xresult-ref=refs/heads/foo, that >> changes it to: >> >> - update refs/heads/foo and return 0 >> - return 1, not updating any refs >> >> That would mean that it would only work for noninteractive use. In the >> conflicting case, the driving script would need to remember what it >> wanted to merge so as have the information when finally committing. > > That would be fine with me. On IRC you said you would like a version that always acts as --no-commit, and simply returns the conflict/no conflict bit as usual. The caller would then proceed using commit-tree itself. I think that is probably a saner solution than this "output ref" idea. -- Thomas Rast trast@{inf,student}.ethz.ch ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] merge -Xindex-only 2013-07-09 20:01 ` Thomas Rast @ 2013-10-26 14:40 ` Thomas Rast 2013-10-26 14:43 ` [PATCH 1/3] merge-recursive: remove dead conditional in update_stages() Thomas Rast 0 siblings, 1 reply; 15+ messages in thread From: Thomas Rast @ 2013-10-26 14:40 UTC (permalink / raw) To: Michael Haggerty; +Cc: git discussion list, Junio C Hamano Thomas Rast <trast@inf.ethz.ch> writes: > Michael Haggerty <mhagger@alum.mit.edu> writes: > > On IRC you said you would like a version that always acts as > --no-commit, and simply returns the conflict/no conflict bit as usual. > The caller would then proceed using commit-tree itself. I think that is > probably a saner solution than this "output ref" idea. I just had a huge facepalm moment. We already have this option. It is called git-merge-recursive. That is, git merge-recursive $(git merge-base --all HEAD other) -- HEAD other will internally do all the work that 'git merge other' would do, but not update any refs. With this series, you can therefore say git merge-recursive --index-only $(git merge-base --all HEAD other) -- HEAD other and get an *index-only* merge of HEAD and other. Can you see if this is enough to build git-imerge on top of it? Otherwise I'm glad to help with building the git-merge infrastucture to support it. I'll send v2 of the series in a minute; the only change is that I changed the internal flag semantics as per Junio's comment in http://thread.gmane.org/gmane.comp.version-control.git/229787/focus=229797 -- Thomas Rast tr@thomasrast.ch ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] merge-recursive: remove dead conditional in update_stages() 2013-10-26 14:40 ` Thomas Rast @ 2013-10-26 14:43 ` Thomas Rast 2013-10-26 14:43 ` [PATCH 2/3] merge-recursive: internal flag to avoid touching the worktree Thomas Rast 2013-10-26 14:43 ` [PATCH 3/3] merge-recursive: -Xindex-only to leave worktree unchanged Thomas Rast 0 siblings, 2 replies; 15+ messages in thread From: Thomas Rast @ 2013-10-26 14:43 UTC (permalink / raw) To: git; +Cc: Michael Haggerty, Junio C Hamano, Thomas Rast From: Thomas Rast <trast@inf.ethz.ch> 650467c (merge-recursive: Consolidate different update_stages functions, 2011-08-11) changed the former argument 'clear' to always be true. Remove the useless conditional. Signed-off-by: Thomas Rast <trast@inf.ethz.ch> --- Unchanged. merge-recursive.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index dbb7104..bdf69cb 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -545,11 +545,9 @@ static int update_stages(const char *path, const struct diff_filespec *o, * would_lose_untracked). Instead, reverse the order of the calls * (executing update_file first and then update_stages). */ - int clear = 1; int options = ADD_CACHE_OK_TO_ADD | ADD_CACHE_SKIP_DFCHECK; - if (clear) - if (remove_file_from_cache(path)) - return -1; + if (remove_file_from_cache(path)) + return -1; if (o) if (add_cacheinfo(o->mode, o->sha1, path, 1, 0, options)) return -1; -- 1.8.4.1.841.gb1dcd95 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/3] merge-recursive: internal flag to avoid touching the worktree 2013-10-26 14:43 ` [PATCH 1/3] merge-recursive: remove dead conditional in update_stages() Thomas Rast @ 2013-10-26 14:43 ` Thomas Rast 2013-10-26 14:43 ` [PATCH 3/3] merge-recursive: -Xindex-only to leave worktree unchanged Thomas Rast 1 sibling, 0 replies; 15+ messages in thread From: Thomas Rast @ 2013-10-26 14:43 UTC (permalink / raw) To: git; +Cc: Michael Haggerty, Junio C Hamano, Thomas Rast From: Thomas Rast <trast@inf.ethz.ch> o->call_depth has a double function: a nonzero call_depth means we want to construct virtual merge bases, but it also means we want to avoid touching the worktree. Introduce a new flag o->no_worktree to trigger only the latter. Signed-off-by: Thomas Rast <trast@inf.ethz.ch> --- Adapted as per Junio's comments in http://thread.gmane.org/gmane.comp.version-control.git/229787/focus=229797 I'm not sure which way is better; the downside of this version is that we now have the burden of ensuring that o->no_worktree always remains a strict subset of o->call_depth whenever we touch this code. merge-recursive.c | 37 +++++++++++++++++++++---------------- merge-recursive.h | 1 + 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index bdf69cb..922a259 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -408,10 +408,10 @@ static void record_df_conflict_files(struct merge_options *o, int i; /* - * If we're merging merge-bases, we don't want to bother with - * any working directory changes. + * If we're working in-core only (e.g., merging merge-bases), + * we don't want to bother with any working directory changes. */ - if (o->call_depth) + if (o->call_depth || o->no_worktree) return; /* Ensure D/F conflicts are adjacent in the entries list. */ @@ -724,7 +724,7 @@ static void update_file_flags(struct merge_options *o, int update_cache, int update_wd) { - if (o->call_depth) + if (o->call_depth || o->no_worktree) update_wd = 0; if (update_wd) { @@ -931,7 +931,8 @@ static struct merge_file_info merge_file_1(struct merge_options *o, result.clean = merge_submodule(result.sha, one->path, one->sha1, a->sha1, b->sha1, - !o->call_depth); + !(o->call_depth || + o->no_worktree)); } else if (S_ISLNK(a->mode)) { hashcpy(result.sha, a->sha1); @@ -1003,7 +1004,7 @@ static void handle_change_delete(struct merge_options *o, const char *change, const char *change_past) { char *renamed = NULL; - if (dir_in_way(path, !o->call_depth)) { + if (dir_in_way(path, !(o->call_depth || o->no_worktree))) { renamed = unique_path(o, path, a_sha ? o->branch1 : o->branch2); } @@ -1128,10 +1129,10 @@ static void handle_file(struct merge_options *o, char *add_name = unique_path(o, rename->path, other_branch); update_file(o, 0, add->sha1, add->mode, add_name); - remove_file(o, 0, rename->path, 0); + remove_file(o, 0, rename->path, o->call_depth || o->no_worktree); dst_name = unique_path(o, rename->path, cur_branch); } else { - if (dir_in_way(rename->path, !o->call_depth)) { + if (dir_in_way(rename->path, !(o->call_depth || o->no_worktree))) { dst_name = unique_path(o, rename->path, cur_branch); output(o, 1, _("%s is a directory in %s adding as %s instead"), rename->path, other_branch, dst_name); @@ -1238,7 +1239,7 @@ static void conflict_rename_rename_2to1(struct merge_options *o, * merge base just undo the renames; they can be detected * again later for the non-recursive merge. */ - remove_file(o, 0, path, 0); + remove_file(o, 0, path, o->call_depth || o->no_worktree); update_file(o, 0, mfi_c1.sha, mfi_c1.mode, a->path); update_file(o, 0, mfi_c2.sha, mfi_c2.mode, b->path); } else { @@ -1246,7 +1247,7 @@ static void conflict_rename_rename_2to1(struct merge_options *o, char *new_path2 = unique_path(o, path, ci->branch2); output(o, 1, _("Renaming %s to %s and %s to %s instead"), a->path, new_path1, b->path, new_path2); - remove_file(o, 0, path, 0); + remove_file(o, 0, path, o->call_depth || o->no_worktree); update_file(o, 0, mfi_c1.sha, mfi_c1.mode, new_path1); update_file(o, 0, mfi_c2.sha, mfi_c2.mode, new_path2); free(new_path2); @@ -1405,6 +1406,7 @@ static int process_renames(struct merge_options *o, * add-source case). */ remove_file(o, 1, ren1_src, + o->call_depth || o->no_worktree || renamed_stage == 2 || !was_tracked(ren1_src)); hashcpy(src_other.sha1, ren1->src_entry->stages[other_stage].sha); @@ -1601,7 +1603,7 @@ static int merge_content(struct merge_options *o, o->branch2 == rename_conflict_info->branch1) ? pair1->two->path : pair1->one->path; - if (dir_in_way(path, !o->call_depth)) + if (dir_in_way(path, !(o->call_depth || o->no_worktree))) df_conflict_remains = 1; } mfi = merge_file_special_markers(o, &one, &a, &b, @@ -1621,7 +1623,7 @@ static int merge_content(struct merge_options *o, path_renamed_outside_HEAD = !path2 || !strcmp(path, path2); if (!path_renamed_outside_HEAD) { add_cacheinfo(mfi.mode, mfi.sha, path, - 0, (!o->call_depth), 0); + 0, !(o->call_depth || o->no_worktree), 0); return mfi.clean; } } else @@ -1722,7 +1724,8 @@ static int process_entry(struct merge_options *o, if (a_sha) output(o, 2, _("Removing %s"), path); /* do not touch working file if it did not exist */ - remove_file(o, 1, path, !a_sha); + remove_file(o, 1, path, + o->call_depth || o->no_worktree || !a_sha); } else { /* Modify/delete; deleted side may have put a directory in the way */ clean_merge = 0; @@ -1753,7 +1756,7 @@ static int process_entry(struct merge_options *o, sha = b_sha; conf = _("directory/file"); } - if (dir_in_way(path, !o->call_depth)) { + if (dir_in_way(path, !(o->call_depth || o->no_worktree))) { char *new_path = unique_path(o, path, add_branch); clean_merge = 0; output(o, 1, _("CONFLICT (%s): There is a directory with name %s in %s. " @@ -1781,7 +1784,8 @@ static int process_entry(struct merge_options *o, * this entry was deleted altogether. a_mode == 0 means * we had that path and want to actively remove it. */ - remove_file(o, 1, path, !a_mode); + remove_file(o, 1, path, + o->call_depth || o->no_worktree || !a_mode); } else die(_("Fatal merge failure, shouldn't happen.")); @@ -1807,7 +1811,8 @@ int merge_trees(struct merge_options *o, return 1; } - code = git_merge_trees(o->call_depth, common, head, merge); + code = git_merge_trees(o->call_depth || o->no_worktree, + common, head, merge); if (code != 0) { if (show(o, 4) || o->call_depth) diff --git a/merge-recursive.h b/merge-recursive.h index 9e090a3..d8dd7a1 100644 --- a/merge-recursive.h +++ b/merge-recursive.h @@ -15,6 +15,7 @@ struct merge_options { const char *subtree_shift; unsigned buffer_output : 1; unsigned renormalize : 1; + unsigned no_worktree : 1; /* do not touch worktree */ long xdl_opts; int verbosity; int diff_rename_limit; -- 1.8.4.1.841.gb1dcd95 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/3] merge-recursive: -Xindex-only to leave worktree unchanged 2013-10-26 14:43 ` [PATCH 1/3] merge-recursive: remove dead conditional in update_stages() Thomas Rast 2013-10-26 14:43 ` [PATCH 2/3] merge-recursive: internal flag to avoid touching the worktree Thomas Rast @ 2013-10-26 14:43 ` Thomas Rast 1 sibling, 0 replies; 15+ messages in thread From: Thomas Rast @ 2013-10-26 14:43 UTC (permalink / raw) To: git; +Cc: Michael Haggerty, Junio C Hamano, Thomas Rast From: Thomas Rast <trast@inf.ethz.ch> Using the new no_worktree flag from the previous commit, we can teach merge-recursive to leave the worktree untouched. Expose this with a new strategy option so that scripts can use it. --- Unchanged. Documentation/merge-strategies.txt | 4 ++++ merge-recursive.c | 2 ++ t/t3030-merge-recursive.sh | 13 +++++++++++++ 3 files changed, 19 insertions(+) diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt index 49a9a7d..b663a2e 100644 --- a/Documentation/merge-strategies.txt +++ b/Documentation/merge-strategies.txt @@ -92,6 +92,10 @@ subtree[=<path>];; is prefixed (or stripped from the beginning) to make the shape of two trees to match. +index-only;; + Write the merge result only to the index; do not touch the + worktree. + octopus:: This resolves cases with more than two heads, but refuses to do a complex merge that needs manual resolution. It is diff --git a/merge-recursive.c b/merge-recursive.c index 922a259..addfb72 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -2096,6 +2096,8 @@ int parse_merge_opt(struct merge_options *o, const char *s) if ((o->rename_score = parse_rename_score(&score)) == -1 || *score != 0) return -1; } + else if (!strcmp(s, "index-only")) + o->no_worktree = 1; else return -1; return 0; diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh index 2f96100..2f3a16c 100755 --- a/t/t3030-merge-recursive.sh +++ b/t/t3030-merge-recursive.sh @@ -296,6 +296,19 @@ test_expect_success 'merge-recursive result' ' ' +test_expect_success 'merge-recursive --index-only' ' + + rm -fr [abcd] && + git checkout -f "$c2" && + test_expect_code 1 git merge-recursive --index-only "$c0" -- "$c2" "$c1" && + git ls-files -s >actual && + # reuses "expected" from previous test! + test_cmp expected actual && + git diff HEAD >actual-diff && + : >expected-diff && + test_cmp expected-diff actual-diff +' + test_expect_success 'fail if the index has unresolved entries' ' rm -fr [abcd] && -- 1.8.4.1.841.gb1dcd95 ^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2013-10-26 14:43 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-07-07 18:02 [PATCH 0/3] merge -Xindex-only Thomas Rast 2013-07-07 18:02 ` [PATCH 1/3] merge-recursive: remove dead conditional in update_stages() Thomas Rast 2013-07-07 18:02 ` [PATCH 2/3] merge-recursive: untangle double meaning of o->call_depth Thomas Rast 2013-07-07 18:37 ` Junio C Hamano 2013-07-07 18:02 ` [PATCH 3/3] merge-recursive: -Xindex-only to leave worktree unchanged Thomas Rast 2013-07-08 15:21 ` [PATCH 0/3] merge -Xindex-only Michael Haggerty 2013-07-08 15:44 ` Thomas Rast 2013-07-09 9:44 ` Michael Haggerty 2013-07-09 12:08 ` Thomas Rast 2013-07-09 19:38 ` Michael Haggerty 2013-07-09 20:01 ` Thomas Rast 2013-10-26 14:40 ` Thomas Rast 2013-10-26 14:43 ` [PATCH 1/3] merge-recursive: remove dead conditional in update_stages() Thomas Rast 2013-10-26 14:43 ` [PATCH 2/3] merge-recursive: internal flag to avoid touching the worktree Thomas Rast 2013-10-26 14:43 ` [PATCH 3/3] merge-recursive: -Xindex-only to leave worktree unchanged Thomas Rast
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.