Closed
Bug 298431
Opened 20 years ago
Closed 19 years ago
Rewrite xforms build system so it works with XULRunner
Categories
(Core Graveyard :: XForms, enhancement)
Core Graveyard
XForms
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: allan, Assigned: allan)
References
()
Details
(Keywords: fixed1.8)
Attachments
(5 files, 5 obsolete files)
(deleted),
patch
|
benjamin
:
review+
asa
:
approval1.8b4+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
chase
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jag+mozilla
:
review+
jag+mozilla
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
benjamin
:
review+
benjamin
:
approval1.8rc1+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
benjamin
:
review+
benjamin
:
approval1.8rc1+
|
Details | Diff | Splinter Review |
To make xforms work with XULRunner and just generally work better, we should
implement the same techniques as used by the DOM Inspector, ie. XPI_NAME.
* extensions/inspector/Makefile.in
* extensions/inspector/jar.mn
Comment 1•20 years ago
|
||
XPI_NAME ships files to dist/xpi-stage/<XPI_NAME> instead of to dist/bin, which
helps keep the extension build separate from the main app. It can also be used
in conjunction with a couple other vars you find in
extensions/inspector/Makefile.in to build a chrome.manifest from a jar.mn file
and to automatically package that xpi-stage directory into a XPI file and/or
ship it to dist/bin/extensions/<extension-ID>.
Assignee | ||
Comment 2•20 years ago
|
||
Ok, this patch:
* adds the magic to Makefile.in
* moves package/install.rdf to root dir.
* kills package/
This means that the extension (and the XPI) is compiled to dist/bin/xpi-stage/.
Problems:
* building the xpi for suite not working
But actually we should drop suite shouldn't we?
* schemavalidation is not part of the xpi
But has this not always been a bit weird? We could create a
schemavalidation.xpi? Having to install two xpi's are not fantastic, but that's
the most generic approach.
Assignee | ||
Comment 3•20 years ago
|
||
btw, I haven't actually tested it with XULRunner yet. I'm trying to figure out
how extensions actually work for XULRunner.. currently building...
Assignee | ||
Comment 4•20 years ago
|
||
(In reply to comment #3)
> btw, I haven't actually tested it with XULRunner yet. I'm trying to figure out
> how extensions actually work for XULRunner.. currently building...
It works with XULRunner.
Assignee | ||
Comment 5•20 years ago
|
||
(In reply to comment #4)
> (In reply to comment #3)
> > btw, I haven't actually tested it with XULRunner yet. I'm trying to figure out
> > how extensions actually work for XULRunner.. currently building...
>
> It works with XULRunner.
Ok. I might have said that a little bit too early.
'make' in extensions/xforms works, and also builds an xpi. But I cannot seem to
figure out how to install the XPI. unzipping under
~/.mozillatest/mybrowser/<profile>/extensions/{cf2812dc-6a7c-4402-b639-4d277dac4c36}
or dist/bin/extensions/{cf2812dc-6a7c-4402-b639-4d277dac4c36} does not work :(
Assignee | ||
Comment 6•20 years ago
|
||
I'll just continue the discussion with myself here :-) ... XULRunner possibly
needs to be an application in install.rdf? Would make sense.
Assignee | ||
Comment 7•20 years ago
|
||
(In reply to comment #6)
> I'll just continue the discussion with myself here :-) ... XULRunner possibly
> needs to be an application in install.rdf? Would make sense.
from irc:
<bsmedberg> beaufour: you need what we don't have yet... an em:targetApplication
marker for "the toolkit" in general.
:(
Assignee | ||
Updated•20 years ago
|
Depends on: toolkit@mozilla.org
Comment 8•20 years ago
|
||
(In reply to comment #2)
> Created an attachment (id=188296) [edit]
> First try
>
> Ok, this patch:
> * adds the magic to Makefile.in
> * moves package/install.rdf to root dir.
> * kills package/
>
> This means that the extension (and the XPI) is compiled to dist/bin/xpi-stage/.
>
>
> Problems:
> * building the xpi for suite not working
> But actually we should drop suite shouldn't we?
>
> * schemavalidation is not part of the xpi
> But has this not always been a bit weird? We could create a
> schemavalidation.xpi? Having to install two xpi's are not fantastic, but that's
> the most generic approach.
For suite, all we need to do is package install.js and the 2 contents.rdf files
- I believe that is possible.
We really should have just one XPI. Perhaps change the schema validation build
system to check if XForms is built and push the files to the stage dir? Not
that I know much about the new build system
No longer depends on: toolkit@mozilla.org
Assignee | ||
Comment 9•20 years ago
|
||
*** Bug 286202 has been marked as a duplicate of this bug. ***
Comment 10•20 years ago
|
||
I'd like to see suite support continue as long as it is just a pakaging/build
issue. I could see dropping support for it if it starts to interfere with code
logic, requires supporting code paths or ifdefs just for it, etc. It still has
faithful users.
I'd like to keep it all one .xpi as long as we can. Seperate xpi's lead to
versioning and compatibility issues down the road, plus support and test issues.
Assignee | ||
Comment 11•20 years ago
|
||
(In reply to comment #10)
> I'd like to see suite support continue as long as it is just a pakaging/build
> issue. I could see dropping support for it if it starts to interfere with code
> logic, requires supporting code paths or ifdefs just for it, etc. It still has
> faithful users.
I agree. But already, our preferences for whitelists does not work for suite...
nevertheless, I'll see what is needed for suite. It's probably just a
configuration issue.
> I'd like to keep it all one .xpi as long as we can. Seperate xpi's lead to
> versioning and compatibility issues down the road, plus support and test issues.
Where did I say that I wanted seperate XPIs? I didn't mean that.
Doron: Why did you drop the dep. for bug 299716?
Depends on: toolkit@mozilla.org
Assignee | ||
Comment 12•20 years ago
|
||
(In reply to comment #11)
> Where did I say that I wanted seperate XPIs? I didn't mean that.
My brain's still on vacation :(... seperate XPIs for schemavalidation and
xforms, of course, yes. I kept reading "seperate XPIs for suite and firefox"...
sorry for that.
As for a seperate XPI for schemavalidation: What if somebody else wants to use
schemavalidation, and already has it installed? Or just want to install that,
and not xforms?
Assignee | ||
Comment 13•20 years ago
|
||
Mr. cut'n'paste does it again... this one should work for suite, including
install.js. I've also "templatized" install.rdf so versions, uuid, etc. only
needs to be changed in Makefile.in.
There's still the "schemavalidation.xpi" problem though.
Comment 14•20 years ago
|
||
Sorry about removing the dependency, not sure how that happened.
http://lxr.mozilla.org/seamonkey/source/calendar/lightning/Makefile.in#51
Using DIRS seems to do what we want, to get the schema validation files.
Comment 15•20 years ago
|
||
(In reply to comment #12)
> (In reply to comment #11)
> > Where did I say that I wanted seperate XPIs? I didn't mean that.
>
> My brain's still on vacation :(... seperate XPIs for schemavalidation and
> xforms, of course, yes. I kept reading "seperate XPIs for suite and firefox"...
> sorry for that.
>
> As for a seperate XPI for schemavalidation: What if somebody else wants to use
> schemavalidation, and already has it installed? Or just want to install that,
> and not xforms?
Yeah, that is a tough nut to crack. I personally think that if anyone else
wants to use schema-validation, then we should probably see about getting it
into the core. I think that it'll be more widely used than just XForms in the
future. W3C specs to come will probably require it.
Can an extension put a dependency on another extension and prompt the user to
get it if it isn't there (but not if it is)? Maybe somehow indicate the need in
the Extensions dialog? Also if we decide to do that, we'll have to get the
nightly builds to build it as an xpi and put it in the xpi dir.
Assignee | ||
Comment 16•20 years ago
|
||
(In reply to comment #14)
> Using DIRS seems to do what we want, to get the schema validation files.
As far as I can see it just adds schema-validation directory to the build list.
I think EXTRA_COMPONENTS is our friend for packaging.
Status: NEW → ASSIGNED
Assignee | ||
Comment 17•20 years ago
|
||
(In reply to comment #15)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > Where did I say that I wanted seperate XPIs? I didn't mean that.
> >
> > My brain's still on vacation :(... seperate XPIs for schemavalidation and
> > xforms, of course, yes. I kept reading "seperate XPIs for suite and firefox"...
> > sorry for that.
> >
> > As for a seperate XPI for schemavalidation: What if somebody else wants to use
> > schemavalidation, and already has it installed? Or just want to install that,
> > and not xforms?
>
>
> Yeah, that is a tough nut to crack. I personally think that if anyone else
> wants to use schema-validation, then we should probably see about getting it
> into the core. I think that it'll be more widely used than just XForms in the
> future. W3C specs to come will probably require it.
Ok. Until somebody complains about it, let's keep packaging together with
xforms.xpi.
Assignee | ||
Comment 18•20 years ago
|
||
This patch builds and packages schema-validation too. It _should_ work on all
platforms, but it needs testing.
It has one bug, which comes from the packaging of schema-validation: When
schema-validation is built it is installed in dist/bin/components. This is not
necessary as it is packaged with xforms, which is installed in
dist/extensions/{cf...}. So it actually ends up being installed twice, but I do
not think it should create problems for us?
I haven't given it much thought, but instead of using DIRS we could possibly
call 'make -f extensions/schema-validation/Makefile NO_DIST_INSTALL=1' to build
schemavalidation and then package the files from extensions/schema-validation
instead? (and include correct magic for creation of Makefiles there)
Attachment #188296 -
Attachment is obsolete: true
Attachment #188415 -
Attachment is obsolete: true
Attachment #188929 -
Flags: review?(doronr)
Comment 19•20 years ago
|
||
cc: pedemonte to do mac testing :)
Comment 20•20 years ago
|
||
Comment on attachment 188929 [details] [diff] [review]
Patch
>diff -X patch-excludes -uprN -U8 xforms/install.rdf xforms.xpi/install.rdf
>--- xforms/install.rdf 1970-01-01 01:00:00.000000000 +0100
>+++ xforms.xpi/install.rdf 2005-07-06 11:02:30.000000000 +0200
>@@ -0,0 +1,41 @@
>+<?xml version="1.0"?>
>+
>+#filter substitution
>+
>+<RDF xmlns="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
>+ xmlns:em="http://www.mozilla.org/2004/em-rdf#">
>+
>+ <Description about="urn:mozilla:install-manifest">
>+ <em:id>@ID@</em:id>
>+ <em:version>@PACKAGE_VERSION@</em:version>
>+
>+ <em:targetApplication>
>+ <!-- Firefox -->
>+ <Description>
>+ <em:id>{ec8030f7-c20a-464f-9b0e-13a3a9e97384}</em:id>
>+ <em:minVersion>@FIREFOX_VERSION@</em:minVersion>
>+ <em:maxVersion>@FIREFOX_VERSION@</em:maxVersion>
>+ </Description>
>+ </em:targetApplication>
>+
>+ <em:targetApplication>
>+ <!-- Mozilla -->
>+ <Description>
>+ <em:id>{86c18b42-e466-45a9-ae7a-9b95ba6f5640}</em:id>
>+ <em:minVersion>@MOZILLA_VERSION@</em:minVersion>
>+ <em:maxVersion>@MOZILLA_VERSION@</em:maxVersion>
>+ </Description>
>+ </em:targetApplication>
>+
>+ <em:name>Mozilla XForms</em:name>
>+ <em:description>XForms support for Mozilla</em:description>
>+ <em:homepageURL>http://www.mozilla.org/projects/xforms/</em:homepageURL>
>+ <em:file>
>+ <Description about="urn:mozilla:extension:file:xforms.jar">
>+ <em:package>content/xforms/</em:package>
>+ <em:locale>locale/en-US/xforms/</em:locale>
>+ </Description>
>+ </em:file>
>+ </Description>
>+</RDF>
>+
Hmm, what about en-US, should that perhaps be configurable? So that someone
could build a say japanese locale into the xpi? Probably don't have to worry
about it now.
Attachment #188929 -
Flags: superreview?(benjamin)
Comment 21•20 years ago
|
||
bsmedberg points out on irc that em:file is no longer needed since we now use
chrome.manifest, and we don't need ff 1.0 compat.
Comment 22•20 years ago
|
||
Comment on attachment 188929 [details] [diff] [review]
Patch
The schemavalidation stuff looks wrong to me: you should probably
"export XPI_NAME = xforms"
and avoid doing the separate EXTRA_COMPONENTS bits that install it from
dist/bin.
It seems to me that your install.jst needs a registerChrome instruction for
en-US.
As doron said, the install.rdf does not need (should not have) the em:file
bits.
Attachment #188929 -
Flags: superreview?(benjamin) → superreview-
Assignee | ||
Updated•20 years ago
|
Attachment #188929 -
Flags: review?(doronr)
Assignee | ||
Comment 23•20 years ago
|
||
(In reply to comment #22)
> (From update of attachment 188929 [details] [diff] [review] [edit])
> The schemavalidation stuff looks wrong to me:
I didn't like it much either.
> you should probably "export XPI_NAME = xforms"
Ah, isn't "export" a lovely little word :-) I obviously missed the point of it
in the link Doron sent.
Fixed that, killed em:file, and included a registerChrome for the locale in
install.js.
Attachment #188929 -
Attachment is obsolete: true
Attachment #189030 -
Flags: superreview?(benjamin)
Attachment #189030 -
Flags: review?(benjamin)
Comment 24•20 years ago
|
||
Comment on attachment 189030 [details] [diff] [review]
Patch w/Benjamin's comments addressed
I'm not sure what the deal is with approvals and xforms, but if this needs
approval it will have it after b3 gets released.
Attachment #189030 -
Flags: superreview?(benjamin)
Attachment #189030 -
Flags: review?(benjamin)
Attachment #189030 -
Flags: review+
Attachment #189030 -
Flags: approval1.8b4?
Assignee | ||
Comment 25•20 years ago
|
||
(In reply to comment #24)
> (From update of attachment 189030 [details] [diff] [review] [edit])
> I'm not sure what the deal is with approvals and xforms, but if this needs
> approval it will have it after b3 gets released.
Normally we do not need approval, but we need to get the procedure for the
nightly building of the XPIs changed to handle the new build procedure. Is that
you Chase?
Updated•20 years ago
|
Attachment #189030 -
Flags: approval1.8b4? → approval1.8b4+
Assignee | ||
Comment 26•20 years ago
|
||
The change in the build system is that instead of the xforms.xpi being created
in extensions/xforms/stage it now ends up in dist/xpi-stage.
Assignee | ||
Comment 27•19 years ago
|
||
This needs a patch:
http://lxr.mozilla.org/mozilla/source/tools/tinderbox/post-mozilla-rel.pl#330
Assignee | ||
Comment 28•19 years ago
|
||
Here's a patch to post-mozilla-rel.pl, changes:
* schemavalidation is built automatically through the xforms Makefile
* xpi-file is now stored in dist/xpi-stage
Attachment #198290 -
Flags: review?(chase)
Comment 29•19 years ago
|
||
Comment on attachment 198290 [details] [diff] [review]
Tinderbox patch
coop will land.
Attachment #198290 -
Flags: review?(chase) → review+
Comment 30•19 years ago
|
||
(In reply to comment #29)
> (From update of attachment 198290 [details] [diff] [review] [edit])
> coop will land.
>
Landed.
Assignee | ||
Comment 31•19 years ago
|
||
Comment on attachment 189030 [details] [diff] [review]
Patch w/Benjamin's comments addressed
Checked in to trunk.
It has an oooold approval for 1.8b4, which was waiting for the tinderbox patch
which went in yesterday. Re-requesting approval for rc1.
Attachment #189030 -
Flags: approval1.8rc1?
Assignee | ||
Updated•19 years ago
|
No longer depends on: toolkit@mozilla.org
Assignee | ||
Updated•19 years ago
|
Whiteboard: xf-to-branch
Assignee | ||
Comment 32•19 years ago
|
||
Argh. This has a bad influence on make distclean :(
As extensions/xforms now builds extensions/schema-validation, a distclean in
xforms also cleans schema-validation, which makes a global 'make distclean' fail
because the Makefile is cleaned from schema-validation by xforms.
Assignee | ||
Comment 33•19 years ago
|
||
Definately not a beauty, but it should fix the distclean problem
Comment 34•19 years ago
|
||
beaufour didn't get the right diff, but here's what he checked in with my
blessing, provided he won't rest (much) until he's found a better solution.
Attachment #200060 -
Attachment is obsolete: true
Attachment #200061 -
Flags: superreview+
Attachment #200061 -
Flags: review+
Assignee | ||
Comment 35•19 years ago
|
||
(In reply to comment #34)
> Created an attachment (id=200061) [edit]
> Bustage fix, for real
>
> beaufour didn't get the right diff
Definately not my day...
Assignee | ||
Comment 36•19 years ago
|
||
(In reply to comment #35)
> (In reply to comment #34)
> > Created an attachment (id=200061) [edit] [edit]
> > Bustage fix, for real
> >
> > beaufour didn't get the right diff
>
> Definately not my day...
AIX on SeaMonkey-Ports keeps hating my guts...:
gmake[3]: Entering directory
`/home/tbox/sb/tinderbox/AIX_5.1_Clobber/mozilla/obj-tinder/extensions/xforms'
/bin/sh: syntax error at line 1 : `;' unexpected
My guess is that it is this line:
http://lxr.mozilla.org/seamonkey/source/config/rules.mk#323
the "DIRS=" hack does not undefine DIRS on AIX, expanding LOOP_OVER_DIRS.
Anybody got an idea here?
Comment 37•19 years ago
|
||
That's because the ifdef is applied at parse-time, while the DIRS= variable
override is applied at rule execution time. Hack forthwith.
Comment 38•19 years ago
|
||
I don't have an old shell that doesn't like "for x in ;" to test this with, but
it should work.
Attachment #200079 -
Flags: review?(allan)
Comment 39•19 years ago
|
||
Please move the distclean rule back down below rules.mk: it won't do to have
"make -C extensions/xforms" doing distclean by default...
Comment 40•19 years ago
|
||
bsmedberg: that'd be my fault. The GNU make manual said that double-colon rules
apply in order found, so I figured maybe the problem was that DIRS didn't get
cleared in time.
Anyway, what I'd much rather see is that schema-validation doesn't get built
from xforms so we can do away with this hackery. I believe that through
configure and client.mk you can make sure schema-validation gets built before
xforms gets built. That should at least limit the need for hackery to just the
packaging step, though I'd like to see a clean(er) solution for that too.
Comment 41•19 years ago
|
||
It is not sufficient to simply build schema-validation, you have to build it with
export XPI_NAME = xforms
So that the results end up as part of the xforms XPI.
Comment 42•19 years ago
|
||
Comment on attachment 189030 [details] [diff] [review]
Patch w/Benjamin's comments addressed
please re-request approval for this not part of the build change after you've
got everything working and verified.
Attachment #189030 -
Flags: approval1.8rc1?
Comment 43•19 years ago
|
||
There must be some other way to get those files into the xforms XPI. I remember
there being a way to merge multiple XPIs into one, could we use that?
Assignee | ||
Comment 44•19 years ago
|
||
(In reply to comment #38)
> Created an attachment (id=200079) [edit]
> Use shell to test DIRS before looping
>
> I don't have an old shell that doesn't like "for x in ;" to test this with, but
> it should work.
I do not think it does, it's only one line in the shell and I think a (stupid)
shell will evaluate it all before doing any thing, which means that "for x in ;"
will still generate an error.
I'm even less happy about the approach, but I could just do
distclean:: LOOP_OVER_DIRS=
instead... that would work.
Assignee | ||
Comment 45•19 years ago
|
||
(In reply to comment #44)
> I do not think it does, it's only one line in the shell and I think a (stupid)
> shell will evaluate it all before doing any thing,
Hmmm, forget the "stupid" part...I guess it would have to pretty darn smart to
skip sections with syntax errors in fact if the test fails...
Assignee | ||
Comment 46•19 years ago
|
||
(In reply to comment #39)
> Please move the distclean rule back down below rules.mk: it won't do to have
> "make -C extensions/xforms" doing distclean by default...
It won't be, it's a prerequisite as it has no commands.
Assignee | ||
Comment 47•19 years ago
|
||
Comment on attachment 200079 [details] [diff] [review]
Use shell to test DIRS before looping
(In reply to comment #44)
> (In reply to comment #38)
> > Created an attachment (id=200079) [edit] [edit]
> > Use shell to test DIRS before looping
> >
> > I don't have an old shell that doesn't like "for x in ;" to test this with, but
> > it should work.
>
> I do not think it does, it's only one line in the shell and I think a (stupid)
> shell will evaluate it all before doing any thing, which means that "for x in ;"
> will still generate an error.
that would be r-..
Attachment #200079 -
Flags: review?(allan) → review-
Comment 48•19 years ago
|
||
(In reply to comment #44)
> (In reply to comment #38)
> > Created an attachment (id=200079) [edit] [edit]
> > Use shell to test DIRS before looping
> >
> > I don't have an old shell that doesn't like "for x in ;" to test this with, but
> > it should work.
>
> I do not think it does, it's only one line in the shell and I think a (stupid)
> shell will evaluate it all before doing any thing, which means that "for x in ;"
> will still generate an error.
>
> I'm even less happy about the approach, but I could just do
> distclean:: LOOP_OVER_DIRS=
> instead... that would work.
Having "distclean:: LOOP_OVER_DIRS=" instead of "distclean:: DIRS=" resolves the
issue.
Comment 49•19 years ago
|
||
(In reply to comment #48)
> Having "distclean:: LOOP_OVER_DIRS=" instead of "distclean:: DIRS=" resolves the
> issue.
Allan - are you comfortable with this fix?
Assignee | ||
Comment 50•19 years ago
|
||
(In reply to comment #49)
> (In reply to comment #48)
> > Having "distclean:: LOOP_OVER_DIRS=" instead of "distclean:: DIRS=" resolves the
> > issue.
>
> Allan - are you comfortable with this fix?
I have already started walking down the hacky path... why turn around? :)
It's ok I guess, though I haven't looked through "make distclean" to see if
LOOP_OVER_DIRS is used for something else.
Assignee | ||
Comment 51•19 years ago
|
||
Attachment #200344 -
Flags: review?(benjamin)
Updated•19 years ago
|
Attachment #200344 -
Flags: review?(benjamin) → review+
Assignee | ||
Updated•19 years ago
|
Attachment #200079 -
Attachment is obsolete: true
Assignee | ||
Comment 52•19 years ago
|
||
I'm really having fun with this.... :( extensions/xforms/package needs to be
killed from allmakefiles.sh too.
Assignee | ||
Updated•19 years ago
|
Attachment #200347 -
Flags: review?(benjamin)
Comment 53•19 years ago
|
||
Comment on attachment 200347 [details] [diff] [review]
remove extensions/xforms/package
Extra/missing files don't really hurt anyone (it's just an annoying warning at
configure-time).
Attachment #200347 -
Flags: review?(benjamin)
Attachment #200347 -
Flags: review+
Attachment #200347 -
Flags: approval1.8rc1+
Updated•19 years ago
|
Attachment #200344 -
Flags: approval1.8rc1?
Comment 54•19 years ago
|
||
Attachment 200344 [details] [diff] checked in to trunk.
Checking in Makefile.in;
/cvsroot/mozilla/extensions/xforms/Makefile.in,v <-- Makefile.in
new revision: 1.43; previous revision: 1.42
done
Updated•19 years ago
|
Attachment #200344 -
Flags: approval1.8rc1? → approval1.8rc1+
Assignee | ||
Comment 55•19 years ago
|
||
Attachment 200347 [details] [diff] checked in to trunk.
Checking in allmakefiles.sh;
/cvsroot/mozilla/allmakefiles.sh,v <-- allmakefiles.sh
new revision: 1.588; previous revision: 1.587
done
Assignee | ||
Comment 56•19 years ago
|
||
Checked in on 1.8 branch
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
Whiteboard: xf-to-branch
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•