Closed
Bug 453840
Opened 16 years ago
Closed 16 years ago
make it possible to create files in the final place for releases
Categories
(Firefox Build System :: General, defect, P2)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: kairo, Assigned: bhearsum)
References
Details
(Keywords: fixed1.9.1, Whiteboard: [hg-automation])
Attachments
(6 files, 6 obsolete files)
17.31 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
13.60 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
3.86 KB,
patch
|
Details | Diff | Splinter Review | |
6.69 KB,
patch
|
Details | Diff | Splinter Review | |
10.96 KB,
patch
|
ted
:
review+
beltzner
:
approval1.9.1+
|
Details | Diff | Splinter Review |
1.25 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
The Firefox release process places files with "pretty" names into platform and locale subdirectories, we should create file with those names and directories right away in the build system package process.
Reporter | ||
Comment 1•16 years ago
|
||
This patch adds a MOZ_PKG_PRETTYNAMES variable that enables using the release-style "pretty" package names. As those require special subdirectories, we also need to add better handling of package directories to the build system - in many places this only parameterizes previously hardcoded directories. For achieving the complete set of namings we e.g. have for Shiretoko Alpha 2, we'd need the mozconfig (or the environment) to contain the following settings: ------------------------- MOZ_PKG_PRETTYNAMES=1 MOZ_PKG_VERSION=alpha2 MOZ_PKG_LONGVERSION=Alpha 2 ------------------------- (the "Shiretoko" name is known to the build from MOZ_APP_DISPLAYNAME already) For a Firefox 3.1 Beta 1, we'd probably need something like: ------------------------- MOZ_PKG_PRETTYNAMES=1 MOZ_PKG_VERSION=3.1beta1 MOZ_PKG_LONGVERSION=3.1 Beta 1 ------------------------- For a Firefox 3.1 final, we can go with: ------------------------- MOZ_PKG_PRETTYNAMES=1 ------------------------- as both version strings then will just be set to the internally known Firefox version number. The one part I haven't figured out yet is partial update naming, I can't really figure out how this is set right now, what I see in update-packaging/Makefile.in sounds a bit strange to me.
Attachment #337071 -
Flags: review?(ted.mielczarek)
Reporter | ||
Comment 2•16 years ago
|
||
Two additional comments: 1) This needs testing, I only have Linux here and haven't tried all the cases even there yet. 2) Checking in the patch this way shouldn't break Thunderbird or SeaMonkey currently, but they should be fixed up the same way as browser/ - I will look into that once I get confirmation that those variables and their names are the right way to go.
Reporter | ||
Comment 3•16 years ago
|
||
Sorry for the fast patch update here, but I ran into a few obvious bugs while testing on Linux with the actual "pretty" names. What I have tested and found to be working on Linux now (on a trunk build with --enable-update-packaging and --with-l10n-base set to a path that contains a current copy of de): make package MOZ_PKG_PRETTYNAMES=1 MOZ_PKG_VERSION="3.1beta1" MOZ_PKG_LONGVERSION="3.1 Beta 1" make -C tools/update-packaging MOZ_PKG_PRETTYNAMES=1 MOZ_PKG_VERSION="3.1beta1" MOZ_PKG_LONGVERSION="3.1 Beta 1" make -C browser/locales/ installers-de MOZ_PKG_PRETTYNAMES=1 MOZ_PKG_VERSION="3.1beta1" MOZ_PKG_LONGVERSION="3.1 Beta 1" make -C tools/update-packaging MOZ_PKG_PRETTYNAMES=1 MOZ_PKG_VERSION="3.1beta1" MOZ_PKG_LONGVERSION="3.1 Beta 1" AB_CD=de PACKAGE_BASE_DIR=../../dist/l10n-stage This results in the following files: dist/langpack/de.xpi dist/linux-i686/de/minefield-3.1beta1.tar.bz2 dist/linux-i686/en-US/minefield-3.1beta1.tar.bz2 dist/update/linux-i686/de/minefield-3.1beta1.complete.mar dist/update/linux-i686/en-US/minefield-3.1beta1.complete.mar Of course, the long version hasn't been used anywhere here, as that's only used on Windows and Mac, testing there would be appreciated a lot! bsmedberg couldn't point me to the bug about langpacks currently being inside the platform dirs when actually they aren't platform-specific, but he confirmed it is considered a bug and so I fixed that here. I also incorporated a fix so that we only need one single override var to tell update-packaging about out build being somewhere else than dist/ for L10n builds. Before that, we needed to override DIST which meant also overriding STAGE_DIR and MAR_BIN to point to the places outside l10n-stage again. With the new PACKAGE_BASE_DIR var, this is not needed any more, so doing L10n complete MARs gets easier for everyone.
Attachment #337071 -
Attachment is obsolete: true
Attachment #337098 -
Flags: review?(ted.mielczarek)
Attachment #337071 -
Flags: review?(ted.mielczarek)
Comment 4•16 years ago
|
||
Comment on attachment 337098 [details] [diff] [review] [checked in] add MOZ_PKG_PRETTYNAMES and deal with subdirs, v1.1 Looks good, r=me.
Attachment #337098 -
Flags: review?(ted.mielczarek) → review+
Reporter | ||
Comment 5•16 years ago
|
||
This is a straight-forward port of the browser/ changes to mail/ and suite/ so we can supports that new style there as well.
Attachment #339115 -
Flags: review?(ted.mielczarek)
Updated•16 years ago
|
Attachment #339115 -
Flags: review?(ted.mielczarek) → review+
Reporter | ||
Comment 6•16 years ago
|
||
main patch pushed as http://hg.mozilla.org/mozilla-central/rev/b375e5cab4dc and mail/suite part as http://hg.mozilla.org/comm-central/rev/417d21c160a9 I'm marking this fixed even though names of partial MARs aren't touched by this. I'm willing to look into them as well, but I couldn't figure out the naming etc. well enough to include them here.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 7•16 years ago
|
||
I think I've found a bug here on Windows. I tried doing: make installer MOZ_PKG_PRETTYNAMES=1 MOZ_PKG_VERSION="3.1 Beta 1" and got an error because preprocessor.pl is getting passed arguments without quotes: for i in packages-static ; do \ /bin/perl /e/builds/moz2_debug_slave/mozil g/preprocessor.pl -DAB_CD=en-US -DPKG_BASENAME=Minef ASENAME=Minefield Setup 3.1 Beta 1 -DMOZ_APP_VERSION efox -DMOZ_APP_DISPLAYNAME=Minefield -DMOZILLA_VERSI NNT5.2\" -DOSARCH=WINNT -D_CRT_SECURE_NO_DEPRECATE=1 =1 -DWINVER=0x500 -D_WIN32_WINNT=0x500 -D_WIN32_ I've got a potenial patch to fix this running through the try server right now (just testing that I don't break anything obvious - can't do full tests there, of course).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 8•16 years ago
|
||
This patch is dead simple - it just quotes all the uses of PKG_BASENAME and PKG_INST_BASENAME that need to be quoted. I skipped usages like: FOO = blahblah/$PKG_BASENAME/blah because IIRC spaces don't matter in these. I'll prepare a comm-central patch once I know this is okay.
Attachment #341936 -
Flags: review?(ted.mielczarek)
Updated•16 years ago
|
Attachment #341936 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 9•16 years ago
|
||
Comment on attachment 341936 [details] [diff] [review] [backed out] follow-up to support win32 e96a238b497b
Attachment #341936 -
Attachment description: follow-up to support win32 → [checked in] follow-up to support win32
Assignee | ||
Updated•16 years ago
|
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
Comment 10•16 years ago
|
||
nag, CC me on bugs changing the locales makefiles? Thanks. That way I don't end up surprised to fix merge conflicts.
Assignee | ||
Comment 11•16 years ago
|
||
(In reply to comment #10) > nag, CC me on bugs changing the locales makefiles? Thanks. That way I don't end > up surprised to fix merge conflicts. Will do
Comment 12•16 years ago
|
||
I think this completely busted windows, at least the quoting piece. mozilla@WINDOWS-XP-VM /c/workdir/orig-repacks/browser/locales $ make repackage-win32-installer-fr make[1]: Entering directory `/c/workdir/orig-repacks/browser/locales' Verifying fr installer variable usage Repackaging into /c/workdir/orig-repacks/dist/install/sea/firefox-3.1b2pre.fr.win32.installer.exe. /local/bin/make -C ../installer/windows export make[2]: Entering directory `/c/workdir/orig-repacks/browser/installer/windows' /c/mozilla-build//python25/python /z/src/central/mozilla-central/config/nsinstall.py -D ../../../dist/branding cp /z/src/central/mozilla-central/browser/installer/windows/nsis/branding.nsi ../../../dist/branding/branding.nsi cp /z/src/central/mozilla-central/browser/installer/windows/wizHeader.bmp ../../../dist/branding/wizHeader.bmp cp /z/src/central/mozilla-central/browser/installer/windows/wizHeaderRTL.bmp ../../../dist/branding/wizHeaderRTL.bmp cp /z/src/central/mozilla-central/browser/installer/windows/wizWatermark.bmp ../../../dist/branding/wizWatermark.bmp make[2]: Leaving directory `/c/workdir/orig-repacks/browser/installer/windows' if test ! -d "/c/workdir/orig-repacks/dist/install/sea/; then \ /c/mozilla-build//python25/python /z/src/central/mozilla-central/config/nsinstall.py -D "/c/workdir/orig-repacks/dist/install/sea/; \ fi /bin/sh: -c: line 3: syntax error near unexpected token `fi' /bin/sh: -c: line 3: `fi' make[1]: *** [repackage-win32-installer] Error 2 make[1]: Leaving directory `/c/workdir/orig-repacks/browser/locales' make: *** [repackage-win32-installer-fr] Error 2 See the unbalanced quote? If I unquote the variable in the rule again, I get passed that point, but corrupt when trying to unpack the installer. The wget rule seems to be broken, too, looking at http://tinderbox.mozilla.org/showbuilds.cgi?tree=Mozilla-l10n
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 13•16 years ago
|
||
Backing out.
Reporter | ||
Updated•16 years ago
|
Attachment #341936 -
Attachment description: [checked in] follow-up to support win32 → [backed out] follow-up to support win32
Reporter | ||
Updated•16 years ago
|
Attachment #337098 -
Attachment description: add MOZ_PKG_PRETTYNAMES and deal with subdirs, v1.1 → [checked in] add MOZ_PKG_PRETTYNAMES and deal with subdirs, v1.1
Reporter | ||
Updated•16 years ago
|
Attachment #339115 -
Attachment description: apply browser/ changes to mail/ and suite/ as well → [checked in] apply browser/ changes to mail/ and suite/ as well
Reporter | ||
Comment 14•16 years ago
|
||
Would adding the quotes to the affected *_BASENAME vars in package-name.mk work, or do tools get confused with a path descriptor ending up as dist/win32/en-US/"Firefox Setup 3.1b1".exe?
Comment 15•16 years ago
|
||
preprocessor.pl is funky about quotes in itself, maybe it would help to convert over to PreProcessor.py there. We should in general only quote variables when we pass them to external commands, and not as makefile variables, IMHO. That's just a hack that goes bad easily. For example one reason was that $(dir "/some/path/to/file.ext") returns "/some/path/to. With just one quote. Not sure what $(dir ) does with filenames with spaces. One quote should probably go to http://mxr.mozilla.org/mozilla-central/source/browser/installer/windows/Makefile.in#69, for example. I expect those to be just "", not \"\". I expect the latter to make it through to the preprocessor individual args.
Assignee | ||
Comment 16•16 years ago
|
||
I learned the hard way that Makefiles don't support quoting. It seems there isn't really any nice way to make them deal with spaces in filenames. The best I could find is this s/ /?/ trick. If there's a better way, I'm happy to use it. Most of this patch is s/ /?/ and quoting (inside of targets, where it actually works). The other notable changes are in package-name.mk. I had to special case Windows separately from Mac - because we want the installer in "pretty" format but not the zip file. I've hardcoded MOZ_APP_VERSION in all of the "pretty" version filenames. It looks like package-name.mk was originally designed to let you override the version number in the package name - by using MOZ_APP_VERSION I've removed that option when in "pretty" mode. Since releases are the only consumers of this code I think that's OK though. I'm 99% sure this code works as intended. I've tested it on all 3 platforms with en-US and a locale. I'm still no Makefile expert though, and am open to suggestions (that don't extend beyond the scope of this bug).
Attachment #341936 -
Attachment is obsolete: true
Attachment #344041 -
Flags: review?(ted.mielczarek)
Assignee | ||
Updated•16 years ago
|
Attachment #344041 -
Flags: review?(l10n)
Comment 17•16 years ago
|
||
Comment on attachment 344041 [details] [diff] [review] try #2 - tested on all three platforms, en-US + de >diff -r adf33dbb59d0 browser/installer/windows/Makefile.in <...> > DEFINES += \ > -DAB_CD=$(AB_CD) \ >- -DPKG_BASENAME=$(PKG_BASENAME) \ >- -DPKG_INST_BASENAME=$(PKG_INST_BASENAME) \ >+ -DPKG_BASENAME=$(subst, $(space),?,$(PKG_BASENAME)) \ >+ -DPKG_INST_BASENAME=$(subst, $(space),?,$(PKG_INST_BASENAME)) \ This looks odd. Are these used anywhere? I'd expect quotes to work here, and I don't see any corresponding unescaping in the patch, thus I wonder if these defines are actually used. >diff -r adf33dbb59d0 browser/locales/Makefile.in <...> > DEFINES += \ > -DAB_CD=$(AB_CD) \ > -DMOZ_LANGPACK_EID=langpack-$(AB_CD)@firefox.mozilla.org \ > -DMOZ_APP_VERSION=$(MOZ_APP_VERSION) \ > -DLOCALE_SRCDIR=$(call core_abspath,$(LOCALE_SRCDIR)) \ >- -DPKG_BASENAME=$(PKG_BASENAME) \ >- -DPKG_INST_BASENAME=$(PKG_INST_BASENAME) \ >+ -DPKG_BASENAME=$(subst $(space),?,$(PKG_BASENAME)) \ >+ -DPKG_INST_BASENAME=$(subst $(space),?,$(PKG_INST_BASENAME)) \ > $(NULL) Same here. <...> >-repackage-win32-installer: WIN32_INSTALLER_OUT=$(_ABS_DIST)/$(PKG_INST_PATH)$(PKG_INST_BASENAME).exe >+repackage-win32-installer: WIN32_INSTALLER_OUT=$(_ABS_DIST)/$(PKG_INST_PATH)$(subst $(space),?,$(PKG_INST_BASENAME)).exe > repackage-win32-installer: $(WIN32_INSTALLER_IN) $(SUBMAKEFILES) <...> >- app.7z > $(WIN32_INSTALLER_OUT) >+ app.7z > "$(subst ?,$(space),$(WIN32_INSTALLER_OUT))" > chmod 0755 $(WIN32_INSTALLER_OUT) What happens if you don't escape above, and just quote both the redirect, and the chmod. The chmod works because the ? are expanded as wildcards? The rest of the patch doesn't pose any questions on my side, leaving the review open for more information.
Assignee | ||
Comment 18•16 years ago
|
||
(In reply to comment #17) > (From update of attachment 344041 [details] [diff] [review]) > >diff -r adf33dbb59d0 browser/installer/windows/Makefile.in > <...> > > DEFINES += \ > > -DAB_CD=$(AB_CD) \ > >- -DPKG_BASENAME=$(PKG_BASENAME) \ > >- -DPKG_INST_BASENAME=$(PKG_INST_BASENAME) \ > >+ -DPKG_BASENAME=$(subst, $(space),?,$(PKG_BASENAME)) \ > >+ -DPKG_INST_BASENAME=$(subst, $(space),?,$(PKG_INST_BASENAME)) \ > > This looks odd. Are these used anywhere? I'd expect quotes to work here, and I > don't see any corresponding unescaping in the patch, thus I wonder if these > defines are actually used. > These all get passed to preprocessor.pl - which will bail with spaces. I'm pretty sure this didn't work with quotes, but I'll test it again. > >-repackage-win32-installer: WIN32_INSTALLER_OUT=$(_ABS_DIST)/$(PKG_INST_PATH)$(PKG_INST_BASENAME).exe > >+repackage-win32-installer: WIN32_INSTALLER_OUT=$(_ABS_DIST)/$(PKG_INST_PATH)$(subst $(space),?,$(PKG_INST_BASENAME)).exe > > repackage-win32-installer: $(WIN32_INSTALLER_IN) $(SUBMAKEFILES) > <...> > >- app.7z > $(WIN32_INSTALLER_OUT) > >+ app.7z > "$(subst ?,$(space),$(WIN32_INSTALLER_OUT))" > > chmod 0755 $(WIN32_INSTALLER_OUT) > > What happens if you don't escape above, and just quote both the redirect, and > the chmod. The chmod works because the ? are expanded as wildcards? > I have to use $(subst) at the top of this section - quotes absolutely don't work there. Thus, I have to unquote it for the redirect, otherwise I got a "no such file or a directory" error. The chmod is fine with the wildcards because they are expanded, as you said.
Comment 19•16 years ago
|
||
preprocessor.pl may need something as funky as -DPKG_BASENAME="\"$(PKG_BASENAME)\"" but seriously, if subst-ing the spaces doesn't break anything, you should be able to remove those defines alltogether. Regarding the second, I was thinking about repackage-win32-installer: WIN32_INSTALLER_OUT=$(_ABS_DIST)/$(PKG_INST_PATH)$(PKG_INST_BASENAME).exe ... app.7z > "$(WIN32_INSTALLER_OUT)" chmod 0755 "$(WIN32_INSTALLER_OUT)" I.e., no quotes in the rule definition, but only in the shell commands.
Assignee | ||
Comment 20•16 years ago
|
||
Comment on attachment 344041 [details] [diff] [review] try #2 - tested on all three platforms, en-US + de So, it looks like my checkouts were before bug 458014 landed, and thus I didn't test against the right code. Updated patch to come later.
Attachment #344041 -
Flags: review?(ted.mielczarek)
Attachment #344041 -
Flags: review?(l10n)
Reporter | ||
Comment 21•16 years ago
|
||
What are those changes in package-name.mk about? they effectively change what filenames we generate and make us generate something different than what we did before. They also introduce capital letter in unix names which is something we never did.
Reporter | ||
Comment 22•16 years ago
|
||
Actually, the changes to package-name.mk completely are undoing some very much intended possibilities of overriding variables when calling this, IMHO this is unacceptable.
Assignee | ||
Comment 23•16 years ago
|
||
Okay, Axel's latest locales/Makefile.in patch has complicated matters there so this patch only addresses en-US. The interesting stuff is in package-name.mk. I've given that file the ability to generate the pretty names now - this eliminates the need to pass in MOZ_PKG_VERSION (but, it is still possible to, if you wanted to override the version number). Kairo, I'm asking ted to review this but I'd still like to know if this addresses your concerns.
Attachment #344041 -
Attachment is obsolete: true
Attachment #344499 -
Flags: review?(ted.mielczarek)
Reporter | ||
Comment 24•16 years ago
|
||
Comment on attachment 344499 [details] [diff] [review] pretty names, en-US only >diff -r f4cecdaa6e78 toolkit/mozapps/installer/package-name.mk >--- a/toolkit/mozapps/installer/package-name.mk Thu Oct 23 08:30:45 2008 -0400 >+++ b/toolkit/mozapps/installer/package-name.mk Thu Oct 23 13:51:51 2008 -0400 >@@ -105,26 +105,35 @@ > > else # "pretty" release package names > > ifndef MOZ_PKG_APPNAME > MOZ_PKG_APPNAME = $(MOZ_APP_DISPLAYNAME) > endif > MOZ_PKG_APPNAME_LC = $(shell echo $(MOZ_PKG_APPNAME) | tr '[A-Z]' '[a-z]') > >+ > ifndef MOZ_PKG_LONGVERSION nit: why two empty line if one should be enough? >-MOZ_PKG_LONGVERSION = $(MOZ_PKG_VERSION) >+MOZ_PKG_LONGVERSION = $(shell echo $(MOZ_PKG_VERSION) |\ >+ sed -e 's/a\([0-9][0-9]*\)/ Alpha \1/' |\ >+ sed -e 's/b\([0-9][0-9]*\)/ Beta \1/' |\ >+ sed -e 's/rc\([0-9][0-9]*\)/ RC \1/') > endif nice :) I still wonder how you generate "shiretoko-alpha1.tar.bz2" etc., I guess you still need to pass both different version numbers there, there's probably no way around that. >-ifeq (,$(filter-out Darwin OS2 WINNT, $(OS_ARCH))) >+ifeq (,$(filter-out Darwin OS2, $(OS_ARCH))) # Mac and OS2 > PKG_BASENAME = $(MOZ_PKG_APPNAME) $(MOZ_PKG_LONGVERSION) >+PKG_INST_BASENAME = $(MOZ_PKG_APPNAME) Setup $(MOZ_PKG_LONGVERSION) >+else >+ifeq (,$(filter-out WINNT, $(OS_ARCH))) # Windows >+PKG_BASENAME = $(MOZ_PKG_APPNAME_LC)-$(MOZ_PKG_VERSION) > PKG_INST_BASENAME = $(MOZ_PKG_APPNAME) Setup $(MOZ_PKG_LONGVERSION) > else # unix (actually, not Windows, Mac or OS/2) > PKG_BASENAME = $(MOZ_PKG_APPNAME_LC)-$(MOZ_PKG_VERSION) > PKG_INST_BASENAME = $(MOZ_PKG_APPNAME_LC)-setup-$(MOZ_PKG_VERSION) >+endif > endif Not sure about the different treatment of Windows zip and installer here, is there any specific reason for that? And it sounds a bit strange that OS/2 and Windows are treated differently ;-)
Assignee | ||
Comment 25•16 years ago
|
||
Whoops, the extra space was unintentional. You're absolutely right that this doesn't handle Alphas. We'll cross that bridge when we get to it. (We may fix that be making alpha dir structure and filename the same as beta, dunno.) As for the zip files...that's just how we distribute them. I'm not seeking to change that right now. The actual reason is probably more historical than logical, dunno!
Reporter | ||
Comment 26•16 years ago
|
||
(In reply to comment #25) > As for the zip files...that's just how we distribute them. I'm not seeking to > change that right now. The actual reason is probably more historical than > logical, dunno! Erm, it's news to me that Firefox or Thunderbird even distribute zip files in releases - and nightlies as well as SeaMonkey releases don't use pretty names ;-)
Assignee | ||
Comment 27•16 years ago
|
||
That's a good point. We do need the en-US zip file to do repacks (I really don't know why, that's just how the system works). Maybe we can drop them alltogether at some point? hopefully.
Comment 28•16 years ago
|
||
What's actually breaking for non-en-US? Regarding the zips on Windows, that's currently the easiest format to unpack, and I use that to identify the source stamp of the repackaged build. That step is probably not needed for release builds, as those build off tags, and thus can get their known sources anyway.
Reporter | ||
Comment 29•16 years ago
|
||
I think it's good that we are actually packaging zips, I know people who regularly use them for nightlies and for SeaMonkey, so I'd rather not get rid of them. But if we aren't actually shipping them for releases, I don't think we need to special-case them in package-name.mk and can merge the Mac, OS/2 and Windows cases on the prettyname side again ;-)
Assignee | ||
Comment 30•16 years ago
|
||
We do use them for releases, though. They are used in l10n repacks. As I mentioned in comment#25 I'm not looking to change that as part of this bug. Once we have working infrastructure for l10n + releases on Mercurial we can look at that. (It kindof sucks that package-name.mk is shared across all projects.)
Assignee | ||
Comment 31•16 years ago
|
||
(In reply to comment #28) > What's actually breaking for non-en-US? > We talked about this on IRC, remember? :) bug 461340 - browser/locales/Makefile.in broken when not using wget-* targets.
Comment 32•16 years ago
|
||
Comment on attachment 344499 [details] [diff] [review] pretty names, en-US only +MOZ_PKG_LONGVERSION = $(shell echo $(MOZ_PKG_VERSION) |\ + sed -e 's/a\([0-9][0-9]*\)/ Alpha \1/' |\ + sed -e 's/b\([0-9][0-9]*\)/ Beta \1/' |\ + sed -e 's/rc\([0-9][0-9]*\)/ RC \1/') Could you make these regexps end with '$'? (I know the quoting will probably get ugly in the makefile.)
Attachment #344499 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 33•16 years ago
|
||
same as before, with the terminating $s changeset: 20812:1a342e4331ff
Assignee: kairo → bhearsum
Attachment #344499 -
Attachment is obsolete: true
Status: REOPENED → ASSIGNED
Assignee | ||
Updated•16 years ago
|
Whiteboard: [hg-automation]
Assignee | ||
Comment 35•16 years ago
|
||
Going to try and get this working for locales now...
Priority: -- → P2
Assignee | ||
Comment 36•16 years ago
|
||
Alright, so this was less troublesome than I thought it would be. Some of this patch is just quoting where necessary, but there are some other changes: * Use ZIP_IN for repackage-zip stuff (and set ZIP_IN to default to $(_ABS_DIST)/$(PACKAGE) as to not break existing code that doesn't pass it) * Don't use $(dir) when testing for existence in repackage-win32-installer-%. This is necessary because $(dir) does not work with spaces in the filename. * When using MOZ_PKG_PRETTYNAMES we have to move the unpacked package from a directory named after $(MOZ_APP_NAME) to one named after $(MOZ_PKG_APPNAME). If we don't do tools/update-packaging won't be able to find the directory to create a MAR from. Mac does this in it's $(UNMAKE_PACKAGE) anyways, so we only do this for Linux and Windows. I also discovered that the regexps in package-name.mk are broken, so I fixed them.
Attachment #348007 -
Flags: review?(ted.mielczarek)
Attachment #348007 -
Flags: review?(l10n)
Assignee | ||
Comment 37•16 years ago
|
||
Comment on attachment 348007 [details] [diff] [review] MOZ_PKG_PRETTYNAMES support for l10n wrong patch, will reattach
Attachment #348007 -
Attachment is obsolete: true
Attachment #348007 -
Flags: review?(ted.mielczarek)
Attachment #348007 -
Flags: review?(l10n)
Assignee | ||
Comment 38•16 years ago
|
||
See my previous comment for details.
Attachment #348008 -
Flags: review?(ted.mielczarek)
Attachment #348008 -
Flags: review?(l10n)
Comment 39•16 years ago
|
||
Comment on attachment 348008 [details] [diff] [review] MOZ_PKG_PRETTYNAMES support for l10n +ifndef ZIP_IN +ZIP_IN = $(_ABS_DIST)/$(PACKAGE) +endif You can write this as ZIP_IN ?= $(_ABS_DIST)/$(PACKAGE)
Attachment #348008 -
Flags: review?(ted.mielczarek) → review+
Comment 40•16 years ago
|
||
Comment on attachment 348008 [details] [diff] [review] MOZ_PKG_PRETTYNAMES support for l10n r=me, with ted's nit. As per IRC, MOZ_PKG_PRETTYNAMES only works when ZIP_IN is explicitly set to something non-pretty. Add a comment to that extent, too?
Attachment #348008 -
Flags: review?(l10n) → review+
Assignee | ||
Comment 41•16 years ago
|
||
Carrying review forward here.
Attachment #348008 -
Attachment is obsolete: true
Assignee | ||
Comment 42•16 years ago
|
||
Comment on attachment 348192 [details] [diff] [review] [checked in] MOZ_PKG_PRETTYNAMES for l10n - comments addressed Mike, this is NPOT en-US Build and would really help me with 3.1b2 l10n builds. Any chance I can get this in for 3.1b2?
Attachment #348192 -
Flags: approval1.9.1b2?
Assignee | ||
Updated•16 years ago
|
Attachment #348192 -
Flags: approval1.9.1b2?
Assignee | ||
Comment 43•16 years ago
|
||
Comment on attachment 348192 [details] [diff] [review] [checked in] MOZ_PKG_PRETTYNAMES for l10n - comments addressed Mossop landed this for me in b7a7180130d5. I'll be watching Mozilla-l10n closely for breakage.
Attachment #348192 -
Attachment description: MOZ_PKG_PRETTYNAMES for l10n - comments addressed → [checked in] MOZ_PKG_PRETTYNAMES for l10n - comments addressed
Assignee | ||
Comment 45•16 years ago
|
||
I just saw a run of 'uk' go by fine on bsmedberg's l10n buildbot. I'll be watching the l10n Buildbot on staging-master too, but I'm pretty confident everything will be OK.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 46•16 years ago
|
||
So, it turns out that all my testing was done by repacking nightly builds. The big difference here is that nightly en-US builds are packaged without MOZ_PKG_PRETTYNAMES, which means the containing directories are 'firefox'. Release builds are (now) packaged with MOZ_PKG_PRETTYNAMES, which means the containing directory is 'Firefox'. What we should probably do here is removing the 'mv firefox Firefox' that's done in STAGE_DIST and simply declare that if the en-US was packaged with MOZ_PKG_PRETTYNAMES that you must package the l10n builds that way as well. I'll be looking into this more after 3.1b2.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 47•16 years ago
|
||
This problem bit me again in our update testing scripts. It also caused some confusion to folks who downloaded the Linux tarball. I also wonder if there are Linux packagers who take our tarball (gentoo?) - this could break them as well. I'm not going back to fix it for 3.1b2 at this point but going forward I think we should revert back to 'firefox'. This seems to be an unintended change, anyways.
Reporter | ||
Comment 48•16 years ago
|
||
Yes, this is unintended, it shouldn't be different than before... did we have this named "shiretoko" or "firefox" for e.g. alpha 1 (or for BonEcho alphas)?
Assignee | ||
Comment 49•16 years ago
|
||
Just to be clear, I'm talking about the first level directory inside of a tarball or zip. Other than for this build I don't know of any build of Firefox which has named this directory differently. Eg: bitters-2:tmp bhearsum$ tar -jvxf shiretoko-alpha2.tar.bz2 firefox/
Reporter | ||
Comment 50•16 years ago
|
||
Here's a patch for the in-package (and in-dist) directory to not end up with the display name used for the package name even with "pretty" names. For this, I've created another var in packager.mk named MOZ_PKG_DIR because I felt the code would be harder to understand if I'd place MOZ_APPNAME there, as a side effect, it can be overridden - for whatever reason someone might want that. I've only tested this on Linux, where a prettynames package that calls itself "minefield-something.tar.bz2" correctly ended up with a firefox/ directory inside.
Attachment #350399 -
Flags: review?(ted.mielczarek)
Assignee | ||
Comment 51•16 years ago
|
||
Given that we're fixing the directories inside of the packages to remain lowercase we don't need this special case anymore.
Attachment #351550 -
Flags: review?(ted.mielczarek)
Attachment #351550 -
Flags: review?(l10n)
Updated•16 years ago
|
Attachment #350399 -
Flags: review?(ted.mielczarek) → review+
Updated•16 years ago
|
Attachment #351550 -
Flags: review?(ted.mielczarek) → review+
Updated•16 years ago
|
Attachment #351550 -
Flags: review?(l10n)
Comment 52•16 years ago
|
||
Comment on attachment 351550 [details] [diff] [review] [checked in] remove MOZ_PKG_PRETTYNAMES special case in Makefile.in I don't really understand how this interacts with the universe, so I'll go with Ted's r, and I'm canceling this review on me.
Assignee | ||
Updated•16 years ago
|
Attachment #350399 -
Attachment description: create a MOZ_PKG_NAME for the in-package dir → [checked in] create a MOZ_PKG_NAME for the in-package dir
Assignee | ||
Comment 53•16 years ago
|
||
Comment on attachment 350399 [details] [diff] [review] [checked in] create a MOZ_PKG_NAME for the in-package dir o changeset: 22375:53326bcc0158
Attachment #351550 -
Attachment description: remove MOZ_PKG_PRETTYNAMES special case in Makefile.in → [checked in] remove MOZ_PKG_PRETTYNAMES special case in Makefile.in
Assignee | ||
Comment 54•16 years ago
|
||
Comment on attachment 351550 [details] [diff] [review] [checked in] remove MOZ_PKG_PRETTYNAMES special case in Makefile.in @ changeset: 22376:5b62f3c644cb
Assignee | ||
Comment 55•16 years ago
|
||
I'm pretty sure this is done now (finally)
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 56•16 years ago
|
||
Ugh, I forgot that I still need to land these on mozilla-1.9.1
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Updated•16 years ago
|
Attachment #350399 -
Flags: approval1.9.1?
Assignee | ||
Comment 57•16 years ago
|
||
Comment on attachment 350399 [details] [diff] [review] [checked in] create a MOZ_PKG_NAME for the in-package dir This patch is required for release automation going forward. It gently touches the packaging code on all platforms but has cycled green on mozilla-central already. Given that, I feel it is very low risk.
Updated•16 years ago
|
Attachment #350399 -
Flags: approval1.9.1? → approval1.9.1+
Comment 58•16 years ago
|
||
a=beltzner
Assignee | ||
Comment 59•16 years ago
|
||
Comment on attachment 351550 [details] [diff] [review] [checked in] remove MOZ_PKG_PRETTYNAMES special case in Makefile.in changeset: 22217:89bf1ad2d30f (mozilla-1.9.1)
Assignee | ||
Updated•16 years ago
|
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Keywords: fixed1.9.1
Target Milestone: --- → mozilla1.9.1b3
Assignee | ||
Updated•16 years ago
|
Target Milestone: mozilla1.9.1b3 → ---
Assignee | ||
Comment 60•15 years ago
|
||
Comment on attachment 351550 [details] [diff] [review] [checked in] remove MOZ_PKG_PRETTYNAMES special case in Makefile.in Turns out I forgot to land this in mozilla-1.9.1. Landed now in: changeset: 22788:a28d67a980fe
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•