Jason Orendorff [:jorendorff] Reporter
	Description • 15 years ago

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).

	Jason Orendorff [:jorendorff] Reporter
	Comment 1 • 15 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.

	Jason Orendorff [:jorendorff] Reporter
	Comment 2 • 15 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).

	Jim Blandy :jimb
	Comment 3 • 12 years ago

I'm not interested in working on this any more. De-assigning bug.

	Ian Stakenvicius
	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

	Ian Stakenvicius
	Comment 5 • 12 years ago

...  currently the patch doesn't add LICENSE, LEGAL and README.txt from $(topsrcdir), it should do that.

	Jim Blandy :jimb
	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.

	Jim Blandy :jimb
	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.

	Ian Stakenvicius
	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...

	Ian Stakenvicius
	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.

	Ian Stakenvicius
	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.

	Ian Stakenvicius
	Comment 11 • 12 years ago

fixed header of patch to un-ambiguate which Makefile.in it applies to.  Last update, honest!

	Gregory Szorc [:gps]
	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

	Ian Stakenvicius
	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.

	Mike Hommey [:glandium]
	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.

	Ian Stakenvicius
	Comment 15 • 12 years ago

Yes, that would also work and ensure no duplication.  Very friendly.  Will go that route.

	Ted Campbell [:tcampbell]
	Comment 16 • 6 years ago

Bug 517765 added js/src/make-source-package.sh which solves this. (Closing stale sm.embedding blockers)