Closed
Bug 1407285
Opened 7 years ago
Closed 6 years ago
Support spaces in MOZ_MACBUNDLE_NAME
Categories
(Firefox Build System :: General, defect)
Tracking
(firefox58 wontfix, firefox59 fixed)
RESOLVED
FIXED
mozilla59
People
(Reporter: Sylvestre, Assigned: Sylvestre)
References
(Depends on 1 open bug)
Details
Attachments
(3 files, 3 obsolete files)
Otherwise, ./mach package is failing
Assignee | ||
Comment 1•7 years ago
|
||
Assignee | ||
Comment 2•7 years ago
|
||
https://hg.mozilla.org/projects/oak/rev/b9704f9b94d0ad49f1dd9e8e3fb46454ea5935df Bug 1407285 - Support spaces in MOZ_MACBUNDLE_NAME
Assignee | ||
Updated•7 years ago
|
Attachment #8917028 -
Attachment is obsolete: true
Assignee | ||
Comment 3•7 years ago
|
||
https://hg.mozilla.org/projects/oak/rev/13953f71fa28da1348802444b1ed3b305897cfb8 Bug 1407285 - Handle path with spaces in tools/update-packaging/
Assignee | ||
Comment 4•7 years ago
|
||
https://hg.mozilla.org/projects/oak/rev/9cfe8735803804b8545b18af322d538e1fcc6cd9 Bug 1407285 - Handle path with spaces in ident: in browser/locales
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → sledru
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
status-firefox59:
--- → affected
Comment 8•6 years ago
|
||
mozreview-review |
Comment on attachment 8928888 [details] Bug 1407285 - Support spaces in MOZ_MACBUNDLE_NAME and in various Makefile and tools https://reviewboard.mozilla.org/r/200200/#review205438 These changes all look fine to me, but you will want to get an r+ from someone in "Build and Release Tools"[1] and/or "Build Config"[2] for these changes. [1] https://wiki.mozilla.org/Modules/All#Build_and_Release_Tools [2] https://wiki.mozilla.org/Modules/All#Build_Config
Attachment #8928888 -
Flags: review?(spohl.mozilla.bugs)
Comment 9•6 years ago
|
||
mozreview-review |
Comment on attachment 8928889 [details] Bug 1407285 - Handle path with spaces in tools/update-packaging/ https://reviewboard.mozilla.org/r/200202/#review205440
Attachment #8928889 -
Flags: review?(spohl.mozilla.bugs)
Comment 10•6 years ago
|
||
mozreview-review |
Comment on attachment 8928890 [details] Bug 1407285 - Handle path with spaces in the ident: target Makefile in browser/locales https://reviewboard.mozilla.org/r/200204/#review205434 ::: commit-message-ea1c8:1 (Diff revision 1) > +Bug 1407285 - Handle path with spaces in ident: in browser/locales r?spohl nit: fix commit message
Attachment #8928890 -
Flags: review?(spohl.mozilla.bugs)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 14•6 years ago
|
||
mozreview-review |
Comment on attachment 8928888 [details] Bug 1407285 - Support spaces in MOZ_MACBUNDLE_NAME and in various Makefile and tools https://reviewboard.mozilla.org/r/200200/#review208826 ::: browser/app/Makefile.in:86 (Diff revision 2) > MAC_BUNDLE_VERSION = $(shell $(PYTHON) $(srcdir)/macversion.py --version=$(MOZ_APP_VERSION) --buildid=$(DEPTH)/buildid.h) > > .PHONY: repackage > tools repackage:: $(DIST)/bin/$(MOZ_APP_NAME) features > rm -rf $(dist_dest) > - $(MKDIR) -p $(dist_dest)/Contents/MacOS > + $(MKDIR) -p "$(dist_dest)/Contents/MacOS" Please use single quotes. ::: old-configure.in:4505 (Diff revision 2) > MOZ_ARG_WITH_STRING(macbundlename-prefix, > [ --with-macbundlename-prefix=prefix > Prefix for MOZ_MACBUNDLE_NAME], > [ MOZ_MACBUNDLE_NAME_PREFIX="$withval"]) > > -MOZ_MACBUNDLE_NAME=`echo $MOZ_APP_DISPLAYNAME | tr -d ' '` > +MOZ_MACBUNDLE_NAME=`echo $MOZ_APP_DISPLAYNAME` No need for `echo` ::: old-configure.in:4511 (Diff revision 2) > if test "$MOZ_MACBUNDLE_NAME_PREFIX"; then > MOZ_MACBUNDLE_NAME="${MOZ_MACBUNDLE_NAME_PREFIX}${MOZ_MACBUNDLE_NAME}" > fi > > if test "$MOZ_DEBUG"; then > MOZ_MACBUNDLE_NAME=${MOZ_MACBUNDLE_NAME}Debug.app This should probably change to add a space. Same for MOZ_MACBUNDLE_NAME_PREFIX.
Attachment #8928888 -
Flags: review?(mh+mozilla)
Comment 15•6 years ago
|
||
mozreview-review |
Comment on attachment 8928889 [details] Bug 1407285 - Handle path with spaces in tools/update-packaging/ https://reviewboard.mozilla.org/r/200202/#review208828 This should be folded with the previous patch.
Attachment #8928889 -
Flags: review?(mh+mozilla) → review+
Comment 16•6 years ago
|
||
mozreview-review |
Comment on attachment 8928890 [details] Bug 1407285 - Handle path with spaces in the ident: target Makefile in browser/locales https://reviewboard.mozilla.org/r/200204/#review208830 ::: browser/locales/Makefile.in:198 (Diff revision 2) > endif > > ident: > @printf 'fx_revision ' > @$(PYTHON) $(topsrcdir)/config/printconfigsetting.py \ > - $(STAGEDIST)/application.ini App SourceStamp > + "$(STAGEDIST)"/application.ini App SourceStamp single quotes
Attachment #8928890 -
Flags: review?(mh+mozilla) → review+
Comment 17•6 years ago
|
||
mozreview-review |
Comment on attachment 8928890 [details] Bug 1407285 - Handle path with spaces in the ident: target Makefile in browser/locales https://reviewboard.mozilla.org/r/200204/#review208832 This should be folded too.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Attachment #8928889 -
Attachment is obsolete: true
Assignee | ||
Updated•6 years ago
|
Attachment #8928890 -
Attachment is obsolete: true
Comment 19•6 years ago
|
||
mozreview-review |
Comment on attachment 8928888 [details] Bug 1407285 - Support spaces in MOZ_MACBUNDLE_NAME and in various Makefile and tools https://reviewboard.mozilla.org/r/200200/#review210064
Attachment #8928888 -
Flags: review?(mh+mozilla) → review+
Comment 20•6 years ago
|
||
Pushed by sledru@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9fff8b2fcf78 Support spaces in MOZ_MACBUNDLE_NAME and in various Makefile and tools r=glandium
Comment 21•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9fff8b2fcf78
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Comment 22•6 years ago
|
||
Backed out for L10n bustage: https://hg.mozilla.org/mozilla-central/rev/88b2d7276416f8b69191ca5fb1b5c670ec8178b8 Push with l10n bustage: https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=5be384bcf00191f97d32b4ac3ecd1b85ec7b18e1&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable From IRC: Pike: Callek_cloud9: any idea what changed the package name to Firefox Nightly.app instead of FirefoxNightly.app?
Status: RESOLVED → REOPENED
Flags: needinfo?(sledru)
Resolution: FIXED → ---
Target Milestone: mozilla59 → ---
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 26•6 years ago
|
||
(In reply to Sylvestre Ledru [:sylvestre] from comment #25) > Why I didn't have this issue in autoland? Try and autoland don't build l10n builds by default, Callek's working on that as we speak. I've kicked off some try-magic on your review request on https://treeherder.mozilla.org/#/jobs?repo=try&revision=f77d00b63077ef671ff307ba8e845d277a49c63a, just to make sure we're not missing stuff.
Comment 27•6 years ago
|
||
Sylvestre, can you rebase these on top of a good m-c changeset? I'd like to see results on Windows, and they're busted in signing, on top of the Android nightly that's busted for things underneath.
Flags: needinfo?(sledru)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 30•6 years ago
|
||
Voila!
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(sledru)
Comment 32•6 years ago
|
||
mozreview-review |
Comment on attachment 8934486 [details] Bug 1407285 - Support spaces in toolkit/locales/l10n.mk https://reviewboard.mozilla.org/r/205402/#review211386
Attachment #8934486 -
Flags: review?(l10n) → review+
Comment 33•6 years ago
|
||
Thanks for the try builds. I didn't find any windows codepaths that have spaces. Windows is sadly way more erratic on quoting, but as long as we don't go down there, this should be good.
Flags: needinfo?(l10n)
Comment 34•6 years ago
|
||
Pushed by sledru@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6f62a6e41778 Support spaces in toolkit/locales/l10n.mk r=Pike https://hg.mozilla.org/integration/autoland/rev/72a55f390248 Support spaces in MOZ_MACBUNDLE_NAME and in various Makefile and tools r=glandium
Comment 35•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6f62a6e41778 https://hg.mozilla.org/mozilla-central/rev/72a55f390248
Status: REOPENED → RESOLVED
Closed: 6 years ago → 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Comment hidden (mozreview-request) |
Assignee | ||
Comment 37•6 years ago
|
||
Tom, please open a new bug for follow up changes. Thanks
Flags: needinfo?(mozilla)
Comment 38•6 years ago
|
||
mozreview-review |
Comment on attachment 8935233 [details] Bug 1407285: Use `'` consistently in make, and fix package-compare; https://reviewboard.mozilla.org/r/206110/#review212048
Attachment #8935233 -
Flags: review?(mh+mozilla) → review+
Comment 39•6 years ago
|
||
Pushed by sledru@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0bc64e08d846 Use `'` consistently in make, and fix package-compare; r=glandium
Updated•6 years ago
|
Flags: needinfo?(mozilla)
Comment 40•6 years ago
|
||
Pushed by mozilla@hocat.ca: https://hg.mozilla.org/comm-central/rev/564d515b57a1 Port bug 1407285 to Thunderbird and SeaMonkey; rs=bustage-fix
Comment 41•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0bc64e08d846
Comment 42•6 years ago
|
||
It looks like this breaks updates on OS X in at least one case. We're seeing this on maple when generating complete MARs: [task 2017-12-07T22:18:03.511Z] 22:18:03 INFO - ~/workspace/build/src/obj-firefox/dist/firefox/Firefox Nightly.app ~/workspace/build/src/obj-firefox/tools/update-packaging [task 2017-12-07T22:18:03.512Z] 22:18:03 INFO - ~/workspace/build/src/obj-firefox/tools/update-packaging [task 2017-12-07T22:18:03.512Z] 22:18:03 INFO - Adding type instruction to update manifests [task 2017-12-07T22:18:03.512Z] 22:18:03 INFO - type complete [task 2017-12-07T22:18:03.513Z] 22:18:03 INFO - Adding file add instructions to update manifests [task 2017-12-07T22:18:03.513Z] 22:18:03 INFO - add "Contents/Resources/updater.ini" [task 2017-12-07T22:18:03.513Z] 22:18:03 INFO - /builds/worker/workspace/build/src/tools/update-packaging/common.sh: line 93: [: too many arguments [task 2017-12-07T22:18:03.513Z] 22:18:03 INFO - add-if-not "Contents/Resources/update-settings.ini" "Contents/Resources/update-settings.ini" [task 2017-12-07T22:18:03.514Z] 22:18:03 INFO - /builds/worker/workspace/build/src/tools/update-packaging/common.sh: line 127: $filev3: ambiguous redirect [task 2017-12-07T22:18:03.514Z] 22:18:03 INFO - add "Contents/Resources/update-settings.ini" (forced) [task 2017-12-07T22:18:03.515Z] 22:18:03 INFO - add "Contents/Resources/res/cursors/zoomOut@2x.png" [task 2017-12-07T22:18:03.515Z] 22:18:03 INFO - /builds/worker/workspace/build/src/tools/update-packaging/common.sh: line 93: [: too many arguments [task 2017-12-07T22:18:03.515Z] 22:18:03 INFO - add "Contents/Resources/res/cursors/zoomOut.png" ... and we end up with an empty updatev3.manifest (which ends up removing the entire install). It seems like this code has successfully been used for mozilla-central nightlies already, so I'm not quite sure what the difference is. We're doing Betas and DevEdition on maple, so it could be something that only shows up after an uplift?
Comment 43•6 years ago
|
||
Full log is available at https://public-artifacts.taskcluster.net/OP5T5uKuSGiey7G-JKusEA/0/public/logs/live_backing.log
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
•