Closed Bug 1686700 Opened 3 years ago Closed 3 years ago

Thunderbird windows builds busted - port bug 1684110 - /builds/worker/checkouts/gecko/comm/mail/components/search/nsMailWinSearchHelper.cpp(108,7): error: call to deleted function 'OpenSCManager'

Categories

(Thunderbird :: Upstream Synchronization, task, P1)

Product:
Thunderbird
Component:
Upstream Synchronization
Type:
task

Tracking

(thunderbird_esr78 unaffected, thunderbird85 unaffected)

Status:
RESOLVED FIXED
Milestone:
86 Branch
Tracking Flags:
Tracking Status
thunderbird_esr78 --- unaffected
thunderbird85 --- unaffected

People

(Reporter: mkmelin, Assigned: it)

References

(Regression)

Details

Attachments

(1 file, 4 obsolete files)

take2.patch - really final
19.27 KB, patch
mkmelin
: review+
Details | Diff | Splinter Review

There's also

[task 2021-01-14T10:22:46.808Z] 10:22:46 INFO - /builds/worker/checkouts/gecko/comm/mailnews/addrbook/src/nsMapiAddressBook.cpp(35,27): error: call to deleted function 'LoadLibrary'
[task 2021-01-14T10:22:46.808Z] 10:22:46 INFO - HMODULE libraryHandle = LoadLibrary("MAPI32.DLL");
[task 2021-01-14T10:22:46.808Z] 10:22:46 INFO - ^~~~~~~~~~~
[task 2021-01-14T10:22:46.808Z] 10:22:46 INFO - /builds/worker/workspace/obj-build/dist/stl_wrappers/windows.h(969,1): note: candidate function has been explicitly deleted
[task 2021-01-14T10:22:46.808Z] 10:22:46 INFO - LoadLibrary(LPCTSTR a0)
[task 2021-01-14T10:22:46.808Z] 10:22:46 INFO - ^
[task 2021-01-14T10:22:46.808Z] 10:22:46 INFO - /builds/worker/checkouts/gecko/comm/mailnews/addrbook/src/nsMapiAddressBook.cpp(44,21): error: call to deleted function 'LoadLibrary'
[task 2021-01-14T10:22:46.808Z] 10:22:46 INFO - libraryHandle = LoadLibrary("MAPI32BAK.DLL");
[task 2021-01-14T10:22:46.808Z] 10:22:46 INFO - ^~~~~~~~~~~
[task 2021-01-14T10:22:46.808Z] 10:22:46 INFO - /builds/worker/workspace/obj-build/dist/stl_wrappers/windows.h(969,1): note: candidate function has been explicitly deleted
[task 2021-01-14T10:22:46.808Z] 10:22:46 INFO - LoadLibrary(LPCTSTR a0)
[task 2021-01-14T10:22:46.808Z] 10:22:46 INFO - ^
[task 2021-01-14T10:22:46.808Z] 10:22:46 INFO - 2 errors generated.
[task 2021-01-14T10:22:46.808Z] 10:22:46 ERROR - make[4]: *** [/builds/worker/checkouts/gecko/config/rules.mk:674: nsMapiAddressBook.obj] Error 1
[task 2021-01-14T10:22:46.808Z] 10:22:46 INFO - make[4]: Leaving directory '/builds/worker/workspace/obj-build/comm/mailnews/addrbook/src'

:toshi, any ideas? Could bug 1684532 have caused this?

Flags: needinfo?(tkikuchi)

OpenSCManager and LoadLibrary are Windows functions "wrapped" in https://searchfox.org/mozilla-central/source/__GENERATED__/dist/stl_wrappers/windows.h. Something happened to that wrapping? Would it be permissible to use the "native" A/W versions instead?

Oh, I see now. Bug 1684110.

Regressed by: 1684110
Summary: Thunderbird windows builds busted - /builds/worker/checkouts/gecko/comm/mail/components/search/nsMailWinSearchHelper.cpp(108,7): error: call to deleted function 'OpenSCManager' → Thunderbird windows builds busted - port bug 1684110 - /builds/worker/checkouts/gecko/comm/mail/components/search/nsMailWinSearchHelper.cpp(108,7): error: call to deleted function 'OpenSCManager'
Flags: needinfo?(tkikuchi)

So the calls should be like LoadLibraryW(L"foo.dll"); now, instead of LoadLibrary?
https://searchfox.org/comm-central/search?q=LoadLibrary%28&path=mail&case=false&regexp=false

Richard, are you able to check if that works?

If you get me a patch I can try it.

Attached patch 1686700-bustage.patch (obsolete) — — Splinter Review

This is incomplete, there are far more issues.

I don't get the loadLibrary error but like this one now:
1:20.65 comm/mailnews/mapi/mapihook/src
1:21.13 z:/Mozilla/comm-central/comm/mailnews/import/src/MapiApi.cpp(918,22): error: no matching function for call to 'strlen'
1:21.13 int strLen = strlen(lpStr);
1:21.14 ^~~~~~
1:21.15 C:\Program Files (x86)\Windows Kits\10\include\10.0.17134.0\ucrt\string.h(219,16): note: candidate function not viable: no known conversion from 'LPCTSTR' (aka 'const wchar_t *') to 'const char *' for 1st argument
1:21.16 size_t __cdecl strlen(

Attached patch take2.patch (obsolete) — — Splinter Review

This compiles so far. This will need revisiting since we believe that like in the first patch, UNICODE should be set, but that causes a heap of other issues.

Hmm, still doesn't compile. We'll take another look.

Attached patch take2.patch (obsolete) — — Splinter Review

Somehow dependencies are not set up correctly, so mailnews/mapi/mapihook/src was missed in our prior build.

Attachment #9197069 - Attachment is obsolete: true
Attached patch take2.patch - final (obsolete) — — Splinter Review

another tweak.

Attachment #9197104 - Attachment is obsolete: true

OK so I need to figure out if this is what is going to land. Need to figure out if the SeaMonkey suite builds successfully with what the final fix is or if I need to do a followup fix the suite patch.

Comment on attachment 9197106 [details] [diff] [review]
take2.patch - final

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

::: mailnews/addrbook/src/nsWabAddressBook.cpp
@@ +32,5 @@
>    DWORD keyType = 0;
>    ULONG byteCount = sizeof(wabDLLPath);
>    HKEY keyHandle = NULL;
>    wabDLLPath[MAX_PATH - 1] = 0;
> +  // W-version not defined in WabApi.h.

`L"" WAB_DLL_PATH_KEY` will work.

::: mailnews/import/src/MapiMimeTypes.cpp
@@ +42,5 @@
>  BOOL CMimeTypes::GetMimeTypeFromReg(const nsCString& ext, LPBYTE* ppBytes) {
>    HKEY extensionKey;
>    BOOL result = FALSE;
>    *ppBytes = NULL;
> +  if (GetKey(HKEY_CLASSES_ROOT, NS_ConvertUTF8toUTF16(ext).get(),

Please do not (unless `ext` will never contain non-ASCII characters). Use `NS_CopyNativeToUnicode`.
Comment on attachment 9197106 [details] [diff] [review]
take2.patch - final

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

::: mailnews/import/src/MapiMimeTypes.cpp
@@ +42,5 @@
>  BOOL CMimeTypes::GetMimeTypeFromReg(const nsCString& ext, LPBYTE* ppBytes) {
>    HKEY extensionKey;
>    BOOL result = FALSE;
>    *ppBytes = NULL;
> +  if (GetKey(HKEY_CLASSES_ROOT, NS_ConvertUTF8toUTF16(ext).get(),

The only caller of CMimeTypes::GetMimeTypeFromReg is CMimeTypes::GetMimeType(const nsCString&) and the only caller of CMimeTypes::GetMimeType(const nsCString&) is CMimeTypes::GetMimeType(const nsString&) and it calls LossyCopyUTF16toASCII. So `ext` is converted UTF-16 to ASCII and then converted back to UTF-16. Could you please just remove these useless conversions?

Yes, we came to the same conclusion.

Attachment #9197106 - Attachment is obsolete: true
See Also: → 1686850
Comment on attachment 9197235 [details] [diff] [review]
take2.patch - really final

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

Looks like success, thx! r=mkmelin
Attachment #9197235 - Flags: review+
Assignee: nobody → it
Status: NEW → ASSIGNED
status-thunderbird85: --- → unaffected
status-thunderbird_esr78: --- → unaffected
Target Milestone: --- → 86 Branch
Attachment #9197061 - Attachment is obsolete: true

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/9ca01b9e6433
Port bug 1684110: Switch to wide char for some Windows interfaces. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/3c3a5e26805c
followup - apply clang-format. rs=clang-format
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: