1168053 - GMP Service include headers depend on unifed build order

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[James Cheng[:JamesCheng]]

I added some my own cpps like in gmp/moz.build before GMPService.cpp like below

UNIFIED_SOURCES += [
    ....
    'GMPMYOWN1.cpp'
    'GMPMYOWN2.cpp'
    ....
    'GMPService.cpp',
    'GMPServiceChild.cpp',
    'GMPServiceParent.cpp',

I got lots of compile errors.

The root cause is that, 

GMPService.cpp and GMPServiceParent/Child.cpp are separated into different unified build chunks.

I found GMPServiceParent.cpp does not include GMPServiceParent.h 

Instead, dom/media/gmp/GMPService.cpp includes GMPServiceParent.h.

Once GMPService.cpp and GMPServiceParent/Child.cpp are not in the same unified unit, compile error will occur since GMPServiceParent.h is included in GMPService.cpp not GMPService.h.

Is this on purpose?

If so, how can I solve the compile errors if I add some cpp before GMPService.cpp.

Comment 1

[James Cheng[:JamesCheng]]

Hi Josh,

Could you please give me some suggestions about how to prevent from those errors?

Currently, I rename my cpp to GMPZxxx.cpp which is unified after GMPService.cpp to avoid the compile errors.

Thanks.

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

I don't think there's anything intentional here. We should just fix the non-unified build.

Comment 3

[James Cheng[:JamesCheng]]

I forcely cut all the included headers from GMPService.cpp and paste into GMPService.h.

There are lots of compile errors.

Currently, I have no idea how to solve the compile errors. 

So I just name my cpp with prefix "Z" to avoid those errors.

Comment 4

[Josh Aas]

I don't really work on this any more, don't know what's going on here. Sorry. I think Chris has pretty much taken over.

Comment 5

[James Cheng[:JamesCheng]]

Hi Chris,

Do you have any idea about how to elegantly solve this problems?

I have tried as comment3 mentioned and cannot solve those errors.

Should anyone meet this problem before?

Thanks.

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch: Fix unifed build in dom/media/gmp

This patch fixes the unified build in dom/media/gmp.

James: The easiest way to fix unified build failures in a directory is to edit that directory's moz.build file and delete the "UNIFIED" in "UNIFIED_SOURCES", so that effectively you disable unified build. You then need to fix the compile errors one by one, by either adding #includes, or otherwise making sure things are declared/defined in the right translation units.

This builds for me on Windows, I haven't tried elsewhere locally.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=194f16461c33

Comment 7

[JW Wang [:jwwang] [:jw_wang]]

https://hg.mozilla.org/mozilla-central/file/ff2e07228041/python/mozbuild/mozbuild/test/frontend/data/unified-sources-non-unified/moz.build#l21

You can also try FILES_PER_UNIFIED_FILE=1 to disable unified build.

Comment 8

[James Cheng[:JamesCheng]]

Comment on attachment 8611022 [details] [diff] [review]
Patch: Fix unifed build in dom/media/gmp

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

seems there is a typo here.
build fails on try server

I will apply this patch later.

Thanks for your feedback.

::: dom/media/gmp/GMPContentParent.cpp
@@ +11,5 @@
>  #include "GMPVideoDecoderParent.h"
>  #include "GMPVideoEncoderParent.h"
>  #include "mozIGeckoMediaPluginService.h"
>  #include "mozilla/Logging.h"
> +#include "mozilla/Unused.h"

unused.h typo

Comment 9

[James Cheng[:JamesCheng]]

Hi Chris,

The patch works fine on me.

Thanks~

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/423b4de51aa5

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/423b4de51aa5