Skip to content

contrib/buildsystems: fix Visual Studio Debug configuration #348

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

SyntevoAlex
Copy link

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]

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]>
@gitgitgadget
Copy link

gitgitgadget bot commented Sep 19, 2019

Welcome to GitGitGadget

Hi @SyntevoAlex, and welcome to GitGitGadget, the GitHub App to send patch series to the Git mailing list from GitHub Pull Requests.

Please make sure that this Pull Request has a good description, as it will be used as cover letter.

Also, it is a good idea to review the commit messages one last time, as the Git project expects them in a quite specific form:

  • the lines should not exceed 76 columns,
  • the first line should be like a header and typically start with a prefix like "tests:" or "commit:", and
  • the commit messages' body should be describing the "why?" of the change.
  • Finally, the commit messages should end in a Signed-off-by: line matching the commits' author.

It is in general a good idea to await the automated test ("Checks") in this Pull Request before contributing the patches, e.g. to avoid trivial issues such as unportable code.

Contributing the patches

Before you can contribute the patches, your GitHub username needs to be added to the list of permitted users. Any already-permitted user can do that, by adding a PR comment of the form /allow <username>.

Once on the list of permitted usernames, you can contribute the patches to the Git mailing list by adding a PR comment /submit.

After you submit, GitGitGadget will respond with another comment that contains the link to the cover letter mail in the Git mailing list archive. Please make sure to monitor the discussion in that thread and to address comments and suggestions.

If you do not want to subscribe to the Git mailing list just to be able to respond to a mail, you can download the mbox ("raw") file corresponding to the mail you want to reply to from the Git mailing list. If you use GMail, you can upload that raw mbox file via:

curl -g --user "<EMailAddress>:<Password>" --url "imaps://imap.gmail.com/INBOX" -T /path/to/raw.txt

@SyntevoAlex
Copy link
Author

@dscho I assume that you might want to review that?

@dscho
Copy link
Member

dscho commented Sep 22, 2019

/allow

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 22, 2019

User SyntevoAlex is now allowed to use GitGitGadget.

@SyntevoAlex
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 23, 2019

Error: Could not determine full name of SyntevoAlex

@SyntevoAlex
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 23, 2019

Submitted as [email protected]

@@ -79,7 +79,8 @@ sub createProject {
if (!$static_library) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Alexandr Miloslavskiy wrote (reply to this):

Johannes, would you please review?

@@ -79,7 +79,8 @@ sub createProject {
if (!$static_library) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Johannes Schindelin wrote (reply to this):

Hi Alexandr,

On Mon, 23 Sep 2019, Alexandr Miloslavskiy via GitGitGadget wrote:

> 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]>

ACK!
Johannes

> ---
>  contrib/buildsystems/Generators/Vcxproj.pm | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/contrib/buildsystems/Generators/Vcxproj.pm b/contrib/builds=
ystems/Generators/Vcxproj.pm
> index 576ccabe1d..7b1e277eca 100644
> --- a/contrib/buildsystems/Generators/Vcxproj.pm
> +++ b/contrib/buildsystems/Generators/Vcxproj.pm
> @@ -79,7 +79,8 @@ sub createProject {
>      if (!$static_library) {
>        $libs_release =3D join(";", sort(grep /^(?!libgit\.lib|xdiff\/lib=
\.lib|vcs-svn\/lib\.lib)/, @{$$build_structure{"$prefix${name}_LIBS"}}));
>        $libs_debug =3D $libs_release;
> -      $libs_debug =3D~ s/zlib\.lib/zlibd\.lib/;
> +      $libs_debug =3D~ s/zlib\.lib/zlibd\.lib/g;
> +      $libs_debug =3D~ s/libcurl\.lib/libcurl-d\.lib/g;
>      }
>
>      $defines =3D~ s/-D//g;
> @@ -119,13 +120,13 @@ sub createProject {
>      <VCPKGArch Condition=3D"'\$(Platform)'=3D=3D'Win32'">x86-windows</V=
CPKGArch>
>      <VCPKGArch Condition=3D"'\$(Platform)'!=3D'Win32'">x64-windows</VCP=
KGArch>
>      <VCPKGArchDirectory>$cdup\\compat\\vcbuild\\vcpkg\\installed\\\$(VC=
PKGArch)</VCPKGArchDirectory>
> -    <VCPKGBinDirectory Condition=3D"'\(Configuration)'=3D=3D'Debug'">\$=
(VCPKGArchDirectory)\\debug\\bin</VCPKGBinDirectory>
> -    <VCPKGLibDirectory Condition=3D"'\(Configuration)'=3D=3D'Debug'">\$=
(VCPKGArchDirectory)\\debug\\lib</VCPKGLibDirectory>
> -    <VCPKGBinDirectory Condition=3D"'\(Configuration)'!=3D'Debug'">\$(V=
CPKGArchDirectory)\\bin</VCPKGBinDirectory>
> -    <VCPKGLibDirectory Condition=3D"'\(Configuration)'!=3D'Debug'">\$(V=
CPKGArchDirectory)\\lib</VCPKGLibDirectory>
> +    <VCPKGBinDirectory Condition=3D"'\$(Configuration)'=3D=3D'Debug'">\=
$(VCPKGArchDirectory)\\debug\\bin</VCPKGBinDirectory>
> +    <VCPKGLibDirectory Condition=3D"'\$(Configuration)'=3D=3D'Debug'">\=
$(VCPKGArchDirectory)\\debug\\lib</VCPKGLibDirectory>
> +    <VCPKGBinDirectory Condition=3D"'\$(Configuration)'!=3D'Debug'">\$(=
VCPKGArchDirectory)\\bin</VCPKGBinDirectory>
> +    <VCPKGLibDirectory Condition=3D"'\$(Configuration)'!=3D'Debug'">\$(=
VCPKGArchDirectory)\\lib</VCPKGLibDirectory>
>      <VCPKGIncludeDirectory>\$(VCPKGArchDirectory)\\include</VCPKGInclud=
eDirectory>
> -    <VCPKGLibs Condition=3D"'\(Configuration)'=3D=3D'Debug'">$libs_debu=
g</VCPKGLibs>
> -    <VCPKGLibs Condition=3D"'\(Configuration)'!=3D'Debug'">$libs_releas=
e</VCPKGLibs>
> +    <VCPKGLibs Condition=3D"'\$(Configuration)'=3D=3D'Debug'">$libs_deb=
ug</VCPKGLibs>
> +    <VCPKGLibs Condition=3D"'\$(Configuration)'!=3D'Debug'">$libs_relea=
se</VCPKGLibs>
>    </PropertyGroup>
>    <Import Project=3D"\$(VCTargetsPath)\\Microsoft.Cpp.Default.props" />
>    <PropertyGroup Condition=3D"'\$(Configuration)'=3D=3D'Debug'" Label=
=3D"Configuration">
> --
> gitgitgadget
>

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 30, 2019

This branch is now known as am/visual-studio-config-fix.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 30, 2019

This patch series was integrated into pu via git@aa54614.

@gitgitgadget gitgitgadget bot added the pu label Sep 30, 2019
@SyntevoAlex
Copy link
Author

Oh, you even detect when the git mailing list patch was accepted?
@dscho this tool is great, thanks for creating it!

@dscho
Copy link
Member

dscho commented Oct 1, 2019

@SyntevoAlex glad you like it!

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 2, 2019

This patch series was integrated into pu via git@e7e35d6.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 3, 2019

This patch series was integrated into pu via git@2b0a651.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 4, 2019

This patch series was integrated into pu via git@b745da6.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 4, 2019

This patch series was integrated into next via git@135d931.

@gitgitgadget gitgitgadget bot added the next label Oct 4, 2019
@gitgitgadget
Copy link

gitgitgadget bot commented Oct 6, 2019

This patch series was integrated into pu via git@7c9e852.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 7, 2019

This patch series was integrated into pu via git@25e3df1.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 8, 2019

This patch series was integrated into pu via git@27abb18.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 9, 2019

This patch series was integrated into pu via git@0277fe5.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 9, 2019

This patch series was integrated into pu via git@042a54d.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 9, 2019

This patch series was integrated into next via git@042a54d.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 9, 2019

This patch series was integrated into master via git@042a54d.

@gitgitgadget gitgitgadget bot added the master label Oct 9, 2019
@gitgitgadget
Copy link

gitgitgadget bot commented Oct 9, 2019

Closed via 042a54d.

@gitgitgadget gitgitgadget bot closed this Oct 9, 2019
@@ -79,7 +79,8 @@ sub createProject {
if (!$static_library) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Philip Oakley wrote (reply to this):

On 23/09/2019 09:28, Alexandr Miloslavskiy via GitGitGadget wrote:
> 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.
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`.

Philip

Why compiling:
[1a] https://www.sourcetrail.com is a cross-platform source explorer 
that helps you get productive on unfamiliar source code.
[1b] Sourcetrail is now free and open-source 
https://www.sourcetrail.com/blog/open_source/
>
> Signed-off-by: Alexandr Miloslavskiy <[email protected]>
> ---
>   contrib/buildsystems/Generators/Vcxproj.pm | 15 ++++++++-------
>   1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/contrib/buildsystems/Generators/Vcxproj.pm b/contrib/buildsystems/Generators/Vcxproj.pm
> index 576ccabe1d..7b1e277eca 100644
> --- a/contrib/buildsystems/Generators/Vcxproj.pm
> +++ b/contrib/buildsystems/Generators/Vcxproj.pm
> @@ -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">

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Alexandr Miloslavskiy wrote (reply to this):

On 27.11.2019 18:56, Philip Oakley wrote:
> 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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Philip Oakley wrote (reply to this):

Hi Alexandr,

On 27/11/2019 18:59, Alexandr Miloslavskiy wrote:
> On 27.11.2019 18:56, Philip Oakley wrote:
>> 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.

I already have the clone of vcpkg from 30/03/2019 with those three files 
already present. Have they been updated since then?

The error report is specifically that the .lib file is missing (which I 
can't find..)

However the plain `libcurl.lib` is present with matching dll's. It just 
feels a little unusual.

On a side note there's a few (not many) Stackoverflow questions about 
building libcurl-d.lib, but they feel almost tangential
https://stackoverflow.com/questions/51418238/libcurl-a-debug-lib-libcurl-a-lib-are-generated-instead-of-libcurld-lib-libcur
https://stackoverflow.com/questions/37126943/libcurld-dll-is-missing-from-your-computer-adding-libcurl-to-visual-studio-pr

quick look..
not sure if the assertion about choosing "ONE" (of debug/release) in 
https://github.com/git-for-windows/git/blob/master/compat/vcbuild/README#L26-L30 
is part of the issue.
That README doesn't really cover the update method for the vcpkg 
repository. It presumes you start from a fresh clone, which can be slow.
I fetched/pulled the vcpkg repoo but no sign of an update.

Philip



Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Philip Oakley wrote (reply to this):



On 27/11/2019 18:59, Alexandr Miloslavskiy wrote:
> On 27.11.2019 18:56, Philip Oakley wrote:
>> 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)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Alexandr Miloslavskiy wrote (reply to this):

On 28.11.2019 1:34, Philip Oakley wrote:

> 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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Philip Oakley wrote (reply to this):

On 28/11/2019 10:07, Alexandr Miloslavskiy wrote:
> On 28.11.2019 1:34, Philip Oakley wrote:
>
>> 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.

Philip

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Philip Oakley wrote (reply to this):

On 29/11/2019 11:53, Philip Oakley wrote:
> On 28/11/2019 10:07, Alexandr Miloslavskiy wrote:
>> On 28.11.2019 1:34, Philip Oakley wrote:
>>
>>> 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.

Issue submitted as https://github.com/microsoft/vcpkg/issues/9148
--
Philip

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Philip Oakley wrote (reply to this):

On 28/11/2019 10:07, Alexandr Miloslavskiy wrote:
> On 28.11.2019 1:34, Philip Oakley wrote:
>
>> 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 was searching for how Visual Studio managed to decide if the vcpkg 
needed installing. I think I've found it in 384a61bc6a 
("contrib/buildsystems: add a backend for modern Visual Studio 
versions", 2019-07-29) where dscho says:

    we initialize the `vcpkg` conditionally, in the `libgit` project's
    `PreBuildEvent`. To allow for parallel building of the projects, we
    therefore put `libgit` at the bottom of the project hierarchy.


What's not clear is if the conditional pre-build can have an if-else 
option so that we can have an 'update' check if already installed. 
Adding the vcpkg update to the vcpkg_install.bat didn't work :-(

Philip

PS. the https://github.com/CoatiSoftware/Sourcetrail visualiser is 
looking nice.


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants