Closed Bug 479475 Opened 16 years ago Closed 6 years ago

SpiderMonkey source distribution makefile target

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: jorendorff, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 3 obsolete files)

Like this: $ cd $OBJDIR $ make sdist ... $ ls js-*.tar.gz js-1.8.1.tar.gz The source distribution should include whatever's needed for `configure && make all` to work. So, configure and imacros.c.out, but not js-config.h or jsautokw.h. We want this before releasing SpiderMonkey 1.8.1 (bug 479473).
Assignee: nobody → jim
How this was done up to SM 1.8: http://www.mozilla.org/js/spidermonkey/release-notes/spidermonkey-releases.html I think we should feel free to change a few things. At least the CVS metadata directories will be gone; we should not include a .hg directory (as that would be crazy). Our 1.6, 1.7, and 1.8 release tarballs all unzip to js/src (and js/jsd). I think we really want to unzip to spidermonkey-1.8.1/js/src or something. Do we want jsd and liveconnect to be in future releases? I ask because don't know how to test either one of these.
I think js/tests should be included (perhaps excluding many of the individual tests, such as lc2, lc3, spidermonkey-n.tests, and spidermonkey-1.8.1-n.tests).
I'm not interested in working on this any more. De-assigning bug.
Assignee: jimb → nobody
Long time coming. It may not be the most ideal or integrated method for rolling a source package within the mozilla build system, but it works and it's a lot better than the current "nothing" we have now. This patch is currently dependent on the versioning that has been added in the atachments of bug 812265
Attachment #688802 - Flags: review?(jimb)
... currently the patch doesn't add LICENSE, LEGAL and README.txt from $(topsrcdir), it should do that.
Looking this over, thanks! Minor note: it's best to make patches relative to the root of the Mozilla tree, so that I can tell this is patching js/src/Makefile.in, and not some other Makefile.in.
Comment on attachment 688802 [details] [diff] [review] Add 'source-package' target to spidermonkey makefile (currently linux only) Review of attachment 688802 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the patch! How are you planning to deal with NSPR? ::: Makefile.in @@ +960,5 @@ > + > +source-package:: clean > + mkdir -p $(SOURCEDISTNAME)/js/src > + find $(srcdir)/../src -maxdepth 1 -not -name src -not -name $(SOURCEDISTNAME) \ > + -exec cp -r --target-directory=$(SOURCEDISTNAME)/js/src {} \+ Isn't '$(srcdir)/../src' just '$(srcdir)'? If that's there to make sure the '-not -name src' trick works even when building in the source tree (where '$(srcdir)' is '.'), you could instead say: find $(srcdir) -maxdepth 1 -mindepth 1 -not -name $(SOURCEDISTNAME) \ ... @@ +961,5 @@ > +source-package:: clean > + mkdir -p $(SOURCEDISTNAME)/js/src > + find $(srcdir)/../src -maxdepth 1 -not -name src -not -name $(SOURCEDISTNAME) \ > + -exec cp -r --target-directory=$(SOURCEDISTNAME)/js/src {} \+ > + $(RM) $(patsubst %,$(SOURCEDISTNAME)/js/src/%,$(DIST_GARBAGE)) That's a surprising use of $(DIST_GARBAGE), but it makes sense. But what will you do to clear out the .o files? Looking at my build dir, I also see executables like host_jskwgen, host_jsoplengen, and js-config; Makefiles; ... I think you're going to need an opt-in approach, not an opt-out approach.
Attachment #688802 - Flags: review?(jimb)
(In reply to Jim Blandy :jimb from comment #7) > Comment on attachment 688802 [details] [diff] [review] > Add 'source-package' target to spidermonkey makefile (currently linux only) > > Review of attachment 688802 [details] [diff] [review]: > ----------------------------------------------------------------- > > Thanks for the patch! > > How are you planning to deal with NSPR? Don't know yet. Works fine with external (system) nspr; may force that. > ::: Makefile.in > @@ +960,5 @@ > > + > > +source-package:: clean > > + mkdir -p $(SOURCEDISTNAME)/js/src > > + find $(srcdir)/../src -maxdepth 1 -not -name src -not -name $(SOURCEDISTNAME) \ > > + -exec cp -r --target-directory=$(SOURCEDISTNAME)/js/src {} \+ > > Isn't '$(srcdir)/../src' just '$(srcdir)'? If that's there to make sure the > '-not -name src' trick works even when building in the source tree (where > '$(srcdir)' is '.'), you could instead say: > > find $(srcdir) -maxdepth 1 -mindepth 1 -not -name $(SOURCEDISTNAME) \ > ... > ...yes, that is cruft, your solution will be used instead. > @@ +961,5 @@ > > +source-package:: clean > > + mkdir -p $(SOURCEDISTNAME)/js/src > > + find $(srcdir)/../src -maxdepth 1 -not -name src -not -name $(SOURCEDISTNAME) \ > > + -exec cp -r --target-directory=$(SOURCEDISTNAME)/js/src {} \+ > > + $(RM) $(patsubst %,$(SOURCEDISTNAME)/js/src/%,$(DIST_GARBAGE)) > > That's a surprising use of $(DIST_GARBAGE), but it makes sense. But what > will you do to clear out the .o files? Looking at my build dir, I also see > executables like host_jskwgen, host_jsoplengen, and js-config; Makefiles; ... > > I think you're going to need an opt-in approach, not an opt-out approach. Opt-in would be ideal but there's nothing in the build system right now that lists all the required files (that I could tell). However the above test (along with the 'clean' prerequisite, which should have taken care of *.o etc for you as it was) did not suffice. I have reworked this. New patch to follow...
fixed 'find' call, improved cleaning, use build system to ensure cleanup of in-source builds and skip when using out-of-source builds, confirmed tarball output, added LICENSE etc.
Attachment #689068 - Flags: review?(jimb)
Attachment #688802 - Attachment is obsolete: true
Same patch as before except that the prefix on the AUTHORS etc. cp command is fixed so it properly ignores the copy errors when files are missing.
Attachment #689068 - Attachment is obsolete: true
Attachment #689068 - Flags: review?(jimb)
Attachment #689190 - Flags: review?(jimb)
fixed header of patch to un-ambiguate which Makefile.in it applies to. Last update, honest!
Attachment #689190 - Attachment is obsolete: true
Attachment #689190 - Flags: review?(jimb)
Attachment #689224 - Flags: review?(jimb)
I'm not sure if a make target is the best place for this. See also bug 818970. But, I could be convinced to turn a blind eye ;) If you implemented this as a mach command, you could do everything in Python and you wouldn't have to worry about compatibility of commands across platforms (in theory). You would also be able to run this command from a fresh source tree without running configure. http://docs.python.org/2/library/archiving.html https://developer.mozilla.org/en-US/docs/Developer_Guide/mach#Adding_Features_to_mach
The same conclusion was reached on irc today (actually that's the reason bug 818970 was filed).. I'm looking into it. Perhaps supporting both methods -- a script for releng, and this for jsapi dev's or others? Given it will be a pain without the build system to ensure a clean sourcedir, I';m considering to have the script execute 'make source-package' iff 'Makefile' and probably config.status exists, and otherwise roll it on its own.
(In reply to Ian Stakenvicius from comment #13) > Given it will be a pain without the build system to ensure a clean > sourcedir, I';m considering to have the script execute 'make source-package' > iff 'Makefile' and probably config.status exists, and otherwise roll it on > its own. That would duplicate the source-making logic, and would likely lead to discrepancies if there are changes to be made some day. It would be better for such a make source-package rule to just use the script.
Yes, that would also work and ensure no duplication. Very friendly. Will go that route.
Attachment #689224 - Flags: review?(jimb)
Blocks: sm-embedding
Product: Core → Firefox Build System
Bug 517765 added js/src/make-source-package.sh which solves this. (Closing stale sm.embedding blockers)
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: