'bc3' Beyond Compare 3 tool incorrect executable

[Werner BEROUX]

I like the fact that bc3 is not supported built-in. However it seems
to be using the wrong version.

BUG DESCRIPTION:

When a BC3 is open and a 'git mergetool' is initiated, BC3 will return
immediately and Git will ask "Was merge successfull? [y/n]".

FIX:

libexec\git-core\git-mergetool--lib @ 11
translate_merge_tool_path () {
case "$1" in
araxis)
echo compare
;;
bc3)
- echo bcompare
+ echo bcomp
;;

EXPLANATION:

From BC3 help BCompare will wait if not instance was opened. If an
instance was opened, it'll return immediately.
"BComp.com" on the other side will run in console mode and wait always
until the tab/window is closed. Additionally "BComp.exe" always
returns immediately but .com should come before .exe.

[Johannes Schindelin]

Hi Werner,

Cool. Could you please do two things now:

- verify that there is no version of BeyondCompare where we have bcompare
but not bcomp (because if there is a user of such a version, our fix
would break her setup)

- make this a patch that I can apply easily by using

git format-patch -1

?

Thank you,
Johannes

[Werner BEROUX]

---

 git-mergetool--lib |    2 +-

 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-mergetool--lib b/git-mergetool--lib

index 91f90ac..5eae490 100644

--- a/git-mergetool--lib

+++ b/git-mergetool--lib

@@ -14,7 +14,7 @@ translate_merge_tool_path () {

                echo compare

                ;;

        bc3)

-               echo bcompare

+               echo bcomp

                ;;

        emerge)

                echo emacs

--

1.7.6.msysgit.0

BComp.com/BComp.exe is provided with all BC3 versions.

Werner

[Pat Thoyts]

On 13 October 2011 10:33, Werner BEROUX <wer...@beroux.com> wrote:
>
> ---
>  git-mergetool--lib |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> diff --git a/git-mergetool--lib b/git-mergetool--lib
> index 91f90ac..5eae490 100644
> --- a/git-mergetool--lib
> +++ b/git-mergetool--lib
> @@ -14,7 +14,7 @@ translate_merge_tool_path () {
>                 echo compare
>                 ;;
>         bc3)
> -               echo bcompare
> +               echo bcomp
>                 ;;
>         emerge)
>                 echo emacs
> --
> 1.7.6.msysgit.0

That looks ok except you did not include a signoff. Also, it helps to
include as full a discussion for the reason for the commit as you are
able. I suggest adding the following to the commit message:

BCompare will wait if no instance is already opened. If an instance is
already running then it will return immediately. However, "BComp.com"
runs in console mode and always waits until the tab/window is

closed. Additionally "BComp.exe" always returns immediately but .com

executables are preferred over .exe as per the default PATHEXT
environment setting.

which summarizes the earlier email and helps to discourage some future
maintainer from deciding that 'bcompare' was fine all along.
It is worth reading the git/Documentation/SubmittingPatches for the
rationale behind commit messages and sign-off lines.

Pat Thoyts

[Werner BEROUX]

You're absolutely right. Thanks for your clarification. It's the first time I use that way.

As a side note, I really like Github and it makes it so much easier to submit patches.

Werner

[Pat Thoyts]

On 13 October 2011 10:54, Werner BEROUX <wer...@beroux.com> wrote:
> You're absolutely right. Thanks for your clarification. It's the first time
> I use that way.
> As a side note, I really like Github and it makes it so much easier to
> submit patches.
> Werner

Ah - what I had not realized is this is really a patch for upstream
and beyondcompare exists for Unix too where bcompare does the right
thing. So this fix needs applying only for Windows. Further,
git-mergetool--lib is generated from git-mergetool--lib.sh so its
fixing the wrong file.

However more important -- the structure for the merge-tools has
changed for 1.7.7 and this has pointed out a problem in this Git for
Windows release. The mergetools are now defined in a subdirectory
git/mergetools/ and we need to add this into our installer too.
Currently "git mergetool" is comprehensively broken in Git for Windows
1.7.7

[Werner BEROUX]

Oh cool to clean that up. As msysgit is Windows and Git documentation didn't seem to support BC3 I expected a Windows-only solution.

Can't wait to see the next release come out (even if I'm more waiting for an automatic "git-svn --mergeinfo" as I've fixed BC3 locally).

Regards,
Werner

[Pat Thoyts]

The following patch should resolve this either way:

From 7e4bf67e6a51c7200407017c69b4a67687fe609e Mon Sep 17 00:00:00 2001
From: Pat Thoyts <patt...@users.sourceforge.net>
Date: Thu, 13 Oct 2011 12:54:35 +0100
Subject: [PATCH] mergetools: use the correct tool for Beyond Compare 3
on Windows

On Windows the bcompare tool launches a graphical program and does
not wait for it to terminate. A separate 'bcomp' tool is provided which
will wait for the view to exit so we use this instead.

Reported-by: Werner BEROUX <wer...@beroux.com>
Signed-off-by: Pat Thoyts <patt...@users.sourceforge.net>
---
mergetools/bc3 | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/mergetools/bc3 b/mergetools/bc3
index 27b3dd4..b642bf2 100644
--- a/mergetools/bc3
+++ b/mergetools/bc3
@@ -16,5 +16,12 @@ merge_cmd () {
}

translate_merge_tool_path() {
- echo bcompare
+ case $(uname -s) in
+ *MINGW*)
+ echo bcomp
+ ;;
+ *)
+ echo bcompare
+ ;;
+ esac
}
--
1.7.7.msysgit.0

[Sebastian Schuberth]

On 13.10.2011 13:56, Pat Thoyts wrote:

> On Windows the bcompare tool launches a graphical program and does
> not wait for it to terminate. A separate 'bcomp' tool is provided which
> will wait for the view to exit so we use this instead.

Thanks for fixing this. Being a BC3 user myself we should probably point
out that on Windows, *.com files take precedence over *.exe files when
no extension is specified on execution.

How about this:

"On Windows, 'bcompare.exe' launches a graphical program and does not
wait for it to terminate. A separate 'bcomp.com' tool is provided which
will wait for the view to exit so we use this instead. Just using
'bcomp' is sufficient although 'bcomp.exe' is also supplied as *.com
files take precedence over *.exe files if no extension is specified."

--
Sebastian Schuberth

[Pat Thoyts]

I already pushed up something like that to devel:
https://github.com/msysgit/git/commit/7e4bf67e6a51c7200407017c69b4a67687fe609e

[David Aguilar]

Can you please send the final patch to g...@vger.kernel.org
and Cc: everyone here as well? We (I) want this upstream.

Sorry for breaking msysgit with the extra mergetools/ directory!
I presume it's the install.nss script that had to be taught
about the new directory?

Thanks all,
--
David

[David Aguilar]

On Sat, Oct 15, 2011 at 09:30:19PM -0700, David Aguilar wrote:
>
> Can you please send the final patch to g...@vger.kernel.org
> and Cc: everyone here as well? We (I) want this upstream.

Sorry for the noise -- I'm just reading my later emails and
realized you had already sent the patch to git.
I will read my emails newest to oldest from now on.
Thank you!
--
David

[Sebastian Schuberth]

On Sun, Oct 16, 2011 at 06:30, David Aguilar <dav...@gmail.com> wrote:

> Sorry for breaking msysgit with the extra mergetools/ directory!
> I presume it's the install.nss script that had to be taught
> about the new directory?

What's an nss script? A mixture between an nsi (Nullsoft Installer)
and an iss (Inno Setup) script? ;-)

Just for the record, in msysGit we're using Inno Setup, and no, it was
not a problem with the setup script, but with a separate shell script
that copies the files to be packaged into a temporary directory. We
should probably make use of "make install" into a custom directory
instead ...

--
Sebastian Schuberth

[Johannes Schindelin]

Hi,

That would not be enough, as we

- need all kinds of support libraries and executables, too (i.e. bash)

- need to get rid of a few executables which can be compiled but just do
not make sense (archimport and others)

For the record, Pat's commit (which he made barely an hour before I came
around to commit, but then not push, mine):

https://github.com/msysgit/msysgit/commit/e04fe5497ae08ad7334dbc035321e5e42e458615

Ciao,
Dscho

[Sebastian Schuberth]

On Mon, Oct 17, 2011 at 04:45, Johannes Schindelin
<Johannes....@gmx.de> wrote:

>> that copies the files to be packaged into a temporary directory. We
>> should probably make use of "make install" into a custom directory
>> instead ...
>
> That would not be enough, as we
>
> - need all kinds of support libraries and executables, too (i.e. bash)
>
> - need to get rid of a few executables which can be compiled but just do
>  not make sense (archimport and others)

I know. I was probably more thinking about how it could be done in my
mingwGitDevEnv project. There, bash etc. already are in a directory
layout that can be packaged as-is, and using "make install" for Git
plus some post-processing to filter out unneeded files would maybe
make more sense.

--
Sebastian Schuberth