* BUG: merge -s theirs is not in effect (does the same as -s ours) @ 2017-09-25 0:02 Yaroslav Halchenko 2017-09-25 1:08 ` Junio C Hamano 0 siblings, 1 reply; 19+ messages in thread From: Yaroslav Halchenko @ 2017-09-25 0:02 UTC (permalink / raw) To: git@vger.kernel.org My interest was to get remote branch "merge" the changes in the branch taking the branch's version (primarily alternative symlinks for git-annex'ed content) over the version in master (previous merge of a similar branch). Unfortunately -s theirs seems to do actually -s ours -- symlinks and content is taken from the 'ours' branch instead of theirs. workaround -- perform -s ours of master within the branch, and then ff of master to that state: $> git --version git version 2.14.1.729.g59c0ea183a $> rm -rf /tmp/repo1; mkdir /tmp/repo1; cd /tmp/repo1; git init .; ln -s sym1 link; echo 1 > file; git add file link; git commit -m 'common'; git co -b b1 ; ln -sf b1link link; echo "b1 file" >| file; git commit -m 'b2 changes' -a; git co master; ln -sf masterlink link; echo "master file" >| file; git commit -m 'also modified in master' -a; git merge -s theirs --no-edit b1; ls -l link; cat file E: could not determine git repository root warning: templates not found /home/yoh/share/git-core/templates Initialized empty Git repository in /tmp/repo1/.git/ [master (root-commit) b6a69d0] common 2 files changed, 2 insertions(+) create mode 100644 file create mode 120000 link Switched to a new branch 'b1' [b1 739eb85] b2 changes 2 files changed, 2 insertions(+), 2 deletions(-) Switched to branch 'master' [master 18a2da4] also modified in master 2 files changed, 2 insertions(+), 2 deletions(-) args: b6a69d0c0c2500530cba8bc2987a1f79998b5e74 -- HEAD 739eb853c480b729ec07da533610243e3a6d69ee Merge made by the 'theirs' strategy. lrwxrwxrwx 1 yoh yoh 10 Sep 24 19:58 link -> masterlink master file -- Yaroslav O. Halchenko Center for Open Neuroscience http://centerforopenneuroscience.org Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755 Phone: +1 (603) 646-9834 Fax: +1 (603) 646-1419 WWW: http://www.linkedin.com/in/yarik ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: BUG: merge -s theirs is not in effect (does the same as -s ours) 2017-09-25 0:02 BUG: merge -s theirs is not in effect (does the same as -s ours) Yaroslav Halchenko @ 2017-09-25 1:08 ` Junio C Hamano 2017-09-25 3:17 ` Yaroslav Halchenko 0 siblings, 1 reply; 19+ messages in thread From: Junio C Hamano @ 2017-09-25 1:08 UTC (permalink / raw) To: Yaroslav Halchenko; +Cc: git@vger.kernel.org Yaroslav Halchenko <yoh@onerussian.com> writes: > My interest was to get remote branch "merge" the changes in the > branch taking the branch's version (primarily alternative symlinks > for git-annex'ed content) over the version in master (previous > merge of a similar branch). Unfortunately -s theirs seems to do > actually -s ours What does ls $(git --exec-path) | grep git-merge say? The official Git never shipped "git-merge-theirs" as far as I know, and it should not exist (neither should "git merge -s theirs"; you can use "git reset --hard theirs" instead). ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: BUG: merge -s theirs is not in effect (does the same as -s ours) 2017-09-25 1:08 ` Junio C Hamano @ 2017-09-25 3:17 ` Yaroslav Halchenko 2017-09-25 5:33 ` Re* " Junio C Hamano 0 siblings, 1 reply; 19+ messages in thread From: Yaroslav Halchenko @ 2017-09-25 3:17 UTC (permalink / raw) To: git@vger.kernel.org On Mon, 25 Sep 2017, Junio C Hamano wrote: > Yaroslav Halchenko <yoh@onerussian.com> writes: > > My interest was to get remote branch "merge" the changes in the > > branch taking the branch's version (primarily alternative symlinks > > for git-annex'ed content) over the version in master (previous > > merge of a similar branch). Unfortunately -s theirs seems to do > > actually -s ours > What does > ls $(git --exec-path) | grep git-merge NB when running git just built, --exec-path reports some non existing dir in ~: $> git --exec-path /home/yoh/libexec/git-core $> ls -l /home/yoh/libexec/git-core ls: cannot access '/home/yoh/libexec/git-core': No such file or directory $> which git /home/yoh/proj/misc/git/git > say? > The official Git never shipped "git-merge-theirs" as far as I know, > and it should not exist (neither should "git merge -s theirs"; you > can use "git reset --hard theirs" instead). d'oh, indeed there is no git-merge-theirs neither in debian pkg or a freshly built git and I found a rogue script in the PATH (which did nothing apparently, sorry!). BUT I was originally mislead by the --help/manpage: MERGE STRATEGIES The merge mechanism (git merge and git pull commands) allows the backend merge strategies to be chosen with -s option. Some strategies can also take their own options, which can be passed by giving -X<option> arguments to git merge and/or git pull. ... recursive This can only resolve two heads using a 3-way merge algorithm. When there is more than one common ancestor that can be used for 3-way merge, it creates a merged tree of the common ancestors and uses that as the reference tree for the 3-way merge. This has been reported to result in fewer merge conflicts without causing mismerges by tests done on actual merge commits taken from Linux 2.6 kernel development history. Additionally this can detect and handle merges involving renames. This is the default merge strategy when pulling or merging one branch. The recursive strategy can take the following options: ours This option forces conflicting hunks to be auto-resolved cleanly by favoring our version. ... theirs This is the opposite of ours. (Documentation/merge-strategies.txt in the sources I guess) PS thanks for CCing me in replies! -- Yaroslav O. Halchenko Center for Open Neuroscience http://centerforopenneuroscience.org Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755 Phone: +1 (603) 646-9834 Fax: +1 (603) 646-1419 WWW: http://www.linkedin.com/in/yarik ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re* BUG: merge -s theirs is not in effect (does the same as -s ours) 2017-09-25 3:17 ` Yaroslav Halchenko @ 2017-09-25 5:33 ` Junio C Hamano 2017-09-25 14:30 ` -X theirs does not resolve symlink conflict Was: BUG: merge -s theirs is not in effect Yaroslav Halchenko 2017-09-25 14:40 ` -s theirs use-case(s) Was: BUG: merge -s theirs is not in effect Yaroslav Halchenko 0 siblings, 2 replies; 19+ messages in thread From: Junio C Hamano @ 2017-09-25 5:33 UTC (permalink / raw) To: Yaroslav Halchenko; +Cc: git@vger.kernel.org Yaroslav Halchenko <yoh@onerussian.com> writes: > d'oh, indeed there is no git-merge-theirs neither in debian pkg or a freshly > built git and I found a rogue script in the PATH (which did nothing > apparently, sorry!). BUT I was originally mislead by the --help/manpage: Ahh, you're right. The text does make readers expect "-s theirs" to exist. -- >8 -- Subject: merge-strategies: avoid implying that "-s theirs" exists The description of `-Xours` merge option has a parenthetical note that tells the readers that it is very different from `-s ours`, which is correct, but the description of `-Xtheirs` that follows it carelessly says "this is the opposite of `ours`", giving a false impression that the readers also need to be warned that it is very different from `-s theirs`, which in reality does not even exist. Clarify it a bit to avoid misleading readers. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- * I hope this should help things a bit. It is a different matter to resurrect the age old discussion that happend in the summer of 2008 if '-s theirs' should or should not exist. In short, the previous discussion can be summarised to "we don't want '-s theirs' as it encourages the wrong workflow". https://public-inbox.org/git/alpine.DEB.1.00.0807290123300.2725@eeepc-johanness/ https://public-inbox.org/git/7vtzen7bul.fsf@gitster.siamese.dyndns.org/ https://public-inbox.org/git/20080720192130.6117@nanako3.lavabit.com/ It is OK for people to come with new perspective and bring new ideas to the table. We learned from experience while using Git for longer and are wiser than what we were back then, and might be able to make a better decision ;-) Documentation/merge-strategies.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt index 2eb92b9327..a09d597463 100644 --- a/Documentation/merge-strategies.txt +++ b/Documentation/merge-strategies.txt @@ -39,7 +39,8 @@ even look at what the other tree contains at all. It discards everything the other tree did, declaring 'our' history contains all that happened in it. theirs;; - This is the opposite of 'ours'. + This is the opposite of 'ours'; note that, unlike 'ours', there is + no 'theirs' merge stragegy to confuse this merge option with. patience;; With this option, 'merge-recursive' spends a little extra time ^ permalink raw reply related [flat|nested] 19+ messages in thread
* -X theirs does not resolve symlink conflict Was: BUG: merge -s theirs is not in effect 2017-09-25 5:33 ` Re* " Junio C Hamano @ 2017-09-25 14:30 ` Yaroslav Halchenko 2017-09-26 1:56 ` Junio C Hamano 2017-09-25 14:40 ` -s theirs use-case(s) Was: BUG: merge -s theirs is not in effect Yaroslav Halchenko 1 sibling, 1 reply; 19+ messages in thread From: Yaroslav Halchenko @ 2017-09-25 14:30 UTC (permalink / raw) To: git@vger.kernel.org On Mon, 25 Sep 2017, Junio C Hamano wrote: > Yaroslav Halchenko <yoh@onerussian.com> writes: > > d'oh, indeed there is no git-merge-theirs neither in debian pkg or a freshly > > built git and I found a rogue script in the PATH (which did nothing > > apparently, sorry!). BUT I was originally mislead by the --help/manpage: > Ahh, you're right. The text does make readers expect "-s theirs" to > exist. > ... > * I hope this should help things a bit. yes it does. Thanks. And that is where I realized that I should have used -X theirs (not -s theirs), as the instruction on the option for the (recursive) merge. And now problem is more specific: - conflict within file content editing was resolved as instructed (taking "theirs" version) - BUT symlink was not taken from "theirs" and left as unresolved conflict: $> rm -rf /tmp/repo1; mkdir /tmp/repo1; cd /tmp/repo1; git init .; ln -s sym1 link; echo 1 > file; git add file link; git commit -m 'common'; git co -b b1 ; ln -sf b1link link; echo "b1 file" >| file; git commit -m 'b2 changes' -a; git co master; ln -sf masterlink link; echo "master file" >| file; git commit -m 'also modified in master' -a; git merge -X theirs --no-edit b1; ls -l link; cat file warning: templates not found /home/yoh/share/git-core/templates Initialized empty Git repository in /tmp/repo1/.git/ [master (root-commit) f0b75bc] common 2 files changed, 2 insertions(+) create mode 100644 file create mode 120000 link Switched to a new branch 'b1' [b1 45c93ca] b2 changes 2 files changed, 2 insertions(+), 2 deletions(-) Switched to branch 'master' [master 0ee6db2] also modified in master 2 files changed, 2 insertions(+), 2 deletions(-) Auto-merging link CONFLICT (content): Merge conflict in link Auto-merging file Automatic merge failed; fix conflicts and then commit the result. lrwxrwxrwx 1 yoh yoh 10 Sep 25 10:21 link -> masterlink b1 file changes on filesystem: link | Unmerged cached/staged changes: file | 2 +- link | Unmerged PS I will followup on -s theirs in a split thread PSS Thanks for CCing me your replies -- Yaroslav O. Halchenko Center for Open Neuroscience http://centerforopenneuroscience.org Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755 Phone: +1 (603) 646-9834 Fax: +1 (603) 646-1419 WWW: http://www.linkedin.com/in/yarik ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: -X theirs does not resolve symlink conflict Was: BUG: merge -s theirs is not in effect 2017-09-25 14:30 ` -X theirs does not resolve symlink conflict Was: BUG: merge -s theirs is not in effect Yaroslav Halchenko @ 2017-09-26 1:56 ` Junio C Hamano 2017-09-26 2:16 ` Junio C Hamano 0 siblings, 1 reply; 19+ messages in thread From: Junio C Hamano @ 2017-09-26 1:56 UTC (permalink / raw) To: Yaroslav Halchenko; +Cc: git@vger.kernel.org Yaroslav Halchenko <yoh@onerussian.com> writes: > yes it does. Thanks. And that is where I realized that I should have used -X > theirs (not -s theirs), as the instruction on the option for the > (recursive) merge. And now problem is more specific: > > - conflict within file content editing was resolved as instructed > (taking "theirs" version) > > - BUT symlink was not taken from "theirs" and left as unresolved conflict: I wouldn't call it working-as-intended, but this unfortunately is expected. You'd encounter exactly the same behaviour when changes to a binary file conflicts. It is because -X<ours|theirs> _ONLY_ kicks in (i.e. that is how it is defined) when we would otherwise throw the half-merged result: <<<<<<< our version looks like this ======= their version looks like this >>>>>>> and ask you to edit that to a correct resolution. Because you would not normally be given something like the above when merging conflicted changes to symbolic links or to binary files, -X<ours|theirs> has no chance of affecting the outcome. I do not recall people talking about symbolic links but the case of binary files has been on the wishlist for a long time, and I do not know of anybody who is working on (or is planning to work on) it. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: -X theirs does not resolve symlink conflict Was: BUG: merge -s theirs is not in effect 2017-09-26 1:56 ` Junio C Hamano @ 2017-09-26 2:16 ` Junio C Hamano 2017-09-26 2:39 ` Junio C Hamano 0 siblings, 1 reply; 19+ messages in thread From: Junio C Hamano @ 2017-09-26 2:16 UTC (permalink / raw) To: Yaroslav Halchenko; +Cc: git@vger.kernel.org Junio C Hamano <gitster@pobox.com> writes: > I do not recall people talking about symbolic links but the case of > binary files has been on the wishlist for a long time, and I do not > know of anybody who is working on (or is planning to work on) it. Ah, I misremembered. We've addressed the "binary files" case back in 2012 with a944af1d ("merge: teach -Xours/-Xtheirs to binary ll-merge driver", 2012-09-08). I do not know offhand if it is just as easy to plumb the MERGE_FAVOR_{OURS,THEIRS} bits thru the symbolic link codepath, like that patch did to the binary file codepath. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: -X theirs does not resolve symlink conflict Was: BUG: merge -s theirs is not in effect 2017-09-26 2:16 ` Junio C Hamano @ 2017-09-26 2:39 ` Junio C Hamano 2017-09-26 13:37 ` Yaroslav Halchenko 0 siblings, 1 reply; 19+ messages in thread From: Junio C Hamano @ 2017-09-26 2:39 UTC (permalink / raw) To: Yaroslav Halchenko; +Cc: git@vger.kernel.org Junio C Hamano <gitster@pobox.com> writes: > Junio C Hamano <gitster@pobox.com> writes: > >> I do not recall people talking about symbolic links but the case of >> binary files has been on the wishlist for a long time, and I do not >> know of anybody who is working on (or is planning to work on) it. > > Ah, I misremembered. > > We've addressed the "binary files" case back in 2012 with a944af1d > ("merge: teach -Xours/-Xtheirs to binary ll-merge driver", > 2012-09-08). I do not know offhand if it is just as easy to plumb > the MERGE_FAVOR_{OURS,THEIRS} bits thru the symbolic link codepath, > like that patch did to the binary file codepath. Perhaps the attached (totally untested) patch might be a good starting point. I do not know if you are interested in hacking on Git, and I do not feel offended if you are not, but perhaps somebody else might get interested in seeing if this #leftoverbits is a good direction to go in, and finishing it with docs and tests if it is ;-) merge-recursive.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index 1d3f8f0d22..3605275ca3 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1026,10 +1026,19 @@ static int merge_file_1(struct merge_options *o, &b->oid, !o->call_depth); } else if (S_ISLNK(a->mode)) { - oidcpy(&result->oid, &a->oid); - - if (!oid_eq(&a->oid, &b->oid)) - result->clean = 0; + switch (o->recursive_variant) { + case MERGE_RECURSIVE_NORMAL: + oidcpy(&result->oid, &a->oid); + if (!oid_eq(&a->oid, &b->oid)) + result->clean = 0; + break; + case MERGE_RECURSIVE_OURS: + oidcpy(&result->oid, &a->oid); + break; + case MERGE_RECURSIVE_THEIRS: + oidcpy(&result->oid, &b->oid); + break; + } } else die("BUG: unsupported object type in the tree"); } ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: -X theirs does not resolve symlink conflict Was: BUG: merge -s theirs is not in effect 2017-09-26 2:39 ` Junio C Hamano @ 2017-09-26 13:37 ` Yaroslav Halchenko 2017-10-16 5:38 ` [PATCH] merge: teach -Xours/-Xtheirs to symbolic link merge Junio C Hamano 0 siblings, 1 reply; 19+ messages in thread From: Yaroslav Halchenko @ 2017-09-26 13:37 UTC (permalink / raw) To: git@vger.kernel.org On Tue, 26 Sep 2017, Junio C Hamano wrote: > >> I do not recall people talking about symbolic links but the case of > >> binary files has been on the wishlist for a long time, and I do not > >> know of anybody who is working on (or is planning to work on) it. > > Ah, I misremembered. > > We've addressed the "binary files" case back in 2012 with a944af1d > > ("merge: teach -Xours/-Xtheirs to binary ll-merge driver", > > 2012-09-08). I do not know offhand if it is just as easy to plumb > > the MERGE_FAVOR_{OURS,THEIRS} bits thru the symbolic link codepath, > > like that patch did to the binary file codepath. > Perhaps the attached (totally untested) patch might be a good > starting point. I do not know if you are interested in hacking on > Git, and I do not feel offended if you are not, but perhaps somebody I would have felt honored to "hack on Git" but neither my C-foo is up to par, neither there would be more time I could adequately allocate for such endeavor. So meanwhile I am trying to contribute in hopefully constructive "whining" while exploiting git. > else might get interested in seeing if this #leftoverbits is a good > direction to go in, and finishing it with docs and tests if it is > ;-) > merge-recursive.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > >...< This patch worked beautifully in my usecase!: $> rm -rf /tmp/repo1; mkdir /tmp/repo1; cd /tmp/repo1; git init .; ln -s sym1 link; echo 1 > file; git add file link; git commit -m 'common'; git co -b b1 ; ln -sf b1link link; echo "b1 file" >| file; git commit -m 'b2 changes' -a; git co master; ln -sf masterlink link; echo "master file" >| file; git commit -m 'also modified in master' -a; git merge -X theirs --no-edit b1; ls -l link; cat file warning: templates not found /home/yoh/share/git-core/templates Initialized empty Git repository in /tmp/repo1/.git/ [master (root-commit) d2e9010] common 2 files changed, 2 insertions(+) create mode 100644 file create mode 120000 link Switched to a new branch 'b1' [b1 a2b1321] b2 changes 2 files changed, 2 insertions(+), 2 deletions(-) Switched to branch 'master' [master fbb4ba7] also modified in master 2 files changed, 2 insertions(+), 2 deletions(-) Auto-merging link Auto-merging file Merge made by the 'recursive' strategy. file | 2 +- link | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) lrwxrwxrwx 1 yoh yoh 6 Sep 26 09:32 link -> b1link b1 file I also tried -s ours and no explicit -s, and they did as prescribed as well PS thanks for the CCs -- Yaroslav O. Halchenko Center for Open Neuroscience http://centerforopenneuroscience.org Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755 Phone: +1 (603) 646-9834 Fax: +1 (603) 646-1419 WWW: http://www.linkedin.com/in/yarik ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] merge: teach -Xours/-Xtheirs to symbolic link merge 2017-09-26 13:37 ` Yaroslav Halchenko @ 2017-10-16 5:38 ` Junio C Hamano 2017-12-29 2:49 ` Elijah Newren 2018-01-25 4:35 ` external diff driver is not used for diff --stat? Yaroslav Halchenko 0 siblings, 2 replies; 19+ messages in thread From: Junio C Hamano @ 2017-10-16 5:38 UTC (permalink / raw) To: git@vger.kernel.org; +Cc: Yaroslav Halchenko The -Xours/-Xtheirs merge options were originally defined as a way to "force" the resolution of 3way textual merge conflicts to take one side without using your editor, hence did not even trigger in situations where you would normally not get the <<< === >>> conflict markers. This was improved for binary files back in 2012 with a944af1d ("merge: teach -Xours/-Xtheirs to binary ll-merge driver", 2012-09-08). Teach a similar trick to the codepath that deals with merging two conflicting changes to symbolic links. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- * Looks like I queued this on 'pu' but never sent it out to the list for extra eyeballs. On the tests are new, relative to what was sent out earlier and archived at: https://public-inbox.org/git/xmqqa81ichdu.fsf@gitster.mtv.corp.google.com merge-recursive.c | 17 +++++++++++++---- t/t6037-merge-ours-theirs.sh | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index 1494ffdb82..ed529f2ceb 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1002,10 +1002,19 @@ static int merge_file_1(struct merge_options *o, &b->oid, !o->call_depth); } else if (S_ISLNK(a->mode)) { - oidcpy(&result->oid, &a->oid); - - if (!oid_eq(&a->oid, &b->oid)) - result->clean = 0; + switch (o->recursive_variant) { + case MERGE_RECURSIVE_NORMAL: + oidcpy(&result->oid, &a->oid); + if (!oid_eq(&a->oid, &b->oid)) + result->clean = 0; + break; + case MERGE_RECURSIVE_OURS: + oidcpy(&result->oid, &a->oid); + break; + case MERGE_RECURSIVE_THEIRS: + oidcpy(&result->oid, &b->oid); + break; + } } else die("BUG: unsupported object type in the tree"); } diff --git a/t/t6037-merge-ours-theirs.sh b/t/t6037-merge-ours-theirs.sh index 3889eca4ae..0aebc6c028 100755 --- a/t/t6037-merge-ours-theirs.sh +++ b/t/t6037-merge-ours-theirs.sh @@ -73,4 +73,36 @@ test_expect_success 'pull passes -X to underlying merge' ' git reset --hard master && test_must_fail git pull -s recursive -X bork . side ' +test_expect_success SYMLINKS 'symlink with -Xours/-Xtheirs' ' + git reset --hard master && + git checkout -b two master && + ln -s target-zero link && + git add link && + git commit -m "add link pointing to zero" && + + ln -f -s target-two link && + git commit -m "add link pointing to two" link && + + git checkout -b one HEAD^ && + ln -f -s target-one link && + git commit -m "add link pointing to one" link && + + # we expect symbolic links not to resolve automatically, of course + git checkout one^0 && + test_must_fail git merge -s recursive two && + + # favor theirs to resolve to target-two? + git reset --hard && + git checkout one^0 && + git merge -s recursive -X theirs two && + git diff --exit-code two HEAD link && + + # favor ours to resolve to target-one? + git reset --hard && + git checkout one^0 && + git merge -s recursive -X ours two && + git diff --exit-code one HEAD link + +' + test_done -- 2.15.0-rc1-172-gbfe4246c99 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] merge: teach -Xours/-Xtheirs to symbolic link merge 2017-10-16 5:38 ` [PATCH] merge: teach -Xours/-Xtheirs to symbolic link merge Junio C Hamano @ 2017-12-29 2:49 ` Elijah Newren 2017-12-29 4:41 ` Yaroslav Halchenko 2018-01-25 4:35 ` external diff driver is not used for diff --stat? Yaroslav Halchenko 1 sibling, 1 reply; 19+ messages in thread From: Elijah Newren @ 2017-12-29 2:49 UTC (permalink / raw) To: Junio C Hamano; +Cc: git@vger.kernel.org, Yaroslav Halchenko On Sun, Oct 15, 2017 at 10:38 PM, Junio C Hamano <gitster@pobox.com> wrote: > The -Xours/-Xtheirs merge options were originally defined as a way > to "force" the resolution of 3way textual merge conflicts to take > one side without using your editor, hence did not even trigger in > situations where you would normally not get the <<< === >>> conflict > markers. > > This was improved for binary files back in 2012 with a944af1d > ("merge: teach -Xours/-Xtheirs to binary ll-merge driver", > 2012-09-08). > > Teach a similar trick to the codepath that deals with merging two > conflicting changes to symbolic links. Saw this change referenced in the "what's cooking" emails and decided to review this. The code changes look obviously correct to me, and the testcase looks good too. Reviewed-by: Elijah Newren <newren@gmail.com> (and perhaps we should also add in "Tested-by: Yaroslav Halchenko <yoh@onerussian.com>" ? At least, that was my thought based on https://public-inbox.org/git/20170926133703.7gtk5ztkhqvfxszh@hopa.kiewit.dartmouth.edu/ ) ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] merge: teach -Xours/-Xtheirs to symbolic link merge 2017-12-29 2:49 ` Elijah Newren @ 2017-12-29 4:41 ` Yaroslav Halchenko 0 siblings, 0 replies; 19+ messages in thread From: Yaroslav Halchenko @ 2017-12-29 4:41 UTC (permalink / raw) To: Elijah Newren; +Cc: Junio C Hamano, git@vger.kernel.org On Thu, 28 Dec 2017, Elijah Newren wrote: > > Teach a similar trick to the codepath that deals with merging two > > conflicting changes to symbolic links. > Saw this change referenced in the "what's cooking" emails and decided > to review this. The code changes look obviously correct to me, and > the testcase looks good too. > Reviewed-by: Elijah Newren <newren@gmail.com> > (and perhaps we should also add in "Tested-by: Yaroslav Halchenko > <yoh@onerussian.com>" ? At least, that was my thought based on > https://public-inbox.org/git/20170926133703.7gtk5ztkhqvfxszh@hopa.kiewit.dartmouth.edu/ > ) I would be honored to wear a badge of the git-tested-by-er! FWIW I can reconfirm, that the patch did work out nicely for me back then Thanks! -- Yaroslav O. Halchenko Center for Open Neuroscience http://centerforopenneuroscience.org Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755 Phone: +1 (603) 646-9834 Fax: +1 (603) 646-1419 WWW: http://www.linkedin.com/in/yarik ^ permalink raw reply [flat|nested] 19+ messages in thread
* external diff driver is not used for diff --stat? 2017-10-16 5:38 ` [PATCH] merge: teach -Xours/-Xtheirs to symbolic link merge Junio C Hamano 2017-12-29 2:49 ` Elijah Newren @ 2018-01-25 4:35 ` Yaroslav Halchenko 1 sibling, 0 replies; 19+ messages in thread From: Yaroslav Halchenko @ 2018-01-25 4:35 UTC (permalink / raw) To: git@vger.kernel.org Dear Git Peoples, I am torturing git and git-annex here trying to compare some logs from a run of a software recorded in two different branches. As many other tools, software often logs its version, elapsed times etc, so diff becomes not of interest to me: $> PATH=~/proj/misc/git/INSTALL-2.16.1/bin:$PATH git diff test-18.0.09 test-18.0.05+git24-gb25b21054_dfsg.1-1_nd90+1 -- AFNI_data6/FT_analysis/FT.results/out.allcostX.txt-git diff --git a/AFNI_data6/FT_analysis/FT.results/out.allcostX.txt-git b/AFNI_data6/FT_analysis/FT.results/out.allcostX.txt-git index db85c9be..5f4a704d 100644 --- a/AFNI_data6/FT_analysis/FT.results/out.allcostX.txt-git +++ b/AFNI_data6/FT_analysis/FT.results/out.allcostX.txt-git @@ -1,4 +1,4 @@ -++ 3dAllineate: AFNI version=AFNI_18.0.09 (Jan 19 2018) [64-bit] +++ 3dAllineate: AFNI version=Debian-18.0.05+git24-gb25b21054~dfsg.1-1~nd90+1 (Jan 23 2018) [64-bit] ++ Authored by: Zhark the Registrator ++ Source dataset: ./anat_final.FT+tlrc.HEAD ++ Base dataset: ./final_epi_vr_base_min_outlier+tlrc.HEAD @@ -28,5 +28,5 @@ volume 0 lpa = 0.921773 lpc+ = 0.310739 ncd = 0.967007 -++ 3dAllineate: total CPU time = 0.0 sec Elapsed = 1.5 +++ 3dAllineate: total CPU time = 0.0 sec Elapsed = 1.3 so I came up with a simple differ to exclude those: $> cat ~/bin/git-annex-diff-wrapper #!/usr/bin/env bash LANG=C diff --color=always --ignore-matching-lines="\(AFNI version=\|time.*Elapsed\)" -u "$2" "$5" which works as it should (sorry for long lines, just wanted to not cut out anything which might be of relevance) $> PATH=~/proj/misc/git/INSTALL-2.16.1/bin:$PATH GIT_EXTERNAL_DIFF='git-annex diffdriver -- ~/bin/git-annex-diff-wrapper --' git diff --ext-diff test-18.0.09 test-18.0.05+git24-gb25b21054_dfsg.1-1_nd90+1 -- AFNI_data6/FT_analysis/FT.results/out.allcostX.txt-git # no output received (and even on annexed files -- whoohoo). The problem comes that --stat seems to be not using the external diff (it is line the same as above just with --stat): $> PATH=~/proj/misc/git/INSTALL-2.16.1/bin:$PATH GIT_EXTERNAL_DIFF='git-annex diffdriver -- ~/bin/git-annex-diff-wrapper --' git diff --ext-diff test-18.0.09 test-18.0.05+git24-gb25b21054_dfsg.1-1_nd90+1 --stat -- AFNI_data6/FT_analysis/FT.results/out.allcostX.txt-git AFNI_data6/FT_analysis/FT.results/out.allcostX.txt-git | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) A shortcoming or somehow "by design"? ;) PS Please CC me in replies Cheers! -- Yaroslav O. Halchenko Center for Open Neuroscience http://centerforopenneuroscience.org Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755 Phone: +1 (603) 646-9834 Fax: +1 (603) 646-1419 WWW: http://www.linkedin.com/in/yarik ^ permalink raw reply [flat|nested] 19+ messages in thread
* -s theirs use-case(s) Was: BUG: merge -s theirs is not in effect 2017-09-25 5:33 ` Re* " Junio C Hamano 2017-09-25 14:30 ` -X theirs does not resolve symlink conflict Was: BUG: merge -s theirs is not in effect Yaroslav Halchenko @ 2017-09-25 14:40 ` Yaroslav Halchenko 2017-09-26 3:45 ` Junio C Hamano 1 sibling, 1 reply; 19+ messages in thread From: Yaroslav Halchenko @ 2017-09-25 14:40 UTC (permalink / raw) To: git@vger.kernel.org On Mon, 25 Sep 2017, Junio C Hamano wrote: > It is a different matter to resurrect the age old discussion that > happend in the summer of 2008 if '-s theirs' should or should not > exist. In short, the previous discussion can be summarised to > "we don't want '-s theirs' as it encourages the wrong workflow". > https://public-inbox.org/git/alpine.DEB.1.00.0807290123300.2725@eeepc-johanness/ > https://public-inbox.org/git/7vtzen7bul.fsf@gitster.siamese.dyndns.org/ > https://public-inbox.org/git/20080720192130.6117@nanako3.lavabit.com/ > It is OK for people to come with new perspective and bring new > ideas to the table. We learned from experience while using Git > for longer and are wiser than what we were back then, and might > be able to make a better decision ;-) FWIW 1. As a workaround for absence of -m theirs I using mtheirs git alias: (I believe provided to me awhile back here on the list): mtheirs = !sh -c 'git merge -s ours --no-commit $1 && git read-tree -m -u $1' - and it worked fine for my usecases 2. I think that if there is a reason for -s ours to exist, so there for -s theirs since it is just the directionality of merges which changes between the two 3. My most frequently used use-case for -m theirs strategy is repositories such as http://datasets.datalad.org/openfmri/ds000001/.git where we construct "datalad dataset" by crawling the web resource(s), and workflow consists of 3 branches: incoming -- content from the web "as is" incoming-processed -- content from the web "processed" (fully automatically), e.g. tarballs extracted etc master -- the "final" result, delivered to public incoming-processed is formed by -s theirs --no-commit incoming, then all content needed to be extracted/processed (since last such merge point) is processed and commit is done. Such "merge" allows us to establish a point of previous "processing state" so we could react appropriately whenever anything in "incoming" branch changes (so that there is a new commit). And then incoming-processed is merged (regular recursive) into the master branch, which might have further "manual" tune ups. PS thanks for CCing replies -- Yaroslav O. Halchenko Center for Open Neuroscience http://centerforopenneuroscience.org Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755 Phone: +1 (603) 646-9834 Fax: +1 (603) 646-1419 WWW: http://www.linkedin.com/in/yarik ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: -s theirs use-case(s) Was: BUG: merge -s theirs is not in effect 2017-09-25 14:40 ` -s theirs use-case(s) Was: BUG: merge -s theirs is not in effect Yaroslav Halchenko @ 2017-09-26 3:45 ` Junio C Hamano 2017-09-26 13:32 ` Yaroslav Halchenko 0 siblings, 1 reply; 19+ messages in thread From: Junio C Hamano @ 2017-09-26 3:45 UTC (permalink / raw) To: Yaroslav Halchenko; +Cc: git@vger.kernel.org Yaroslav Halchenko <yoh@onerussian.com> writes: > 1. As a workaround for absence of -m theirs I using mtheirs git alias: > (I believe provided to me awhile back here on the list): > > mtheirs = !sh -c 'git merge -s ours --no-commit $1 && git read-tree -m -u $1' - > > and it worked fine for my usecases > > 2. I think that if there is a reason for -s ours to exist, so there for -s theirs > since it is just the directionality of merges which changes between the two Just on this point. They are not exactly symmetric. Imagine there are some undesirable changes you want to vanquish from the world, but they have already built on useful changes on top of the undesirable changes. A hypothetical history might look like this: B---C / X---X---A / ---o---o your mainline where 'X' denotes those unwanted changes. With a "-s ours" merge, you can declare that changes on the other branch will never be merged to your branch, i.e. B---C / X---X---A / \ ---o---o---M your mainline and then you can safely merge A and C into your branch, without having to worry about them bringing the unwanted changes to your tree state. B---C / \ X---X---A \ / \ \ \ ---o---o---M---N---O your mainline That is the primary reason why "-s ours" exists, i.e. you do not control the branch where mistakes X were made because that is somebody else's history. The symmetiric case where _you_ have wrong changes do not need "-s theirs". These mistakes X are yours, so are the changes depend on them: B---C / X---X---A / ---o---o their mainline and you can just rebase A, B and C on top of their mainline while getting rid of Xs yourself before publishing. B'--C' / ---o---o---A' The reason why ours and theirs are not symmetric is because you are you and not them---the control and ownership of our history and their history is not symmetric. There may be valid workflows that benefit from "-s theirs", and I would not be surprised at all if we found more of them in the past 9 years since we had the "why -s theirs does not exist" discussion in 2008. But "because -s ours can be used in reverse to emulate" is not a valid excuse to add "-s theirs". It can be used a rationale against adding it (e.g. "-s theirs generally is discouraged because it forsters a bad workflow, but in a very rare case where it might be useful, you can always check out their branch and merge yours using '-s ours' to emulate it, so we do not lose any functionality even if we did not add it"), though. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: -s theirs use-case(s) Was: BUG: merge -s theirs is not in effect 2017-09-26 3:45 ` Junio C Hamano @ 2017-09-26 13:32 ` Yaroslav Halchenko 2017-09-27 0:09 ` Junio C Hamano 0 siblings, 1 reply; 19+ messages in thread From: Yaroslav Halchenko @ 2017-09-26 13:32 UTC (permalink / raw) To: git@vger.kernel.org On Tue, 26 Sep 2017, Junio C Hamano wrote: > Yaroslav Halchenko <yoh@onerussian.com> writes: > > 1. As a workaround for absence of -m theirs I using mtheirs git alias: > > (I believe provided to me awhile back here on the list): > > mtheirs = !sh -c 'git merge -s ours --no-commit $1 && git read-tree -m -u $1' - > > and it worked fine for my usecases > > 2. I think that if there is a reason for -s ours to exist, so there for -s theirs > > since it is just the directionality of merges which changes between the two > Just on this point. They are not exactly symmetric. > Imagine there are some undesirable changes you want to vanquish from > the world, but they have already built on useful changes on top of > the undesirable changes. A hypothetical history might look like > this: > B---C > / > X---X---A > / > ---o---o your mainline > where 'X' denotes those unwanted changes. > >...< > The symmetiric case where _you_ have wrong changes do not need "-s > theirs". These mistakes X are yours, so are the changes depend on > them: > B---C > / > X---X---A > / > ---o---o their mainline > and you can just rebase A, B and C on top of their mainline while > getting rid of Xs yourself before publishing. and that is where the gotcha comes -- what if "my" changes were already published? then I would like to avoid the rebase, and would -s theirs to choose "their" solution in favor of mine and be able to push so others could still "fast-forward" to the new state. So -- as to me it remains 'symmetric' ;) > There may be valid workflows that benefit from "-s theirs", and I > would not be surprised at all if we found more of them in the past 9 > years since we had the "why -s theirs does not exist" discussion in > 2008. But "because -s ours can be used in reverse to emulate" is > not a valid excuse to add "-s theirs". It can be used a rationale > against adding it (e.g. "-s theirs generally is discouraged because > it forsters a bad workflow, but in a very rare case where it might > be useful, you can always check out their branch and merge yours > using '-s ours' to emulate it, so we do not lose any functionality > even if we did not add it"), though. sure, git is flexible, so workarounds could always be found, but often many options are just a matter of convenience. And here -s theirs would be one of them besides my other use case where it is a somewhat "by design" workflow, and -s theirs is use to take their exact state I would improve upon (before committing the "merge") -- Yaroslav O. Halchenko Center for Open Neuroscience http://centerforopenneuroscience.org Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755 Phone: +1 (603) 646-9834 Fax: +1 (603) 646-1419 WWW: http://www.linkedin.com/in/yarik ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: -s theirs use-case(s) Was: BUG: merge -s theirs is not in effect 2017-09-26 13:32 ` Yaroslav Halchenko @ 2017-09-27 0:09 ` Junio C Hamano 2017-09-27 5:19 ` Yaroslav Halchenko 0 siblings, 1 reply; 19+ messages in thread From: Junio C Hamano @ 2017-09-27 0:09 UTC (permalink / raw) To: Yaroslav Halchenko; +Cc: git@vger.kernel.org Yaroslav Halchenko <yoh@onerussian.com> writes: > and that is where the gotcha comes -- what if "my" changes were already > published? then I would like to avoid the rebase, and would -s theirs > to choose "their" solution in favor of mine and be able to push so > others could still "fast-forward" to the new state. > > So -- as to me it remains 'symmetric' ;) I do not necessarily agree. Once you decide that their history is the mainline, you'd rather want to treat your line of development as a side branch and make a merge in that direction, i.e. the first parent of the resulting merge is a commit on their history and the second parent is the last bad one of your history. So you would end up using "checkout their-history && merge -s ours your-history" to keep the first-parenthood sensible. And at that point, use of "-s ours" is no longer a workaround for lack of "-s theirs". It is a proper part of the desired semantics, i.e. from the point of view of the surviving canonical history line, you want to preserve what it did, nullifying what the other line of history did. So I still do not think the above scenario justifies "-s theirs". ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: -s theirs use-case(s) Was: BUG: merge -s theirs is not in effect 2017-09-27 0:09 ` Junio C Hamano @ 2017-09-27 5:19 ` Yaroslav Halchenko 2017-09-27 20:21 ` Yaroslav Halchenko 0 siblings, 1 reply; 19+ messages in thread From: Yaroslav Halchenko @ 2017-09-27 5:19 UTC (permalink / raw) To: git@vger.kernel.org On Wed, 27 Sep 2017, Junio C Hamano wrote: > > and that is where the gotcha comes -- what if "my" changes were already > > published? then I would like to avoid the rebase, and would -s theirs > > to choose "their" solution in favor of mine and be able to push so > > others could still "fast-forward" to the new state. > > So -- as to me it remains 'symmetric' ;) > I do not necessarily agree. Once you decide that their history is > the mainline, you'd rather want to treat your line of development as > a side branch and make a merge in that direction, i.e. the first > parent of the resulting merge is a commit on their history and the > second parent is the last bad one of your history. So you would end > up using "checkout their-history && merge -s ours your-history" to > keep the first-parenthood sensible. > And at that point, use of "-s ours" is no longer a workaround for > lack of "-s theirs". It is a proper part of the desired semantics, > i.e. from the point of view of the surviving canonical history line, > you want to preserve what it did, nullifying what the other line of > history did. > So I still do not think the above scenario justifies "-s theirs". ok, when you describe it like this (in my case I rarely cared about the side of the merge), then indeed I might better do the entire dance with git reset --hard theirstate; git merge -s ours HEAD@{1} and live happily with the left side being the one always correct and hide "my" mistakes ;) will keep it in mind -- Yaroslav O. Halchenko Center for Open Neuroscience http://centerforopenneuroscience.org Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755 Phone: +1 (603) 646-9834 Fax: +1 (603) 646-1419 WWW: http://www.linkedin.com/in/yarik ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: -s theirs use-case(s) Was: BUG: merge -s theirs is not in effect 2017-09-27 5:19 ` Yaroslav Halchenko @ 2017-09-27 20:21 ` Yaroslav Halchenko 0 siblings, 0 replies; 19+ messages in thread From: Yaroslav Halchenko @ 2017-09-27 20:21 UTC (permalink / raw) To: git@vger.kernel.org On Wed, 27 Sep 2017, Yaroslav Halchenko wrote: > > And at that point, use of "-s ours" is no longer a workaround for > > lack of "-s theirs". It is a proper part of the desired semantics, > > i.e. from the point of view of the surviving canonical history line, > > you want to preserve what it did, nullifying what the other line of > > history did. > > So I still do not think the above scenario justifies "-s theirs". > ok, when you describe it like this (in my case I rarely cared about the > side of the merge), then indeed I might better do the entire dance with > git reset --hard theirstate; git merge -s ours HEAD@{1} > and live happily with the left side being the one always correct and > hide "my" mistakes ;) will keep it in mind ha -- was about to use it, which reminded me about this use case! NB pardon me for not so wonderful ascii-diagrams as yours but hopefully still helps x-o-o-o-x-o-o debian / / x-------x----- releases -- should just linearly sweep through releases I care about / / R1 R2 various release branches/tags A B which have differing commits / / ---o---o----o----o masteg For packaging some packages for Debian, with my git-rotten-soul, I am trying to keep my debian packaging (in a branch "debian") on top of upstream releases. But the problem comes whenever upstream releases from "release branches", which are never merged into master and might have different and conflicting changes. So, it becomes impossible to maintain a linearly progressing "debian" branch without adding a middle-man between upstream releases (in release branches), and debian branch (should progress linearly forward) -- "releases" branch. See e.g. http://github.com/neurodebian/pandas and its releases branch. so I use -s theirs for "linearizing" the branched up development history -- Yaroslav O. Halchenko Center for Open Neuroscience http://centerforopenneuroscience.org Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755 Phone: +1 (603) 646-9834 Fax: +1 (603) 646-1419 WWW: http://www.linkedin.com/in/yarik ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2018-01-25 4:35 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-09-25 0:02 BUG: merge -s theirs is not in effect (does the same as -s ours) Yaroslav Halchenko 2017-09-25 1:08 ` Junio C Hamano 2017-09-25 3:17 ` Yaroslav Halchenko 2017-09-25 5:33 ` Re* " Junio C Hamano 2017-09-25 14:30 ` -X theirs does not resolve symlink conflict Was: BUG: merge -s theirs is not in effect Yaroslav Halchenko 2017-09-26 1:56 ` Junio C Hamano 2017-09-26 2:16 ` Junio C Hamano 2017-09-26 2:39 ` Junio C Hamano 2017-09-26 13:37 ` Yaroslav Halchenko 2017-10-16 5:38 ` [PATCH] merge: teach -Xours/-Xtheirs to symbolic link merge Junio C Hamano 2017-12-29 2:49 ` Elijah Newren 2017-12-29 4:41 ` Yaroslav Halchenko 2018-01-25 4:35 ` external diff driver is not used for diff --stat? Yaroslav Halchenko 2017-09-25 14:40 ` -s theirs use-case(s) Was: BUG: merge -s theirs is not in effect Yaroslav Halchenko 2017-09-26 3:45 ` Junio C Hamano 2017-09-26 13:32 ` Yaroslav Halchenko 2017-09-27 0:09 ` Junio C Hamano 2017-09-27 5:19 ` Yaroslav Halchenko 2017-09-27 20:21 ` Yaroslav Halchenko
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).