Closed Bug 732976 Opened 8 years ago Closed 8 years ago

SingleSourceFactory should generate checksums file

Categories

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

All
Linux

Tracking

(firefox-esr10 fixed)

RESOLVED FIXED
Tracking Status
firefox-esr10 --- fixed

People

(Reporter: rail, Assigned: rail)

References

Details

(Whiteboard: [releases][automation][wait till 3.6 EOL])

Attachments

(4 files, 7 obsolete files)

Attached patch generate checksums (obsolete) — Splinter Review
We have switched to generating MD5/SHA1/SHA512SUMS files out of checksums files recently. Unfortunately "make source-package" doesn't generate checksums file and we can't use "make upload" to upload source files. Additionally we generate an hg bundle without using a build system target.

The following patch generates and signs checksums for this factory. Worked fine in staging
Attachment #602908 - Flags: review?(bhearsum)
Comment on attachment 602908 [details] [diff] [review]
generate checksums

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

Can't we fix the build system to do this?
Attached patch build system part (obsolete) — Splinter Review
(In reply to Ben Hearsum [:bhearsum] from comment #1)
> Can't we fix the build system to do this?

Actually I thought about that, but for some reason chose the easiest way.  :)

Attached patch implements build system part. Automation patch incoming.

Still need to be be tested against all supported release branches.
Attachment #603513 - Flags: feedback?(bhearsum)
Attached patch buildbotcustom (obsolete) — Splinter Review
* uses hgtool
* 1 files changed, 28 insertions(+), 91 deletions(-) :)
Attachment #602908 - Attachment is obsolete: true
Attachment #603515 - Flags: feedback?(bhearsum)
Attachment #602908 - Flags: review?(bhearsum)
Attached patch build system part (obsolete) — Splinter Review
+ signing
Attachment #603513 - Attachment is obsolete: true
Attachment #603673 - Flags: review?(bhearsum)
Attachment #603513 - Flags: feedback?(bhearsum)
Attached patch buildbotcustom (obsolete) — Splinter Review
+ signing
Attachment #603674 - Flags: feedback?(bhearsum)
Attachment #603515 - Attachment is obsolete: true
Attachment #603515 - Flags: feedback?(bhearsum)
In case if we want to land everything before 3.6 EOL.
Try run for 4df7f4cbbeea is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=4df7f4cbbeea
Results (out of 12 total builds):
    success: 12
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/raliiev@mozilla.com-4df7f4cbbeea
 Timed out after 06 hours without completing.
Comment on attachment 603673 [details] [diff] [review]
build system part

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

I'd like to see what Ted has to say, too.

::: toolkit/mozapps/installer/packager.mk
@@ +1011,5 @@
> +HG_BUNDLE_FILE = $(DIST)/$(PKG_SRCPACK_PATH)$(PKG_BUNDLE_BASENAME).bundle
> +SOURCE_CHECKSUM_FILE = $(DIST)/$(PKG_SRCPACK_PATH)$(PKG_SRCPACK_BASENAME).checksums
> +SOURCE_UPLOAD_FILES = $(SOURCE_TAR)
> +
> +HG = hg

I think you might want HG =: hg, because IIRC that lets it be overridable.

@@ +1034,3 @@
>  # source-package creates a source tarball from the files in MOZ_PKG_SRCDIR,
>  # which is either set to a clean checkout or defaults to $topsrcdir
> +source-package: hg-bundle

Hmmm...it seems a bit wrong to require the hg bundle in order to create the source package. Is there any way this can be done without requiring it? Maybe there's some common functionality that can be factored out into a smaller dependency?
Attachment #603673 - Flags: review?(ted.mielczarek)
Comment on attachment 603674 [details] [diff] [review]
buildbotcustom

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

::: process/factory.py
@@ +4410,3 @@
>                   mozconfigBranch="production", appVersion=None, **kwargs):
>          ReleaseFactory.__init__(self, buildSpace=buildSpace, **kwargs)
>  

I think you can drop some parameters from this factory now. Eg, stage*
Attachment #603674 - Flags: feedback?(bhearsum) → feedback+
(In reply to Ben Hearsum [:bhearsum] from comment #8)
> I think you might want HG =: hg, because IIRC that lets it be overridable.

or HG ?= hg
 
> Hmmm...it seems a bit wrong to require the hg bundle in order to create the
> source package. Is there any way this can be done without requiring it?
> Maybe there's some common functionality that can be factored out into a
> smaller dependency?

hg-bundle is a no-op step unless you set CREATE_HG_BUNDLE
(In reply to Rail Aliiev [:rail] from comment #10)
> (In reply to Ben Hearsum [:bhearsum] from comment #8)
> > I think you might want HG =: hg, because IIRC that lets it be overridable.
> 
> or HG ?= hg

Er, yeah.

> > Hmmm...it seems a bit wrong to require the hg bundle in order to create the
> > source package. Is there any way this can be done without requiring it?
> > Maybe there's some common functionality that can be factored out into a
> > smaller dependency?
> 
> hg-bundle is a no-op step unless you set CREATE_HG_BUNDLE

Ah. Why does source-bundle depend on it at all then? Just to make sure both get created from the call of one target?
(In reply to Ben Hearsum [:bhearsum] from comment #11)
> Ah. Why does source-bundle depend on it at all then? Just to make sure both
> get created from the call of one target?

I added that dependency mostly to simplify concatenation of SOURCE_UPLOAD_FILES.

I can remove that dependency and run "make hg-bundle source-package CREATE_HG_BUNDLE=1 HG_BUNDLE_REVISION=$rev" instead.
* removed hg-bundle dependency from source-package
* HG ?= hg
* used UPLOAD_HG_BUNDLE to populate SOURCE_UPLOAD_FILES instead of CREATE_HG_BUNDLE (which is created by explicit "make hg-bundle" call)
Attachment #603673 - Attachment is obsolete: true
Attachment #604963 - Flags: review?(ted.mielczarek)
Attachment #603673 - Flags: review?(ted.mielczarek)
Attachment #603673 - Flags: review?(bhearsum)
Attached patch buildbotcustom (obsolete) — Splinter Review
Adjusted to the last build system patch. Carry on f+.
Attachment #603674 - Attachment is obsolete: true
Attachment #604965 - Flags: feedback+
Passed staging, btw.
Component: Release Engineering: Releases → Release Engineering: Automation
QA Contact: bhearsum → catlee
Comment on attachment 604963 [details] [diff] [review]
build system part

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

r=me with two changes

::: toolkit/mozapps/installer/packager.mk
@@ +1039,5 @@
> +	(cd $(MOZ_PKG_SRCDIR) && $(CREATE_SOURCE_TAR) - $(DIR_TO_BE_PACKAGED)) | bzip2 -vf > $(SOURCE_TAR)
> +	$(SIGN_SOURCE_TAR_CMD)
> +
> +hg-bundle:
> +	mkdir -p $(DIST)/$(PKG_SRCPACK_PATH)

Can you use $(MKDIR) here? (It's faster in PyMake.) Also fix the command above where you copied from while you're there.

@@ +1045,5 @@
> +	$(SIGN_HG_BUNDLE_CMD)
> +
> +upload-source: UPLOAD_FILES=$(SOURCE_UPLOAD_FILES)
> +upload-source: CHECKSUM_FILE=$(SOURCE_CHECKSUM_FILE)
> +upload-source: upload

Urgh. We generally don't like to use target-specific variables, they're fiddly. Can you just do:
upload-source:
  $(MAKE) upload UPLOAD_FILES=... CHECKSUM_FILE=...
instead?
Attachment #604963 - Flags: review?(ted.mielczarek) → review+
Whiteboard: [releases][automation] → [releases][automation][Leave open after merge]
Comment on attachment 604963 [details] [diff] [review]
build system part

The patch doesn't affect CI, the new build system target are used only by release automation. The patch simplifies the releng side code and fixes an issue with missing checksums of source tarball/bundle in the MD5/SHA1/SHA512SUMS files.

Regression caused by (bug #): the current bug
Testing completed (on m-c, etc.): tested in dev environment by running staging releases 
Risk to taking this patch (and alternatives if risky): very low
String changes made by this patch: none
Attachment #604963 - Flags: approval-mozilla-beta?
Comment on attachment 604963 [details] [diff] [review]
build system part

[Triage comment]
approved: low risk, releases only.
Attachment #604963 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Having a minor regression in 12.0b1 build2 build. Signing on demand now happens earlier, during make source-package step, so we need to get a signing token/nonce earlier. attachment 606159 [details] [diff] [review] should fix the problem.
Just checked the impact of the patch in 12.0b1 build2. Since signing is not enabled (yet), the patch didn't change the build procedure, as expected.
Comment on attachment 604963 [details] [diff] [review]
build system part

I would like to land this patch on esr10 branch. No need to land it on mozilla-release since the releng part can't be landed until 3.x EOL, so mozilla-beta will be migrated to m-r. The patch will be noop for esr10. It will be used once we land the patch for buildbotcustom.
Attachment #604963 - Flags: approval-mozilla-esr10?
Comment on attachment 604963 [details] [diff] [review]
build system part

[Triage Comment]
sounds good, low risk, please go ahead and land.
Attachment #604963 - Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
Priority: P2 → P3
Whiteboard: [releases][automation][Leave open after merge] → [releases][automation][wait till 3.6 EOL]
Bulk move of bugs to Release Automation component.
Component: Release Engineering: Automation (General) → Release Engineering: Automation (Release Automation)
Flags: checked-in+
Flags: approval-mozilla-esr10+
Flags: approval-mozilla-beta+
No longer blocks: hg-automation
Mass move of bugs to Release Automation component.
No longer blocks: hg-automation
Comment on attachment 604963 [details] [diff] [review]
build system part

checked‑in status was lost during the migration to the new bugzilla component, restoring the state and copying the history of the commits.

http://hg.mozilla.org/integration/mozilla-inbound/rev/804103f3020c
https://hg.mozilla.org/mozilla-central/rev/804103f3020c
http://hg.mozilla.org/releases/mozilla-beta/rev/fc318c39e820
http://hg.mozilla.org/releases/mozilla-esr10/rev/6c61683ed9cb
Attachment #604963 - Flags: checked-in+
Depends on: 744200
Attached patch fix source-upload target name (obsolete) — Splinter Review
I'm not sure how it happened, but I suspect that I "transplanted" the patch from my user repo (where I tested the patch in staging release environment) with some manual intervention, so I uses upload-source name instead of source-upload :/

http://hg.mozilla.org/mozilla-central/file/63f78a93ae5c/toolkit/mozapps/installer/packager.mk#l1063
vs
http://hg.mozilla.org/mozilla-central/file/63f78a93ae5c/browser/build.mk#l101
http://hg.mozilla.org/mozilla-central/file/63f78a93ae5c/xulrunner/build.mk#l76

I realized the typo when I was testing the refreshed buildbotcustom part using freshly cloned repo... :(
Attachment #614023 - Flags: review?(ted.mielczarek)
Comment on attachment 614023 [details] [diff] [review]
fix source-upload target name

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

Yike!
Attachment #614023 - Flags: review?(ted.mielczarek) → review+
This is the same patch from https://hg.mozilla.org/mozilla-central/rev/0d1e86660e49 with extra headers, so we can use it for autoland.

Carrying on r=ted.
Attachment #614023 - Attachment is obsolete: true
Attachment #614404 - Flags: review+
Attachment #614404 - Flags: checked-in+
Comment on attachment 614404 [details] [diff] [review]
fix source-upload target name

I would like to land the followup to aurora, beta and esr10, so release automation could use new makefile targets. The patch is simple and tested in staging. Feel free to use Autoland (I re-uploaded the same patch with proper HG headers for this purpose, carrying on ted's r+).

Regression caused by (bug #): the current bug
User impact if declined: None, 
Testing completed (on m-c, etc.): in m-c since last week. I also tested it in staging release of Firefox/Fennec/XulRunner 12.0b6.
Risk to taking this patch (and alternatives if risky): very low, it touches only release related target names in makefiles and has impact on CI.
String changes made by this patch: None
Attachment #614404 - Flags: approval-mozilla-esr10?
Attachment #614404 - Flags: approval-mozilla-beta?
Attachment #614404 - Flags: approval-mozilla-aurora?
Comment on attachment 614404 [details] [diff] [review]
fix source-upload target name

[Triage Comment]
Our expectation is that any fallout from this landing would be immediately obvious as part of the build. Approving for all branches.
Attachment #614404 - Flags: approval-mozilla-esr10?
Attachment #614404 - Flags: approval-mozilla-esr10+
Attachment #614404 - Flags: approval-mozilla-beta?
Attachment #614404 - Flags: approval-mozilla-beta+
Attachment #614404 - Flags: approval-mozilla-aurora?
Attachment #614404 - Flags: approval-mozilla-aurora+
Attached patch buildbotcustomSplinter Review
Refreshed buildbotcustom patch
Attachment #604965 - Attachment is obsolete: true
Attachment #615452 - Flags: review?(bhearsum)
Autoland Warning:
	Only landing on branch(es): mozilla-beta
	Already landing patches 614404 on branch mozilla-aurora.
	Reviewer doesn't have correct permissions for mozilla-esr10 on patch(es): 614404
Autoland Patchset:
	Patches: 614404
	Branch: mozilla-aurora => try
Patchset could not be applied and pushed.
Failed to push
Attachment #615452 - Flags: review?(bhearsum) → review+
deployed in production
Worked fine in Firefox/Fennec 12.0b6 build1.
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.