This has been separated from the wget patch from Bug 445254 The make target sets a variable UPLOAD_DIR which is a folder where we copy the recently generated repackages, xpi (langpacks) and/or windows installers to the specified folder to uploaded from there to the ftp server in the nightly section The structure followed is (in this case windows): - dist/upload/latest/firefox-3.0.2pre.af.win32.zip - dist/upload/latest/firefox-3.0.2pre.af.win32.installer.exe - dist/upload/latest/windows-xpi/af.xpi In the prepare-upload-dated, we specify the dated folder we want: UPLOAD_DIR = $(UPLOAD_DIR)/upload/.`date "+%y-%m-%d-%H"`.-$(MOZ_APP_VERSION)-l10n It seems that the dated folder is where we pull from in releases(check the mentioned Bug 445254)

BTW, I have separated this patch from the(In reply to comment #0) > Created an attachment (id=334791) [details] > prepare repackages in correct structure to upload to ftp server > > This has been separated from the wget patch from Bug 445254 > BTW, I have separated this patch from the wget because I had to do more changes in this one and the wget one was ready to be landed

(In reply to comment #0) > It seems that the dated folder is where we pull from in releases(check the > mentioned Bug 445254) > As mentioned in bug 451225#c3 >If your code pushes to the right place on stage to begin with, eg, >/home/ftp/pub/firefox/nightly/2008-07-24-20-firefox3.0.2-l10n/, these steps >will work just fine.

It generates the folders that I need, dated and latest

I forgot to add the chmod line which is in use in staging-1.9 and I have added comments and the order of the make targets

Comment on attachment 334977 [details] [diff] [review] prepare repackages in correct structure to upload to ftp server +prepare-upload-dated-%: + @$(MAKE) prepare-repackages-$* UPLOAD_DIR=$(DIST)/upload/`date "+%Y-%m-%d-%H"`-$(MOZ_PKG_APPNAME)$(MOZ_APP_VERSION)-l10n Is using 'date' really the right thing to do here? Are the dated dirs just supposed to be "whatever the current date is", or should they be based on the BuildID or something? +# Link to the langpack + ln -s $(DIST)/install/firefox-$(MOZ_APP_VERSION).$*.langpack.xpi \ + $(UPLOAD_DIR)/$(XPI_DESTINATION)/$*.xpi I don't think you should use 'ln -s' explicitly here. Why not use $(INSTALL)? That should symlink on platforms that support it. +# Set the permissions that the folders will have in ftp once uploaded + chmod -vR 755 $(UPLOAD_DIR) I really doubt that this will work on Windows. Have you tried it there? MSYS doesn't really have a good concept of unix permissions.

(In reply to comment #5) > (From update of attachment 334977 [details] [diff] [review]) > +prepare-upload-dated-%: > + @$(MAKE) prepare-repackages-$* UPLOAD_DIR=$(DIST)/upload/`date > "+%Y-%m-%d-%H"`-$(MOZ_PKG_APPNAME)$(MOZ_APP_VERSION)-l10n > > Is using 'date' really the right thing to do here? Are the dated dirs just > supposed to be "whatever the current date is", or should they be based on the > BuildID or something? > This is where is gets generated the folder name http://mxr.mozilla.org/mozilla/source/tools/tinderbox/post-mozilla-rel.pl#1397 1390 if ($Settings::BuildLocales) { 1391 my ($c_hour,$c_day,$c_month,$c_year,$c_yday) = (localtime(time))[2,3,4,5,7]; 1392 $c_year = $c_year + 1900; # ftso perl 1393 $c_month = $c_month + 1; # ftso perl 1394 $c_hour = pad_digit($c_hour); 1395 $c_day = pad_digit($c_day); 1396 $c_month = pad_digit($c_month); 1397 $datestamp = "$c_year-$c_month-$c_day-$c_hour-$Settings::milestone"; and 1411 $package_dir = "$datestamp"; which is passed to: 1496 unless (pushit(server => $Settings::ssh_server, remote_path => $ftp_path, package_name => $package_dir, > +# Link to the langpack > + ln -s $(DIST)/install/firefox-$(MOZ_APP_VERSION).$*.langpack.xpi \ > + $(UPLOAD_DIR)/$(XPI_DESTINATION)/$*.xpi > > I don't think you should use 'ln -s' explicitly here. Why not use $(INSTALL)? > That should symlink on platforms that support it. > I think I could keep it as "move" as I had it before since it makes no sense to want to prepare the folder structure for nightly when we are doing a release. Like calling "prepare-upload-latest" and just after that "prepare-upload-dated" > +# Set the permissions that the folders will have in ftp once uploaded > + chmod -vR 755 $(UPLOAD_DIR) > > I really doubt that this will work on Windows. Have you tried it there? MSYS > doesn't really have a good concept of unix permissions. > This has been working in staging 1.9 for all 3 platforms We have as well using chmod in the tinderbox code: http://mxr.mozilla.org/mozilla/source/tools/tinderbox/post-mozilla-rel.pl#1115 I have realized that is 775 instead of 755

I have fixed the chmod and changed the "ln -s" for "mv" instead

is the whiteboard note correct? [needs checkin on 1.9.1 and 1.9.0]??

CVS HEAD: Checking in browser/locales/Makefile.in; /cvsroot/mozilla/browser/locales/Makefile.in,v <-- Makefile.in new revision: 1.62; previous revision: 1.61 done

Comment on attachment 335130 [details] [diff] [review] prepare repackages in correct structure to upload to ftp server This doesn't work. /builds/tinderbox/Fx-Trunk/Darwin_8.8.4_Depend/build/universal/ppc/config/nsinstall -L /builds/tinderbox/Fx-Trunk/Darwin_8.8.4_Depend/build/universal/ppc/browser/components/build -m 644 /builds/tinderbox/Fx-Trunk/Darwin_8.8.4_Depend/mozilla/browser/components/build/nsBrowserCompsCID.h ../../../dist/include/browsercomps /builds/tinderbox/Fx-Trunk/Darwin_8.8.4_Depend/build/universal/ppc/config/nsinstall -L /builds/tinderbox/Fx-Trunk/Darwin_8.8.4_Depend/build/universal/ppc/browser/components -m 644 /builds/tinderbox/Fx-Trunk/Darwin_8.8.4_Depend/mozilla/browser/components/nsIBrowserHandler.idl /builds/tinderbox/Fx-Trunk/Darwin_8.8.4_Depend/mozilla/browser/components/nsIBrowserGlue.idl ../../dist/idl /builds/tinderbox/Fx-Trunk/Darwin_8.8.4_Depend/build/universal/ppc/config/nsinstall -L /builds/tinderbox/Fx-Trunk/Darwin_8.8.4_Depend/build/universal/ppc/browser/components -m 644 _xpidlgen/nsIBrowserHandler.h _xpidlgen/nsIBrowserGlue.h ../../dist/include/browsercomps Makefile:340: Extraneous text after `else' directive Makefile:342: Extraneous text after `else' directive Makefile:342: *** only one `else' per conditional. Stop. make[5]: *** [export] Error 2 make[4]: *** [export_tier_app] Error 2 make[3]: *** [tier_app] Error 2 make[2]: *** [alldep] Error 2 make[1]: *** [alldep] Error 2 make: *** [alldep] Error 2

I have made no changes to the patch, the only things is that appears at a higher line number since there has been a commit since last time Is the log that you show from a FF3.0 build? The only problem that I can see from the previous patch was that it would have not applied correctly. It doesn't make sense the bustage Let me use the try server and see if I can get the same log you got. could this please be checked in to hg? I will ask to be checked in to CVS HEAD once there is approval1.9.0.x rather than before

Comment on attachment 336402 [details] [diff] [review] prepare repackages in correct structure to upload to ftp server could someone please check this in to hg?

I think the error in comment #10 (which is from a 3.0 build) is not a patch issue, it's make 3.80 objecting to anything following |else|. Or to put it another way, 3.80 only supports conditional-directive text-if-one-is-true else text-if-false endif and not conditional-directive text-if-one-is-true else conditional-directive text-if-true else text-if-false endif We have make 3.80 on Fx3.0 and older macs; 3.81 on the pool for mozilla-central et al. We could land different patches in the two places, but I think it's better to land the same patch in both. Can you rewrite this to achieve the same effect ?

(In reply to comment #13) > We could land different patches in the two places, but I think it's > better to land the same patch in both. Can you rewrite this to achieve the same > effect ? I will rewrite it

fixed replace (else if condition) for (else if condition)

Comment on attachment 337773 [details] [diff] [review] prepare repackages in correct structure to upload to ftp server [Checkin: Comment 28] Changing review to owner of this code (the build system).

(In reply to comment #16) > (From update of attachment 337773 [details] [diff] [review]) > Changing review to owner of this code (the build system). Armen: can you give Ted a quick rundown of the testing you've done and the results you've seen trying this patch locally since reed's comment 10? It might expedite a review. Thanks.

(In reply to comment #17) > (In reply to comment #16) > > (From update of attachment 337773 [details] [diff] [review] [details]) > > Changing review to owner of this code (the build system). > > Armen: can you give Ted a quick rundown of the testing you've done and the > results you've seen trying this patch locally since reed's comment 10? It might > expedite a review. Thanks. They are all green. These are the logs of the patch for cvs: http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1221020658.1221022703.16694.gz&fulltext=1 http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1221024251.1221026606.26881.gz&fulltext=1 http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry These are the logs for hg: http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1221019107.1221023674.18970.gz&fulltext=1 http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1221019214.1221021752.14613.gz&fulltext=1 http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1221019107.1221022939.17263.gz&fulltext=1

please checkin-needed for cvs head and hg

There has been a commit in hg that has not happened in cvs since attachment 33773 [details] [diff] [review] was created.

(In reply to comment #20) > Created an attachment (id=338871) [details] > the same patch as the previous one but for CVS > > There has been a commit in hg that has not happened in cvs since attachment > 33773 [details] was created. We can carry forward the review from Ted, but we still need to get approval to land this on CVS trunk.

(In reply to comment #19) > please checkin-needed for cvs head and hg Can we land attachment#337773 [details] [diff] [review] in hg? Its been r+'d, and having this landed in hg would help with the approval1.9.0.3? request.

I've been trying, but the tree has been red and/or closed since Tuesday. I'm monitoring tinderbox and will check it in at the first available opportunity.

(In reply to comment #23) > I've been trying, but the tree has been red and/or closed since Tuesday. I'm > monitoring tinderbox and will check it in at the first available opportunity. cool, thanks coop! Armen and I are just sorting through all these l10n bugs, making sure they are uptodate...

Comment on attachment 338871 [details] [diff] [review] [checked in]the same patch as the previous one but for CVS Approved for 1.9.0.3, a=dveditz for release-drivers

Comment on attachment 338871 [details] [diff] [review] [checked in]the same patch as the previous one but for CVS Checking in Makefile.in; /cvsroot/mozilla/browser/locales/Makefile.in,v <-- Makefile.in new revision: 1.66; previous revision: 1.65 done

Still need to land in hg, but the change has been pushed to CVS HEAD now.

Comment on attachment 337773 [details] [diff] [review] prepare repackages in correct structure to upload to ftp server [Checkin: Comment 28] http://hg.mozilla.org/mozilla-central/rev/ccce859a94aa I "fixed" [ Hunk #1 FAILED at 333 ] which was caused by the context only. I removed some trailing spaces too.

Comment on attachment 337773 [details] [diff] [review] prepare repackages in correct structure to upload to ftp server [Checkin: Comment 28] >+# This target will generate a UPLOAD_DIR folder with >+# l10n repackages in the way that we offer l10n nightlies >+# 1) ./ the binary >+# 2) ./{linux,mac,windows}-xpi/locale.xpi using {linux,mac,windows}-xpi/ is actually a bug, as langpacks aren't different for the platforms. At least on trunk, we should move away from that. >+ mv $(DIST)/install/firefox-$(MOZ_APP_VERSION).$*.langpack.xpi \ >+ $(UPLOAD_DIR)/$(XPI_DESTINATION)/$*.xpi >+# Move the repackage >+ mv $(DIST)/firefox-$(MOZ_APP_VERSION).$*.* \ >+ $(UPLOAD_DIR)/. >+# Move the windows installer >+ifeq (WINNT, $(OS_ARCH)) >+ mv $(DIST)/install/sea/firefox-$(MOZ_APP_VERSION).$*.win32.installer.exe \ >+ $(UPLOAD_DIR)/. Ugh. Could you please update those to use the new variables for the paths and package names as set by package-name.mk?

(In reply to comment #29) > (From update of attachment 337773 [details] [diff] [review]) > >+# This target will generate a UPLOAD_DIR folder with > >+# l10n repackages in the way that we offer l10n nightlies > >+# 1) ./ the binary > >+# 2) ./{linux,mac,windows}-xpi/locale.xpi > > using {linux,mac,windows}-xpi/ is actually a bug, as langpacks aren't different > for the platforms. At least on trunk, we should move away from that. > I am so glad I can hear comments like this (revealing more light to my un-knowledge) ;) Is there a bug filed for this? > >+ mv $(DIST)/install/firefox-$(MOZ_APP_VERSION).$*.langpack.xpi \ > >+ $(UPLOAD_DIR)/$(XPI_DESTINATION)/$*.xpi > >+# Move the repackage > >+ mv $(DIST)/firefox-$(MOZ_APP_VERSION).$*.* \ > >+ $(UPLOAD_DIR)/. > >+# Move the windows installer > >+ifeq (WINNT, $(OS_ARCH)) > >+ mv $(DIST)/install/sea/firefox-$(MOZ_APP_VERSION).$*.win32.installer.exe \ > >+ $(UPLOAD_DIR)/. > > Ugh. Could you please update those to use the new variables for the paths and > package names as set by package-name.mk? I filed bug 456351 for this

(In reply to comment #30) > > using {linux,mac,windows}-xpi/ is actually a bug, as langpacks aren't different > > for the platforms. At least on trunk, we should move away from that. > > > I am so glad I can hear comments like this (revealing more light to my > un-knowledge) ;) > Is there a bug filed for this? I don't think so - I fixed it for the case of releases in my "pretty" filename work for the hg-based release, I don't think there's a bug for nightly boxes, but we should go and just only upload them from one platform (e.g. Linux) and into a generic place (just next to the other L10n files?), for hg trunk at least. > I filed bug 456351 for this Thanks, more comments there.