The commit from #1343058: Add support for extracting .tar.bz2 files introduced a regression in some simple get downloads in makefiles. I haven't been able to track the cause down just yet, and it might not even be make-specific.

To replicate, use this make file (which contains a snippet from Drupal Commons)

libraries[getid3][download][type] = "get"
libraries[getid3][destination] = "libraries"
libraries[getid3][download][url] = "http://downloads.sourceforge.net/project/getid3/getID3%28%29%201.x/1.9.1/getid3-1.9.1-20110810.zip
?r=http%3A%2F%2Fsourceforge.net%2Fprojects%2Fgetid3%2Ffiles%2FgetID3%2528%2529%25201.x%2F1.9.1%2F&ts=1320871534"
libraries[getid3][directory_name] = "getid3"
; http://drupal.org/node/1336886
libraries[getid3][patch][] = "http://drupal.org/files/getid3-remove-demos-1.9.1.patch"
CommentFileSizeAuthor
#4 drush-1721610.patch2.98 KBjonhattan
#2 drush-tar-regression-1721610-02.patch1.34 KBjhedstrom

Comments

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented

Tracked this down to an issue with this logic in make_file_download_unpack(), which makes any download that doesn't pass in (the non-required) $subtree parameter get treated like a single file download.


    if ($subtree) {
      $source = dirname($filename) . DIRECTORY_SEPARATOR . $subtree;
    }
    else {
      // If there's just one file, get the path to that file.
      // If there are multiple files, get the path to the wrapper directory.
      $slash = strpos(reset($listing), DIRECTORY_SEPARATOR);
      $source = dirname($filename) . DIRECTORY_SEPARATOR . ($slash === FALSE ? reset($listing) : substr(reset($listing), 0, $slash);
    }
jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Active » Needs work
StatusFileSize
new drush-tar-regression-1721610-02.patch1.34 KB

Here's a start on a fix. The issue is that archives are being extracted directly into the temporary build directory, and in the case of the example in the original post, there is no subdirectory in the archive, so all files are just dumped into the base build directory. I'm actually surprised this hasn't caused an issue with concurrency yet.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented

Marked #1502940: Drush make build fails with can't cd to /tmp/make_tmp... as a duplicate.

jonhattan’s picture

jonhattan
Primary language Spanish
Location Plasencia
commented
StatusFileSize
new drush-1721610.patch2.98 KB

With this patch only two tests are failing. I don't see the relation:

There were 2 failures:

1) makeMakefileCase::testMakePatch
Test patching and writing of PATCHES.txt file - build md5 matches expected value: 27403b34b599af1cbdb50417e6ea626f
Failed asserting that ' >> Project context contains 3 modules: context_layouts, context_ui, context.
56f1613fc8b6a9f03ab62cfa0300df4c' contains "27403b34b599af1cbdb50417e6ea626f".

/usr/src/drush/tests/makeTest.php:41
/usr/src/drush/tests/makeTest.php:62

2) makeMakefileCase::testMakeRecursion
Unexpected exit code: /usr/local/bin/drush --nocolor make /usr/src/drush/tests/makefiles/recursion.make --no-core --contrib-destination=profiles/drupal_forum --test --md5=print
Failed asserting that 1 matches expected 0.

/usr/src/drush/tests/drush_testcase.inc:323
/usr/src/drush/tests/drush_testcase.inc:390
/usr/src/drush/tests/makeTest.php:36
/usr/src/drush/tests/makeTest.php:70
jonhattan’s picture

jonhattan
Primary language Spanish
Location Plasencia
commented
Status: Needs work » Needs review

Same two failures without the patch.

jonhattan’s picture

jonhattan
Primary language Spanish
Location Plasencia
commented
Status: Needs review » Fixed

committed.

Status: Fixed » Closed (fixed)

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