/Zc:wchar_t- is no longer required

RESOLVED FIXED in mozilla1.9.2b1

Status

()

Core
Build Config
RESOLVED FIXED
8 years ago
6 years ago

People

(Reporter: emk, Assigned: Jacek Caban)

Tracking

({dev-doc-needed})

Trunk
mozilla1.9.2b1
x86
Windows 7
dev-doc-needed
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-bs)

Attachments

(6 attachments, 12 obsolete attachments)

1.70 KB, patch
emk
: review+
Details | Diff | Splinter Review
1.61 KB, patch
Details | Diff | Splinter Review
10.13 KB, patch
dougt
: review+
Details | Diff | Splinter Review
1.20 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
1.29 KB, patch
bsmedberg
: review+
Details | Diff | Splinter Review
2.92 KB, patch
ted
: review+
Details | Diff | Splinter Review
(Reporter)

Description

8 years ago
Per bug 324842, it was added due to "This conflicts with various SDK headers that still want to define WCHAR". Is it still true with Vista SDK or later? I don't see any SDK errors without /Zc:wchar_t-.
I'm tired to see gazillions of MinGW build errors related to wchar_t.
(Reporter)

Comment 1

8 years ago
Created attachment 393180 [details] [diff] [review]
(Av1) patch in updater
[Moved to bug 507338]
(Reporter)

Comment 2

8 years ago
I could build fine even with 2003SP1 SDK (and all patches of dependent bugs).
So we have no reason to insist on using /Zc:wchar_t- anymore.
Summary: Is /Zc:wchar_t- still required? → /Zc:wchar_t- is no longer required
(Reporter)

Comment 3

8 years ago
Comment on attachment 393180 [details] [diff] [review]
(Av1) patch in updater
[Moved to bug 507338]

This will fix inconsistency with archivereader.h and readstrings.h
Attachment #393180 - Flags: review?(robert.bugzilla)
Attachment #393180 - Flags: review?(robert.bugzilla) → review+
Comment on attachment 393180 [details] [diff] [review]
(Av1) patch in updater
[Moved to bug 507338]

Looks good and thanks
(Reporter)

Updated

8 years ago
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/cabad98babdb
Assignee: nobody → VYV03354
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2b1
(Assignee)

Comment 6

8 years ago
It's not fixed yet. The committed patch fixed only a bug that this bug depends on.
(Reporter)

Comment 7

8 years ago
Yes, I should have say that a follow-up patch will come. Reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 393180 [details] [diff] [review]
(Av1) patch in updater
[Moved to bug 507338]

Backed out... This doesn't work on WinCE.

http://tinderbox.mozilla.org/showlog.cgi?log=Mobile/1249892930.1249893343.6278.gz
Attachment #393180 - Attachment description: patch in updater → patch in updater (backed out)
(Assignee)

Comment 9

8 years ago
It looks like windows.h (or winnt.h) needs to be included before progressui.h in progressui_null.cpp (or in progressui.h). I don't have setup to test it.
I'm working on getting the progress ui working on WinCE / WinMo and could take care of this over in bug 507338. Sorry I didn't catch this error... turns out my work on bug 507338 makes it so this patch compiles fine at least with Firefox WinCE.
(Reporter)

Updated

8 years ago
Depends on: 507338
bug 507338 landed on trunk and should land on the 1.9.2 branch in a couple of days
(Reporter)

Comment 12

8 years ago
Created attachment 394524 [details] [diff] [review]
(Bv1) m-c patch

Although this patch works with mozilla-central alone, it will break c-c + m-c build.
We will need a temporary configure option which controls /Zc:wchar_t- flag.
Attachment #393180 - Attachment is obsolete: true
(Reporter)

Comment 13

8 years ago
Created attachment 394664 [details] [diff] [review]
(Cv1-CC) comm-central patch

This is a temporary patch to avoid c-c + m-c build being broken. The else clause will be eventually removed.
Attachment #394664 - Flags: review?(kairo)

Updated

8 years ago
Attachment #394664 - Flags: review?(kairo) → review+

Comment 14

8 years ago
Comment on attachment 394664 [details] [diff] [review]
(Cv1-CC) comm-central patch

Oh, I see, you force this for now, then remove it from m-c then remove the else clause and make bot consistent again.

r=me on the condition that you will post a followup patch when the main one has landed on m-c to remove the else clause. Nit: please add a "(see bug 508905)" or so to the comment lines so that it's clear what this does.
(Reporter)

Comment 15

8 years ago
Created attachment 394682 [details] [diff] [review]
(Cv1a-CC) added bug number per review comment

Thank you!
Attachment #394664 - Attachment is obsolete: true
Attachment #394682 - Flags: review+
(Reporter)

Comment 16

8 years ago
Created attachment 394683 [details] [diff] [review]
(Dv1-CC) followup-patch after m-c landing
(Reporter)

Comment 17

8 years ago
Comment on attachment 394524 [details] [diff] [review]
(Bv1) m-c patch

This patch needs to be landed after attachment 394682 [details] [diff] [review] and before 394683.
Attachment #394524 - Attachment description: WIP → m-c patch
Attachment #394524 - Flags: review?(ted.mielczarek)
(Reporter)

Updated

8 years ago
Keywords: checkin-needed
Whiteboard: [attachment 394682] [don't close after landing]
(Reporter)

Updated

8 years ago
Status: REOPENED → ASSIGNED
(Reporter)

Updated

8 years ago
Depends on: 493280
(Reporter)

Comment 18

8 years ago
Comment on attachment 394524 [details] [diff] [review]
(Bv1) m-c patch

pending review until a build error caused by 493280 is fixed.
Attachment #394524 - Flags: review?(ted.mielczarek)
Attachment #393180 - Attachment description: patch in updater (backed out) → patch in updater [Moved to bug 507338]
Whiteboard: [attachment 394682] [don't close after landing] → [attachment 394682 to c-c] [don't close after landing]
(Reporter)

Comment 19

8 years ago
Comment on attachment 394524 [details] [diff] [review]
(Bv1) m-c patch

The fix was pushed. Requesting review again.
Attachment #394524 - Flags: review?(ted.mielczarek)
Attachment #394524 - Flags: review?(ted.mielczarek) → review+
(Reporter)

Updated

8 years ago
Whiteboard: [attachment 394682 to c-c] [don't close after landing] → [First land attachment 394682 to c-c, then land 394524 to m-c] [don't close after landing]
Comment on attachment 394682 [details] [diff] [review]
(Cv1a-CC) added bug number per review comment

Could you remove the MOZILLA_1_9_1_BRANCH checks now that c-c has branched? Thanks.
(Reporter)

Comment 21

8 years ago
Unfortunately, trunk will be broken again if the patch is applied :(
Keywords: checkin-needed
Whiteboard: [First land attachment 394682 to c-c, then land 394524 to m-c] [don't close after landing]
(Reporter)

Comment 22

8 years ago
Created attachment 408276 [details] [diff] [review]
(Cv2-CC) removed MOZILLA_1_9_1_BRANCH checks

Someone seems to have fixed offending codes.
Carrying forward r+.
Attachment #394682 - Attachment is obsolete: true
Attachment #408276 - Flags: review+
(Reporter)

Comment 23

8 years ago
Created attachment 408277 [details] [diff] [review]
(Dv2-CC) followup-patch after m-c landing
Attachment #394683 - Attachment is obsolete: true
(Reporter)

Updated

8 years ago
Attachment #408277 - Attachment is patch: true
Attachment #408277 - Attachment mime type: application/octet-stream → text/plain
(Reporter)

Updated

8 years ago
Keywords: checkin-needed
Whiteboard: [attachment 408276] [don't close after landing]
Could you summarize which patch will land where/when/why? Thanks.
(Reporter)

Comment 25

8 years ago
(In reply to comment #24)
1. Land attachment #408276 [details] [diff] [review] on comm-central first. This patch will force /Zc:wchar_t- to subdir.
2. If tinderbox doesn't burn, land attachment #394524 [details] [diff] [review] on mozilla-central. This patch removes /Zc:wchar_t- from m-c. c-c will be built with /Zc:wchar_t- at this point.
3. If tinderbox doesn't burn, land attachment #408277 [details] [diff] [review] on comm-central. /Zc:wchar_t- will be removed from c-c also.
The order is important to avoid c-c build being broken.
Whiteboard: [attachment 408276] [don't close after landing] → [see comment #25]
(In reply to comment #25)

> (In reply to comment #24)
> 1. Land attachment #408276 [details] [diff] [review] on comm-central first. This patch will force
> /Zc:wchar_t- to subdir.

If you're unsure that this patch will pass, then it should probably be submitted to c-c Try server first, should it not?

> 2. If tinderbox doesn't burn, land attachment #394524 [details] [diff] [review] on mozilla-central. This
> patch removes /Zc:wchar_t- from m-c. c-c will be built with /Zc:wchar_t- at
> this point.

Was that tried at least locally, for m-c alone then c-c + m-c?

> 3. If tinderbox doesn't burn, land attachment #408277 [details] [diff] [review] on comm-central.
> /Zc:wchar_t- will be removed from c-c also.
> The order is important to avoid c-c build being broken.

That said,
isn't it possible to do all this the other way round: just removing existing code(s)?
(I guess not, but why?)
(Reporter)

Comment 27

8 years ago
(In reply to comment #26)
> If you're unsure that this patch will pass,
I believe the patch will pass. I just worried about unexpected contingency.

> then it should probably be
> submitted to c-c Try server first, should it not?
How to get a try server access? (Sorry for the dumb question.)

> Was that tried at least locally, for m-c alone then c-c + m-c?
Yes. I've tested all of combinations locally.

> That said,
> isn't it possible to do all this the other way round: just removing existing
> code(s)?
I'm concerned about timing issue. Is it possible to push the patches to c-c and m-c at the same time?
(In reply to comment #27)
> (In reply to comment #26)

> > submitted to c-c Try server first, should it not?
> How to get a try server access? (Sorry for the dumb question.)

https://wiki.mozilla.org/Thunderbird/Infrastructure/TryServer#Gaining_Access

> > isn't it possible to do all this the other way round: just removing existing
> > code(s)?
> I'm concerned about timing issue. Is it possible to push the patches to c-c and
> m-c at the same time?

Yes, each patch could be pushed to its respective repo one (right) after the other.

Unless there is some kind of need for clobber or depend builds that would brake and not recover otherwise, the two target patches should just be pushed directly, at the "same" time if you say so ;-)
(A transient red/orange (on c-c) would be no problem.)
attachment 394524 [details] [diff] [review] breaks WinCE and WinMo on the tryserver:

http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1260732762.1260733959.9775.gz&fulltext=1
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1260732762.1260733773.7638.gz&fulltext=1
Keywords: checkin-needed
(Assignee)

Comment 30

8 years ago
Created attachment 417377 [details] [diff] [review]
(Ev1) fix for wince

The attached patch should fix the problem, but I don't have compile environment to test it. (Also there are some duplications between environment.h and mozce_shunt.h and GetEnvironmentVariableW returns char instead of DWORD, but these are not related problems).
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1260737382.1260738263.26275.gz&fulltext=1
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1260737382.1260738263.26276.gz&fulltext=1

Isn't poking at things blindly fun? :)
(Assignee)

Comment 32

8 years ago
Yeah :) I guess I might try setting up an environment if there will be more problems. I will attach another try that should fix this issue using wchar_t instead of WCHAR in header files as I've found that mozce_shunt.h is included by command line options, so WCHAR is not yet defined then.
(Assignee)

Comment 33

8 years ago
Created attachment 417387 [details] [diff] [review]
(Ev2) fix for wince

Another try
Attachment #417377 - Attachment is obsolete: true
Another 100+ errors - http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1260742122.1260742781.12598.gz&fulltext=1
(Assignee)

Comment 35

8 years ago
Created attachment 417929 [details] [diff] [review]
(Ev3) Final fix

I've setup an environment for WinCE building and the attached patch works for me.
Attachment #417387 - Attachment is obsolete: true
(Assignee)

Updated

8 years ago
Attachment #417929 - Flags: review?(mozbugz)
Comment on attachment 417929 [details] [diff] [review]
(Ev3) Final fix

at quick glance, this is a "unsigned short" -> WCHAR change.  Why is it important?
(Assignee)

Comment 37

8 years ago
wchar_t is usually a builtin compiler type, so it's not the same as unsigned short and explicit casts are needed when we mix wchar_t and unsigned short. It's not true in MSC if you pass /Zc:wchar_t- option and that's what's currently done. It causes patches without explicit casts to be accepted. MinGW uses builtin type and thus compilation on mingw break often. We want to get rid of /Zc:wchar_t- so that both compilers will tread wchar_t the same way (another argument is a technical correctness).
(Assignee)

Comment 38

7 years ago
Doug, do you need more explanation? Could you please review the patch?
Comment on attachment 417929 [details] [diff] [review]
(Ev3) Final fix

why do we need to use both WCHAR and wchar_t?

I think the functions that we expose should match whatever msdn does.  For example:

_wgetcwd  should be 

wchar_t *_wgetcwd( 
   wchar_t *buffer,
   int maxlen 
);

that is, wchar_t not WCHAR.  is there a problem doing it this way?
(Assignee)

Comment 40

7 years ago
Created attachment 421656 [details] [diff] [review]
(Ev4) fixed wchar_t/WCHAR
[Checked in: Comment 57]

Thanks for review, I've changed it. We can't change wchar_t->WCHAR in headers of functions like GetEnvironmentVariable in headers, because they are used in context that doesn't have WCHAR (and LPWSTR/LPCWSTR) defined. I don't think it's something we should care about.

There are other things that differ from MSDN like char instead of BOOL (that is defined as int), but it's out of scope of the problem this patch is meant to fix.
Attachment #417929 - Attachment is obsolete: true
Attachment #421656 - Flags: review?(mozbugz)
Attachment #417929 - Flags: review?(mozbugz)

Updated

7 years ago
Attachment #421656 - Flags: review?(mozbugz) → review+
(Assignee)

Comment 41

7 years ago
Created attachment 424234 [details] [diff] [review]
(Bv2) updated m-c patch

Thanks for review. I'm attaching updated m-c patch, that may be landed after "fixed wchar_t/WCHAR" (preferable with try server check first).
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
(Assignee)

Updated

7 years ago
Depends on: 542091
With both these patches applied; I pushed a try build, and it failed on windows:

http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1272347054.1272350184.647.gz&fulltext=1

e:/builds/moz2_slave/win32-hg/build/gfx/cairo/cairo/src/cairo-dwrite-font.cpp(280) : error C2664: 'DWriteFactory::FindSystemFontFamily' : cannot convert parameter 1 from 'uint16_t *' to 'const WCHAR *'
        Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast

(among other warnings).

Once it is safe to push please add the keyword again.
Keywords: checkin-needed
(In reply to comment #42)
> With both these patches applied; I pushed a try build, and it failed on
> windows:

Interesting: how do you add a m-c patch to a MozillaTry build?
(In reply to comment #43)
> Interesting: how do you add a m-c patch to a MozillaTry build?

Never mind: I misread MozillaTry as ThunderbirdTry :-<
(Assignee)

Updated

7 years ago
Depends on: 449292
(Assignee)

Comment 45

7 years ago
Created attachment 451008 [details] [diff] [review]
(Fv1) cairo-dwrite fix

The attached patch fixes the cairo problem. This patch together with my fix from bug 449292 allows landing this bug (tested on try server).
Attachment #451008 - Flags: review?(jmuizelaar)
Comment on attachment 451008 [details] [diff] [review]
(Fv1) cairo-dwrite fix

>diff --git a/gfx/cairo/cairo/src/cairo-dwrite-font.cpp b/gfx/cairo/cairo/src/cairo-dwrite-font.cpp
>index 127ea85..0f12fbd 100644
>--- a/gfx/cairo/cairo/src/cairo-dwrite-font.cpp
>+++ b/gfx/cairo/cairo/src/cairo-dwrite-font.cpp
>@@ -277,7 +277,7 @@ _cairo_dwrite_font_face_create_for_toy (cairo_toy_font_face_t   *toy_face,
>     status = _cairo_utf8_to_utf16 (toy_face->family, -1,
> 				   &face_name, &face_name_len);
> 
>-    IDWriteFontFamily *family = DWriteFactory::FindSystemFontFamily(face_name);
>+    IDWriteFontFamily *family = DWriteFactory::FindSystemFontFamily(reinterpret_cast<wchar_t*>(face_name));
>     if (!family) {
> 	*font_face = (cairo_font_face_t*)&_cairo_font_face_nil;
> 	return CAIRO_STATUS_FONT_TYPE_MISMATCH;

Perhaps we can use MultiByteToWideChar instead of _cairo_utf8_to_utf16 so that we get the right type and don't need to cast?
Attachment #451008 - Flags: review?(jmuizelaar) → review-
(Assignee)

Comment 47

7 years ago
Created attachment 451923 [details] [diff] [review]
(Fv2) cairo-dwrite fix x.1.1
[Checked in: Comment 57]

Thanks for the review. The attached version uses MultiByteToWideChar (also fixes memory leak in error path).
Attachment #451008 - Attachment is obsolete: true
Attachment #451923 - Flags: review?(jmuizelaar)
Attachment #451923 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 48

7 years ago
Patches from comment 40 and comment 47 can be checked in. After landing the patch from bug 449292 comment 146, the final patch from comment 41 can be landed. I'd appreciate quick checkin to avoid another breakages preventing landing the final patch.
Keywords: checkin-needed
Whiteboard: [see comment #25] → [see comment #48]
Created attachment 452219 [details] [diff] [review]
(Gv1) Harfbuzz fix
[Moved to bug 449292]

hb_buffer_add_utf16 expects uint16_t* but aString is a PRUnichar*
Attachment #452219 - Flags: review?(jdaggett)
Comment on attachment 424234 [details] [diff] [review]
(Bv2) updated m-c patch

>@@ -558,7 +558,6 @@ case "$target" in
>             _CC_SUITE=7
>         elif test "$_CC_MAJOR_VERSION" = "14"; then
>             _CC_SUITE=8
>-            CXXFLAGS="$CXXFLAGS -Zc:wchar_t-"
Would it be possible to add CXXFLAGS="$CXXFLAGS -Zc:wchar_t" to the _CC_SUITE=7 block? (-Zc:wchar_t- is the default for that version.)

Updated

7 years ago
Attachment #452219 - Flags: review?(jdaggett) → review+
(Assignee)

Comment 51

7 years ago
(In reply to comment #49)
> Created an attachment (id=452219) [details]
> Harfbuzz fix
> 
> hb_buffer_add_utf16 expects uint16_t* but aString is a PRUnichar*

This is what bug 449292 comment 146 is about.

(In reply to comment #50)
> (From update of attachment 424234 [details] [diff] [review])
> >@@ -558,7 +558,6 @@ case "$target" in
> >             _CC_SUITE=7
> >         elif test "$_CC_MAJOR_VERSION" = "14"; then
> >             _CC_SUITE=8
> >-            CXXFLAGS="$CXXFLAGS -Zc:wchar_t-"
> Would it be possible to add CXXFLAGS="$CXXFLAGS -Zc:wchar_t" to the _CC_SUITE=7
> block? (-Zc:wchar_t- is the default for that version.)

I think it should be possible, but I don't have one to test.
(In reply to comment #51)
> (In reply to comment #49)
> > Created an attachment (id=452219)
> > Harfbuzz fix
> > hb_buffer_add_utf16 expects uint16_t* but aString is a PRUnichar*
> This is what bug 449292 comment 146 is about.
Sorry for the duplication.

> (In reply to comment #50)
> > (From update of attachment 424234 [details] [diff] [review])
> > >@@ -558,7 +558,6 @@ case "$target" in
> > >             _CC_SUITE=7
> > >         elif test "$_CC_MAJOR_VERSION" = "14"; then
> > >             _CC_SUITE=8
> > >-            CXXFLAGS="$CXXFLAGS -Zc:wchar_t-"
> > Would it be possible to add CXXFLAGS="$CXXFLAGS -Zc:wchar_t" to the _CC_SUITE=7
> > block? (-Zc:wchar_t- is the default for that version.)
> I think it should be possible, but I don't have one to test.
How do you think I found the Harfbuzz compile error ;-)

(In reply to comment #0)
> Per bug 324842, it was added due to "This conflicts with various SDK headers
> that still want to define WCHAR". Is it still true with Vista SDK or later? I
> don't see any SDK errors without /Zc:wchar_t-.
I think VC7.1 was partly converted to wchar_t - WinNT.h uses it, but MAPIDefS.h and WabDefs.h still used WORD; the 2003 SDK uses wchar_t throughout.
(Assignee)

Comment 53

7 years ago
Created attachment 452314 [details] [diff] [review]
(Bv3) the patch with wchar_t enabled for _CC_SUITE=7
[Backed out: Comment 63]

> > > Would it be possible to add CXXFLAGS="$CXXFLAGS -Zc:wchar_t" to the _CC_SUITE=7
> > > block? (-Zc:wchar_t- is the default for that version.)
> > I think it should be possible, but I don't have one to test.
> How do you think I found the Harfbuzz compile error ;-)

Let's add it then :)
Attachment #452314 - Flags: review?(ted.mielczarek)
(Reporter)

Comment 54

7 years ago
Reassigning to an actual current contributer. Thank you, Jacek!
(In reply to comment #52)
> (In reply to comment #0)
> > Per bug 324842, it was added due to "This conflicts with various SDK headers
> > that still want to define WCHAR". Is it still true with Vista SDK or later? I
> > don't see any SDK errors without /Zc:wchar_t-.
> I think VC7.1 was partly converted to wchar_t - WinNT.h uses it, but MAPIDefS.h
> and WabDefs.h still used WORD; the 2003 SDK uses wchar_t throughout.
I only considered VC8 or later because /Zc:wchar_t- was the default for VC7.1.
I don't oppose adding a option for VC7.1 because it seems harmless.
Assignee: VYV03354 → jacek
Attachment #452314 - Flags: review?(ted.mielczarek) → review+
(Reporter)

Comment 55

7 years ago
Note that this patch will conflict with a patch for bug 570365.
(Assignee)

Comment 56

7 years ago
Created attachment 454286 [details] [diff] [review]
(Hv1) jetpack fix

Jetpack added new PRUnichar->jschar problem. The attached patch fixes it.
Attachment #454286 - Flags: review?
(Assignee)

Updated

7 years ago
Attachment #454286 - Flags: review? → review?(benjamin)
(Assignee)

Comment 57

7 years ago
http://hg.mozilla.org/mozilla-central/rev/9ca328e60bdd
http://hg.mozilla.org/mozilla-central/rev/dceb0c65850a

The final configure.in patch is blocked by jetpack fix.
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
Whiteboard: [see comment #48]
Comment on attachment 454286 [details] [diff] [review]
(Hv1) jetpack fix

talk about ugly :-(
Attachment #454286 - Flags: review?(benjamin) → review+
(Assignee)

Comment 59

7 years ago
(In reply to comment #58)
> Comment on attachment 454286 [details] [diff] [review]
> jetpack fix
> 
> talk about ugly :-(

Thanks for the review. There is bug 578340 in hope to make things less ugly...
(Assignee)

Updated

7 years ago
Attachment #452314 - Flags: approval2.0?
Have you tested that sizeof(PRUnichar) is still 2 with this patch?

I think nsString.cpp should perhaps contain:

PR_STATIC_ASSERT(sizeof(PRUnichar) == 2)
PR_STATIC_ASSERT(sizeof(nsString::char_type) == 2)
PR_STATIC_ASSERT(sizeof(nsCString::char_type) == 1)

to double-check that we don't break those.
Attachment #452314 - Flags: approval2.0? → approval2.0+
I'll deal with comment 60 in a different bug, though.
(Assignee)

Comment 62

7 years ago
Thanks. Pushed to m-c:

http://hg.mozilla.org/mozilla-central/rev/f10fc9e3be99
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago7 years ago
Resolution: --- → FIXED
(Assignee)

Comment 63

7 years ago
Reverted due to failure:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1281204510.1281205892.28536.gz

http://hg.mozilla.org/mozilla-central/rev/45e0643ad28a
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 452314 [details] [diff] [review]
(Bv3) the patch with wchar_t enabled for _CC_SUITE=7
[Backed out: Comment 63]

(bookkeeping)
Attachment #452314 - Flags: approval2.0+ → approval2.0-
Attachment #394524 - Attachment is obsolete: true
Attachment #393180 - Attachment description: patch in updater [Moved to bug 507338] → (Av1) patch in updater [Moved to bug 507338]
Attachment #394524 - Attachment description: m-c patch → (Bv1) m-c patch
Attachment #394664 - Attachment description: comm-central patch → (Cv1) comm-central patch
Attachment #394682 - Attachment description: added bug number per review comment → (Cv1a) added bug number per review comment
Attachment #394664 - Attachment description: (Cv1) comm-central patch → (Cv1-CC) comm-central patch
Attachment #394682 - Attachment description: (Cv1a) added bug number per review comment → (Cv1a-CC) added bug number per review comment
Attachment #394683 - Attachment description: followup-patch after m-c landing → (Dv1-CC) followup-patch after m-c landing
Attachment #408276 - Attachment description: removed MOZILLA_1_9_1_BRANCH checks → (Cv2-CC) removed MOZILLA_1_9_1_BRANCH checks
Attachment #408277 - Attachment description: followup-patch after m-c landing → (Dv2-CC) followup-patch after m-c landing
Attachment #417377 - Attachment description: fix for wince → (Ev1) fix for wince
Attachment #417387 - Attachment description: fix for wince → (Ev2) fix for wince
Attachment #417929 - Attachment description: Final fix → (Ev3) Final fix
Attachment #421656 - Attachment description: fixed wchar_t/WCHAR → (Ev4) fixed wchar_t/WCHAR
Attachment #424234 - Attachment description: updated m-c patch → (Bv2) updated m-c patch
Attachment #451008 - Attachment description: cairo-dwrite fix → (Fv1) cairo-dwrite fix
Attachment #451923 - Attachment description: cairo-dwrite fix x.1.1 → (Fv2) cairo-dwrite fix x.1.1
Attachment #452219 - Attachment description: Harfbuzz fix → (Gv1) Harfbuzz fix
Attachment #452219 - Attachment description: (Gv1) Harfbuzz fix → (Gv1) Harfbuzz fix [Moved to bug 449292]
Attachment #452219 - Attachment is obsolete: true
Attachment #452314 - Attachment description: the patch with wchar_t enabled for _CC_SUITE=7 → (Bv3) the patch with wchar_t enabled for _CC_SUITE=7
Attachment #424234 - Attachment is obsolete: true
Attachment #454286 - Attachment description: jetpack fix → (Hv1) jetpack fix
Attachment #451923 - Attachment description: (Fv2) cairo-dwrite fix x.1.1 → (Fv2) cairo-dwrite fix x.1.1 [Checked in: Comment 57]
Attachment #421656 - Attachment description: (Ev4) fixed wchar_t/WCHAR → (Ev4) fixed wchar_t/WCHAR [Checked in: Comment 57]
Attachment #452314 - Attachment description: (Bv3) the patch with wchar_t enabled for _CC_SUITE=7 → (Bv3) the patch with wchar_t enabled for _CC_SUITE=7 [Backed out: Comment 63]
(Reporter)

Comment 65

6 years ago
Created attachment 529312 [details] [diff] [review]
(Bv4) unbitrotted

Let's try again.
Tryserver log:
http://tbpl.mozilla.org/?tree=Try&rev=53cdd1b87b28
Jetpack failures are known. Non-windows test failures are obviously unrelated.
Attachment #452314 - Attachment is obsolete: true
(Reporter)

Updated

6 years ago
Attachment #529312 - Flags: review?(ted.mielczarek)
Comment on attachment 529312 [details] [diff] [review]
(Bv4) unbitrotted

Review of attachment 529312 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #529312 - Flags: review?(ted.mielczarek) → review+
(Reporter)

Updated

6 years ago
Keywords: checkin-needed
(Assignee)

Comment 67

6 years ago
Pushed to b-s:

http://hg.mozilla.org/projects/build-system/rev/f8849773f7f4
Keywords: checkin-needed
Whiteboard: fixed-in-bs
http://hg.mozilla.org/mozilla-central/rev/f8849773f7f4
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago6 years ago
Resolution: --- → FIXED
Is it expected that my build will need a clobber after updating to this push? I had a few link errors due to mismatched types (wchar_t vs. unsigned short).
(Assignee)

Comment 70

6 years ago
Yes, it's quite likely, every file using wchar_t needs to be recompiled

Updated

6 years ago
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.