Closed Bug 741014 Opened 9 years ago Closed 8 years ago
pymake is busted with make l10n-check
see bug 593585#c30 and below
I can't reproduce the issue in comment 30 locally. Instead, I get a different issue, ending with sed: can't read components/*.manifest: No such file or directory c:\Users\Sid\mozilla\mozilla-central\toolkit\locales\l10n.mk:147:0: command '(cd c:/Users/Sid/mozilla/fxto/browser/locales/../../dist/l10n-stage/firefox && c:/Users/Sid/mozilla/fxto/_virtualenv/Scripts/python.exe c:/Users/Sid/mozilla/fxto/browser/locales/../../../mozilla-central/config/optimizejars.py --deoptimize c:/Users/Sid/mozilla/fxto/browser/locales/../../jarlog//x-test ./ ./ && unzip -o omni.ja && rm -f components/binary.manifest && for m in components/*.manifest; do sed -e 's/^#binary-component/binary-component/' $m > tmp.manifest && mv tmp.manifest $m; done)' failed, return code 2
Oh, nvm, I can reproduce with MOZ_PKG_PRETTYNAMES=1.
The problem is that $(WIN32_INSTALLER_IN) at https://mxr.mozilla.org/mozilla-central/source/browser/locales/Makefile.in#159 is "Firefox Setup 17.0a1.exe" without quotes (for example), which is escaped by ESCAPE_SPACE to "Firefox\ Setup\ 17.0a1.exe" without quotes. Pymake parses this not as one requirement but as three.
The three requirements being "Firefox\", "Setup\" and "17.0a1.exe" all without quotes.
This probably isn't the exact issue we're seeing, but it makes pymake crash immediately so it's a little more useful :)
Of course, a reasonable argument could be made that GNU make was never meant to support spaces, so we shouldn't even bother and should instead rename the setup file. See <http://savannah.gnu.org/bugs/?712>.
We work around this with QUOTED_WILDCARD elsewhere, not sure if that will work in this case: http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/packager.mk#973 (It only really works if the file already exists.)
Yeah, I think it's assumed that the file should already exists.
So... I've switched to escaping via wildcards (for a comparison, see http://www.cmcrossroads.com/ask-mr-make/7859-gnu-make-meets-file-names-with-spaces-in-them). This is different from QUOTED_WILDCARD -- neither GNU make nor pymake recognizes escaping via quotes. There was also an underspecified dependency: repackage-win32-installer-% depends on repackage-zip-% because the repackage-zip step is the one that unzips the data needed to create the repackaged installer. It was pure chance that was making it work earlier. With these two problems fixed, pymake l10n-check seems to work great.
Comment on attachment 648196 [details] [diff] [review] patch v1 Given how hairy and fragile this code is, I'd like both khuey and ted to look at it (unless one of you feels your review is enough).
I believe the quoting part of QUOTED_WILDCARD is just to make it work properly when we pass it on the commandline to upload.py. The escaping with wildcard characters is to make $(wildcard) happy.
Yeah, shell has somewhat different semantics. What a mess.
This patch might be wrong. I'll investigate tomorrow.
The dependency is actually the other way around (repackage-zip depends on repackage-win32-installer files generated in the l10ngen directory), though both depend on $(STAGEDIST).
Comment on attachment 648577 [details] [diff] [review] better patch Review of attachment 648577 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/mozapps/installer/packager.mk @@ +976,2 @@ > > # This variable defines which OpenSSL algorithm to use to Kill the trailing space on this line while you're here?
Attachment #648577 - Flags: review?(ted.mielczarek) → review+
Assignee: nobody → sagarwal
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.