Refresh MAPI include files in automation so that attachment 9044579 [details] [diff] [review] compiles

RESOLVED FIXED in Thunderbird 68.0

Status

defect
RESOLVED FIXED
2 months ago
23 days ago

People

(Reporter: jorgk, Assigned: rjl)

Tracking

Thunderbird 68.0
x86
Windows 7
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Reporter

Description

2 months ago

+++ This bug was initially created as a clone of Bug #1527450 +++

In bug 1527450, attachment 9044579 [details] [diff] [review], Mike suggested to add some static asserts to make sure the our hand-rolled MAPI structures are the same as the ones that come from Mirosoft's MAPI headers.

Look like the patch doesn't compile in automation, see:

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=fdb855cedcaa09d981df1d9c65ae3cfcefc01dd6
/mailnews/mapi/mapiDLL/MapiDll.cpp(46,49): error: unknown type name 'MapiFileDescW'; did you mean 'MapiFileDesc'?

most likely since the MAPI header files need a refresh, see bug 1527450 comment #14 for details. Locally the patch didn't compile with the "standard" setup, it only compiled after removing an old version of MAPI.h from \shared that obscured a newer on in \um.

This looks like a build issue. I remember the days when Tom migrated Thunderbird to TaskCluster, one of the tasks was to make the MAPI includes available on Windows. I can't see any reference to MAPI in our TaskCluster setup.

Flags: needinfo?(rob)
Flags: needinfo?(mozilla)
Reporter

Comment 2

2 months ago

So they are downloaded from the MS site? Every time? Hmm, I want to "refresh" MAPI.h. Can we not simply put the ZIP file into the tree somewhere?

The zipfile is stored in tooltool which has a zipfile that is a repackaging of the linked file from microsoft. I don't think the files can be included in the tree, as I doubt they are licensed in a way that is redistributable by Mozilla.

Comment 4

2 months ago

And can't one of the files (MAPI.h) be just excluded? Its updated version is present in the platform SDK used by build anyway.

The include search path could perhaps be changed, so that the header from here is after the one from the platform SDK. Or a new repack could be made that excluded a header. I would be inclined to try the first solution first.

Reporter

Comment 6

2 months ago

On Windows 10, MAPI.h already ships with the Windows SDK:
C:\Program Files (x86)\Windows Kits\10\Include\10.0.15063.0\um\MAPI.h

So for Windows 10 we could just remove it from the archive or change the include path, for Windows 7 we need to get the newer version into the archive. In general, we need a way forward since we might not be on Outlook 2010(!!) headers forever.

Comment 7

2 months ago

On WIndows 7? What is the compiler baseline? I was under impression that it is VS 2017 with Windows 10 SDK (which perfectly builds for Windows 7)?

Comment 8

2 months ago

https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Windows_Prerequisites:

Windows 10 SDK (<N>) for Desktop C++ x86 and x64. 10.0.15063.0 is the minimum supported N

Reporter

Comment 9

2 months ago

Hmm, maybe our tree herder is lying: https://treeherder.mozilla.org/#/jobs?repo=comm-central
Windows 7 debug
Windows 7 opt
Windows 10 x64 debug
Windows 10 x64 opt
Windows 2012 debug
Windows 2012 opt
Windows 2012 x64 debug
Windows 2012 x64 opt

Looks like the actual builds run on the machines tagged "Windows 2012", so that's the server version of Windows 8.

Comment 10

2 months ago

I suppose that whatever OS is used, using Windows 10 SDK is possible (and required as baseline) on all supported Win OSes, and all of them thus should have the header.

Assignee

Comment 11

a month ago

Building is done on Win2k12, tests on Win7 and Win10.

It makes more sense to me to update the MAPI header zip file in tooltool rather than change the include path. That way our "local" include files (MAPI headers) don't get overridden by "system" include files (SDK).

Flags: needinfo?(rob)
Assignee

Updated

a month ago
Assignee: nobody → rob
Reporter

Comment 12

a month ago

The trouble is that the old Outlook 2010 headers are downloaded from here http://www.microsoft.com/en-us/download/details.aspx?id=12905 as per comment #1.

There is no package available I could see that includes "refreshed" headers. So we'd have to get and repack them or store our own copy. Also see Tom's comment #3 and comment #5.

Assignee

Comment 13

a month ago

We already store our own copy in tooltool. Someone (Tom?) downloaded the EXE from Microsoft, repackaged it as a ZIP and put it in tooltool as mapiheader.zip.
I'm suggesting we update that zip file rather than changing include paths because IMHO application includes should override system and sdk headers.

Reporter

Comment 14

a month ago
Posted file MAPI.zip

All yours. Here's the file we need.

Comment 15

a month ago

(In reply to Rob Lemley [:rjl] from comment #13)

I'm suggesting we update that zip file rather than changing include paths because IMHO application includes should override system and sdk headers.

But what would be the recommended procedure for Simple Thunderbird Build? would you change instructions to point to your zip?

Assignee

Comment 16

a month ago

I don't have a good answer for that. My assumption is that an individual doing a Simple Build does not have tooltool available and would have to make adjustments manually.

Assignee

Comment 17

a month ago

I took a look at modifying $(INCLUDEPATH) as suggested in comment 5. The mapiheader directory winds up being the only thing in it though, so there's nothing to reorder.

21:39:07     INFO -    INCLUDE=;z:\build\build\src\mapiheader

I'll look at the modifying the zip file some more.

Assignee

Updated

a month ago
Depends on: 1544554
Assignee

Comment 18

a month ago

Updated tooltool manifest for win32 and win64 that grabs the patched version of the MS MAPI headers.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=9225d6ef8e0560cc12d9b9b3545c4dd24a104e66

21:37:27     INFO -   0:01.45 materialized mozbuild.action.tooltool.FileRecord(filename='mapiheader-20190415.zip', size=87349, digest='c4a9362fb688ceab7959ff7b48770f56ea1f626f640f32dab03a14f0f5908b4fa1da811593afa3003c5f15ef4a63880c6ed790863e69a9938c4357d0534dfc1e', algorithm='sha512', visibility=None)
21:37:40     INFO -   0:14.94 Downloading mapiheader-20190415.zip
21:37:40     INFO -   0:14.94 attempt 1/5
21:37:40     INFO -   0:14.94 Downloading to temporary location c:\builds\tooltool_cache\c4a9362fb688ceab7959ff7b48770f56ea1f626f640f32dab03a14f0f5908b4fa1da811593afa3003c5f15ef4a63880c6ed790863e69a9938c4357d0534dfc1e
21:37:41     INFO -   0:15.08 Downloaded artifact to c:\builds\tooltool_cache\c4a9362fb688ceab7959ff7b48770f56ea1f626f640f32dab03a14f0f5908b4fa1da811593afa3003c5f15ef4a63880c6ed790863e69a9938c4357d0534dfc1e
21:37:41     INFO -   0:15.08 hashed u'c:\\builds\\tooltool_cache\\c4a9362fb688ceab7959ff7b48770f56ea1f626f640f32dab03a14f0f5908b4fa1da811593afa3003c5f15ef4a63880c6ed790863e69a9938c4357d0534dfc1e' with sha512 to be c4a9362fb688ceab7959ff7b48770f56ea1f626f640f32dab03a14f0f5908b4fa1da811593afa3003c5f15ef4a63880c6ed790863e69a9938c4357d0534dfc1e
21:38:27     INFO -   1:01.59 unzipping "z:\build\build\src\mapiheader-20190415.zip"
Attachment #9058814 - Flags: review?(geoff)
Assignee

Comment 19

a month ago

Try build referenced in comment 18 was run with the MapiDll.cpp patch in attachment 9044579 [details] [diff] [review] applied.

This is a try build without that patch and is expected to work.
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=61d03a4d53e0cb3dc79551d65d369e4d4643f372

Reporter

Comment 20

a month ago
Comment on attachment 9058814 [details] [diff] [review]
mapiheader_tooltool_update.patch

Yes, this will do the trick, thanks.
Attachment #9058814 - Flags: review?(geoff) → review+

Comment 21

a month ago

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/3cac0cf62b8e
Refresh MS MAPI headers in tooltool. r=jorgk

Status: NEW → RESOLVED
Last Resolved: a month ago
Resolution: --- → FIXED
Reporter

Updated

a month ago
Severity: critical → normal
Target Milestone: --- → Thunderbird 68.0

Comment 22

24 days ago

What do I have to do so that I can build again?

https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Thunderbird_build#MAPI_Headers

... still recommends the headers from MS.

(In reply to Jorg K (GMT+2) from comment #14)

Created attachment 9056669 [details]
MAPI.zip

All yours. Here's the file we need.

Sorry, you are not authorized to access attachment #9056669.

  • Install ToolTool? OK - how?
Reporter

Comment 23

24 days ago

(In reply to Alfred Peters from comment #22)

What do I have to do so that I can build again?

Yes, we need to update the documentation. The issue is that MAPI.h is already included in the Windows SDK.

Taking bug 1546491 into account, you need to install the 10.0.17134.0 SDK using the MSVC installer.

Then copy these 17 MAPI headers

18/10/2010  16:11             7,334 MAPIAux.h
02/06/2009  17:02             7,938 MAPICode.h
02/06/2009  17:02            22,960 MAPIDbg.h
02/06/2009  17:02            84,644 MAPIDefS.h
02/06/2009  17:02            27,840 MAPIForm.h
02/06/2009  17:02            11,880 MAPIGuid.h
02/06/2009  17:02             2,648 MAPIHook.h
02/06/2009  17:02             5,359 MAPINls.h
02/06/2009  17:02             2,743 MAPIOID.h
02/06/2009  17:02            32,978 MAPISPI.h
02/06/2009  17:02            54,395 MAPITags.h
02/06/2009  17:02            26,467 MAPIUtil.h
02/06/2009  17:02            97,301 MAPIVal.h
02/06/2009  17:02             9,334 MAPIWin.h
02/06/2009  17:02             1,906 MAPIWz.h
02/06/2009  17:02            18,277 MAPIX.h
02/06/2009  17:02             5,012 MSPST.h

into C:\Program Files (x86)\Windows Kits\10\Include\10.0.17134.0\shared. Do NOT copy MAPI.h, it is already in C:\Program Files (x86)\Windows Kits\10\Include\10.0.17134.0\um\MAPI.h.

BTW, that was discussed in bug 1527450 comment #14, but no one expects you to know that.

Comment 24

23 days ago

(In reply to Jorg K (GMT+2) from comment #23)

into C:\Program Files (x86)\Windows Kits\10\Include\10.0.17134.0\shared. Do NOT copy MAPI.h, it is already in C:\Program Files (x86)\Windows Kits\10\Include\10.0.17134.0\um\MAPI.h.

Yes, that did it. Thanks.

You need to log in before you can comment on or make changes to this bug.