715586 - checksums.py should generate sha1 and md5 checksums

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[Rail Aliiev [:rail]]

Attachment: generate sha1, sha512 and md5 checksums

This would simplify MD5SUMS/SHA1SUMS generation.

Comment 1

[Rail Aliiev [:rail]]

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

Comment 2

[John Ford [:jhford] CET/CEST Berlin Time]

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

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

Looks good to me.

Comment 3

[Rail Aliiev [:rail]]

Attachment: generate sha1, sha512 and md5 checksums

Updated docstring (digets -> digests)

Comment 4

[(not currently active) Ted Mielczarek]

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.

Comment 5

[Rail Aliiev [:rail]]

Attachment: 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

Comment 6

[(not currently active) Ted Mielczarek]

We already require Python 2.5 for building Mozilla, I don't see the point in keeping 2.4-isms around in our codebase.

Comment 7

[Rail Aliiev [:rail]]

Attachment: 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)

Comment 8

[Rail Aliiev [:rail]]

Attachment: generate sha1, sha512 and md5 checksums

Use with statement

Comment 9

[[e-mail] (:bhearsum)]

Comment on attachment 587359 [details] [diff] [review]
generate sha1, sha512 and md5 checksums

http://hg.mozilla.org/mozilla-central/rev/1b89605ede03

Comment 10

[Rail Aliiev [:rail]]

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

Comment 11

[Alex Keybl [:akeybl]]

Comment on attachment 587359 [details] [diff] [review]
generate sha1, sha512 and md5 checksums

[Triage Comment]
Approved for Aurora along with bug 714542.

Comment 12

[[e-mail] (:bhearsum)]

Comment on attachment 587359 [details] [diff] [review]
generate sha1, sha512 and md5 checksums

http://hg.mozilla.org/releases/mozilla-aurora/rev/76c3d4e9cd54

Comment 13

[Rail Aliiev [:rail]]

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.

Comment 14

[Lukas Blakk [:lsblakk] use ?needinfo]

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 :)

Comment 15

[Rail Aliiev [:rail]]

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