Similar to but 785738, c-c now fails on openbsd/clang due to prtime being different from int64_t, and probably others. /var/buildslave-mozilla/comm-central-amd64/build/mailnews/base/src/nsMsgPurgeService.cpp:199:17: error: no matching function for call to 'PR_ParseTimeString' PR_ParseTimeString(curFolderLastPurgeTimeString.get(), false, &theTime); ^~~~~~~~~~~~~~~~~~ ../../../mozilla/dist/include/prtime.h:244:20: note: candidate function not viable: no known conversion from 'int64_t *' (aka 'long long *') to 'PRTime *' (aka 'long *') for 3rd argument; NSPR_API(PRStatus) PR_ParseTimeString ( ^ /var/buildslave-mozilla/comm-central-amd64/build/mailnews/base/src/nsMsgPurgeService.cpp:290:11: error: no matching function for call to 'PR_ParseTimeString' PR_ParseTimeString(curJunkFolderLastPurgeTimeString.get(), false, &theTime); ^~~~~~~~~~~~~~~~~~ ../../../mozilla/dist/include/prtime.h:244:20: note: candidate function not viable: no known conversion from 'int64_t *' (aka 'long long *') to 'PRTime *' (aka 'long *') for 3rd argument; NSPR_API(PRStatus) PR_ParseTimeString ( ^ 2 errors generated. Working on a patch...

Dunno if i should ask a c-c peer .. this diff fixes the build for me on openbsd/clang

/home/landry/src/comm-central/mailnews/imap/src/nsAutoSyncManager.cpp:108:20: error: cannot initialize a parameter of type 'PRTime *' (aka 'long *') with an rvalue of type 'int64_t *' (aka 'long long *') /home/landry/src/comm-central/mailnews/local/src/nsMsgBrkMBoxStore.cpp:177:38: error: cannot initialize a parameter of type 'PRTime *' (aka 'long *') with an rvalue of type 'int64_t *' (aka 'long long *') /home/landry/src/comm-central/mailnews/local/src/nsMsgMaildirStore.cpp:613:11: error: call to member function 'AppendInt' is ambiguous newName.AppendInt(PR_Now()); Were the corresponding failure messages.

comm-aurora with gcc 4.2 is affected too, the same fix will probably do the trick : /usr/ports/pobj/comm-aurora/mailnews/base/src/nsMsgPurgeService.cpp: In member function 'nsresult nsMsgPurgeService::PerformPurge()': /usr/ports/pobj/comm-aurora/mailnews/base/src/nsMsgPurgeService.cpp:199: error: invalid conversion from 'int64_t*' to 'PRTime*' /usr/ports/pobj/comm-aurora/mailnews/base/src/nsMsgPurgeService.cpp:290: error: invalid conversion from 'int64_t*' to 'PRTime*'

[Approval Request Comment] Regression caused by (bug #): 579517 User impact if declined: c-a failure on OpenBSD/gcc 4.2.1 Testing completed (on c-c, etc.): in progress Risk to taking this patch (and alternatives if risky): none, wont affect tier1 platforms for which those types are the same.

Comment on attachment 658384 [details] [diff] [review] Fix build on OpenBSD after bug 579517 Review of attachment 658384 [details] [diff] [review]: ----------------------------------------------------------------- I'm technically not a peer of anything in c-c, but I know some of them. ;-)

Comment on attachment 658463 [details] [diff] [review] Fix build on OpenBSD after bug 579517 (c-a patch) Review of attachment 658463 [details] [diff] [review]: ----------------------------------------------------------------- (You don't need to ask for a separate review on branch patches if they're practically the same as trunk patches...)

it seems i've missed one, dunno how it wasnt catched.. or the PRInt64 was brought by new code ? /var/buildslave-mozilla/comm-central-amd64/build/mailnews/imap/src/nsImapMailFolder.cpp:8261:26: error: cannot initialize a parameter of type 'int64_t *' (aka 'long long *') with an rvalue of type 'PRInt64 *' (aka 'long *') seekable->Tell(&curOfflineStorePos); ^~~~~~~~~~~~~~~~~~~

Comment on attachment 658384 [details] [diff] [review] Fix build on OpenBSD after bug 579517 Review of attachment 658384 [details] [diff] [review]: ----------------------------------------------------------------- So I know this already landed, but I'm not sure why it is ONLY failing for you, when it works fine on all the official mozilla machines. I don't have the best comprehension of this C++ magic, so I want Neil [or another real peer of this code] to give it a proper look before *anyone* approves it on the branch ::: mailnews/local/src/nsMsgMaildirStore.cpp @@ +609,5 @@ > } > > // generate new file name > nsAutoCString newName; > + newName.AppendInt(static_cast<int64_t>(PR_Now())); In addition to the above this worries me, PR_Now returns PRTime, which is typedefed to PRInt64 which is *not* always an int64_t compat type, where it can be long long, among other things (see: http://mxr.mozilla.org/comm-central/source/mozilla/nsprpub/pr/include/prtypes.h#340 ) So this static_cast feels dangerous if its needed, since it circumvents the normal checks and bounds here

(In reply to Justin Wood (:Callek) from comment #9) > Comment on attachment 658384 [details] [diff] [review] > Fix build on OpenBSD after bug 579517 > > Review of attachment 658384 [details] [diff] [review]: > ----------------------------------------------------------------- > > So I know this already landed, but I'm not sure why it is ONLY failing for > you, when it works fine on all the official mozilla machines. Because on OpenBSD PRInt64 != int64_t, and on all official mozilla machines the types are equivalent and thus don't produce those failures. See bug 785738 and bug 634793 for the root cause of all my pain. > In addition to the above this worries me, PR_Now returns PRTime, which is > typedefed to PRInt64 which is *not* always an int64_t compat type, where it > can be long long, among other things (see: > http://mxr.mozilla.org/comm-central/source/mozilla/nsprpub/pr/include/ > prtypes.h#340 ) > > So this static_cast feels dangerous if its needed, since it circumvents the > normal checks and bounds here The same 'fixes' were applied on m-c.

That one fixes the aforementioned error. c-c builds again for me on openbsd. Pending discussion on the other issue....

Comment on attachment 659464 [details] [diff] [review] fix another print64/int64_t mixup I am not an expert so I still want Neil to look at this bug but this command anyway until he does

(In reply to Justin Wood (:Callek) from comment #12) > Comment on attachment 659464 [details] [diff] [review] > fix another print64/int64_t mixup > > I am not an expert so I still want Neil to look at this bug but this command > anyway until he does Bah - THIS is why I should never try to review from mobile. Let me repeat what I said, only clearer and correctly: I'm no expert with Cpp unfortunately, so I'd still like to keep the previous review request on Neil open to help check my sanity in my above comments. However given the other patch landed and this unblocks you, lets land this self-contained change anyway.

https://hg.mozilla.org/comm-central/rev/e3c08e7b8ebc i of course need to recheck aurora to see if this chunk needs to be added to the patch waiting approval...

Comment on attachment 658384 [details] [diff] [review] Fix build on OpenBSD after bug 579517 >- newName.AppendInt(PR_Now()); >+ newName.AppendInt(static_cast<int64_t>(PR_Now())); Callek: AppendInt takes an int32_t, uint32_t, int64_t or uint64_t. If PR_Now() doesn't return one of those, you'll get an ambiguous cast, so you have to cast it manually.

(In reply to Landry Breuil (:gaston) from comment #14) > https://hg.mozilla.org/comm-central/rev/e3c08e7b8ebc > > i of course need to recheck aurora to see if this chunk needs to be added to > the patch waiting approval... That pruint64 is not present in comm-aurora, so my approval-comm-aurora request stands for attachment 658463 [details] [diff] [review].

Callek, ping comm-aurora a? would be a pity to miss next uplift.

(In reply to Landry Breuil (:gaston) from comment #17) > Callek, ping comm-aurora a? would be a pity to miss next uplift. I can't approve mailnews/ patches -- standard8: ping

So is this bug fixed now? Also, exactly which patches need approval?

att 658463 needs approval, the bug is fixed in c-c but pending in c-a.

Ok, thanks, if its fixed in comm-central, then we normally mark the bug as fixed - branches are tracked by the status field.

Comment on attachment 659464 [details] [diff] [review] fix another print64/int64_t mixup After a recheck, it seems this chunk is needed too on comm-aurora. In the hope i can slip it in before the next uplift... [Approval Request Comment] Regression caused by (bug #): 579517 User impact if declined: c-a failure on OpenBSD/gcc 4.2.1 Testing completed (on c-c, etc.): done, fixes build Risk to taking this patch (and alternatives if risky): none, wont affect tier1 platforms for which those types are the same.

Comment on attachment 659464 [details] [diff] [review] fix another print64/int64_t mixup Review of attachment 659464 [details] [diff] [review]: ----------------------------------------------------------------- I can't officially set this [yet - filed Bug 798996 for that], but I'm giving it a+=me