Created attachment 8711110 [details] [diff] [review] 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?
Attachment #8711110 - Flags: review?(ted)
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. :)
Attachment #8711110 - Flags: review?(ted) → review+
Created attachment 8714823 [details] [diff] [review] 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?
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 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.
Created attachment 8717666 [details] [diff] [review] 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
Attachment #8717666 - Flags: review?(mh+mozilla) → review+
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.