I think it would be a good to be able to specify md5 checksums of downloads and have drush make verify them. update.drupal.org and features servers already provide md5 checksums for files, it is straight forward to verify these on download. It should also be possible to specify and verify md5 hashes for tarballs, patches and libraries. There may also be cases where a user may want to disable the verification of these checksums.

CommentFileSizeAuthor
#14 drush_make-882236-md5-verification-14.patch13.29 KBskwashd
#7 drush_make-882236-md5-verification-1.patch8.43 KBskwashd
#1 drush_make-882236-md5-verification.patch8.43 KBskwashd

Comments

skwashd’s picture

skwashd commented
StatusFileSize
new drush_make-882236-md5-verification.patch8.43 KB

Patch to do the above. Docs updated too. I would write some test for this, but the drush_make test framework doesn't seem to support failing tests.

I forgot to mention in the initial content, if a md5 hash is specified for a project on updates.drupal.org or a feature server is trumps the server provided value. This seemed to be the sanest way of dealing with this.

skwashd’s picture

skwashd commented
Status: Active » Needs review

Forgot to update status

Caseledde’s picture

Caseledde commented

I tried to run your patch, but i wasn't very successful.

My Mini-Drush-Make-Script:

core = 6.x

projects[cocomore][type] = "core"
projects[cocomore][download][type] = "get"
projects[cocomore][download][url] = "http://drupal.cocomore.com/system/files/cdc-6.19.0.tar.gz"
projects[cocomore][download][md5]= "Hello World"

Drush Make still install the core. It seems that the md5 option has no effect regardless of which value.

What is wrong?

skwashd’s picture

skwashd commented

Are you sure you've applied the patch properly? I ran your make file here and got:

$ drush make core.make core
cocomore downloaded from                                             [ok]
http://drupal.cocomore.com/system/files/cdc-6.19.0.tar.gz.
Checksum verification failed for cdc-6.19.0.tar.gz. Expected Hello   [error]
World, received de3710ead8828780029c956264cae2ab.
An error occurred at function : drush_drush_make_make                [error]
Caseledde’s picture

Caseledde commented

I checked again and now everything works fine.

thx for the patch.

kars-t’s picture

kars-t commented
Status: Needs review » Needs work

Hi

Caseledde and me did test the patch and we think the patch is nice and running well. Thanks! :)

But there are some minor coding style things that are not conform to the drupal standards:

+++ drush_make.download.inc	13 Aug 2010 11:53:57 -0000
@@ -21,6 +21,20 @@
+    if ( $md5 !== $checksum ) {

I think your coding style is nice and clean and there is nothing against it in the standards. But the rest of the module doesn't use spaces after and before the if( ) braces. I would advise to change this.

+++ drush_make.download.inc	13 Aug 2010 11:53:57 -0000
@@ -284,7 +303,11 @@
+        } else {

Please do a linebreak before else {

http://drupal.org/coding-standards#controlstruct

+++ drush_make.drush.inc	13 Aug 2010 11:53:57 -0000
@@ -156,6 +162,11 @@
+    } else if ( !empty($md5) ) {

Standard is like this:

}
elseif {

http://drupal.org/coding-standards#controlstruct

Powered by Dreditor.

Otherwise its RTBC.

skwashd’s picture

skwashd commented
Status: Needs work » Needs review
StatusFileSize
new drush_make-882236-md5-verification-1.patch8.43 KB

@kars-T Thanks for the review. I'd never noticed the coding standard for else before - geez that looks ugly. Anyway it is rerolled with the whitespace around the brackets cleaned up and should now comply with the coding standard.

Caseledde’s picture

Caseledde commented
Status: Needs review » Reviewed & tested by the community

Works and its RTBC.

agileware’s picture

agileware commented

Subscribe. Great work :)

dmitrig01’s picture

dmitrig01 commented
Status: Reviewed & tested by the community » Needs work

We need a test to make sure this works. It shouldn't be very hard to write.

skwashd’s picture

skwashd commented

I'd appreciate some guidance on how to implement failing tests using the current drush make testing framework.

dmitrig01’s picture

dmitrig01 commented

build => FALSE i think

dixon_’s picture

dixon_
he/him
Primary language English
Location France
commented

Subscribing.

skwashd’s picture

skwashd commented
StatusFileSize
new drush_make-882236-md5-verification-14.patch13.29 KB

Here is the patch with the tests.

I have also include a couple of fixes/tidy ups:
* Handle missing/empty make files a little more graciously and exit earlier
* drush_make_libraries() and DrushMakeProject::applyPatches() exit early rather than having the whole method in an if()

Note: The test suite was failing before applying this patch, my included test work. I don't know what is going on there.

skwashd’s picture

skwashd commented
Status: Needs work » Needs review

Forgot to set it to needs review.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Reviewed & tested by the community

Works for me:

md5.make builds
md5-fail.make fails
md5-fail.make --ignore-checksums works

dmitrig01’s picture

dmitrig01 commented
Status: Reviewed & tested by the community » Fixed

Awesome, thanks

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.