Some library dependencies (such as wkhtmltopdf, required by print.module) are available only in .tar.bz2 format. It would be handy to support extracting these .tar.bz2 files automatically in a Drush Makefile.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

helmo’s picture

Status: Active » Closed (won't fix)

[ Powered by #1115636: Issue Macros and Templates - Drush Make]

Drush make is being merged into drush core (discussed in issue:#1310130: Put drush make in drush core)
This means that the issue queue is also moving. The Drush project has a component called 'Make' for this purpose.

We would like to take this opportunity to leave behind old/obsolete issues, allowing us to focus on a stable make command in core. E.g. one of the major tasks ahead is making more use of the Drush core code for handling downloads and decompression.

If you feel that this issue is still relevant, feel free to re-open and move it to the Drush queue.

More information will be posted on the drush_make and drush project pages.

NOTE: This will probably be part of #1267228: Drush Make should use Drush core's native download abilities concurrently

smokris’s picture

Project: Drush Make » Drush
Version: 6.x-2.x-dev »
Component: Code » Make
Status: Closed (won't fix) » Active

This issue is still relevant.

moshe weitzman’s picture

such functionality should probably go into drush_tarball_extract()

smokris’s picture

Status: Active » Needs review
FileSize
2.09 KB

drush_tarball_extract() doesn't seem to be called from Drush Make.

I've attached a patch that adds .tar.bz2 / .tbz support by modifying make_download_file_unpack().

moshe weitzman’s picture

Status: Needs review » Needs work

drush_tarball_extract() is not called from Make, but should be. We're no longer enhancing makes own tar functionality

jhedstrom’s picture

Issue tags: +Needs tests

This will require a test in tests/makeTest.php.

smokris’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
10.71 KB

Revised patch attached.

  • Modified drush_tarball_extract() to also support uncompressed tar, tar/bzip2, and zip.
  • Refactored make_download_file_unpack() to use drush_tarball_extract() instead of its own functions.
  • Removed the now-unused make_download_file_unpack_*() functions.
  • Includes a test in tests/makeTest.php.
jonhattan’s picture

Status: Needs review » Needs work
FileSize
11.05 KB

That's a great change to make!

Added a check for bzip2 executable to avoid the bz2 if the command is not available.

This two tests are failing for me with this patch:

makeMakefileCase::testMakeGet
makeMakefileCase::testMakeLimitLibraries

jhedstrom’s picture

The breakage in tests is due to temporary build directories being left in place while the md5 sum is calculated, thus changing the hashes. Working now on figuring out why these aren't being removed.

jhedstrom’s picture

This patch removes the downloaded tar file, so that md5 sums match. However, I'm still seeing 3 failing tests (down from 5).

Time: 55 seconds, Memory: 12.50Mb

There were 3 failures:

1) makeMakefileCase::testMakeBZ2
bzip2 - build md5 matches expected value: 05b6935122a499375a7ea46879a133ab
Failed asserting that '5ec081203131a1a3277c8b23f9ddb995' contains "05b6935122a499375a7ea46879a133ab".

/home/jhedstrom/work/contributions/utilities/drush/tests/makeTest.php:41
/home/jhedstrom/work/contributions/utilities/drush/tests/makeTest.php:182

2) makeMakefileCase::testMakeSubtree
Failed asserting that file "/tmp/drush-sandbox/subtree/sites/all/libraries/nivo-slider/jquery.nivo.slider.js" exists.

/home/jhedstrom/work/contributions/utilities/drush/tests/makeTest.php:231

3) makeMakefileCase::testMakeFileExtract
Unexpected exit code: /usr/local/bin/drush --nocolor make /home/jhedstrom/work/contributions/utilities/drush/tests/makefiles/file-extract.make --no-core --concurrency=1 --test --md5=print
Failed asserting that 1 matches expected 0.
jhedstrom’s picture

Oops, last patch missed the added test file.

smokris’s picture

Status: Needs work » Needs review
FileSize
16.31 KB

New patch!

  • Failure #1 in comment #10 is because I pasted in the wrong md5sum (oops). Fixed.
  • Fixed issue handling archives whose first entry wasn't the wrapper folder. Also added a test for that.
  • Added a test to make sure that tarballs containing a single file (with no wrapper folder) also work.
  • Fixed testMakeSubtree. Note: this slightly changes the behavior of the "subtree" directive, to make it more self-consistent: "subtree" is now always relative to the root of the archive — instead of (as it was previously) relative to the root if there are multiple files/folders in the root of the archive, or relative to the wrapper folder if there is a single folder in the root of the archive.
  • Fixed testMakeFileExtract (fixed handling of Zip files; added file-magic detection for files that don't have recognized extensions).

All tests now pass.

smokris’s picture

The patch in #12 only worked with PHP >= 5.3.0. Revised to also work with PHP 5.2.9.

moshe weitzman’s picture

Assigned: Unassigned » jhedstrom

I'll leave this to jhedstrom or jonhattan to review and commit.

jonhattan’s picture

Status: Needs review » Fixed

Committed as is in 2137767a77cdd03f4a83c6066b60e173ebfe1546

I also did some changes in subsequent commits:
* fix parsing finfo_file. It returns the charset in addition to the mimetype (because of FILEINFO_MIME) and it was failing to identify the correct mime type.

I will create new issues for other tasks I've identified.

smokris’s picture

Awesome. Thanks, @jonhattan, @moshe, and @jhedstrom!

jhedstrom’s picture

This commit introduced a regression #1721610: Regression from support bz2 downloads.

Status: Fixed » Closed (fixed)

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