As mentioned once again in bug 508243 comment #2, we should probably use 'make source-package', introduced in bug 456373, for our source factories in the release harness.

That'll provide tarball with all the hg metadata stripped out, right ? Do you have any plans to extend that to provide a bundle which retains the history, and hence can be updated after the user downloads it ? We provide that for Firefox, ftp://ftp.mozilla.org/pub/mozilla.org/firefox/releases/3.5.2/source/

Yes, it does provide the tarball with hg and, if existing, CVS metadata stripped out (comm-central trees contain the LDAP c-sdk and possibly ChatZilla from CVS). We don't have a build system command for a hg bundle right now, but we still could do that the "old way" from the checkout, as we still need to pull the source in some way even if we run "make source-package" on it.

Mass move of bugs from Release Engineering:Future -> Release Engineering. See http://coop.deadsquid.com/2010/02/kiss-the-future-goodbye/ for more details.

Robert could you please have a look at this bug and check to see if any of the requirements have changed? I want to get a student to work on this. * bundle & packaged source? both? one or the other?

KaiRo is this still wanted?

Yes, we should do this. It doesn't affect KaiRo though, it was filed for our benefit.

Sorry for not reacting earlier - this would be nice for us all, as it would put more logic out of buildbot and into the Mozilla build system. I implemented "make source-package" in the build system, but it only does a source tarball at this time. If we want anything else, we still need to get that done there as well.

bug 437482 covers building hg bundles, so there's no need to worry about that. This sounds like a job for mozharness!

So to be clear, we're basically talking about replacing self.addStep(ShellCommand, command=['tar', '-cj', '--owner=0', '--group=0', '--numeric-owner', '--mode=go-w', '--exclude=.hg*', '--exclude=CVS', '--exclude=.cvs*', '-f', sourceTarball, self.branchName], workdir='.', description=['create tarball'], haltOnFailure=True ) in the CCSourceFactory with a call to 'make source-package'. Would the subsequent upload.py invocation need to change?

dustin, from how I see it, yes, that replacement should just do it, and the upload should in theory not change - that is, if the tarball name and location used now are the same as we defined as a standard in "make source-package".

I want to verify that these will build the same thing, but there are a lot of variables - not all of which I have access to. As I understand it, this build step is responsible for creating e.g., http://releases.mozilla.org/pub/mozilla.org/seamonkey/releases/2.0.10/source/seamonkey-2.0.10.source.tar.bz2 where the filename is generated with '%s-%s.source.tar.bz2' % (productName, version) and the directory name is generated by post_upload.py. On the build machine, the tarball is in the source/ directory. The tarball has a single top-level direntry, from self.branchName (see comment #9), which defaults to the repository name, so comm-central/ or, in the case of the tarball linked above, comm-1.9.1/. The source-package rule (in mozilla-central:toolkit/mozapps/installer/packager.mk) looks like this: ---- CREATE_SOURCE_TAR = $(TAR) -c --owner=0 --group=0 --numeric-owner \ --mode="go-w" --exclude=".hg*" --exclude="CVS" --exclude=".cvs*" -f # source-package creates a source tarball from the files in MOZ_PKG_SRCDIR, # which is either set to a clean checkout or defaults to $topsrcdir source-package: @echo "Packaging source tarball..." (cd $(MOZ_PKG_SRCDIR) && $(CREATE_SOURCE_TAR) - .) | bzip2 -vf > $(DIST)/$(PKG_SRCPACK_PATH)$(PKG_SRCPACK_BASENAME).tar.bz2 --- From my read, PKG_SRCPACK_BASENAME matches the format given above, so the tarball should arrive with the same name. However, there are two differences, one serious, one not. First, the make rule tars up ".", so the resulting tarball will not have a single top-level direntry. Will that change cause pain for users or automated consumers of these tarballs? Second, the make rule drops the tarball in $(DIST) which will evaluate to 'dist' in the comm-central directory, whereas the current buildstep is using an 'upload' directory sibling to the comm-central directory. This could be fixed with the build step, but might it be even better for Buildbot to just run 'make upload', with the appropriate $UPLOAD_ environment variables set? I don't think I have the capacity to stage this change for testing, nor to put it into production. I'm happy to create a patch (or two, to also fix the first problem above), but I'll need help to test and land it.

This is the buildbotcustom patch. I'd still like to hear back about the differently-structured tarballs before going to production.

Oops, -r1 won't work because the Makefiles haven't been set up yet. Hopefully a bare run of ./configure will be enough to get the Makefiles in place so this rule will work.

Comment on attachment 494496 [details] [diff] [review] buildbotcustom-r2.patch configure will likely need either a mozconfig, or a set of command-line options to build what you need. It may also require the env to be set. We aren't setup to run CC releases, in staging or otherwise. Did you mean to be changing SingleSourceFactory instead/as well so we can test and then hopefully have something that KaiRo can use too?

There are a few blockers to setting this up in Buildbot: - source-package rule creates an incorrect source tarball - source-package rule requires a configured source directory - source-package rule will tar up the results of that config From some discussion in #developers, it looks like it would be best if the build scripts can run 'make -f client.mk source-package' without first configuring or building anything. It would simplify things if 'make upload' also ran from client.mk. It would further simplify things if 'make upload' also detected '*.hgbundle' files in the dist/ directory, and even more so if a 'make bundle' command could create such files. I'm not sure who's best-qualified to make those changes.

(In reply to comment #15) > There are a few blockers to setting this up in Buildbot: > - source-package rule creates an incorrect source tarball What does 'incorrect' mean? Different dir structure, missing/extra files?

Different dir structure. See the para beginning with "First," in comment #11. It may also have extra files, although I'm told it shouldn't if you use an objdir.

(In reply to comment #17) > Different dir structure. See the para beginning with "First," in comment #11. Given this, do we need a blocking bug to get the make target fixed to be on par with our release tar command first?

I'm playing with make target right now. At the very least, I'll need to exclude the dist/ dir, because we seem to be recursively packaging there.

This tested fine in staging, generating source packages that match what we currently provide for Firefox and XULRunner. The excludes needed some beefing up since we need to run configure before the make target will work.

I broke the addConfigSteps() part out into it's own def rather than just inlining the steps in case we want to factor those steps out and push them up into MozillaBuildFactory() at some point. Tested in staging in conjunction with the m-c patch.

Comment on attachment 515109 [details] [diff] [review] Fix make source-package target to match what we currently provide as source for releases >diff --git a/toolkit/mozapps/installer/packager.mk b/toolkit/mozapps/installer/packager.mk >--- a/toolkit/mozapps/installer/packager.mk > CREATE_SOURCE_TAR = $(TAR) -c --owner=0 --group=0 --numeric-owner \ >- --mode="go-w" --exclude=".hg*" --exclude="CVS" --exclude=".cvs*" -f >+ --mode="go-w" --exclude="dist/*" --exclude=".hg*" --exclude="CVS" \ >+ --exclude=".cvs*" --exclude=".mozconfig*" --exclude="Makefile" \ >+ --exclude="config.*" -f and autoconf.mk (various locations). [probably more stuff too] How about making this something like an: SRC_TAR_EXCLUDE_PATHS += ... so that any app's (build.mk) can extend it. And then doing $(addprefix ...) in the CREATE_SOURCE_TAR here, for the --exclude=""?

(In reply to comment #22) > and autoconf.mk (various locations). [probably more stuff too] How close is good enough here? After doing some pattern-based exclusions, there a still a bunch of individual files that appear in the source package but not a simple tarball: +mozilla-central/netwerk/necko-config.h +mozilla-central/unallmakefiles +mozilla-central/js/src/js-config.h +mozilla-central/js/src/js-confdefs.h +mozilla-central/js/src/unallmakefiles +mozilla-central/js/src/ctypes/libffi/fficonfig.h +mozilla-central/js/src/ctypes/libffi/libffi.pc +mozilla-central/js/src/ctypes/libffi/include/ffitarget.h +mozilla-central/js/src/ctypes/libffi/include/ffi.h +mozilla-central/js/src/ctypes/libffi/stamp-h1 +mozilla-central/js/src/ctypes/libffi/libtool +mozilla-central/js/src/js-config +mozilla-central/xpcom/xpcom-config.h +mozilla-central/xpcom/xpcom-private.h +mozilla-central/nsprpub/unallmakefiles +mozilla-central/nsprpub/config/nspr-config +mozilla-central/nsprpub/config/nsprincl.mk +mozilla-central/nsprpub/config/nsprincl.sh +mozilla-central/config/doxygen.cfg +mozilla-central/mozilla-config.h +mozilla-central/gfx/cairo/cairo/src/cairo-features.h Should I add an exclude file to track these? Should we just ignore them? > SRC_TAR_EXCLUDE_PATHS += ... > > so that any app's (build.mk) can extend it. > > And then doing $(addprefix ...) in the CREATE_SOURCE_TAR here, for the > --exclude=""? Sounds good. I'll play around with the above to make it work.

I actually wonder how many of the files mentioned in comment #23 are bugs and should be filed. We should try hard not to write to the source dir during a normal build. Also, I wonder how many of those are actually a concern in a source factory, which doesn't do an actual build before calling "make source-package".

The exclude file contains all of the files that are missing from the simple tar file when compared to the package generated by make source-package. The package contents generated by the two methods are now identical, modulo all the .cvsignore ang .hg* files that the simple was/is failing to exclude. I added explicit excludes for $(TOP_LEVEL_DIRNAME)/Makefile and $(TOP_LEVEL_DIRNAME)/dist because they end up being interpreted as wildcards and preventing other files in the tree from being packaged, e.g. we have some Makefiles in the tree that exist prior to running configure. Outputting the package into $(MOZ_PKG_SRCDIR)/../$(PKG_SRCPACK_PATH) seems hacky to be, but it gives up drop-in parity with the existing source tar command for Firefox and XR. Tested in staging for both Firefox and XR. We'll need to backport these changes to 1.9.2 and 1.9.1 before we can land the buildbotcustom changes, naturally.

Comment on attachment 516336 [details] [diff] [review] Use file-based excludes for make source-package any chance we can create two source-package.exclude files, so that toolkit/-based apps don't need to constantly (?!) update their own .exclude file to accomodate, and can then use the toolkit one for everything common? Also, if we are naming this SRC_TAR_EXCLUDE_PATHS do we really want/need to prepend -X to each path ourselves? ($(addprefix -X ...) should work, no?) And finally a nit: I can imagine people getting tripped up from where the SRC_TAR_EXCLUDE_PATHS is relative from, (I would have expected at a glance of only the installer/Makefile.in to be relative to srcdir, not topsrcdir) If this is too much hassle/work for what you are trying to accomplish here, I think the c-c apps can deal, but I'd certainly rather not. (holding out on a specific review, incase you are willing to address those issues)

A small changes from the previous patch: * set MOZ_PKG_PRETTYNAMES and MOZ_PKG_VERSION via env so we can predictably put the tarball into the source/ dir with the bundle, and so that post_upload then puts them in the expected place on stage. The rest is the same.

(In reply to comment #26) > any chance we can create two source-package.exclude files, so that > toolkit/-based apps don't need to constantly (?!) update their own .exclude > file to accomodate, and can then use the toolkit one for everything common? Is there a simple way for me to tell which excludes would go into each file, short of building a bunch of other projects? > Also, if we are naming this SRC_TAR_EXCLUDE_PATHS do we really want/need to > prepend -X to each path ourselves? ($(addprefix -X ...) should work, no?) What happens if a project wants to include non-file-based --excludes here? I know the naming of the SRC_TAR_EXCLUDE_PATHS var is not accurate in that case, but addprefix would preclude that AFAICT. > And finally a nit: I can imagine people getting tripped up from where the > SRC_TAR_EXCLUDE_PATHS is relative from, (I would have expected at a glance of > only the installer/Makefile.in to be relative to srcdir, not topsrcdir) Not sure what to do here. The actual tar command as executed cd's to $(MOZ_PKG_SRCDIR), so relative paths would be broken. > If this is too much hassle/work for what you are trying to accomplish here, I > think the c-c apps can deal, but I'd certainly rather not. (holding out on a > specific review, incase you are willing to address those issues) I'm not a build system hacker. Perhaps that much is obvious. Your pointers are helpful, especially since I'd be punting this bug to the Fx build system if I actually thought they had time to work on it.

(In reply to comment #28) > (In reply to comment #26) > > any chance we can create two source-package.exclude files, so that > > toolkit/-based apps don't need to constantly (?!) update their own .exclude > > file to accomodate, and can then use the toolkit one for everything common? > > Is there a simple way for me to tell which excludes would go into each file, > short of building a bunch of other projects? In short, I think anything in /browser/* is firefox, anything outside can be considered toolkit. There are exceptions in both directions, but they should be few and far between. and *probably* irrelevant for at least the m-c side. > > Also, if we are naming this SRC_TAR_EXCLUDE_PATHS do we really want/need to > > prepend -X to each path ourselves? ($(addprefix -X ...) should work, no?) > > What happens if a project wants to include non-file-based --excludes here? I > know the naming of the SRC_TAR_EXCLUDE_PATHS var is not accurate in that case, > but addprefix would preclude that AFAICT. Fair enough, I think we can keep it then. (I had not considered that and I have no strong feelings on this one). > > And finally a nit: I can imagine people getting tripped up from where the > > SRC_TAR_EXCLUDE_PATHS is relative from, (I would have expected at a glance of > > only the installer/Makefile.in to be relative to srcdir, not topsrcdir) > > Not sure what to do here. The actual tar command as executed cd's to > $(MOZ_PKG_SRCDIR), so relative paths would be broken. Yea, was a nit, as I had a feeling it was something tricky like that, I suppose we could make it an absolute path, but I worry that we'll still be playing with fire attempting to do that.

Same as attachment 516336 [details] [diff] [review] except I took Callek's suggestion and split the exclude list into two files for easier maintenance by other projects.

Hrm, another list of all Makefiles sounds like a maintenance hassle. Why are those ending up in the srcdir anyhow? those should never ever end up in there but only in the objdir. If they appear there, it sounds like the configure was done in the srcdir instead of in an objdir...

(In reply to comment #31) > Hrm, another list of all Makefiles sounds like a maintenance hassle. Why are > those ending up in the srcdir anyhow? those should never ever end up in there > but only in the objdir. If they appear there, it sounds like the configure was > done in the srcdir instead of in an objdir... Without any idea on difficulty (as I haven't thought of this more than 5 minutes), is it possible to source allmakefiles.sh in some way for this exclusion? And/or let the source package error if we have generated Makefiles in it?

(In reply to comment #32) > And/or let the source package error if we have generated Makefiles in it? That's not a bad idea. I'm totally in favor of having "make source-package" error out if srcdir=objdir.

(In reply to comment #31) > Hrm, another list of all Makefiles sounds like a maintenance hassle. Why are > those ending up in the srcdir anyhow? those should never ever end up in there > but only in the objdir. If they appear there, it sounds like the configure was > done in the srcdir instead of in an objdir... How does one avoid doing this? AFAICT, I need to run configure from the srcdir to create the necessary source-package make target.

(In reply to comment #34) > (In reply to comment #31) > > Hrm, another list of all Makefiles sounds like a maintenance hassle. Why are > > those ending up in the srcdir anyhow? those should never ever end up in there > > but only in the objdir. If they appear there, it sounds like the configure was > > done in the srcdir instead of in an objdir... > AFAICT, I need to run configure from the srcdir to create the necessary > source-package make target. We do need to run configure, I'm unsure if we need to run it in srcdir as objdir should be possible, but probably would require buildbot changes. But I do have the solution, basically... |@objdir@/unallmakefiles| in a configure-run setup, which could be @topsrcdir@/unallmakefiles in this case. It lists _all_ the MAKEFILES we generated by running configure, we may need some additional excludes, but we should be able to pass that as an exclude list/file. Note this file is space-delim, not newline delim, I forget if the -X works with that, or if we need to process the file to get a useable exclude list.

(In reply to comment #35) > > AFAICT, I need to run configure from the srcdir to create the necessary > > source-package make target. > > We do need to run configure, I'm unsure if we need to run it in srcdir as > objdir should be possible, but probably would require buildbot changes. I can change the buildbot steps if I need to. Right now I'm running the equivalent of: cd srcdir autoconf-2.13 cd js/src && autoconf-2.13 && cd ../.. ./configure Is the correct way to setup the objdir for this to run "make -f client.mk configure" rather than a straight "./configure" ?

(In reply to comment #36) > Is the correct way to setup the objdir for this to run "make -f client.mk > configure" rather than a straight "./configure" ? I think it should make life easier, but I'd rather let bhearsum (or someone else) tell us if changing to an OBJDIR setup for this step is best. [Since I am not technically even a peer of this code] Note that requiring an object directory would make anyone trying this from a source-directory configure have those Makefiles included.

(In reply to comment #36) > I can change the buildbot steps if I need to. Right now I'm running the > equivalent of: > > cd srcdir > autoconf-2.13 > cd js/src && autoconf-2.13 && cd ../.. > ./configure > > Is the correct way to setup the objdir for this to run "make -f client.mk > configure" rather than a straight "./configure" ? A better way would be: mkdir objdir cd srcdir autoconf-2.13 cd js/src && autoconf-2.13 && cd ../.. cd ../objdir ../srcdir/configure And then execute "make source-package" also from that objdir. Note that the step with the autoconf needs to be subclassable so that SeaMonkey/Thunderbird can deal with their different layout there. Of course, another possibility for the prep steps would be to have some mozconfig we can use and just do a "make -f client.mk configure", which would be the same across all projects again.

(In reply to comment #38) > Of course, another possibility for the prep steps would be to have some > mozconfig we can use and just do a "make -f client.mk configure", which would > be the same across all projects again. I fear we're over-rotating on hypotheticals, so let's talk specifics. Does SeaMonkey have a mozconfig it could pass to the source-package step? That's what I'm strongly leaning towards for Firefox. If I pass the same mozconfig and env that I use for a linux release build for Firefox, the source-package step becomes dead simple, i.e. cd srcdir && make -f client.mk configure cd objdir && make source-package I can do the autoconf+configure dance instead if that's easier for SeaMonkey, it's just replicating the steps in "make -f client.mk configure."

(In reply to comment #39) > That's what I'm strongly leaning towards for Firefox. If I pass the same > mozconfig and env that I use for a linux release build for Firefox, the > source-package step becomes dead simple That's what I was suggesting with the last paragraph of comment #38 - and yes, a linux release mozconfig probably works for everyone, including SeaMonkey or Thunderbird.

Following the plan set out in comment #39 * pass env and objdir to factories * use "make -f client.mk configure" with an objdir to simplify buildbot steps * use mozillaSrcDir and mozillaObjdir (like repack steps) to play nicely with SeaMonkey

A bit of a simpler patch for ted. If we run make source-package from an objdir, we hardly need to exclude anything. Thanks to KaiRo and Callek for helping me realize that. This patch works gives us parity with the current source tarballs we create for Firefox and XR.

Comment on attachment 517554 [details] [diff] [review] Add a source-package target for XR; exclude some transient files from the source-package >+ifdef MOZ_OBJDIR >+SRC_TAR_EXCLUDE_PATHS += --exclude="$(MOZ_OBJDIR)" >+endif This addition only makes sense if MOZ_OBJDIR is under the srcdir, and in which case, it can also be specified with a relative path, lets make this an absolute |--exclude="$(call core_abspath,$(MOZ_OBJDIR))"| > source-package: > @echo "Packaging source tarball..." >- (cd $(MOZ_PKG_SRCDIR) && $(CREATE_SOURCE_TAR) - .) | bzip2 -vf > $(DIST)/$(PKG_SRCPACK_PATH)$(PKG_SRCPACK_BASENAME).tar.bz2 >+ mkdir -p $(MOZ_PKG_SRCDIR)/../$(PKG_SRCPACK_PATH) >+ (cd $(MOZ_PKG_SRCDIR) && $(CREATE_SOURCE_TAR) - ../$(TOP_LEVEL_DIRNAME)) | bzip2 -vf > $(MOZ_PKG_SRCDIR)/../$(PKG_SRCPACK_PATH)$(PKG_SRCPACK_BASENAME).tar.bz2 this is essentially (for seamonkey) now, |(cd @topsrcdir@ && tar .. -f - ../source/mozilla ......| which surely is broken. Because this will change things to only package the MOZILLA directory, can you explain why you need the change from |- .| to |- ../$(TOP_LEVEL_DIRNAME)| since TOP_LEVEL_DIRNAME is actually MOZILLA_SRC_DIRNAME at this point. All in all, looks like a great improvement over the earlier patches, but f- mostly on the sense that this looks like its going to break source packages for comm-central.

Comment on attachment 517532 [details] [diff] [review] Use objdir for making the source-package For what it is worth, I actually am noticing a lot of stuff I expect to fail, be wrong, or just plain painful for this factory in a c-c based app. However, I note that no c-c based app (Thunderbird or SeaMonkey) makes any use of this factory yet, so I think I can safely ignore my concerns with your changes and the existing code for now, and address it when I am able to move us to it.

(In reply to comment #44) > For what it is worth, I actually am noticing a lot of stuff I expect to fail, > be wrong, or just plain painful for this factory in a c-c based app. However, I > note that no c-c based app (Thunderbird or SeaMonkey) makes any use of this > factory yet, so I think I can safely ignore my concerns with your changes and > the existing code for now, and address it when I am able to move us to it. Gah! I thought SeaMonkey *was* using this factory, that's why I spent so much time trying to get it right. It's like the tail wagging the dog otherwise. Are you using the source-package target at least? (In reply to comment #43) (In reply to comment #43) > This addition only makes sense if MOZ_OBJDIR is under the srcdir, and in which > case, it can also be specified with a relative path, lets make this an absolute > |--exclude="$(call core_abspath,$(MOZ_OBJDIR))"| Firefox has habitually used an objdir under the source dir. If the objdir isn't under the source dir, wouldn't it be ignored anyway? The expansion doesn't work for Firefox anyway. It ends up as |--exclude="/builds/slave/coop/source-package/mozilla-central/obj-firefox/browser/installer/obj-firefox"| here. > this is essentially (for seamonkey) now, |(cd @topsrcdir@ && tar .. -f - > ../source/mozilla ......| which surely is broken. Because this will change > things to only package the MOZILLA directory, can you explain why you need the > change from |- .| to |- ../$(TOP_LEVEL_DIRNAME)| since TOP_LEVEL_DIRNAME is > actually MOZILLA_SRC_DIRNAME at this point. Is MOZILLA_SRC_DIRNAME an actual define somewhere? I've changed the |- .| to |- ../$(TOP_LEVEL_DIRNAME)| so that the source package ends up with a single root dir that all the files are packaged into. Can you tell me what *would* work for SeaMonkey then? An example of your ideal source-package command that I could compare with the Firefox/XR one would actually allow me to look for similarities. If your not using the target right now, then maybe it's moot. I'll remove the $(MOZ_PKG_SRCDIR)/../$(PKG_SRCPACK_PATH) and move the file to the final location in buildbot instead. Getting kinda sick of this bug, though. :/

(In reply to comment #45) > Gah! I thought SeaMonkey *was* using this factory, that's why I spent so much > time trying to get it right. It's like the tail wagging the dog otherwise. Are > you using the source-package target at least? Well, the *current* factory doesn't work for us, so we have a very similar one we crafted after it for our purposes because we knew this bug would come along and change it in a way that we could all use the same one. I think it's good we're trying to go some distance here to make this easier and go the last steps when Callek switches us over and can test what's still to do - of course, in a followup. And yes, the source-package target exists in the toplevel on comm-central and should do the right thing already. Not sure if it works 100% fine when called from the mozilla/ level, but from the toplevel IIRC I made it to work correctly. > Firefox has habitually used an objdir under the source dir. If the objdir isn't > under the source dir, wouldn't it be ignored anyway? Yes, that's the easiest way.

Comment on attachment 517532 [details] [diff] [review] Use objdir for making the source-package Going to need to make some changes here to work with the new config file format.

A small delta from the previous patch to: * cope with the new release config file structure * move the tarball to the proper spot for uploading due to reverted changes in the make target

Simplified some defs from the previous patch based on discussions with Callek. SeaMonkey can set |DIR_TO_BE_PACKAGED="."| to have the target work as it used to.

Comment on attachment 518137 [details] [diff] [review] Add a source-package target for XR; exclude some transient files from the source-package, v2 >+TOP_LEVEL_DIRNAME = $(shell basename $(MOZILLA_DIR)) >+ifndef DIR_TO_BE_PACKAGED >+DIR_TO_BE_PACKAGED = ../$(TOP_LEVEL_DIRNAME) >+endif |DIR_TO_BE_PACKAGED ?= ../$(shell basename $(topsrcdir))| should actually support your use case, and do the right thing for comm-central apps. and STILL allow the override if any app thinks this is a bad idea. but looks good either way, (we can always set that ourselves, but I think it would be best to have that here)

Incorporates Justin's suggestion from comment #50. Ted: I'm quite happy to target someone else for review if you want to point me at someone.

I'll give it a shot, I just haven't taken a pass through my review queue this week yet. I will tomorrow or Friday.

Comment on attachment 518180 [details] [diff] [review] Add a source-package target for XR; exclude some transient files from the source-package, v5 >diff --git a/toolkit/mozapps/installer/packager.mk b/toolkit/mozapps/installer/packager.mk >--- a/toolkit/mozapps/installer/packager.mk >+++ b/toolkit/mozapps/installer/packager.mk >@@ -719,16 +719,25 @@ upload: checksum > $(PYTHON) $(MOZILLA_DIR)/build/upload.py --base-path $(DIST) \ > $(UPLOAD_FILES) \ > $(CHECKSUM_FILE) > > ifndef MOZ_PKG_SRCDIR > MOZ_PKG_SRCDIR = $(topsrcdir) > endif > >+DIR_TO_BE_PACKAGED ?= ../$(shell basename $(topsrcdir)) Make has a $(notdir) function that does this: http://www.gnu.org/software/make/manual/make.html#File-Name-Functions >+SRC_TAR_EXCLUDE_PATHS += --exclude=".hg*" --exclude="CVS" --exclude=".cvs*" \ I feel like this might read better if you put a continuation after the +=, and listed one --exclude per line. Looks fine otherwise.

Comment on attachment 518118 [details] [diff] [review] Move tarball into the proper dir for upload I'm having trouble understanding why the build system doesn't generate the tarball in the right place. I thought that was one of the benefits of using source-package instead of doing it manually. Why doesn't it?

(In reply to comment #54) > I'm having trouble understanding why the build system doesn't generate the > tarball in the right place. I thought that was one of the benefits of using > source-package instead of doing it manually. Why doesn't it? There is no "right" place, there's only the spot where we've been putting the bundle and tarball files up until now. That dir happens to be named source/ which lives in the buildbot build dir alongside the actual srcdir. The make targets (understandably) want to create things under the srcdir/objdir, usually under dist/. Until now, we've been creating both bundle and tarball by hand. Unless we create a |make bundle| target, we'll still be doing that by hand anyway, and can direct that bundle output anywhere. If I changed the location on disk where we create the bundle file to match where the tarball gets created, that would avoid the need for a mv command. Is that a better approach?

Ah, sorry. I understand now. My comments were based on the assumption that we were using 'make upload' rather than upload.py, directly. Given that, this looks fine!

Comment on attachment 518180 [details] [diff] [review] Add a source-package target for XR; exclude some transient files from the source-package, v5 I've incorporated Ted's suggestions into a patch that's ready for landing. This needs to land on every branch we release off of (including m-c) with the possible exception of mozilla-2.1 since Fennec doesn't actually use |make source-package|, but we could land it there too for parity. This patch changes release packaging only, so can be landed as NPOTB with DONTBUILD set.

Comment on attachment 518180 [details] [diff] [review] Add a source-package target for XR; exclude some transient files from the source-package, v5 Not this time, non-blocker code-freeze was last week.

Comment on attachment 518180 [details] [diff] [review] Add a source-package target for XR; exclude some transient files from the source-package, v5 Landed on m-c: http://hg.mozilla.org/mozilla-central/rev/7e1308fc3b70

Still waiting on approvals here for all legacy branches. I'll note that this code has been in m-c for over a month now, and aurora since it branched.

Comment on attachment 518180 [details] [diff] [review] Add a source-package target for XR; exclude some transient files from the source-package, v5 Approved for 1.9.2.18 and 1.9.1.20, a=dveditz for release-drivers Not planning more 2.0 based releases, but approved for the mozilla2.0 repository, a=dveditz for release-drivers if you want it.

Comment on attachment 518180 [details] [diff] [review] Add a source-package target for XR; exclude some transient files from the source-package, v5 http://hg.mozilla.org/releases/mozilla-2.0/rev/fce895bf8852 http://hg.mozilla.org/releases/mozilla-1.9.2/rev/c4d0a908b8ae http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b5e3b71282de

Comment on attachment 518118 [details] [diff] [review] Move tarball into the proper dir for upload http://hg.mozilla.org/build/buildbotcustom/rev/463c4e015f20

Needs merging to production-0.8 and a reconfig.

Armen merged and reconfig-ed with this patch this morning.

(In reply to comment #62) > http://hg.mozilla.org/releases/mozilla-2.0/rev/fce895bf8852 > http://hg.mozilla.org/releases/mozilla-1.9.2/rev/c4d0a908b8ae > http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b5e3b71282de Please mark the appropriate "status-foo" fields as fixed when you check in to branch repos, it will help release management a lot.