Closed Bug 339568 Opened 18 years ago Closed 15 years ago

No nightly builds of Mac xforms.xpi for some time

Categories

(Core Graveyard :: XForms, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: allan, Unassigned)

References

()

Details

Attachments

(1 file, 2 obsolete files)

Bug 339198 fixed the trunk builds. 1_8 and 1_8_0 are still failing:

ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla1.8/mac-xpi
xforms.xpi  	220 KB  	09/05/06  	11:42:00

ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla1.8.0/mac-xpi/
xforms.xpi  	220 KB  	16/05/06  	11:58:00

We are syncing the trunk code with branches on bug 339087. That should handle it, but you never know...
No longer depends on: 339087
Summary: No nighly builds of Mac xforms.xpi for some time → No nightly builds of Mac xforms.xpi for some time
(In reply to comment #0)
> Bug 339198 fixed the trunk builds. 1_8 and 1_8_0 are still failing:
ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla1.8/mac-xpi
> xforms.xpi      220 KB          09/05/06        11:42:00
> 
> ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla1.8.0/mac-xpi/
> xforms.xpi      220 KB          16/05/06        11:58:00
> 
> We are syncing the trunk code with branches on bug 339087. That should handle
> it, but you never know...

It did not help.
Blocks: 339855
The Mac 1.8 and 1.8.0 tinderboxes are universal.  They seem to have $BuildXForms turned off.

Unfortunately, fixing this bug isn't as straightforward as turning that variable on.  There's no in-tree support for building extensions as universal binaries yet, so for an extension with binary bits, all you'd get is ppc-only.
(In reply to comment #2)
> The Mac 1.8 and 1.8.0 tinderboxes are universal.  They seem to have
> $BuildXForms turned off.
> 
> Unfortunately, fixing this bug isn't as straightforward as turning that
> variable on.  There's no in-tree support for building extensions as universal
> binaries yet, so for an extension with binary bits, all you'd get is ppc-only.

But, is there a box building ppc for 1.8.0?
I think not any longer.

This can be fixed by making the part of the tinderbox script that builds xforms universal-aware.
Attached patch Patch (untested) (obsolete) — Splinter Review
I can't easily test this right this moment, but someone should!
Attachment #224440 - Flags: review?(rhelmer)
I talked to preed at mozilla.org.  He's fine with the patch as long as we can find someone to test it for us.  Ugh.  I asked on the NG to see if we can get someone to contribute mac builds for us instead.
Took me a while to set up the a test Tinderbox environment, but I'm finally testing this patch.  The "dist/universal/xpi-stage" directory is getting created, but there's nothing in it.  It seems that 'unify' is not merging the two xform.xpis.  Here's the output:

/Users/pedemonte/builds/tinder/Firefox/Darwin_8.7.0_Depend/mozilla/build/macosx/universal/unify /Users/pedemonte/builds/tinder/Firefox/Darwin_8.7.0_Depend/mozilla/objopt/ppc/dist/xpi-stage/xforms.xpi /Users/pedemonte/builds/tinder/Firefox/Darwin_8.7.0_Depend/mozilla/objopt/ppc/../i386/dist/xpi-stage/xforms.xpi /Users/pedemonte/builds/tinder/Firefox/Darwin_8.7.0_Depend/mozilla/objopt/ppc/dist/universal/xpi-stage/xforms.xpi
/Users/pedemonte/builds/tinder/Firefox/Darwin_8.7.0_Depend/mozilla/build/macosx/universal/unify: compareZipArchives: zip archives differ:
  /Users/pedemonte/builds/tinder/Firefox/Darwin_8.7.0_Depend/mozilla/objopt/ppc/dist/xpi-stage/xforms.xpi,
  /Users/pedemonte/builds/tinder/Firefox/Darwin_8.7.0_Depend/mozilla/objopt/ppc/../i386/dist/xpi-stage/xforms.xpi
/Users/pedemonte/builds/tinder/Firefox/Darwin_8.7.0_Depend/mozilla/build/macosx/universal/unify: compareZipArchives: members differ:
  components/libxforms.dylib,
  components/libschemavalidation.dylib
Comment on attachment 224440 [details] [diff] [review]
Patch (untested)

>+        TinderUtils::run_shell_command("cd $builddir/../i386/extensions/xforms; $builddir/../i386/build/autoconf/make-makefile -t $builddir/../i386 -d ../..");
>+        TinderUtils::run_shell_command("make -C $builddir/../i386/extensions/xforms");

I don't think this is necessary.  If the mozconfig already contains the directives to build XForms, then it should be built for both PPC and Intel.

>+        TinderUtils::run_shell_command("$srcdir/build/macosx/universal/unify $builddir/dist/xpi-stage/xforms.xpi $builddir/../i386/dist/xpi-stage/xforms.xpi $builddir/dist/universal/xpi-stage/xforms.xpi");

I tried running 'unify' on the "xpi-stage/xforms" directories, rather than on the XPIs.  But I still got an error:
/Users/pedemonte/builds/tinder/Firefox/Darwin_8.7.0_Depend/mozilla/build/macosx/universal/unify: makeUniversal: symbolic links differ:  /Users/pedemonte/builds/tinder/Firefox/Darwin_8.7.0_Depend/mozilla/objopt/ppc/dist/xpi-stage/xforms/components/libschemavalidation.dylib,
  /Users/pedemonte/builds/tinder/Firefox/Darwin_8.7.0_Depend/mozilla/objopt/ppc/../i386/dist/xpi-stage/xforms/components/libschemavalidation.dylib
Yeah, we want to merge before building the xpi.

> I don't think this is necessary.  If the mozconfig already contains the
> directives to build XForms, then it should be built for both PPC and Intel.

Then why does tinderbox build xforms as it is now?

> I tried running 'unify' on the "xpi-stage/xforms" directories, rather than on
> the XPIs.

That failed because your xpi-stage/xforms directories contained symlinks into their build trees.  unify doesn't follow symlinks, it preserves them.  You can prepare input suitable for unify with rsync:

rsync -av --copy-unsafe-links xpi-stage/xforms/ xpi-stage/xforms.nolinks

Do that for both architectures and then run unify on your new xpi-stage/xforms.nolinks directories.
> Then why does tinderbox build xforms as it is now?

Ah, I see the existing code, now.  I guess the tinderbox doesn't add xforms in its mozconfig.  It seems to me like using mozconfig would be the better way to go, rather than adding special code to the tinderbox script.  But whatever, that's not what this bug is about.  So I rescind my earlier statement.
 
> rsync -av --copy-unsafe-links xpi-stage/xforms/ xpi-stage/xforms.nolinks

I'll give that a shot.
Attached patch patch (tested) (obsolete) — Splinter Review
Added the 'rsync' command as suggested by mento, and 'zip' to repackage the now universal extension.  Also changed the 'make-makefile' calls since they were failing (pointing to $builddir rather than $srcdir).
Attachment #224440 - Attachment is obsolete: true
Attachment #227742 - Flags: review?(rhelmer)
Attachment #224440 - Flags: review?(rhelmer)
Comment on attachment 227742 [details] [diff] [review]
patch (tested)

Why does $srcdir need to be passed to packit()? 

If you look at e.g. lightning.xpi, the build and staging is already done by the time it gets to this part.
(In reply to comment #12)
> Why does $srcdir need to be passed to packit()? 

It is passed in order to get access to 'make-makefile' and 'unify' scripts.  I believe the calls to 'make-makefile' can be avoided if xforms is specified in mozconfig.  However, you still need to merge the PPC and Intel binaries by calling 'unify'.
(In reply to comment #13)
> (In reply to comment #12)
> > Why does $srcdir need to be passed to packit()? 
> 
> It is passed in order to get access to 'make-makefile' and 'unify' scripts.  I
> believe the calls to 'make-makefile' can be avoided if xforms is specified in
> mozconfig.  However, you still need to merge the PPC and Intel binaries by
> calling 'unify'.
> 

Could you move the "make-makefile" and "unify" calls to before packit() is called? This probably belongs in build-seamonkey-util.pl
Attached patch patch v2Splinter Review
Moved the code to build xforms.xpi to build-seamonkey-util.pl.  XForms is built _without_ specifying it in mozconfig; just need to set the $BuildXForms variable in the tinderbox scripts.  Now all that is left in post-mozilla-rel.pl packit() is to pull in the xforms.xpi.  Is this what you had in mind, rhelmer?
Attachment #227742 - Attachment is obsolete: true
Attachment #228438 - Flags: review?(rhelmer)
Attachment #227742 - Flags: review?(rhelmer)
Comment on attachment 228438 [details] [diff] [review]
patch v2

looks good to me!
Attachment #228438 - Flags: review?(rhelmer) → review+
(In reply to comment #15)
> Created an attachment (id=228438) [edit]
> patch v2
> 
> Moved the code to build xforms.xpi to build-seamonkey-util.pl.  XForms is built
> _without_ specifying it in mozconfig; just need to set the $BuildXForms
> variable in the tinderbox scripts.  Now all that is left in post-mozilla-rel.pl
> packit() is to pull in the xforms.xpi.  Is this what you had in mind, rhelmer?
> 

Thanks for making these changes. BTW, I just tried applying the patch to trunk, does not apply cleanly to build-seamonkey-util.pl but should be easy to resolve.

Can you take a look at that and submit a final version? 
Also do you need me to check this in for you when that's done?
Checked in.  In order to enable building the xforms.xpi, you'll need to set "$Settings::BuildXForms".
Status: NEW → RESOLVED
Closed: 18 years ago
Component: XForms → XBL
Resolution: --- → FIXED
Looks like since this patch was checked in, xforms.xpi is no longer being built...see bug 344673
Could this be related to bug 347421? We are seeing an installed xforms extension inside the Mac .dmg since around the 2006-07-13 nightly build.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Yeah, this caused 347421.
The installed xforms extension has a ppc-only libxforms.dylib, where the xpi has the proper fat version.  Note these warnings when doing the final unify:

/builds/tinderbox/Fx-Mozilla1.8/Darwin_8.7.0_Depend/mozilla/build/macosx/universal/unify: warning: makeUniversalDirectory: only in ppc ../build/unifox/ppc/dist/firefox/BonEcho.app/Contents/MacOS/extensions/\{cf2812dc-6a7c-4402-b639-4d277dac4c36\}:
  chrome.manifest,
  components,
  chrome,
  install.js,
  install.rdf

The problem here is somewhat like the problem in bug 328854.  We make in browser/app (only for ppc) after building Talkback in order to get Talkback into the app bundle.  When XForms is also built, this additional make also has the side effect of putting it into the bundle.  (In the case of 328854, it's not another extension being built that's the problem, it's changes to the app that occur while running tests.)

Perhaps we should build Talkback and remake in browser/app immediately after a build completes, instead of waiting until other things like XForms and tests are done.  (We'd still want to wait for tests to complete before uploading symbols...)
That comment was meant for bug 347421.
Well, since this wasn't happening before, the easy fix would be to move the building of XForms back into post-mozilla-rel.pl (the first two patches in this bug).  But it sounds like the correct fix would be what Mark mentioned.  Robert, what do you think?
Blocks: 339198
No longer depends on: 339198
*** Bug 354188 has been marked as a duplicate of this bug. ***
(In reply to comment #24)
> Well, since this wasn't happening before, the easy fix would be to move the
> building of XForms back into post-mozilla-rel.pl (the first two patches in this
> bug).  But it sounds like the correct fix would be what Mark mentioned. 
> Robert, what do you think?

Can you submit a patch for this please?
> Perhaps we should build Talkback and remake in browser/app immediately after a
> build completes, instead of waiting until other things like XForms and tests
> are done.

Instead of running make again in browser/app, couldn't we just simply copy the Talkback files directly into the bundle?  Is there any particular reason it has to be done through browser/app?
(In reply to comment #27)
> > Perhaps we should build Talkback and remake in browser/app immediately after a
> > build completes, instead of waiting until other things like XForms and tests
> > are done.
> 
> Instead of running make again in browser/app, couldn't we just simply copy the
> Talkback files directly into the bundle?  Is there any particular reason it has
> to be done through browser/app?


Yes, that's what mento suggested in bug 347421 comment 7 if I am reading that right. He is more familiar with the universal part of the tinderbox code, so I'll defer to him on the right way to do it.

Sorry I haven't had time to work on this lately, Javier if you put together a patch to do this I will review and make sure it gets in quickly.

I am going to turn off xforms.xpi in our monitoring systems until this bug gets resolved, since I'm getting pinged about it every day :) 
Assignee: xforms → nobody
QA Contact: spride → xbl
This had the right assignee; it had just had the component accidentally switched.

It should also probably be wontfix.
Component: XBL → XForms
QA Contact: xbl → xforms
Mozilla is not producing xforms extensions any more.
Status: REOPENED → RESOLVED
Closed: 18 years ago15 years ago
Resolution: --- → WONTFIX
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: