Closed Bug 738349 Opened 8 years ago Closed 8 years ago

Categories

(Release Engineering :: Release Automation: Other, defect, P1, major)

All
macOS

Tracking

(firefox-esr10 fixed)

RESOLVED FIXED
Tracking Status
firefox-esr10 --- fixed

People

(Reporter: Terry.F1Com, Assigned: rail)

References

()

Details

Attachments

(1 file)

No description provided.
Changing Component to Security?  Continuing to receive Mozilla webmaster complaints about the ASC file missing.  I would think this would receive the attention to get corrected.
Severity: normal → major
Component: Untriaged → Security
QA Contact: untriaged → firefox
Sending over to release engineering
Component: Security → Release Engineering
Product: Firefox → mozilla.org
QA Contact: firefox → release
Version: 11 Branch → other
Component: Release Engineering → Release Engineering: Releases
QA Contact: release → bhearsum
It looks like the .info file got named wrong, and copied into the release directory, too. Rail, any idea why we didn't get gpg sigs here? I feel like that's a bug we already fixed...
I spun off the info file issue into bug 740814.

I _think_ Rail is looking at the GPG issue already.
Assignee: nobody → rail
Component: Release Engineering: Releases → Release Engineering: Automation (Release Automation)
I think that the problem is how we iterate over UPLOAD_EXTRA_FILES here: http://hg.mozilla.org/mozilla-central/file/0cfa44a13b25/toolkit/mozapps/installer/packager.mk#l958

On Mac $(PACKAGE).asc is "mac/en-US/Firefox 11.0.dmg.asc" (INSTALLER_PACKAGE is win32 only iirc), so when we add it here http://hg.mozilla.org/mozilla-central/file/0cfa44a13b25/toolkit/mozapps/installer/packager.mk#l559 and then try to iterate using foreach, this function splits it to "mac/en-US/Firefox" and "11.0.dmg.asc". Then wildcard function properly finds the first part (fixed post 11.0 in bug 731795) and successfully drops the second part (no such file).

I think it would be safer to use UPLOAD_FILES here.
Attachment #610893 - Flags: feedback?(ted.mielczarek)
Attachment #610893 - Flags: feedback?(catlee)
Hum. This is sort of unfortunate. Perhaps we should remove the foreach, and just require things in UPLOAD_EXTRA_FILES to use QUOTED_WILDCARD themselves?
We also use UPLOAD_EXTRA_FILES in release automation to easily pass files: http://mxr.mozilla.org/build/search?string=UPLOAD_EXTRA_FILES
(In reply to Ted Mielczarek [:ted] from comment #6)
> Hum. This is sort of unfortunate. Perhaps we should remove the foreach, and
> just require things in UPLOAD_EXTRA_FILES to use QUOTED_WILDCARD themselves?

I think this is a good idea. Even though the attached patch fixes the current problem, it doesn't guarantees that we won't hit this problem in the future...
One thing I really like in the current implementation that you can pass UPLOAD_EXTRA_FILES and you don't need to care about $(DIST) which may be so unpredicted outside of the build system (especially for universal builds)...
I signed the DMG files manually and uploaded them to stage. Mirrors should pickup them shortly.

Instructions:
https://wiki.mozilla.org/Releases/Firefox_11.0/BuildNotes#Detached_GPG_signing_for_Mac_dmg_files
What do you think about resolving the issue in 2 steps:

1) Apply the patch attached (will be tested, of course). It will fix the current issue, so we don't need to sign the files manually again. It doesn't require any changes in automation and could be landed on all branches after testing.

2) File another bug (yay!) to get rid of "foreach". It requires more testing and release automation changes to properly pass UPLOAD_EXTRA_FILES. It should be landed on all release branches once it hits beta, since we'll need to change the automation logic.
Comment on attachment 610893 [details] [diff] [review]
don't use UPLOAD_EXTRA_FILES

Review of attachment 610893 [details] [diff] [review]:
-----------------------------------------------------------------

Yeah, that works for me.
Attachment #610893 - Flags: feedback?(ted.mielczarek) → feedback+
Attachment #610893 - Flags: feedback?(catlee) → feedback+
I pushed to try, will test release builds next week.
Try run for 4a0bf3bd7f52 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=4a0bf3bd7f52
Results (out of 15 total builds):
    success: 15
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/raliiev@mozilla.com-4a0bf3bd7f52
Comment on attachment 610893 [details] [diff] [review]
don't use UPLOAD_EXTRA_FILES

I've just checked a staging release and everything looks fine. I compared directories of staging and production ftps, they look fine as well.

Time to push it live! :)
Attachment #610893 - Flags: review?(ted.mielczarek)
Priority: -- → P1
Comment on attachment 610893 [details] [diff] [review]
don't use UPLOAD_EXTRA_FILES

Review of attachment 610893 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry, I thought I had r+ed this a while ago. Apparently I didn't hit submit!
Attachment #610893 - Flags: review?(ted.mielczarek) → review+
Comment on attachment 610893 [details] [diff] [review]
don't use UPLOAD_EXTRA_FILES

I would like to push the patch to all branches:

* it fixes release automation so it signs Mac builds properly
* it removes extra manual steps in the release process
* the patch affects the builds system only, it's simple, and the risk is very low

Regression caused by (bug #): the current bug
User impact if declined: N/A, it only affects the files not used by end user
Testing completed (on m-c, etc.): I tested it for a couple of staging releases
Risk to taking this patch (and alternatives if risky): very low
String changes made by this patch: None
Attachment #610893 - Flags: approval-mozilla-release?
Attachment #610893 - Flags: approval-mozilla-esr10?
Attachment #610893 - Flags: approval-mozilla-beta?
Attachment #610893 - Flags: approval-mozilla-aurora?
Comment on attachment 610893 [details] [diff] [review]
don't use UPLOAD_EXTRA_FILES

please go ahead and land on all except m-r, we will have to wait and get this on the merge for m-r since it's npotb and we wouldn't need it in a chemspill either it can wait for the trains.
Attachment #610893 - Flags: approval-mozilla-release?
Attachment #610893 - Flags: approval-mozilla-release-
Attachment #610893 - Flags: approval-mozilla-esr10?
Attachment #610893 - Flags: approval-mozilla-esr10+
Attachment #610893 - Flags: approval-mozilla-beta?
Attachment #610893 - Flags: approval-mozilla-beta+
Attachment #610893 - Flags: approval-mozilla-aurora?
Attachment #610893 - Flags: approval-mozilla-aurora+
Looks fine in 12.0b4:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/12.0b5-candidates/build1/mac/en-US/
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.