I have decided to have a python script and a make target rather than just a script because of the following reasons: * it is easier to build a name like this: firefox-3.5pre.fr.mac.complete.mar through the build system rather than pulling buildbot properties left and right * it allows anyone from the build system just to define three variables and generate the snippet (this could be reduced to only one)

This pretty much gets called like this: self.addStep(ShellCommand, command=['make', WithProperties('generateSnippet-%(locale)s')], workdir='build/%s/%s/locales' % (self.branchName,self.appName), description=['generating', 'complete', 'snippet'], descriptionDone=['snippet', 'generated'], env={'DOWNLOAD_BASE_URL':self.downloadBaseURL, 'BUILDID':WithProperties('%(buildid)s'), 'MILESTONE':self.branchName} )

Comment on attachment 381385 [details] [diff] [review] python script that generates a snippet for a locale "dist/update/complete.update.snippet" >+def main(): >+ parser = OptionParser( >+ usage="%prog <absoluteDistDir> <completeMarFile> <baseUrl> "+ \ >+ "<buildid> <appVersion> <milestone>") >+ (options, args) = parser.parse_args() >+ if len(args)!=6: >+ parser.error("incorrect number of arguments") >+ else: >+ writeSnippetToDisk(args[0], args[1], args[2], args[3], args[4], args[5]) There's really no reason to use optparse if you're not going to use named vars (which I would prefer). Rest looks fine.

Comment on attachment 381383 [details] [diff] [review] adds the buildid in the ident target and a target to call the python script that generates the snippet Is there a reason you're creating a python script and then calling it via the build system? Wouldn't it make more sense to use/extend the existing snippet code in http://hg.mozilla.org/build/buildbotcustom/file/default/steps/updates.py ?

It has been said before, but either way we're doing it, I'd very much prefer if we have a single point in code (wherever it ends up) which creates snippets for as many cases as possible, at least both en-US and L10n, ideally we'd find a way to even have complete and partial snippet generation in one place (but that latter thing is harder from all I know).

Comment on attachment 381385 [details] [diff] [review] python script that generates a snippet for a locale "dist/update/complete.update.snippet" r- for the following: What coop said about the params, too. I don't think this script should be in config. tools/update-packaging sounds more like it. The dated-dir creation belongs into the buildbot logic, that should be shared with whatever is doing the uploads. The hard-coding of 'update' is wrong, please use COMPLETE_MAR in the Makefile to call in here. I'd prefer a single formatted string for the complete.update.snippet like snippet = '''complete %(marDownloadURL)s sha1 %(completeMarHash)s ... ''' % dict.. And then write that in one go.

Note, I'm not a friend of the updates.py step. It's doing the snippet on the master and then downloads it to the slave, that feels a bit icky, and it requires a good deal of preparation steps to get all the properties into the build properties that the master step then uses. Data, that, AFAICT, the slave mostly has locally anyway. Thus the idea to create a script on the slave side that just does that in one go.

(In reply to comment #8) > Note, I'm not a friend of the updates.py step. It's doing the snippet on the > master and then downloads it to the slave, that feels a bit icky, and it > requires a good deal of preparation steps to get all the properties into the > build properties that the master step then uses. Data, that, AFAICT, the slave > mostly has locally anyway. Thus the idea to create a script on the slave side > that just does that in one go. Personally, I'm very happy to get rid of updates.py, too.

Comment on attachment 381383 [details] [diff] [review] adds the buildid in the ident target and a target to call the python script that generates the snippet The paths are wrong here, package-names.mk knows them already. I wonder if the snippet piece should go into http://mxr.mozilla.org/mozilla-central/source/toolkit/locales/l10n.mk#143 Don't echo -n, printf instead, too. Where do the en-US builds get BuildID and Milestone from? I'm wondering if this calls for some factorization of those code paths.

(In reply to comment #5) > (From update of attachment 381383 [details] [diff] [review]) > Is there a reason you're creating a python script and then calling it via the > build system? > > Wouldn't it make more sense to use/extend the existing snippet code in > http://hg.mozilla.org/build/buildbotcustom/file/default/steps/updates.py ? > I tried to make use of it with attachment 380157 [details] [diff] [review] with "Bug 495100 - browser/locales - snippetProperties-% target". As others have mentioned, the info is in the slave and makes more sense to move away from updates.py It seems that we all agree that we want the en-US scenario as well with whatever script we end up having. This means that the goal would basically be missed at this point since I wouldn't like to rush it if we have to test the en-US scenario. Aside of that I would like for next quarter to work on bug 410806 which would make the slaves to generate even the partial updates locally without having to store date folders on ftp.

(In reply to comment #10) > Where do the en-US builds get BuildID and Milestone from? I'm wondering if this > calls for some factorization of those code paths. > BuildID through http://mxr.mozilla.org/build/source/buildbot/buildbot/slave/commands.py#2914 Milestone through http://mxr.mozilla.org/build/source/buildbotcustom/process/factory.py#671

(In reply to comment #11) > It seems that we all agree that we want the en-US scenario as well with > whatever script we end up having. This means that the goal would basically be > missed at this point since I wouldn't like to rush it if we have to test the > en-US scenario. We discussed this becoming a larger project next quarter. Armen: does your python script work in the regular en-US nightly case, i.e. does it generate an identical snippet? No sense writing a script today and then replacing it next month when we do start fixing updates more generally.

Comment on attachment 381383 [details] [diff] [review] adds the buildid in the ident target and a target to call the python script that generates the snippet I'm concerned about BUILDID and MILESTONE too. Do we ever expect this target to be run outside of buildbot? r- for issues Axel has already raised.

(In reply to comment #13) > (In reply to comment #11) > > It seems that we all agree that we want the en-US scenario as well with > > whatever script we end up having. This means that the goal would basically be > > missed at this point since I wouldn't like to rush it if we have to test the > > en-US scenario. > > We discussed this becoming a larger project next quarter. > > Armen: does your python script work in the regular en-US nightly case, i.e. > does it generate an identical snippet? No sense writing a script today and then > replacing it next month when we do start fixing updates more generally. > The script was based on how the en-US snippet gets generated so the logic is the same. I will have to work tomorrow on testing the en-US scenario. (In reply to comment #14) > (From update of attachment 381383 [details] [diff] [review]) > I'm concerned about BUILDID and MILESTONE too. Do we ever expect this target to > be run outside of buildbot? > I will assume that we won't and try to follow Axel's comment#7. I will be back to this bug tomorrow; I am setting up my machine to work for the repackage-on-change scenario.

(In reply to comment #7) > (From update of attachment 381385 [details] [diff] [review]) > The dated-dir creation > belongs into the buildbot logic, that should be shared with whatever is doing > the uploads. > The dated-dir is not passed to the make target that does the upload (if I am not mistaken) it gets calculated in post_upload.py. http://mxr.mozilla.org/mozilla-central/source/browser/locales/Makefile.in#435 Any parameters passed are through self.uploadEnv http://mxr.mozilla.org/build/source/buildbotcustom/process/factory.py#959 (postUploadCmd is defined in http://mxr.mozilla.org/build/source/buildbotcustom/process/factory.py#1159) (Seriously I get lost with so many pieces) With this I think that it is OK for now to have it on the slave side. (In reply to comment #14) > (From update of attachment 381383 [details] [diff] [review]) > I'm concerned about BUILDID and MILESTONE too. Do we ever expect this target to > be run outside of buildbot? > I am going to pull the BUILID and MILESTONE from application.ini so there will be no need of it. Aside of that, have I heard through either IRC or any of our calls that we would prefer not have the make target call the python script? and why not if so? If I did everything from the python script, I would only need the locale and the downloadBaseURL to be passed to it. - buildid and milestone can be extracted from application.ini - how could I determine the platform inside the script? - the product name also appears in application.ini I only feel that I would be doing three subcalls to "python config/printconfigsetting.py" to extract buildid, milestone and product name. (In reply to comment #10) > I wonder if the snippet piece should go into > http://mxr.mozilla.org/mozilla-central/source/toolkit/locales/l10n.mk#143 > Not a bad idea if everytime we generate a MAR file we also want to generate the snippet for it.

(In reply to comment #16) > Aside of that, have I heard through either IRC or any of our calls that we > would prefer not have the make target call the python script? and why not if > so? I can't speak for others, but my concern is consistency. I want the process for l10n updates to look like the process for en-US, with the eventual goal of making nightly and release updates the same as well. I just don't want another code path to wrangle in next quarter.

(In reply to comment #17) > (In reply to comment #16) > > Aside of that, have I heard through either IRC or any of our calls that we > > would prefer not have the make target call the python script? and why not if > > so? > > I can't speak for others, but my concern is consistency. I want the process for > l10n updates to look like the process for en-US, with the eventual goal of > making nightly and release updates the same as well. I just don't want another > code path to wrangle in next quarter. I agree 1000% with this.

(In reply to comment #10) > I wonder if the snippet piece should go into > http://mxr.mozilla.org/mozilla-central/source/toolkit/locales/l10n.mk#143 > Does l10n.mk just exist on mozilla-central?

(In reply to comment #19) > Does l10n.mk just exist on mozilla-central? Currently, yes, but you could discuss getting it landed on 1.9.1 with Axel if it's going to make things easier.

Comment on attachment 382769 [details] [diff] [review] make target that calls the python script to generate a snippet plus change to make ident Looks good to me, but Axel gets final say here.

The good run's log: http://tinderbox.mozilla.org/showlog.cgi?log=MozillaStaging/1244749564.1244749642.26754.gz&fulltext=1 If you search for "snippet" in the log you will be able to see that the snippet points to the right URL: http://staging-stage.build.mozilla.org/pub/mozilla.org/firefox/nightly//2009/06/2009-06-11-03-mozilla-1.9.1-l10n/firefox-3.5pre.fr.linux-i686.complete.mar as well as in here: http://staging-stage.build.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-1.9.1-l10n/ Here is where the snippet was uploaded: http://localhost:8081/aus2/build/0/Firefox/mozilla-1.9.1/Linux_x86-gcc3/20090611030910/fr/complete.txt If the script in staging-stage works and we get a couple of nightly l10n repacks, we should be able to download one and be able to update to the next one. The only problem is that I don't think it is running the black magic. I looked in /opt/aus2/incoming/3/Firefox on staging-stage and I get the feeling that there are only snippets for release tests. I am going to file a bug. I have also realized that the DOWNLOAD_BASE_URL specified in http://mxr.mozilla.org/build/source/buildbot-configs/mozilla2-staging/config.py#22 does not point to http://staging-stage.build.mozilla.org/ but to ftp://ftp.mozilla.org. I don't think that the updates will work on staging if the snippet point to ftp.mozilla.org where the MAR files will not be. I am fixing this locally

Comment on attachment 382767 [details] [diff] [review] generates a snippet for a given locale Like the named options. Some small nits below. >+ print "Error: you must specify the path to the where the MAR files is." "MAR file" singular, I think you mean. >+ print "Error: you must specify the path to the application.ini file." Are these paths absolute or relative (or does it matter)? Same applies to MAR file above. Helpful to add that detail to a comment or error message. >+ marFileName = '%s-%s.%s.%s.complete.mar'% ( Can we get a space between the string and the %, i.e. '%s-%s.%s.%s.complete.mar' % ( r+ with those nits fixed. I'm happy to make those changes myself when landing if Axel doesn't have any other issues.

OK. I have deployed the patches in slaves darwin9-slave03, linux-slave03 and win32-slave04. The patch has been deployed for mozilla-central and mozilla-1.9.1. I have made sure that the URL written in the URL points to staging-stage instead of ftp.mozilla.org. For instance: http://staging-stage.build.mozilla.org/pub/mozilla.org/firefox/nightly//2009/06/2009-06-12-04-mozilla-1.9.1-l10n/firefox-3.5pre.sv-SE.win32.complete.mar I will keep these 3 slaves running all weekend to gather several dated nightly repackages. With this I hope that when I am using a repackage that is a day old it should request me to update to the latest nightly. I will try to test during the weekend since I will have a couple of days worth of repackages.

I have downloaded one of the binaries from staging-master and I have checked the app.update.url which is: https://aus2.mozilla.org/update/3/%PRODUCT%/%VERSION%/%BUILD_ID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/%OS_VERSION%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/update.xml This means that even if I made the snippets to point to the right place (staging-stage) the browser itself will look for updates in the wrong place.

(In reply to comment #27) > I have downloaded one of the binaries from staging-master and I have checked > the app.update.url which is: > https://aus2.mozilla.org/update/3/%PRODUCT%/%VERSION%/%BUILD_ID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/%OS_VERSION%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/update.xml > This means that even if I made the snippets to point to the right place > (staging-stage) the browser itself will look for updates in the wrong place. Can you override the url by hand though? QA might also have a tool to do this automatically.

Using app.update.url.override works to make the update mechanism to check somewhere else. To test this: 1) app.update.log.all - true 2) close browser 3) start from command line 4) check for updates and you should see a line like this in the command line: *** AUS:SVC Checker:checkForUpdates - sending request to: http://staging-stage.build.mozilla.org/update/3/Firefox/3.5pre/20090612053046/Darwin_Universal-gcc3/es-ES/nightly/Darwin%209.7.0/default/default/update.xml?force=1&force=1

I want to modify app.update.url.override to point to the right URL I have the snippet in: /opt/aus2/build/0/Firefox/mozilla-1.9.1/Darwin_Universal-gcc3/20090612053046/es-ES/complete.txt (which is http://staging-stage.build.mozilla.org/aus2/build/0/Firefox/mozilla-1.9.1/Darwin_Universal-gcc3/20090612053046/es-ES/complete.txt) and the URL where it is looking for updates is: http://staging-stage.build.mozilla.org/update/3/Firefox/3.5pre/20090612053046/Darwin_Universal-gcc3/es-ES/nightly/Darwin%209.7.0/default/default/update.xml?force=1&force=1 How do I make them map to each other? Another issue is that I have realised that the complete.txt are having unexpected permissions: -rw------- 1 cltbld cltbld 244 Jun 12 13:39 complete.txt I know that I have been able to see them. In fact, the URL from comment 24 still works: http://staging-stage.build.mozilla.org/aus2/build/0/Firefox/mozilla-1.9.1/Linux_x86-gcc3/20090611030910/fr/complete.txt After checking under /opt/aus2/build/0/Firefox/mozilla-1.9.1 with: ls -l Linux_x86-gcc3/20090*/* | grep complete.txt WINNT has all permissions to: -rw-r--r-- Linux has 90+% of all permissions to: -rw-rw-r-- Darwin has half with -rw-rw-r-- (previous to Apr 21) and the following to -rw-------

Comment on attachment 382767 [details] [diff] [review] generates a snippet for a given locale r- for reasons mentioned inline. >diff --git a/tools/update-packaging/generatesnippet.py b/tools/update-packaging/generatesnippet.py file position seems ok for me. <...> >+def main(): >+ error = False >+ parser = OptionParser( >+ usage="%prog [options]") >+ parser.add_option("--marPath", >+ action="store", >+ dest="marPath", >+ help="Specify the directory where the MAR file is found.") >+ parser.add_option("--applicationIniFile", >+ action="store", >+ dest="applicationIniFile", >+ help="Specify the path to the application.ini file.") >+ parser.add_option("-l", >+ "--locale", >+ action="store", >+ dest="locale", >+ help="Specify which locale we are generating the snippet for.") >+ parser.add_option("-p", >+ "--product", >+ action="store", >+ dest="product", >+ help="This option is used to generate the URL to download the MAR file.") Nit, options are commonly written as --application-ini-file, not CamelCase. Please add a [required] to the end of the help texts to specify that they're not truly options. >+ downloadBaseURL = os.environ.get("DOWNLOAD_BASE_URL") No arguments through the environment, please. This should be a parser option, and the default should be mentioned in the help text. >+ (options, args) = parser.parse_args() >+ if not options.marPath: >+ print "Error: you must specify the path to the where the MAR files is." >+ error = True >+ if not options.applicationIniFile: >+ print "Error: you must specify the path to the application.ini file." >+ error = True >+ if not options.locale: >+ print "Error: you must specify a locale." >+ error = True >+ if not options.product: >+ print "Error: you must specify a product." >+ error = True >+ if not downloadBaseURL: >+ downloadBaseURL = 'http://ftp.mozilla.org/pub/mozilla.org/nightly' >+ >+ if error: >+ sys.exit(1) Factor this, something like for req, msg in (('mar_path', 'the path to where the MAR file is'), ('application_ini_file', '...'), ...): if not hasattr(options, req): parser.error('You must specify %s' % msg) Note the parser.error instead of sys.exit() >+ >+ snippet = generateSnippet(options.marPath, >+ options.applicationIniFile, >+ options.locale, >+ downloadBaseURL, >+ options.product) >+ writeSnippetToDisk( >+ os.path.join(options.marPath, 'complete.update.snippet'), >+ snippet) There's no need to put the three lines of code into a three lines method call without the prospect of reuse of that function here, just inline this back. >+ # Show in our logs what the contents of the snippet are >+ print snippet IMHO, this shouldn't happen by default. I'm happy to have that on if you're adding a -v, --verbose flag and pass that in as part of the build process. >+ >+def generateSnippet(abstDistDir, applicationIniFile, locale, >+ downloadBaseURL, product): >+ # Let's extract information from application.ini >+ c = ConfigParser() >+ c.read([applicationIniFile]) This can be just c.read(applicationIniFile), but either way, this will silently ignore if it can't find applicationIniFile. Maybe switch over to explicitly open() and readfp() with proper error messages on a missing file? >+ buildid = c.get("App", "BuildID") This seems to obsolete getting the BuildID by make ident, right? <...> >+ # Construct the URL to where the MAR file will exist >+ interfix = '' >+ if locale.find('en-US') == -1: >+ interfix = '-l10n' Not very pythonic, instead do interfix = locale != 'en-US' and '-l10n' or '' Yes, there is a good piece of style-guide in here, but I hope that it will make it easier to maintain and read this code for people that haven't written it.

Comment on attachment 382769 [details] [diff] [review] make target that calls the python script to generate a snippet plus change to make ident I think we should get this into l10n.mk (I'll make this depend on that bug), and the fragment to make ident can go, if I read the other patch right.

(In reply to comment #31) > >+ # Construct the URL to where the MAR file will exist > >+ interfix = '' > >+ if locale.find('en-US') == -1: > >+ interfix = '-l10n' > > Not very pythonic, instead do > > interfix = locale != 'en-US' and '-l10n' or '' Granted, python is not my native language, but this construct seems unnecessarily complex. I'm fine with the other suggested changes.

We'll need to get bug 489313 fixed1.9.1 to move the snippets target into l10n.mk.

(In reply to comment #33) > (In reply to comment #31) > > >+ # Construct the URL to where the MAR file will exist > > >+ interfix = '' > > >+ if locale.find('en-US') == -1: > > >+ interfix = '-l10n' > > > > Not very pythonic, instead do > > > > interfix = locale != 'en-US' and '-l10n' or '' > > Granted, python is not my native language, but this construct seems > unnecessarily complex. > > I'm fine with the other suggested changes. Either way the string comparison should work on locale.find() == -1, but either locale == 'en-US' or !=. (Hey, we might at some point end up with en-US-x-ghetto).

PS: however the grammar turns out (ugh on me), don't use .find() == -1 to test for string equality.

What's wrong with if 'en-US' in locale: interfix = '-l10n' else: interfix = ''

(In reply to comment #37) > What's wrong with > > if 'en-US' in locale: > interfix = '-l10n' > else: > interfix = '' a missing 'not' apparently :\ if 'en-US' not in locale: ...

Why test for substrings if we want equality?

(In reply to comment #30) > I want to modify app.update.url.override to point to the right URL You would go to about:config and copy the value of app.update.url. Then create app.update.url.override and paste that in, replacing "https://aus2.mozilla.org" with "http://staging-stage.build.mozilla.org". That's the closest you can get in our staging environment to what nightly users would have (ie the app still does all the substitutions in the URL). > Another issue is that I have realised that the complete.txt are having > unexpected permissions: > -rw------- 1 cltbld cltbld 244 Jun 12 13:39 complete.txt This is our old "friend" the default "umask = None" in buildbot.tac on the slaves. It needs to say "umask = 002". I've fixed moz2-darwin9-slave03/04 and moz2-win32-slave21, the other staging slaves were fine. Did KeepAlive = None while I was at it.

(In reply to comment #31) > (From update of attachment 382767 [details] [diff] [review]) > >+ buildid = c.get("App", "BuildID") > > This seems to obsolete getting the BuildID by make ident, right? > The buildid pulled through make ident is used when rendering + postUploadCmd += ['-i %(buildid)s', on attachment 383342 [details] [diff] [review] from bug 480081

I have fixed all the mentioned corrections. I have also tested that on windows it does generate the snippet with the correct line endings. I will work on the make target

When updates are generated the generate-snippet target gets called as well. This is going to be a weird dance since l10n.mk has to still land on 1.9.1. We can land this first for mozilla-central and later for mozilla-1.9.1.

This patch only applies cleanly on mozilla-central since browser/locales/Makefile.in are different at this moment. The buildid is needed to construct the postupload line (check comment 41)

Comment on attachment 383720 [details] [diff] [review] generatesnippet.py - it generates a snippet for a given locale >+ parser.add_option("--download-base-URL", >+ action="store", >+ dest="downloadBaseURL", >+ help="This option indicates under which.") Your help text is incomplete here. >+ for req, msg in (('marPath', "the absolute path to the where the MAR file is"), >+ ('applicationIniFile', "the absolute path to the application.ini file."), >+ ('locale', "a locale."), >+ ('product', "specify a product.")): >+ if not hasattr(options, req): >+ parser.error('You must specify %s' % msg) Remove the extra 'specify' from the msg for product or you'll end up with "specify specify." r+ with those nits fixed.

Comment on attachment 383917 [details] [diff] [review] l10n.mk - adding generation of snippets Gozer or KaiRo should probably have a look at this one, too.

Comment on attachment 383917 [details] [diff] [review] l10n.mk - adding generation of snippets Looks good, and tested it with one locale. This would need porting over to comm-central/mail/locales/Makefile.in, as we don't seem to use l10n.mk... Kairo ?

Comment on attachment 383917 [details] [diff] [review] l10n.mk - adding generation of snippets looks good. As gozer says though, we need to either port the switch to l10n.mk - which will require changes in l10n.mk as e.g. $(topsrcdir)/tools or $(DEPTH)/tools doesn't work in comm-central, pointing to the wrong location - or we need to port this patch to comm-central/{calendar/sunbird,mail,suite}/locales/Makefile.in I'm not sure which is the best option right now.

All 3 patches can land into mozilla-central * The generatesnippet.py can land in both branches * The l10n.mk patch cannot land in mozilla-1.9.1 until dependent bug is fixed * The make ident can land in mozilla-1.9.1 as well - I will attach the same patch but it actually apply cleanly for mozilla-1.9.1

(In reply to comment #48) > (From update of attachment 383917 [details] [diff] [review]) > looks good. As gozer says though, we need to either port the switch to l10n.mk > - which will require changes in l10n.mk as e.g. $(topsrcdir)/tools or > $(DEPTH)/tools doesn't work in comm-central, pointing to the wrong location - > or we need to port this patch to > comm-central/{calendar/sunbird,mail,suite}/locales/Makefile.in > > I'm not sure which is the best option right now. KaiRo, I don't know why we are not using l10n.mk in comm-central, but wouldn't that be the right long-term solution. As opposed to having to keep porting changes over ?

(In reply to comment #51) > KaiRo, I don't know why we are not using l10n.mk in comm-central, but wouldn't > that be the right long-term solution. As opposed to having to keep porting > changes over ? It would, but 1) bug 489313 first needs to even land l10n.mk on 1.9.1 and 2) l10n.mk probably would need some changes to be able to still refer to the right directories inside the mozilla/ tree, even when included by a comm-central locales/Makefile.

From bug 489313#c12, the only patch that we will need to land in 1.9.1 is the generatesnippet.py one since the other two patches of this bug are included in attachment 386972 [details] [diff] [review].

We should not be reaching the scenario in which the DOWNLOAD_BASE_URL is not set when calling the script but let's fix this for the sake of having it proper for others not to bite the bullet in the future. After this gets checked in I will ask for approval 1.9.1.

Included the last fix and carrying forward the reviews. Can we please get approval to land this in 1.9.1? We need this script to generate nightly updates for L10n on 1.9.1

Comment on attachment 387461 [details] [diff] [review] script to generate snippets for 1.9.1 removing dead flag; please generate a new mozilla-1.9.1 patch and request approval on that.

Requesting blocking1.9.1, we need this to create l10n builds on a nightly update channel for our stable branch.

This bug won't block and I'm pretty sure it doesn't need approval. Isn't this code NPOTB? If it does need approval, please request it on the appropriate patch.

Comment on attachment 387461 [details] [diff] [review] script to generate snippets for 1.9.1 http://hg.mozilla.org/releases/mozilla-1.9.1/rev/cb4fb811239b

Comment on attachment 386994 [details] [diff] [review] fix situation when DOWNLOAD_BASE_URL is not set http://hg.mozilla.org/mozilla-central/rev/38a8f988ddf7

Removing dependency with bug 489313 and moving it to parent bug since it does not block us as it did in the beginning (if it ever really did - I can't remember) coop thanks for checking that in. This bug should be fixed.