Closed Bug 970429 Opened 7 years ago Closed 7 years ago

Limit scope of initguid.h usage to avoid problems in unified sources builds.


(Core :: Widget: Win32, defect)

Windows 7
Not set





(Reporter: jacek, Assigned: jacek)




(1 file, 1 obsolete file)

Attached patch fix (obsolete) — Splinter Review
initguid.h is dangerous in unified builds, because it affects all subsequent includes that contain DEFINE_GUID. I have observed problems in widget/windows mingw builds so far, but we may want to take a look at other cases and perhaps even find a more generic solution. For the problem I've seen (multiple declarations), the attached patch is enough. initguid.h is not needed in nsToolkit.cpp AFAICT (I will push to try to make sure) and we may restore original DEFINE_GUID variant in nsTextStore.h.
Attachment #8373464 - Flags: review?(ehsan)
Do you have a link to the contents of the initguid.h file?
Flags: needinfo?(jacek)
Not for MSVC version, that wouldn't be legal... But I may confirm that it does the same thing as mingw-w64 version:

So it's just two lines:
#define INITGUID
#include <guiddef.h>

The real work is done by guiddef.h:

Note that DEFINE_GUID is in a part of file that is not guarded, so multiple includes will override each other (that's what my patch uses).
Flags: needinfo?(jacek)
Comment on attachment 8373464 [details] [diff] [review]

Review of attachment 8373464 [details] [diff] [review]:

I see, thanks for the explanation.

I think instead of doing a spot fix like this, you want to modify the include template here: <> and fail the build if a source file defines INITGUID.  Then based on the build failures resulting from that patch, take the source files which rely on that macro out of unified builds.
Attachment #8373464 - Flags: review?(ehsan) → review-
OK, here is a patch implementing that. It came out that INITGUID was quite abused in the tree. gfx/2d is a winner for enabling it in Here is a try build (still running):
Attachment #8373464 - Attachment is obsolete: true
Attachment #8374085 - Flags: review?(ehsan)
Comment on attachment 8374085 [details] [diff] [review]
Don't allow using INITGUID in unified sources.patch.diff

Review of attachment 8374085 [details] [diff] [review]:


::: python/mozbuild/mozbuild/backend/
@@ +661,5 @@
>                      'so it cannot be built in unified mode."\n'
>                      '#undef FORCE_PR_LOG\n'
> +                    '#endif\n'
> +                    '#ifdef INITGUID\n'
> +                    '#error "%(cppfile)s forces defines INITGUID, '

Nit: s/forces //
Attachment #8374085 - Flags: review?(ehsan) → review+
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Depends on: 977279
You need to log in before you can comment on or make changes to this bug.