Closed Bug 1538567 Opened 7 years ago Closed 7 years ago

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

Categories

(Thunderbird :: Build Config, defect)

Product:
Thunderbird
Component:
Build Config
Platform:
x86
Windows 7
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 68.0

People

(Reporter: jorgk-bmo, Assigned: rjl)

Details

Attachments

(1 file)

mapiheader_tooltool_update.patch
2.74 KB, patch
jorgk-bmo
: review+
Details | Diff | Splinter Review

+++ 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)

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.

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.

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.

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)?

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

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.

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.

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: nobody → rob

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.

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.

Attached file MAPI.zip —

All yours. Here's the file we need.

(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?

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.

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.

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)

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

Comment on attachment 9058814 [details] [diff] [review] mapiheader_tooltool_update.patch Yes, this will do the trick, thanks.
Attachment #9058814 - Flags: review?(geoff) → review+

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

Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Severity: critical → normal
Target Milestone: --- → Thunderbird 68.0

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?

(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.

(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.

Attachment

General

Creator:
Created:
Updated:
Size: