checksums.py should generate sha1 and md5 checksums

RESOLVED FIXED

Status

Release Engineering
General
P2
normal
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: rail, Assigned: rail)

Tracking

Firefox Tracking Flags

(firefox11 fixed, firefox-esr10 fixed)

Details

(Whiteboard: [releases][automation][signing][qa-])

Attachments

(1 attachment, 4 obsolete attachments)

Created attachment 586147 [details] [diff] [review]
generate sha1, sha512 and md5 checksums

This would simplify MD5SUMS/SHA1SUMS generation.
Comment on attachment 586147 [details] [diff] [review]
generate sha1, sha512 and md5 checksums

It worked fine in staging:

e9718f4b6c01de11fe110654a2e00b2513c264b7 sha1 33367076 mac/af/Firefox 10.0.dmg
b8f454feac929652a59842447c85aaae30a8ff8f676e9889fdf42de2852c8b5d9c50304075ef72b23b666ebd36baaf602b1db80d04c8024946cbdb3e654039d9 sha512 33367076 mac/af/Firefox 10.0.dmg
6485867a99367e449711fd1616812a03 md5 33367076 mac/af/Firefox 10.0.dmg
e9718f4b6c01de11fe110654a2e00b2513c264b7 sha1 33367076 mac/af/Firefox 10.0.dmg
dab3a84b92f0b0c6955ef31dc0e27ed074d4e7e6 sha1 32384557 update/mac/af/firefox-10.0.complete.mar
8b5c7c56b0851e5b05e702b9ad9b1d1e69f43886c020a281fa90225c6b98f4bb50c55b761ebb6214e603178d4cdbcf71bb829c0d8414e527468ceb190e7a58d0 sha512 32384557 update/mac/af/firefox-10.0.complete.mar
70e88f40373f603c0e38b0c7a7e3300b md5 32384557 update/mac/af/firefox-10.0.complete.mar
dab3a84b92f0b0c6955ef31dc0e27ed074d4e7e6 sha1 32384557 update/mac/af/firefox-10.0.complete.mar
3aa990737fc6cdebdc532c50ce51739521ec1371 sha1 255253 mac/xpi/af.xpi
66263e586c129647ce621bacf71b999ced14cfa97391fe79e9ecbc9b2d14b07896e4f26a698cc06d23f837af15568e118ada5311f0f7a841a6c940bf134fe7fd sha512 255253 mac/xpi/af.xpi
fca8356a5aeaee04233c31dcc58e3a8b md5 255253 mac/xpi/af.xpi
3aa990737fc6cdebdc532c50ce51739521ec1371 sha1 255253 mac/xpi/af.xpi
f6573c8006893b03b333c99c09f5119b6102b341 sha1 21029744 update/mac/af/firefox-9.0.1-10.0.partial.mar
9b805942aa1311c15406765e5c3b66cb6478653e8642d2892e4b69c932e41d1fe62423219d5cd3d59b6796d69f6e888418fdc600935fb73dd59f4adfd095824b sha512 21029744 update/mac/af/firefox-9.0.1-10.0.partial.mar
f0f70e071244a26e3baeb279baccba23 md5 21029744 update/mac/af/firefox-9.0.1-10.0.partial.mar
f6573c8006893b03b333c99c09f5119b6102b341 sha1 21029744 update/mac/af/firefox-9.0.1-10.0.partial.mar
ca8b11bebc5bd90e52546c77d46fd7036d0583c9 sha1 189 update/mac/af/firefox-9.0.1-10.0.partial.mar.asc
d8a207880223c436ca4dd132bd7b982696b4795dac21d984a1cdd0040a2b761048bb38eb001f852855d7bebec6751043868698c52f795fcdee523ffaf20e1ebf sha512 189 update/mac/af/firefox-9.0.1-10.0.partial.mar.asc
3a4264da3aa2cc091e1a997d9c865f70 md5 189 update/mac/af/firefox-9.0.1-10.0.partial.mar.asc
ca8b11bebc5bd90e52546c77d46fd7036d0583c9 sha1 189 update/mac/af/firefox-9.0.1-10.0.partial.mar.asc
c456163596d5162a7408f1725fb5ff41f093fdd6 sha1 189 update/mac/af/firefox-10.0.complete.mar.asc
20a8307fd45983f9b6a07cad91028b27d757de11f72527850e4a37c4a679bcd186cd0b196fbe679b5c3a427a64fbf3ac900512a459bd7ae1e389993a6d66bdb5 sha512 189 update/mac/af/firefox-10.0.complete.mar.asc
a56d67357ceafd6c3f29ab377da7288c md5 189 update/mac/af/firefox-10.0.complete.mar.asc
c456163596d5162a7408f1725fb5ff41f093fdd6 sha1 189 update/mac/af/firefox-10.0.complete.mar.asc
Attachment #586147 - Flags: feedback?(jhford)
Comment on attachment 586147 [details] [diff] [review]
generate sha1, sha512 and md5 checksums

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

Looks good to me.
Attachment #586147 - Flags: feedback?(jhford) → feedback+
Attachment #586147 - Flags: review?(ted.mielczarek)
Created attachment 587017 [details] [diff] [review]
generate sha1, sha512 and md5 checksums

Updated docstring (digets -> digests)
Attachment #586147 - Attachment is obsolete: true
Attachment #586147 - Flags: review?(ted.mielczarek)
Attachment #587017 - Flags: review?(ted.mielczarek)
Attachment #587017 - Flags: review?(catlee)
Comment on attachment 587017 [details] [diff] [review]
generate sha1, sha512 and md5 checksums

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

r=me with a few nits.

::: build/checksums.py
@@ +97,4 @@
>                       output_filename)
>      else:
>          logger.debug('Creating a new checksums file "%s"' % output_filename)
>      output = open(output_filename, 'w+')

Would you mind changing this to a with statement while you're here?
with open(output_filename, 'w+') as output:

then indent everything below it, and remove the output.close() line. You'll also need to stick a future import at the top of the file (before the other imports):
from __future__ import with_statement

@@ +106,5 @@
> +                hash = digest_file(file, digest)
> +                if hash is None:
> +                    logger.warn('Unable to generate a hash for %s. ' +
> +                                'Using NOHASH as fallback' % file)
> +                    hash = 'NOHASH'

Seems like in the extremely unlikely case that this fails, you could wind up with multiple NOHASH lines for a file, which would be a little ugly. Maybe we should break out of the digests loop after printing a NOHASH line?

@@ +174,3 @@
>      except ValueError, ve:
>          logger.error('Could not create a "%s" hash object (%s)' %
>                       (options.digest, ve.args[0]))

You need to fix this error message, it's still using options.digest.
Attachment #587017 - Flags: review?(ted.mielczarek) → review+

Updated

6 years ago
Attachment #587017 - Flags: review?(catlee) → review+
Created attachment 587328 [details] [diff] [review]
generate sha1, sha512 and md5 checksums

(In reply to Ted Mielczarek [:ted, :luser] from comment #4)
> Would you mind changing this to a with statement while you're here?

Ruh roh. I prefer to not touch this and keep it python 2.4 compatible.

> @@ +106,5 @@
> > +                hash = digest_file(file, digest)
> > +                if hash is None:
> > +                    logger.warn('Unable to generate a hash for %s. ' +
> > +                                'Using NOHASH as fallback' % file)
> > +                    hash = 'NOHASH'
> 
> Seems like in the extremely unlikely case that this fails, you could wind up
> with multiple NOHASH lines for a file, which would be a little ugly. Maybe
> we should break out of the digests loop after printing a NOHASH line?

Done. Added continue statement and changed the log message.

> @@ +174,3 @@
> >      except ValueError, ve:
> >          logger.error('Could not create a "%s" hash object (%s)' %
> >                       (options.digest, ve.args[0]))
> 
> You need to fix this error message, it's still using options.digest.

Fixed. Using digest instead.

Carry on review. Interdiff is as following:


--- b/build/checksums.py
+++ b/build/checksums.py
@@ -106,8 +106,8 @@
                 hash = digest_file(file, digest)
                 if hash is None:
                     logger.warn('Unable to generate a hash for %s. ' +
-                                'Using NOHASH as fallback' % file)
-                    hash = 'NOHASH'
+                                'Skipping.' % file)
+                    continue
                 if file.startswith(strip):
                     short_file = file[len(strip):]
                     short_file = short_file.lstrip('/')
@@ -173,7 +173,7 @@
             hashlib.new(digest)
     except ValueError, ve:
         logger.error('Could not create a "%s" hash object (%s)' %
-                     (options.digest, ve.args[0]))
+                     (digest, ve.args[0]))
         exit(1)
 
     # Validate the files to checksum
Attachment #587328 - Flags: review+
We already require Python 2.5 for building Mozilla, I don't see the point in keeping 2.4-isms around in our codebase.
Created attachment 587331 [details] [diff] [review]
generate sha1, sha512 and md5 checksums

Ooops, just figured out that OptionParser appends the value for reals! :)

--- b/build/checksums.py
+++ b/build/checksums.py
@@ -142,7 +142,7 @@
     # Parse command line arguments
     parser = OptionParser()
     parser.add_option('-d', '--digest', help='checksum algorithm to use',
-                      action='append', dest='digests', default=['sha1'])
+                      action='append', dest='digests')
     parser.add_option('-o', '--output', help='output file to use',
                       action='store', dest='outfile', default='checksums')
     parser.add_option('-v', '--verbose',
@@ -168,6 +168,8 @@
     logger = logging.getLogger('checksums.py')
 
     # Validate the digest type to use
+    if not options.digests:
+        options.digests = ['sha1']
     try:
         for digest in options.digests:
             hashlib.new(digest)
Attachment #587328 - Attachment is obsolete: true
Attachment #587331 - Flags: review?(catlee)
Attachment #587331 - Flags: review?(catlee)
Created attachment 587359 [details] [diff] [review]
generate sha1, sha512 and md5 checksums

Use with statement
Attachment #587331 - Attachment is obsolete: true
Attachment #587359 - Flags: review?(catlee)

Updated

6 years ago
Attachment #587359 - Flags: review?(catlee) → review+
Attachment #587017 - Attachment is obsolete: true
Comment on attachment 587359 [details] [diff] [review]
generate sha1, sha512 and md5 checksums

http://hg.mozilla.org/mozilla-central/rev/1b89605ede03
Attachment #587359 - Flags: checked-in+
Comment on attachment 587359 [details] [diff] [review]
generate sha1, sha512 and md5 checksums

The patch will be required for Firefox 11.0 release automation where we introduce signing of binaries and MAR files as a part of build process. We need to use *.checksums files to generate MD5SUMS and SHA1SUMS without downloading all files for current releases. It should reduce release end-to-end time.

User impact if declined: None, the "*.checksums" files are used by automation.
Testing completed (on m-c, etc.): the patch passes m-c builds and staging release automation.
Risk to taking this patch (and alternatives if risky): low
Attachment #587359 - Flags: approval-mozilla-aurora?
Comment on attachment 587359 [details] [diff] [review]
generate sha1, sha512 and md5 checksums

[Triage Comment]
Approved for Aurora along with bug 714542.
Attachment #587359 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 587359 [details] [diff] [review]
generate sha1, sha512 and md5 checksums

http://hg.mozilla.org/releases/mozilla-aurora/rev/76c3d4e9cd54
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
status-firefox11: --- → fixed
Whiteboard: [releases][automation][signing] → [releases][automation][signing][qa-]
Comment on attachment 587359 [details] [diff] [review]
generate sha1, sha512 and md5 checksums

It would be great to port this functionality to the ESR branch what would allow RelEng dropping manual signing. This feature has been tested for 4 betas and worked just fine.
Attachment #587359 - Flags: approval-mozilla-esr10?
Comment on attachment 587359 [details] [diff] [review]
generate sha1, sha512 and md5 checksums

[Triage Comment]
As with bug 418815 and bug 714542, please land today preferably - glad to be able to get rid of manual signing across the board :)
Attachment #587359 - Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
Comment on attachment 587359 [details] [diff] [review]
generate sha1, sha512 and md5 checksums

Transplanted to esr10:
http://hg.mozilla.org/releases/mozilla-esr10/rev/2732ad08d0c3
status-firefox-esr10: --- → fixed
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.