653865 - Trunk builds broken by bug 556214: undeclared 'MonitorAutoEnter'

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect)

Description

[Joshua Cranmer [:jcranmer]]

All trunk builds of Thunderbird have been broken by bug 556214, due to the decision to make the default monitor non-reentrant. This will probably require an ifdef MOZILLA_AURORA, if we don't already have that kind of thing set up.

I see failure in IMAP, but it probably exists in other places as well.

Comment 1

[David :Bienvenu]

I'll look into this.

Comment 2

[Serge Gautherie (:sgautherie)]

MOZILLA_5_0_BRANCH is unchanged.
SeaMonkey is affected too, of course.

{
.../mailnews/imap/src/nsImapProtocol.cpp(1164) : error C2065: 'MonitorAutoEnter' : undeclared identifier
[...]
}

http://mxr.mozilla.org/comm-central/search?string=MonitorAutoEnter&case=1&find=%2Fmailnews%2F
"Found 23 matching lines" in that 1 file.

Comment 3

[Philip Chee]

Attachment: Patch v1.0

This patch allows SeaMonkey comm-central to build at least.

Comment 4

[Philip Chee]

Hmm
-using mozilla::MutexAutoLock;
+using mozilla::MonitorAutoLock;
Do we need this?

Comment 5

[Joshua Cranmer [:jcranmer]]

(In reply to comment #3)
> Created attachment 529275 [details] [diff] [review]
> Patch v1.0
> 
> This patch allows SeaMonkey comm-central to build at least.

This would fail on mozilla-aurora, would it not?

I'm almost tempted to do an
#ifndef MOZILLA_5_0_BRANCH
#define Monitor ReentrantMonitor
#define MonitorAutoEnter ReentrantMonitorAutoEnter
// etc.
#endif

Comment 6

[David :Bienvenu]

(In reply to comment #5)
> (In reply to comment #3)
> > Created attachment 529275 [details] [diff] [review]
> > Patch v1.0
> > 
> > This patch allows SeaMonkey comm-central to build at least.
> 
> This would fail on mozilla-aurora, would it not?
> 
> I'm almost tempted to do an
> #ifndef MOZILLA_5_0_BRANCH
> #define Monitor ReentrantMonitor
> #define MonitorAutoEnter ReentrantMonitorAutoEnter
> // etc.
> #endif

right, I'm going to do something like that.

Comment 7

[David :Bienvenu]

Attachment: patch that builds with mozilla 5 0

haven't tried trunk, but I think this should build there as well

Comment 8

[Philip Chee]

> +#ifndef MOZILLA_5_0_BRANCH
> +  typedef mozilla::ReentrantMonitor ReentrantMonitor;
> +#else
> +  typedef mozilla::Monitor Monitor;
> +#endif
Why not:
#else
  typedef mozilla::Monitor ReentrantMonitor;
#endif

Then you could drop the earlier:
> +#define ReentrantMonitor Monitor

Comment 9

[David :Bienvenu]

Attachment: patch for checkin

this addresses Phillip's comment, and adds a comment about what we should do when we don't have to support 5.0 - it builds on trunk as well, so I think I'll just land it.

Comment 10

[David :Bienvenu]

Comment on attachment 529294 [details] [diff] [review]
patch for checkin

checked in

Comment 11

[David :Bienvenu]

fix checked in - http://hg.mozilla.org/comm-central/rev/ca4bc28d9024

Comment 12

[David :Bienvenu]

Comment on attachment 529275 [details] [diff] [review]
Patch v1.0

doesn't build on 5.0

Comment 13

[Serge Gautherie (:sgautherie)]

(TB) Miramar is still green,
SeaMonkey and ThunderbirdTrunk are green again :-)

V.Fixed