Closed Bug 968772 Opened 8 years ago Closed 8 years ago

Don't use pragma for wintrust.lib linkage.

Categories

(Toolkit :: Downloads API, defect)

x86
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: jacek, Assigned: jacek)

Details

Attachments

(1 file)

Attached patch fixSplinter Review
This is not supported on mingw.
Attachment #8371425 - Flags: review?(mmc)
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
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 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
Attachment #8371425 - Flags: review?(mmc)
Attachment #8371425 - Flags: review?(gps)
Attachment #8371425 - Flags: review?(dmajor)
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 :)
Attachment #8371425 - Flags: review?(dmajor) → feedback+
Comment on attachment 8371425 [details] [diff] [review]
fix

HTTP 307 glandium
Reason: wonky library foo
Attachment #8371425 - Flags: review?(gps) → review?(mh+mozilla)
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.
Attachment #8371425 - Flags: review?(mh+mozilla) → review+
Thanks for reviews. I filled a followup bug 969321.

https://hg.mozilla.org/integration/mozilla-inbound/rev/11b6eab06c17
https://hg.mozilla.org/mozilla-central/rev/11b6eab06c17
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.