1241976 - Port xpcom/tests INSTALL_TARGETS to moz.build

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Comment 1

[Michael Shal [:mshal]]

Attachment: 0003-Bug-1241976-port-INSTALL_TARGETS-in-xpcom-tests-to-m.patch

We can use the FINAL_TARGET trick here to get things installed in the right place. Is there a way to get rid of the remaining LDFLAGS filtering in component_no_aslr/Makefile.in?

Comment 2

[(not currently active) Ted Mielczarek]

Comment on attachment 8711110 [details] [diff] [review]
0003-Bug-1241976-port-INSTALL_TARGETS-in-xpcom-tests-to-m.patch

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

I love simple patches that remove whole Makefiles. :)

Comment 3

[Michael Shal [:mshal]]

Attachment: 0001-Bug-1241976-port-INSTALL_TARGETS-in-xpcom-tests-to-m.patch

Unfortunately this patch breaks xpcshell tests. I thought I verified that the files were being installed in the same place, but apparently not. The Makefile bits install the files directly into objdir/_tests/xpcshell/xpcom/tests/unit, but the moz.build equivalent installed them into objdir/_tests/xpcshell/xpcom/tests/unit/components (with the extra '/components' on the end), so the tests can't find them. This happens because XPCOMBinaryComponent() sets IS_COMPONENT=True, and target_binaries.mk appends the '/components' string based on that flag.

Changing XPCOMBinaryComponent to GeckoSharedLibrary effectively just removes IS_COMPONENT=True so that the libraries get installed in the correct location, and xpcshell passes. However, I'm not sure if that's the proper solution here. Thoughts?

Comment 4

[Mike Hommey [:glandium]]

IS_COMPONENT=True adds things to LDFLAGS, so that's required-ish to build an actual component. -ish because it might just work without, and for test components that might not be a big deal. That being said, there's value in identifying what are meant to be xpcom component from moz.build, so the distinction between XPCOMBinaryComponent and GeckoSharedLibrary has its significance.

How about moving the FINAL_TARGET setting in the XPCOMBinaryComponent template, and override that somehow in the relevant moz.builds?

Comment 5

[Michael Shal [:mshal]]

Comment on attachment 8714823 [details] [diff] [review]
0001-Bug-1241976-port-INSTALL_TARGETS-in-xpcom-tests-to-m.patch

I'll try glandium's suggestion and either post a new patch or re-ask for review on this one.

Comment 6

[Michael Shal [:mshal]]

Attachment: 0001-Bug-1241976-port-INSTALL_TARGETS-in-xpcom-tests-to-m.patch

I tried to go with setting FINAL_TARGET in XPCOMBinaryComponent, but mozbuild doesn't seem to like setting/appending anything to it (or even doing print(FINAL_TARGET)). Unless I'm misunderstanding your suggestion, I think to make it work we'd either have to relax the restrictions on FINAL_TARGET, or use some other variable to determine whether or not '/components' is appended. Since this is only for a few tests files, it seems silly to add another mozbuild feature especially for that.

What about this approach? This moves everything into the components/ directory, and just references components/foo.manifest in the js test files. This makes the xpcom/tests moz.build files more similar to js/xpconnect/tests/components/native/moz.build

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fdccd4f00208

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fdccd4f00208