Closed Bug 1288355 Opened 3 years ago Closed 3 years ago

Remove MOZ_UTF16() macro in favor of using u"string" directly

Categories

(MailNews Core :: Backend, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 50.0

People

(Reporter: ewong, Assigned: jorgk)

References

Details

Attachments

(1 file)

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.
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
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)
There were two in /suite. I've got them covered ;-)
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?
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
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
(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.
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.
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)
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
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'
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.
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
Attachment #8773459 - Attachment filename: 1288355.patch → [see comment #8 and comment #10 for patches landed]
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
I think you just push an empty file called CLOBBER.
I clobbered the OS X (and the Win to be sure) machines. I also restarted the OS X builds.
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.
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.
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)
Raised bug 1288651.
Local suite build works fine at first glance. Just briefly tested mail / news. Thanks.
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: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 50.0
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.