968772 - Don't use pragma for wintrust.lib linkage.

Status: RESOLVED FIXED

(Toolkit :: Downloads API, defect)

Description

[Jacek Caban]

Attachment: fix

This is not supported on mingw.

Comment 1

[[:mmc] Monica Chew (no longer reading bugmail)]

Hi Jacek,

Thanks for the patch. Could you please describe how you are building, and paste the breakage? Also, are you able to build the update installer and other binaries that use this? Finally, all non-trivial build file changes require build peer review, cc'ing gps.

http://mxr.mozilla.org/mozilla-central/search?string=wintrust.lib&find=&findi=&filter=%5E%5B%5E%5C0%5D*%24&hitlimit=&tree=mozilla-central

Thank you,
Monica

Comment 2

[Jacek Caban]

Hi Monica,

(In reply to Monica Chew [:mmc] (please use needinfo) from comment #1)
> Hi Jacek,
> 
> Thanks for the patch. Could you please describe how you are building, and
> paste the breakage?

My instructions may be found here:
https://developer.mozilla.org/en-US/docs/Cross_Compile_Mozilla_for_Mingw32
I didn't have enough time lately to upstream all the required fixes for recent breackages, so current m-c won't build for a few reasons (in case you'd like to try).

The build error is here:

../../netwerk/base/src/Unified_cpp_netwerk_base_src0.o: In function `ZN7mozilla3net19BackgroundFileSaver20ExtractSignatureInfoERK18nsAString_internal':
/home/jacek/mozilla/mozilla-central/netwerk/base/src/BackgroundFileSaver.cpp:866: undefined reference to `WinVerifyTrust@12'
/home/jacek/mozilla/mozilla-central/netwerk/base/src/BackgroundFileSaver.cpp:871: undefined reference to `WTHelperProvDataFromStateData@4'
/home/jacek/mozilla/mozilla-central/netwerk/base/src/BackgroundFileSaver.cpp:927: undefined reference to `WinVerifyTrust@12'
 

> Also, are you able to build the update installer and
> other binaries that use this?

No, those are currently unsupported in my builds, because make makensisu (unlike plain makensis) is not available for Linux, so it can't be used for cross compiling.

> Finally, all non-trivial build file changes require build peer review, cc'ing gps.

I'm sure those may be considered as trivial;)

Comment 3

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 8371425 [details] [diff] [review]
fix

Review of attachment 8371425 [details] [diff] [review]:
-----------------------------------------------------------------

Hi Jacek,

Thanks for the information. Sorry to hear about the m-c breakage. I still feel unqualified for review, since that patch was my first Windows patch and I don't know very much about the build system. I pushed your patch to try here:

https://tbpl.mozilla.org/?tree=Try&rev=5df286d6545c

It looks like dmajor may also be a choice, since he was the last person to touch OS_LIBS in that Makefile to include. Redirecting to both him and gps, whoever gets to it first.

Thanks very much,
Monica

Comment 4

[(Away)]

Comment on attachment 8371425 [details] [diff] [review]
fix

Review of attachment 8371425 [details] [diff] [review]:
-----------------------------------------------------------------

Seems fine to me... but I don't dare be the only reviewer on build changes :)

Comment 5

[Gregory Szorc [:gps]]

Comment on attachment 8371425 [details] [diff] [review]
fix

HTTP 307 glandium
Reason: wonky library foo

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 8371425 [details] [diff] [review]
fix

Review of attachment 8371425 [details] [diff] [review]:
-----------------------------------------------------------------

We have many more #pragma comment in the tree, you should do them all in one pass.

Comment 7

[Jacek Caban]

Thanks for reviews. I filled a followup bug 969321.

https://hg.mozilla.org/integration/mozilla-inbound/rev/11b6eab06c17

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/11b6eab06c17