Closed
Bug 479475
Opened 16 years ago
Closed 6 years ago
SpiderMonkey source distribution makefile target
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
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).
Updated•16 years ago
|
Assignee: nobody → jim
Reporter | ||
Comment 1•16 years ago
|
||
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.
Reporter | ||
Comment 2•16 years ago
|
||
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).
Comment 3•13 years ago
|
||
I'm not interested in working on this any more. De-assigning bug.
Assignee: jimb → nobody
Comment 4•12 years ago
|
||
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)
Comment 5•12 years ago
|
||
... currently the patch doesn't add LICENSE, LEGAL and README.txt from $(topsrcdir), it should do that.
Comment 6•12 years ago
|
||
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 7•12 years ago
|
||
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.
Updated•12 years ago
|
Attachment #688802 -
Flags: review?(jimb)
Comment 8•12 years ago
|
||
(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...
Comment 9•12 years ago
|
||
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)
Updated•12 years ago
|
Attachment #688802 -
Attachment is obsolete: true
Comment 10•12 years ago
|
||
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)
Comment 11•12 years ago
|
||
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)
Comment 12•12 years ago
|
||
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
Comment 13•12 years ago
|
||
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.
Comment 14•12 years ago
|
||
(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.
Comment 15•12 years ago
|
||
Yes, that would also work and ensure no duplication. Very friendly. Will go that route.
Updated•12 years ago
|
Attachment #689224 -
Flags: review?(jimb)
Updated•12 years ago
|
Blocks: sm-embedding
Updated•7 years ago
|
Product: Core → Firefox Build System
Comment 16•6 years ago
|
||
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.
Description
•