Make mailnews compile with frozen linkage on debug Windows

RESOLVED FIXED in Thunderbird 11.0

Status

MailNews Core
Build Config
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: neil@parkwaycc.co.uk)

Tracking

(Blocks: 1 bug)

Trunk
Thunderbird 11.0
All
Windows XP
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

6 years ago
+++ This bug was initially created as a clone of Bug #705553 +++

Just a few changes to help with the compilation process with frozen linkage.
(Assignee)

Comment 1

6 years ago
Created attachment 577171 [details] [diff] [review]
Proposed patch

I wasn't actually able to get the Thunderbird version of nsMessengerWinIntegration to compile --with-libxul-sdk so these changes are only sufficient to get SeaMonkey to compile.

I didn't think it was worthwhile implementing nsPrintfCString. (I thought we'd done it somewhere else but I couldn't find it.)

The Makefile.in changes are because debug builds don't like to inline stuff and sometimes want to link to things that aren't even called. Sigh.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #577171 - Flags: review?(dbienvenu)
Comment on attachment 577171 [details] [diff] [review]
Proposed patch

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

::: mailnews/base/util/nsStopwatch.cpp
@@ +162,3 @@
>    if (!ret)
>      NS_ERROR(nsPrintfCString("GetProcessTimes() failed, error=0x%lx.", GetLastError()).get());
>  #endif

Nit: Could that become
{{
  if (!ret) {
#ifdef MOZILLA_INTERNAL_API
    ... with GetLastError()
#else
    // A comment about nsPrintfCString() not being available.
    ... without GetLastError()
#end
  }
}}
fwiw ?
Or could the full error string be created in a different manner?
(Assignee)

Comment 3

6 years ago
(In reply to Serge Gautherie from comment #2)
> (From update of attachment 577171 [details] [diff] [review])
>     // A comment about nsPrintfCString() not being available.
Well, I could just NS_ERROR("GetProcessTimes() failed"); I suppose...

> Or could the full error string be created in a different manner?
Not really, because then you have to start fiddling around with #ifdef DEBUG.

Comment 4

6 years ago
Comment on attachment 577171 [details] [diff] [review]
Proposed patch

debug builds and runs...
Attachment #577171 - Flags: review?(dbienvenu) → review+
(Assignee)

Comment 5

6 years ago
Pushed changeset 9b9931394d81 to comm-central.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Created attachment 578905 [details] [diff] [review]
(Bv1) GetCPUTime(): Report error when not MOZILLA_INTERNAL_API too
[Checked in: See comment 9]

Per comment 2 and 3.
This should additionally fix a new "unused var ret" warning ;->
Attachment #578905 - Flags: review?(dbienvenu)
Flags: in-testsuite-
Target Milestone: --- → Thunderbird 11.0
David, ping for review.
Comment on attachment 578905 [details] [diff] [review]
(Bv1) GetCPUTime(): Report error when not MOZILLA_INTERNAL_API too
[Checked in: See comment 9]

> #ifdef DEBUG
>+  if (!ret)
> #ifdef MOZILLA_INTERNAL_API
>-  if (!ret)
>     NS_ERROR(nsPrintfCString("GetProcessTimes() failed, error=0x%lx.", GetLastError()).get());
>+#else
>+    // nsPrintfCString() is unavailable to report GetLastError().
>+    NS_ERROR("GetProcessTimes() failed.");
> #endif
> #endif

nit: I'd prefer for the if (!ret) to be there twice, as then it doesn't mess up indentation for editors that don't understand ifdefs.

r=me with that fixed.
Attachment #578905 - Flags: review?(dbienvenu) → review+
Comment on attachment 578905 [details] [diff] [review]
(Bv1) GetCPUTime(): Report error when not MOZILLA_INTERNAL_API too
[Checked in: See comment 9]

http://hg.mozilla.org/comm-central/rev/bc7c26552fe4
Bv1, with comment 8 suggestion(s).


"approval-comm-aurora=?":
AURORA_BASE_20111220 happened in the meantime :-/ No risk.
Attachment #578905 - Attachment description: (Bv1) GetCPUTime(): Report error when not MOZILLA_INTERNAL_API too. → (Bv1) GetCPUTime(): Report error when not MOZILLA_INTERNAL_API too [Checked in: See comment 9]
Attachment #578905 - Flags: approval-comm-aurora?
Comment on attachment 578905 [details] [diff] [review]
(Bv1) GetCPUTime(): Report error when not MOZILLA_INTERNAL_API too
[Checked in: See comment 9]

Default configurations will have this as internal API, and we don't even completely support external API on what is aurora now, plus it is debug only. Sorry, but this just isn't necessary on branch.
Attachment #578905 - Flags: approval-comm-aurora? → approval-comm-aurora-
You need to log in before you can comment on or make changes to this bug.