Closed Bug 1098537 Opened 10 years ago Closed 10 years ago

detangle reftest's testing+addon build system

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 948278

People

(Reporter: froydnj, Assigned: glandium)

References

Details

Attachments

(6 files)

      No description provided.
We want to build httpd.js's module so we can include it in the reftest module.
It'd be nice if all the httpd.js knowledge was contained in one place.  This
patch is taking a stab at doing that.
Attachment #8522454 - Flags: feedback?(gps)
This patch might have belonged with part 1...anyway, we're going to need to
build a separate xpt for the reftest addon, and the jar manifest will need to
reflect that.
Attachment #8522455 - Flags: feedback?(gps)
I ran across this limitation when I attempted to have the reftest addon have
both its normal manifest and the manifest from the httpd.js module.  This patch
is the easiest, most straightforward way I could see to fix it.  It may be
completely wrong in parts, particularly as relates to the jar.py bits, which I
have no familiarity with.

However, it does seem to work...although the chrome.manifest for the reftest
addon after this series of patches is not quite identical to what we had
before.  I think the changes are harmless, but I haven't checked.
Attachment #8522456 - Flags: feedback?(gps)
This patch is an explicit hack.  I'd very much welcome better ways to do
this...but register_idl had this optional parameter that was just begging to be
used...

Anyway, the root problem is that the build system as it stands doesn't want IDL
files included in different xpt files.  Which seems perfectly reasonable, but
interferes with what we're trying to do here.
Attachment #8522457 - Flags: feedback?(gps)
This patch is the meat of the change; all the XPI_NAME-dependent bits are moved
to their own directory.

You can see some XXX comments and whatnot.  This patch is also not ideal
insofar as the jar.mn files for reftest and reftest-addon are almost
identical.  Either they should be refactored to share common bits,
or...something else.  Feedback welcome.

I haven't tested this on try.  However, reftests on my machine do appear to
work, so that's one positive thing going for this patch.  We should ask dbaron
at some point whether this all works for him...or just wait for a bugreport and
fix it then. ;)
Attachment #8522458 - Flags: feedback?(gps)
This patch isn't really necessary, but it was a nice cleanup.
I should also point out that part 5 probably doesn't go far enough, as we're still installing the test harness's version of reftest from xpi-stage...which is undesirable as far as directory ordering goes.  I don't have a good idea as to the plan of attack there.

Maybe it's possible we don't need the complete structure of the addon for testing, and we could explicitly list out the files needed there?  More experimentation required on that front.
(In reply to Nathan Froyd (:froydnj) from comment #4)
> Anyway, the root problem is that the build system as it stands doesn't want
> IDL
> files included in different xpt files.  Which seems perfectly reasonable, but
> interferes with what we're trying to do here.

We could, I suppose, have the emitter maintain a list of files that are permitted to exist in multiple modules, and then add a boolean flag on XPIDLFile indicating whether the backend should permit duplicates of this file.  That would be slightly cleaner...
How about this instead: make httpd.js go in its own, separate addon. Then whatever needs it can install it. That would kill all our problems related to multiple things wanting it.
I am totally okay with the suggestion in comment 9. If the IDL/XPT thing is a problem, I think making us always build and package the httpserver IDL with a Firefox build would be fine too.
(In reply to Mike Hommey [:glandium] from comment #9)
> How about this instead: make httpd.js go in its own, separate addon. Then
> whatever needs it can install it. That would kill all our problems related
> to multiple things wanting it.

Can you use things cross-addon like that?  Or is it just that the httpserver addon would expose things via xpcom components, and then reftests (or whoever) could just pick them up?)
Flags: needinfo?(mh+mozilla)
(In reply to Nathan Froyd (:froydnj) from comment #11)
> (In reply to Mike Hommey [:glandium] from comment #9)
> > How about this instead: make httpd.js go in its own, separate addon. Then
> > whatever needs it can install it. That would kill all our problems related
> > to multiple things wanting it.
> 
> Can you use things cross-addon like that?  Or is it just that the httpserver
> addon would expose things via xpcom components, and then reftests (or
> whoever) could just pick them up?)

Either is possible, however httpd.js works. IIRC it's an xpcom component.
Flags: needinfo?(mh+mozilla)
Note that one way to solve the IDL/XPT problem would be to make it a jsm module. But I don't know how it's being used and how much change that would mean for callers.
So, a quick look suggests that:
- it's already been made loadable but is still also an xpcom component
- only reftest.js is using the xpcom interface.
- there's already a loadable copy in resource://testing-common/httpd.js for mochitests. Not sure where it comes from.
(In reply to Mike Hommey [:glandium] from comment #14)
> Not sure where it comes from.

It simply comes from netwerk/test/httpserver/moz.build's TESTING_JS_MODULES.

So it looks to me like we should just add support for the --testing-modules-dir to reftest, like there is for xpcshell and mochitest, and then httpd.js doesn't even need to be an xpcom component at all anymore.
My hacks have inspired glandium to write up something cleaner, so I'm going to mark this bug as assigned to him.  I think this also removes the need for DYNAMIC_FINAL_TARGET (yay!) in bug 1058051, so marking as blocker.
Assignee: nfroyd → mh+mozilla
Blocks: 1058051
Damn, turns out I already had a patch for this, and it's been sitting forever. I'll compare the patch there to my current WiP from here.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
Attachment #8522454 - Flags: feedback?(gps)
Attachment #8522455 - Flags: feedback?(gps)
Attachment #8522456 - Flags: feedback?(gps)
Attachment #8522457 - Flags: feedback?(gps)
Attachment #8522458 - Flags: feedback?(gps)
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: