76050 - JS Debugger related make changes

Status: VERIFIED FIXED

(SeaMonkey :: Build Config, defect)

Description

[Robert Ginda]

The following patch adds the "venkman" extension, aka the JS Debugger to the
unix make system.
This is slightly different from other extensions in that it triggers js/jsd to
make, as well as extensions/venkman.  js/jsd is the c debugger api that jband
wrote sooo long ago.

Comment 1

[Robert Ginda]

Attachment: configure.in and autoconf.mk.in changes

Comment 2

[cls]

It looks like that change is going to conflict slightly with the
--with-extensions=all change that dbaron is proposing in bug 75924.  You should
probably grep for venkman in MOZ_EXTENSIONS after that option has been set. 
Also, it doesn't even look like venkman is being pulled on the mozilla tip.

Comment 3

[Robert Ginda]

oops, I checked venmkan in *after* filing this bug, it should be pulling now.

js/jsd, on the otherhand, is not pulled by default.  I'll ask leaf to put it in
the SeaMonkeyAll module.

In addition to the attached patch, there is a change in js/Makefile.in that I
missed:

Index: Makefile.in
===================================================================
RCS file: /cvsroot/mozilla/js/Makefile.in,v
retrieving revision 1.7
diff -u -r1.7 Makefile.in
--- Makefile.in 2001/02/04 06:27:57     1.7
+++ Makefile.in 2001/04/14 21:46:53
@@ -47,6 +47,10 @@
 
 DIRS           = src/fdlibm src
 
+ifdef MOZ_JSDEBUGGER
+DIRS        += jsd
+endif
+
 ifdef MOZ_JAVA
 DIRS           += jsj
 endif

Comment 4

[John Bandhauer]

In case you don't already have this covered... Please put in place the 
appropriate cvs pull magic to only pull in the subdirs under js/jsd that you 
actually use. We for sure don't want *all* that stuff - adding gobs and gobs of 
unused .java files to everyone's cvs pulls will likely result in death threats.

I *do* think you should add those bits of the stuff you need under js/jsd to the 
default pull *and* build to keep everyone honest and make it easy for everyone 
to benefit from this wonderful new tool you'll be providing.

Comment 5

[Robert Ginda]

Reassigning to myself, attaching updated patch.

New patch removes MOZ_JSD_LIBS (it's no longer needed), and sets JS_DEBUGGER in
the --with-extensions=all case, in anticipation of bug 75924.  Bug 76963 has
been filed re: getting js/jsd into SeaMonkeyAll.

Comment 6

[Robert Ginda]

Attachment: updated diff; configure.in, config/autoconf.mk.in, js/Makefile.in

Comment 7

[chris hofmann]

what is left to do on this bug?  will the changes get landed in the next
week or so?

Comment 8

[Robert Ginda]

waiting for r/sr=
having 76963 fixed would be nice too, but not really necessary to check this in.

Comment 9

[Daniel (Leaf) Nunes]

Attachment: fresh tree changes fast / old patch dies in bugzilla / new patch, same effect

Comment 10

[cls]

The test for JS_DEBUGGER should be outside of the --with-extensions macro. Fix
that and r=cls

Comment 11

[Robert Ginda]

Attachment: like this?

Comment 12

[cls]

yep

Comment 13

[Robert Ginda]

leaf, did I hear an sr=?

Comment 14

[Robert Ginda]

Shaver tells me all I need is an r=cls to check in build stuff.  I was not aware
of that.  I'll check into the next open tree.

Comment 15

[Robert Ginda]

Fix checked in.

Comment 16

[Robert Ginda]

Shrike went red when I checked this in.  I don't understand why, maybe it's just
late.  If anyone has any ideas on what I did wrong feel free to say so.  I've
commented out the DIRS+=jsd from js/Makefile.in until I can figure this out.

gmake[4]: Entering directory
`/builds/tinderbox/SeaMonkey/Linux_2.2.5-22smp_Clobber/mozilla/obj-i686-unknown-linux-gnu/js/jsd/idl'
Creating ../../../dist/idl
../../../config/nsinstall -R -m 444
/builds/tinderbox/SeaMonkey/Linux_2.2.5-22smp_Clobber/mozilla/js/jsd/idl/jsdIDebuggerService.idl
../../../dist/idl
gmake[4]: *** No rule to make target `_xpidlgen/jsdIDebuggerService.h', needed
by `export'. Stop.
gmake[4]: Leaving directory
`/builds/tinderbox/SeaMonkey/Linux_2.2.5-22smp_Clobber/mozilla/obj-i686-unknown-linux-gnu/js/jsd/idl'
gmake[3]: *** [export] Error 2
gmake[3]: Leaving directory
`/builds/tinderbox/SeaMonkey/Linux_2.2.5-22smp_Clobber/mozilla/obj-i686-unknown-linux-gnu/js/jsd'
gmake[2]: *** [export] Error 2
gmake[2]: Leaving directory
`/builds/tinderbox/SeaMonkey/Linux_2.2.5-22smp_Clobber/mozilla/obj-i686-unknown-linux-gnu/js'
gmake[1]: *** [export] Error 2
gmake[1]: Leaving directory
`/builds/tinderbox/SeaMonkey/Linux_2.2.5-22smp_Clobber/mozilla/obj-i686-unknown-linux-gnu'
gmake: *** [build] Error 2

No More Errors

Comment 17

[Robert Ginda]

While trying to figure out the problem, Pike mentioned that allmakefiles.sh
probably should have been updated as well.  Something like this, I presume?

Index: allmakefiles.sh
===================================================================
RCS file: /cvsroot/mozilla/allmakefiles.sh,v
retrieving revision 1.298
diff -u -r1.298 allmakefiles.sh
--- allmakefiles.sh     2001/05/10 03:00:36     1.298
+++ allmakefiles.sh     2001/05/15 08:02:10
@@ -222,6 +222,11 @@
 js/src/xpconnect/tools/idl/Makefile
 "
 
+MAKEFILES_jsdebugger="
+js/jsd/Makefile
+js/jsd/idl/Makefile
+"
+
 MAKEFILES_content="
 content/Makefile
 content/base/Makefile
@@ -1100,6 +1105,7 @@
 $MAKEFILES_jpeg
 $MAKEFILES_jprof
 $MAKEFILES_js
+$MAKEFILES_jsdebugger
 $MAKEFILES_l10n
 $MAKEFILES_l10n_lang
 $MAKEFILES_langpacks

Comment 18

[cls]

Attachment: Build jsd *after* xpidl is built

Comment 19

[cls]

r=cls on the allmakefiles.sh patch.

That error message was far more cryptic than it should have been.  Not sure how
to fix that.

Comment 20

[cls]

Attachment: Update REQUIRES and use the same MODULE name across the module (fix senna bustage)

Comment 21

[John Bandhauer]

A couple of questions...

- Do we really want to directly link the building of js/jsd to the building of 
the debugger front end. Inevitably people will want js/jsd without the debugger 
extension front end. For 4.x we built and shipped js/jsd are part of the 
standard distribution so that there was no doubt that it would be present in 
normal installs (it is small). Why not make them separate and have one depend on 
the other? MOZ_JSD and MOZ_JSDEBUGGER?

- I'm not clear on what happened last night, but... Do we really still need to 
build the js module during the export phase? Years ago it was fairly common for 
people to screwup their makefiles by making the mistake of basing those 
makefiles on the makefile in the js module. We used to have reasons to build 
js/src in the export phase. I don't see that those reasons still apply. Do they?

Comment 22

[cls]

Afaict, js/src is not built during the export phase. All dirs are traversed
during the export phase to generate .h from .idl and place them in $(DIST).  
Afaik, the only things that are compiled during the export phase are nspr, the
config tools & the xpt tools.

Last night's issue was a simple build ordering problem that was only evident in
a clobber build.  I don't exactly remember why but js(,/src) is built before
xpcom.  When jsd was added to js/Makefile.in, it caused us to attempt to do the
.idl->.h generation before xpidl (comes from xpcom) was built.

Comment 23

[John Bandhauer]

> Afaict, js/src is not built during the export phase.

Ah, I guess it is only on Windows that we do this...
http://lxr.mozilla.org/seamonkey/source/js/src/makefile.win#201

I still think it would be good to decouple the flags that control building of 
js/jsd from the flags that control building any one jsdebugger front end.

Comment 24

[Robert Ginda]

MOZ_JSDEBUGGER=1 controls the building of the xpcom component. Appending
"venkman" to MOZ_EXTENSIONS controls the building of the front end. The only
reason they appear tied together is because --with-extensions=venkman triggers
both to occur. But, there is no reason we couldn't have an --enable-jsd flag to
make the component, and leave --with-extensions=venkman to build the FE. It
seems a bit more "correct", though a little more confusing for first time builders.

While hacking together an --enable-jsd flag I noticed that the grep -c test I
copied from xmlterm is broken. test 0 evaluates to true, and so we had been
setting MOZ_JSDEBUGGER even without --enable-extensions=venkman. That explains
why these machines tried to build! (They're not *supposed* to be building with
--with-extensions=all, right?) We can build by default later, if it seems like a
good idea. I'm not comfortable with having it built by default now, if it can be
avoided.

Also, I missed extensions/venkman in allmakefiles.sh.

I'm attaching a patch that combines all of the changes still pending on this
bug, including the --enable-jsd flag, and the fix to the xmlterm test.

Comment 25

[Robert Ginda]

Attachment: pending changes in a single patch

Comment 26

[John Bandhauer]

Rob, FWIW I used to call the junk like js/jsd 'JSDEBUGGING' and the front end 
'JSDEBUGGER'. This might not have been the best choice of names, but it was 
repeatedly useful to have a (sorta) clear distinction between the shared support 
parts and the end user UI parts.

Comment 27

[cls]

Actually, most of the tinderboxes are building using --with-extensions.  Your
comment doesn't match the logic of the --enable-jsd option.  With this patch, it
would be off by default.  Outside of that, r=cls

Comment 28

[chris hofmann]

-> 0.9.2

Comment 29

[Robert Ginda]

already checked in.

Comment 30

[Jon Granrose]

verified.