diff mbox series

vcbuild: fix library name for expat with make MSVC=1

Message ID pull.722.git.1599077798953.gitgitgadget@gmail.com (mailing list archive)
State Accepted
Commit c2f3ef8d8f16bccd08f17d33f6a6e10f0e79cdb3
Headers show
Series vcbuild: fix library name for expat with make MSVC=1 | expand

Commit Message

Jean-Noël Avila via GitGitGadget Sept. 2, 2020, 8:16 p.m. UTC
From: Orgad Shaneh <orgads@gmail.com>

Signed-off-by: Orgad Shaneh <orgads@gmail.com>
---
    vcbuild: fix library name for expat with make MSVC=1
    
    Signed-off-by: Orgad Shaneh orgads@gmail.com [orgads@gmail.com]

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-722%2Forgads%2Fvcbuild-expat-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-722/orgads/vcbuild-expat-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/722

 compat/vcbuild/scripts/clink.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: e19713638985533ce461db072b49112da5bd2042

Comments

Jonathan Nieder Sept. 3, 2020, 12:06 a.m. UTC | #1
(cc-ing a couple of Windows experts)
Orgad Shaneh wrote:

> Subject: vcbuild: fix library name for expat with make MSVC=1

Do you have more details?  For example, what error message does the
build produce without this change?

> Signed-off-by: Orgad Shaneh <orgads@gmail.com>
> ---
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-722%2Forgads%2Fvcbuild-expat-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-722/orgads/vcbuild-expat-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/722
>
>  compat/vcbuild/scripts/clink.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I'm ignorant enough about the platform-specific details involved that
I'd like an Ack from one of the Windows folks.

Thanks,
Jonathan

> diff --git a/compat/vcbuild/scripts/clink.pl b/compat/vcbuild/scripts/clink.pl
> index 61ad084a7b..df167d1e1a 100755
> --- a/compat/vcbuild/scripts/clink.pl
> +++ b/compat/vcbuild/scripts/clink.pl
> @@ -66,7 +66,7 @@
>  		}
>  		push(@args, $lib);
>  	} elsif ("$arg" eq "-lexpat") {
> -		push(@args, "expat.lib");
> +		push(@args, "libexpat.lib");
>  	} elsif ("$arg" =~ /^-L/ && "$arg" ne "-LTCG") {
>  		$arg =~ s/^-L/-LIBPATH:/;
>  		push(@lflags, $arg);
> 
> base-commit: e19713638985533ce461db072b49112da5bd2042
> -- 
> gitgitgadget
Johannes Schindelin Sept. 3, 2020, 1:42 a.m. UTC | #2
Hi Orgad,

On Wed, 2 Sep 2020, Orgad Shaneh via GitGitGadget wrote:

> From: Orgad Shaneh <orgads@gmail.com>
>
> Signed-off-by: Orgad Shaneh <orgads@gmail.com>
> ---
>     vcbuild: fix library name for expat with make MSVC=1
>
>     Signed-off-by: Orgad Shaneh orgads@gmail.com [orgads@gmail.com]

As can be seen at
https://dev.azure.com/git/git/_build/results?buildId=2065&view=artifacts&type=publishedArtifacts,
this change is correct.

ACK.

Thank you,
Dscho

>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-722%2Forgads%2Fvcbuild-expat-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-722/orgads/vcbuild-expat-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/722
>
>  compat/vcbuild/scripts/clink.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/compat/vcbuild/scripts/clink.pl b/compat/vcbuild/scripts/clink.pl
> index 61ad084a7b..df167d1e1a 100755
> --- a/compat/vcbuild/scripts/clink.pl
> +++ b/compat/vcbuild/scripts/clink.pl
> @@ -66,7 +66,7 @@
>  		}
>  		push(@args, $lib);
>  	} elsif ("$arg" eq "-lexpat") {
> -		push(@args, "expat.lib");
> +		push(@args, "libexpat.lib");
>  	} elsif ("$arg" =~ /^-L/ && "$arg" ne "-LTCG") {
>  		$arg =~ s/^-L/-LIBPATH:/;
>  		push(@lflags, $arg);
>
> base-commit: e19713638985533ce461db072b49112da5bd2042
> --
> gitgitgadget
>
Junio C Hamano Sept. 3, 2020, 2:07 a.m. UTC | #3
Jonathan Nieder <jrnieder@gmail.com> writes:

> (cc-ing a couple of Windows experts)
> Orgad Shaneh wrote:
>
>> Subject: vcbuild: fix library name for expat with make MSVC=1
>
> Do you have more details?  For example, what error message does the
> build produce without this change?

Presumably you'd get an error at link time, saying either 'no such
library: expat.lib', or 'symbol X not found' for symbols that were
supposed to come from libexpat.lib.

I do not think we want to see exact error message.  If an empty body
of the log message bothers us, I probably would say that it is
sufficient to write something like

    The name of the expat library is libexpat.lib, not expat.lib;
    otherwise we'd get linkage errors.

> I'm ignorant enough about the platform-specific details involved that
> I'd like an Ack from one of the Windows folks.

I saw Dscho looked at both pull requests and commented on them, but
haven't seen him (or anybody else) acking or nacking the version
that was submitted.  It would be nice to see an Ack from Windows'
side.

Thanks.
diff mbox series

Patch

diff --git a/compat/vcbuild/scripts/clink.pl b/compat/vcbuild/scripts/clink.pl
index 61ad084a7b..df167d1e1a 100755
--- a/compat/vcbuild/scripts/clink.pl
+++ b/compat/vcbuild/scripts/clink.pl
@@ -66,7 +66,7 @@ 
 		}
 		push(@args, $lib);
 	} elsif ("$arg" eq "-lexpat") {
-		push(@args, "expat.lib");
+		push(@args, "libexpat.lib");
 	} elsif ("$arg" =~ /^-L/ && "$arg" ne "-LTCG") {
 		$arg =~ s/^-L/-LIBPATH:/;
 		push(@lflags, $arg);