1199118 - The fix for bug 1177861 breaks mingw-w64 builds

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Georg Koppen]

When trying to build a new release for Tor Browser based on ESR 38.2.1 we accidentally built from the wrong commit. The only positive side-effect is that we found a bug in the one commit excluded in BUILD2. It fails to compile with mingw-w64:

../common-standalone/certificatecheck.o: In function `Z28DoCertificateAttributesMatchPK13_CERT_CONTEXTR20CertificateCheckInfo':
/home/ubuntu/build/tor-browser/toolkit/mozapps/update/common/certificatecheck.cpp:133: undefined reference to `_imp__CertGetNameStringA@24'
/home/ubuntu/build/tor-browser/toolkit/mozapps/update/common/certificatecheck.cpp:149: undefined reference to `_imp__CertGetNameStringW@24'
/home/ubuntu/build/tor-browser/toolkit/mozapps/update/common/certificatecheck.cpp:170: undefined reference to `_imp__CertGetNameStringA@24'
/home/ubuntu/build/tor-browser/toolkit/mozapps/update/common/certificatecheck.cpp:185: undefined reference to `_imp__CertGetNameStringW@24'
../common-standalone/certificatecheck.o: In function `Z25CheckCertificateForPEFilePKwR20CertificateCheckInfo':
/home/ubuntu/build/tor-browser/toolkit/mozapps/update/common/certificatecheck.cpp:44: undefined reference to `_imp__CryptQueryObject@44'
/home/ubuntu/build/tor-browser/toolkit/mozapps/update/common/certificatecheck.cpp:54: undefined reference to `_imp__CryptMsgGetParam@20'
/home/ubuntu/build/tor-browser/toolkit/mozapps/update/common/certificatecheck.cpp:72: undefined reference to `_imp__CryptMsgGetParam@20'
/home/ubuntu/build/tor-browser/toolkit/mozapps/update/common/certificatecheck.cpp:85: undefined reference to `_imp__CertFindCertificateInStore@24'
/home/ubuntu/build/tor-browser/toolkit/mozapps/update/common/certificatecheck.cpp:103: undefined reference to `_imp__CertFreeCertificateContext@4'
/home/ubuntu/build/tor-browser/toolkit/mozapps/update/common/certificatecheck.cpp:106: undefined reference to `_imp__CertCloseStore@8'
/home/ubuntu/build/tor-browser/toolkit/mozapps/update/common/certificatecheck.cpp:109: undefined reference to `_imp__CryptMsgClose@4'
../common-standalone/certificatecheck.o: In function `Z29VerifyCertificateTrustForFilePKw':
/home/ubuntu/build/tor-browser/toolkit/mozapps/update/common/certificatecheck.cpp:240: undefined reference to `WinVerifyTrust@12'
collect2: error: ld returned 1 exit status

glandium pointed to #pragma comment(lib, ...) instances on IRC:

00:45 < glandium> bingo
00:45 < glandium> /home/ubuntu/build/tor-browser/toolkit/mozapps/update/common/certificatecheck.cpp:14:0: warning: ignoring #pragma comment  
                  [-Wunknown-pragmas] #pragma comment(lib, "wintrust.lib") ^
00:45 < glandium> /home/ubuntu/build/tor-browser/toolkit/mozapps/update/common/certificatecheck.cpp:15:0: warning: ignoring #pragma comment  
                  [-Wunknown-pragmas] #pragma comment(lib, "crypt32.lib") ^
00:46 < glandium> there are plenty others, btw, you should take a look at all 
                  of them

Comment 1

[Georg Koppen]

(I thought about blocking bug 1177861 but I don't have the proper clearance. :)

Comment 2

[Jacek Caban]

Attachment: fix

The attached patch fixes the problem for me. Here is try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ac1c739e9bb4

Comment 3

[Mike Hommey [:glandium]]

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

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

Note, as I mentioned on irc, there are many others of those pragmas, it might be worth going through all of them and file bugs when relevant.

Comment 4

[Jacek Caban]

Agreed, I actually meant to do that, that's why this patch was sitting in my tree (and waiting until I have the time to do that) until this bug was filled. We have bug 969321 for that.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/34bbc3cb3e79

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/34bbc3cb3e79