Closed
Bug 1288355
Opened 8 years ago
Closed 8 years ago
Remove MOZ_UTF16() macro in favor of using u"string" directly
Categories
(MailNews Core :: Backend, defect)
MailNews Core
Backend
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 50.0
People
(Reporter: ewong, Assigned: jorgk-bmo)
References
Details
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
There's quite a few mentions in mailnews/ etc..
+++ This bug was initially created as a clone of Bug #1277106 +++
We are currently using MOZ_UTF16() macro to workaround the difference between VC compiler (which uses L"") and other compilers (which accepts u""). This macro can be removed once we drop the support of VS2013, because VS2015 supports u"string" directly.
Assignee | ||
Comment 1•8 years ago
|
||
This was created with:
find . -name *.cpp -exec sed -i -e 's/MOZ_UTF16(\(\".*\"\))/u\1/g' {} \;
run on /mail and /mailnews.
Try:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=05713963be5a
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•8 years ago
|
||
Comment on attachment 8773459 [details] [diff] [review]
[see comment #8 and comment #10 for patches landed]
Anyone wants to review this? It's boring. sed will have done the right thing.
Attachment #8773459 -
Flags: review?(acelists)
Assignee | ||
Comment 3•8 years ago
|
||
There were two in /suite. I've got them covered ;-)
Comment 4•8 years ago
|
||
The parent patch had also changes to NS_LITERAL_STRING, like:
- Write(NS_LITERAL_STRING("\xFFFC"));
+ Write(NS_LITERAL_STRING(u"\xFFFC"));
Do you intent to also do a patch for that?
Assignee | ||
Comment 5•8 years ago
|
||
Kent, no.
Joshua pointed out that I missed a few lines where there were two strings on a line. Fixed now:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=c802c2cc66df
Assignee | ||
Comment 6•8 years ago
|
||
Missed some more.
I got smart and did a local compile on Windows first ;-) Since that worked, here another try:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=14569ad7ebf1
Assignee | ||
Comment 7•8 years ago
|
||
(In reply to Kent James (:rkent) from comment #4)
> The parent patch had also changes to NS_LITERAL_STRING, like:
> - Write(NS_LITERAL_STRING("\xFFFC"));
> + Write(NS_LITERAL_STRING(u"\xFFFC"));
> Do you intent to also do a patch for that?
We don't have that anywhere. I only found this in Mailnews:
int32_t offset = val.Find(NS_LITERAL_STRING("\x0D\x0A"));
offset = val.Find(NS_LITERAL_STRING("\x0D\x0A"), offset + 2);
./import/text/src/nsTextImport.cpp
So I don't think they need treatment.
Assignee | ||
Comment 8•8 years ago
|
||
Landed:
https://hg.mozilla.org/comm-central/rev/a09c05db1261
The patch is 4422 lines long, so frankly, I don't thing anyone would have had the time or inclination to go through it with a fine comb. Plus, apart from a few manual tweaks, it was all sed.
Let's leave the bug open to see whether there are any build/test failures. Built OK on Windows and Linux.
Assignee | ||
Comment 9•8 years ago
|
||
Comment on attachment 8773459 [details] [diff] [review]
[see comment #8 and comment #10 for patches landed]
The patch does *not* reflect exactly what was pushed!
Attachment #8773459 -
Flags: review?(acelists)
Assignee | ||
Comment 10•8 years ago
|
||
Linux went through alright, Windows didn't build for some reason and Mac failed since I had forgotten to fix up .mm files as well. So here's another push:
https://hg.mozilla.org/comm-central/rev/66ffb3c06576
Assignee | ||
Comment 11•8 years ago
|
||
Mac still doesn't compile, looks like more bustage, but in M-C(??):
/builds/slave/tb-c-cen-m64-00000000000000000/build/mozilla/dom/promise/Promise.cpp:744:31: error: no member named 'PromiseNativeHandler' in namespace 'mozilla::dom::prototypes::id'
/builds/slave/tb-c-cen-m64-00000000000000000/build/mozilla/dom/promise/PromiseDebugging.cpp:402:27: error: no viable conversion from 'JS::RootedObject' (aka 'Rooted<JSObject *>') to 'mozilla::dom::Promise'
/builds/slave/tb-c-cen-m64-00000000000000000/build/mozilla/dom/promise/PromiseDebugging.cpp:419:23: error: no viable conversion from 'JS::RootedObject' (aka 'Rooted<JSObject *>') to 'mozilla::dom::Promise'
/builds/slave/tb-c-cen-m64-00000000000000000/build/mozilla/dom/promise/PromiseNativeHandler.cpp:19:10: error: use of undeclared identifier 'PromiseNativeHandlerBinding'; did you mean 'PromiseNativeHandler'?
/builds/slave/tb-c-cen-m64-00000000000000000/build/mozilla/dom/promise/PromiseNativeHandler.cpp:19:39: error: no member named 'Wrap' in 'mozilla::dom::PromiseNativeHandler'
Comment 12•8 years ago
|
||
You need to clobber. Had this with bug 911216 part 30 early this week. Bug has been backed out since then and in again yesterday.
Assignee | ||
Comment 13•8 years ago
|
||
Clobber on Treeherder?? I wouldn't know how to do that, although I can see something here:
https://hg.mozilla.org/mozilla-central/rev/efcf012e6cfc
Assignee | ||
Updated•8 years ago
|
Attachment #8773459 -
Attachment filename: 1288355.patch → [see comment #8 and comment #10 for patches landed]
Assignee | ||
Updated•8 years ago
|
Attachment #8773459 -
Attachment description: 1288355.patch → [see comment #8 and comment #10 for patches landed]
Attachment #8773459 -
Attachment filename: [see comment #8 and comment #10 for patches landed] → 1288355.patch
Comment 14•8 years ago
|
||
I think you just push an empty file called CLOBBER.
Comment 15•8 years ago
|
||
I clobbered the OS X (and the Win to be sure) machines. I also restarted the OS X builds.
Assignee | ||
Comment 16•8 years ago
|
||
Richard, can you please explain what you did exactly to clobber.
Restarted the OS X went via the Treeherder menu, "Add New Jobs" I suppose.
Assignee | ||
Comment 17•8 years ago
|
||
Richard told me privately, that "clobber" is also available on the Treeherder menu.
Re. comment #4 and comment #7: Reading bug 1277106 comment #5 (quote):
Note that a few NS_LITERAL_STRING() callers must be changed when their string literal parameter actually contains escaped Unicode characters like "\x2026".
I've just looked again:
find . -name *.cpp -exec grep '\"\\x[0-9][0-9][0-9]' {} \; -print
The only thing that turned up was:
AddText(L"\x2019"); // Unicode right single quotation mark
./import/outlook/src/rtfMailDecoder.cpp
That L could have been replaced by a u, but since this is compiled for Windows only (Outlook import), it doesn't matter.
Assignee | ||
Comment 18•8 years ago
|
||
Despite looking red on Treeherder, Windows did compile:
http://archive.mozilla.org/pub/thunderbird/tinderbox-builds/comm-central-win32/1469165954/comm-central-win32-bm71-build1-build0.txt.gz
Finished compile (results: 0, elapsed: 59 mins, 4 secs) (at 2016-07-22 00:43:56.597159)
Looks like some packaging failed:
Error: c:\builds\moz2_slave\tb-c-cen-w32-00000000000000000\build\objdir-tb\mail\installer\package-manifest:55: Missing file(s): bin/icudt56l.dat
[snip]
mozmake.exe[3]: *** [stage-package] Error 1
mozmake.exe[2]: *** [make-package] Error 2
mozmake.exe[1]: *** [default] Error 2
Looks like bug 1262731 (SM bug 1288533)
Assignee | ||
Comment 19•8 years ago
|
||
Raised bug 1288651.
Comment 20•8 years ago
|
||
Local suite build works fine at first glance. Just briefly tested mail / news. Thanks.
Assignee | ||
Comment 21•8 years ago
|
||
Yes, I started up TB with the change yesterday and it seemed to work.
Since Mac compiled not, I guess we're done here.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 50.0
Assignee | ||
Comment 22•8 years ago
|
||
Opps: Since Mac compiled NOW, I guess we're done here.
You need to log in
before you can comment on or make changes to this bug.
Description
•