Closed Bug 1168053 Opened 4 years ago Closed 4 years ago

GMP Service include headers depend on unifed build order

Categories

(Core :: Audio/Video, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: JamesCheng, Assigned: cpearce)

Details

Attachments

(1 file)

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.
Summary: GMP Service include headers depends on unifed build order → GMP Service include headers depend on unifed build order
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.
Flags: needinfo?(joshmoz)
I don't think there's anything intentional here. We should just fix the non-unified build.
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.
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.
Flags: needinfo?(joshmoz)
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.
Flags: needinfo?(cpearce)
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
Flags: needinfo?(cpearce)
Attachment #8611022 - Flags: review?(jwwang)
Attachment #8611022 - Flags: feedback?(jacheng)
Attachment #8611022 - Flags: review?(jwwang) → review+
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
Attachment #8611022 - Flags: feedback?(jacheng) → feedback+
Hi Chris,

The patch works fine on me.

Thanks~
https://hg.mozilla.org/mozilla-central/rev/423b4de51aa5
Assignee: nobody → cpearce
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.