contrib/buildsystems: fix Visual Studio Debug configuration

From: Alexandr Miloslavskiy <[email protected]>

Even though Debug configuration builds, the resulting build is incorrect in a subtle way: it mixes up Debug and Release binaries, which in turn causes hard-to-predict bugs.

In my case, when git calls iconv library, iconv sets 'errno' and git then tests it, but in Debug and Release CRT those 'errno' are different memory locations.

This patch addresses 3 connected bugs:
1) Typo in '\(Configuration)'. As a result, Debug configuration condition is always false and Release path is taken instead.
2) Regexp that replaced 'zlib.lib' with 'zlibd.lib' was only affecting the first occurrence. However, some projects have it listed twice. Previously this bug was hidden, because Debug path was never taken. I decided that avoiding double -lz in makefile is fragile and I'd better replace all occurrences instead.
3) In Debug, 'libcurl-d.lib' should be used instead of 'libcurl.lib'. Previously this bug was hidden, because Debug path was never taken.

Signed-off-by Alexandr Miloslavskiy <[email protected]>

---
 contrib/buildsystems/Generators/Vcxproj.pm | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

contrib/buildsystems/Generators/Vcxproj.pm +8 -8

index 576ccabe1d..7b1e277eca 100644
@@ -79,7 +79,8 @@ sub createProject {
     if (!$static_library) {
       $libs_release = join(";", sort(grep /^(?!libgit\.lib|xdiff\/lib\.lib|vcs-svn\/lib\.lib)/, @{$$build_structure{"$prefix${name}_LIBS"}}));
       $libs_debug = $libs_release;
-      $libs_debug =~ s/zlib\.lib/zlibd\.lib/;
+      $libs_debug =~ s/zlib\.lib/zlibd\.lib/g;
+      $libs_debug =~ s/libcurl\.lib/libcurl-d\.lib/g;
     }
 
     $defines =~ s/-D//g;

@@ -119,13 +120,13 @@ sub createProject {
     <VCPKGArch Condition="'\$(Platform)'=='Win32'">x86-windows</VCPKGArch>
     <VCPKGArch Condition="'\$(Platform)'!='Win32'">x64-windows</VCPKGArch>
     <VCPKGArchDirectory>$cdup\\compat\\vcbuild\\vcpkg\\installed\\\$(VCPKGArch)</VCPKGArchDirectory>
-    <VCPKGBinDirectory Condition="'\(Configuration)'=='Debug'">\$(VCPKGArchDirectory)\\debug\\bin</VCPKGBinDirectory>
-    <VCPKGLibDirectory Condition="'\(Configuration)'=='Debug'">\$(VCPKGArchDirectory)\\debug\\lib</VCPKGLibDirectory>
-    <VCPKGBinDirectory Condition="'\(Configuration)'!='Debug'">\$(VCPKGArchDirectory)\\bin</VCPKGBinDirectory>
-    <VCPKGLibDirectory Condition="'\(Configuration)'!='Debug'">\$(VCPKGArchDirectory)\\lib</VCPKGLibDirectory>
+    <VCPKGBinDirectory Condition="'\$(Configuration)'=='Debug'">\$(VCPKGArchDirectory)\\debug\\bin</VCPKGBinDirectory>
+    <VCPKGLibDirectory Condition="'\$(Configuration)'=='Debug'">\$(VCPKGArchDirectory)\\debug\\lib</VCPKGLibDirectory>
+    <VCPKGBinDirectory Condition="'\$(Configuration)'!='Debug'">\$(VCPKGArchDirectory)\\bin</VCPKGBinDirectory>
+    <VCPKGLibDirectory Condition="'\$(Configuration)'!='Debug'">\$(VCPKGArchDirectory)\\lib</VCPKGLibDirectory>
     <VCPKGIncludeDirectory>\$(VCPKGArchDirectory)\\include</VCPKGIncludeDirectory>
-    <VCPKGLibs Condition="'\(Configuration)'=='Debug'">$libs_debug</VCPKGLibs>
-    <VCPKGLibs Condition="'\(Configuration)'!='Debug'">$libs_release</VCPKGLibs>
+    <VCPKGLibs Condition="'\$(Configuration)'=='Debug'">$libs_debug</VCPKGLibs>
+    <VCPKGLibs Condition="'\$(Configuration)'!='Debug'">$libs_release</VCPKGLibs>
   </PropertyGroup>
   <Import Project="\$(VCTargetsPath)\\Microsoft.Cpp.Default.props" />
   <PropertyGroup Condition="'\$(Configuration)'=='Debug'" Label="Configuration">
--

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

Even though Debug configuration builds, the resulting build is incorrect in a subtle way: it mixes up Debug and Release binaries, which in turn causes hard-to-predict bugs.

In my case, when git calls iconv library, iconv sets 'errno' and git then tests it, but in Debug and Release CRT those 'errno' are different memory locations.

This patch addresses 3 connected bugs:
1) Typo in '\(Configuration)'. As a result, Debug configuration condition is always false and Release path is taken instead.
2) Regexp that replaced 'zlib.lib' with 'zlibd.lib' was only affecting the first occurrence. However, some projects have it listed twice. Previously this bug was hidden, because Debug path was never taken. I decided that avoiding double -lz in makefile is fragile and I'd better replace all occurrences instead.
3) In Debug, 'libcurl-d.lib' should be used instead of 'libcurl.lib'. Previously this bug was hidden, because Debug path was never taken.

Signed-off-by Alexandr Miloslavskiy <[email protected]>

---
  contrib/buildsystems/Generators/Vcxproj.pm | 15 ++++++++-------
  1 file changed, 8 insertions(+), 7 deletions(-)

contrib/buildsystems/Generators/Vcxproj.pm +8 -7

index 576ccabe1d..7b1e277eca 100644
@@ -79,7 +79,8 @@ sub createProject {
      if (!$static_library) {
        $libs_release = join(";", sort(grep /^(?!libgit\.lib|xdiff\/lib\.lib|vcs-svn\/lib\.lib)/, @{$$build_structure{"$prefix${name}_LIBS"}}));
        $libs_debug = $libs_release;
-      $libs_debug =~ s/zlib\.lib/zlibd\.lib/;
+      $libs_debug =~ s/zlib\.lib/zlibd\.lib/g;
+      $libs_debug =~ s/libcurl\.lib/libcurl-d\.lib/g;
      }
 
      $defines =~ s/-D//g;

@@ -119,13 +120,13 @@ sub createProject {
      <VCPKGArch Condition="'\$(Platform)'=='Win32'">x86-windows</VCPKGArch>
      <VCPKGArch Condition="'\$(Platform)'!='Win32'">x64-windows</VCPKGArch>
      <VCPKGArchDirectory>$cdup\\compat\\vcbuild\\vcpkg\\installed\\\$(VCPKGArch)</VCPKGArchDirectory>
-    <VCPKGBinDirectory Condition="'\(Configuration)'=='Debug'">\$(VCPKGArchDirectory)\\debug\\bin</VCPKGBinDirectory>
-    <VCPKGLibDirectory Condition="'\(Configuration)'=='Debug'">\$(VCPKGArchDirectory)\\debug\\lib</VCPKGLibDirectory>
-    <VCPKGBinDirectory Condition="'\(Configuration)'!='Debug'">\$(VCPKGArchDirectory)\\bin</VCPKGBinDirectory>
-    <VCPKGLibDirectory Condition="'\(Configuration)'!='Debug'">\$(VCPKGArchDirectory)\\lib</VCPKGLibDirectory>
+    <VCPKGBinDirectory Condition="'\$(Configuration)'=='Debug'">\$(VCPKGArchDirectory)\\debug\\bin</VCPKGBinDirectory>
+    <VCPKGLibDirectory Condition="'\$(Configuration)'=='Debug'">\$(VCPKGArchDirectory)\\debug\\lib</VCPKGLibDirectory>
+    <VCPKGBinDirectory Condition="'\$(Configuration)'!='Debug'">\$(VCPKGArchDirectory)\\bin</VCPKGBinDirectory>
+    <VCPKGLibDirectory Condition="'\$(Configuration)'!='Debug'">\$(VCPKGArchDirectory)\\lib</VCPKGLibDirectory>
      <VCPKGIncludeDirectory>\$(VCPKGArchDirectory)\\include</VCPKGIncludeDirectory>
-    <VCPKGLibs Condition="'\(Configuration)'=='Debug'">$libs_debug</VCPKGLibs>
-    <VCPKGLibs Condition="'\(Configuration)'!='Debug'">$libs_release</VCPKGLibs>
+    <VCPKGLibs Condition="'\$(Configuration)'=='Debug'">$libs_debug</VCPKGLibs>
+    <VCPKGLibs Condition="'\$(Configuration)'!='Debug'">$libs_release</VCPKGLibs>
    </PropertyGroup>
    <Import Project="\$(VCTargetsPath)\\Microsoft.Cpp.Default.props" />
    <PropertyGroup Condition="'\$(Configuration)'=='Debug'" Label="Configuration">

I just bumped against a potential issue like this. I was test compiling [1a,b] the vs/master branch from Git-For-Windows and got the LINK error that the 'libcurl-d.lib' was not found (4 places).

Error    LNK1104    cannot open file 'libcurl-d.lib' git-imap-send C:\git-sdk-64\usr\src\git\git-imap-send\ LINK    1

Having just located this email, I changed the build type to 'Release' and the errors disappeared.

Do we also need to identify where the libcurl-d.lib will be found? i.e. is it something that needs including via the sdk pacman list (I think I'm up to date but maybe not..)

A quick web search didn't show any hits for libcurl-d.lib (with the dash -), though did find a few for libcurld.lib.

Quoted 9 lines · Philip Oakley

I just bumped against a potential issue like this. I was test compiling [1a,b] the vs/master branch from Git-For-Windows and got the LINK error that the 'libcurl-d.lib' was not found (4 places).

Error    LNK1104    cannot open file 'libcurl-d.lib' git-imap-send C:\git-sdk-64\usr\src\git\git-imap-send\ LINK    1

Having just located this email, I changed the build type to 'Release' and the errors disappeared.

Do we also need to identify where the libcurl-d.lib will be found? i.e. is it something that needs including via the sdk pacman list (I think I'm up to date but maybe not..)

A quick web search didn't show any hits for libcurl-d.lib (with the dash -), though did find a few for libcurld.lib.

If you clone git-for-windows and build in VS using git.sln, it will automatically clone git-for-windows\compat\vcbuild\vcpkg and build various things there, resulting in

git-for-windows\compat\vcbuild\vcpkg\buildtrees\curl\x64-windows-dbg\lib\libcurl-d.dll

git-for-windows\compat\vcbuild\vcpkg\installed\x64-windows\debug\bin\libcurl-d.dll

git-for-windows\compat\vcbuild\vcpkg\packages\curl_x64-windows\debug\bin\libcurl-d.dll

Which will be picked up by solution to build git.

I have built Debug many times now and didn't have any issues. If you do, I would suggest to clone a new copy and build it.

Quoted 9 lines · Philip Oakley

I just bumped against a potential issue like this. I was test compiling [1a,b] the vs/master branch from Git-For-Windows and got the LINK error that the 'libcurl-d.lib' was not found (4 places).

Error    LNK1104    cannot open file 'libcurl-d.lib' git-imap-send C:\git-sdk-64\usr\src\git\git-imap-send\ LINK    1

Having just located this email, I changed the build type to 'Release' and the errors disappeared.

Do we also need to identify where the libcurl-d.lib will be found? i.e. is it something that needs including via the sdk pacman list (I think I'm up to date but maybe not..)

A quick web search didn't show any hits for libcurl-d.lib (with the dash -), though did find a few for libcurld.lib.

If you clone git-for-windows and build in VS using git.sln, it will automatically clone git-for-windows\compat\vcbuild\vcpkg and build various things there, resulting in

git-for-windows\compat\vcbuild\vcpkg\buildtrees\curl\x64-windows-dbg\lib\libcurl-d.dll

git-for-windows\compat\vcbuild\vcpkg\installed\x64-windows\debug\bin\libcurl-d.dll

git-for-windows\compat\vcbuild\vcpkg\packages\curl_x64-windows\debug\bin\libcurl-d.dll

Which will be picked up by solution to build git.

I have built Debug many times now and didn't have any issues. If you do, I would suggest to clone a new copy and build it.

Hmm, 45 minutes of cloning and rebuild, but finally it compiled clean (both Release and Debug)

Quoted 1 line · Philip Oakley

Hmm, 45 minutes of cloning and rebuild, but finally it compiled clean (both Release and Debug)

I understand that the issue is resolved now.

Probably your old repo was missing the libraries for whatever reason (like antivirus deleting them, etc), but build script thought that dependencies are properly built, so didn't attempt to rebuild them.

Quoted 7 lines · Alexandr Miloslavskiy

Quoted 1 line · Philip Oakley

Hmm, 45 minutes of cloning and rebuild, but finally it compiled clean (both Release and Debug)

I understand that the issue is resolved now.

Probably your old repo was missing the libraries for whatever reason (like antivirus deleting them, etc), but build script thought that dependencies are properly built, so didn't attempt to rebuild them.

I think I'll report that as an issue to the https://github.com/microsoft/vcpkg folks so that there's better detection for 'out of date' / updated vcpkg issues.

The vcpkg_install.bat in /compat/vcbuild may need updating to do a 'pull' if there is an existing directory. At the moment it's a rather simplistic 'all or nothing' for getting all those extra packages.

Quoted 1 line · Philip Oakley

Hmm, 45 minutes of cloning and rebuild, but finally it compiled clean (both Release and Debug)

I understand that the issue is resolved now.

Probably your old repo was missing the libraries for whatever reason (like antivirus deleting them, etc), but build script thought that dependencies are properly built, so didn't attempt to rebuild them.