If you have a makefile M containing:

projects[] = p_a
projects[] = p_b

And project p_a has a makefile N containing:

projects[] = p_b

Then you get a spurious "Directory not empty" error for p_b. Worse, if p_b is in a git repository, drush_make_download_git fails horribly the second time round: drush make stops in its tracks when it tries to unpack p_b a second time (I think - it basically quits with "error in drush_drush_make_make" immediately after the second "p_b cloned from..." message.)

When you don't have control over the makefiles - if they're in contrib on d.o, or in someone else's module - then there's nothing you can do to prevent this from happening. Two project makefiles could justifiably bring in a CCK dependency, and you'll then get an error message that, it turns out, you can ignore; but if two makefiles try to bring in the same git repository, then you will never be able to build the site using drush make as it stands. This is probably going to become more and more of a problem as Drupal moves to using git.

Please find attached a patch which solves this. The patch does two major things:

1. DrushMakeProject::findDownloadLocation keeps track of what it's been asked to download before. If it recognizes a package, it only reports a warning, not an error, on the directory not being empty.
2. drush_make_download_factory checks for repeat projects, and produces a verbose warning when the second or future calls is attempted.

By still reporting [warning]s, that means that any potential problems can be tracked by looking at the reported drush log, but 1. means that the directory-not-empty isn't reported as a (spurious) error, and 2. means that a lower-level PHP error doesn't kill drush_drush_make_make.

Comments

middric’s picture

Just tested this, works here.

dmitrig01’s picture

Status: Needs review » Needs work

I don't think this code belongs in the dowload factory. the download factory is meant to be something generalized and reusable beyond drush make itself, and if we hardcode projects in there, it becomes non-generalized. in the distant future of a 3.x branch this problem will be solved, but for now please make a new function that keeps a static variable of downloaded projects.

jp.stacey’s picture

I'm not clear what you want to be changed, and where.

There's two functions being changed in the existing patch:

* The factory needs changing somehow to prevent double downloading from different (or the same sources);
* findDownloadLocation needs changing somehow because it tries to create a directory for the download and throws a big red error, regardless of what the factory then finds out later on. But then this is in drush_make.project.inc, so maybe that's OK to be a bit more ad-hoc?

Basically: should I change static $done, or static $previous_locations, or both? And do you want the patch's static arrays to just be replaced by function calls, or instead do you want the interception to happen the next level up?

dmitrig01’s picture

the $done - one level up. Ideally this patch should not touch .download.inc. Thanks!

Xen’s picture

It should also be more intelligent about it. If 2 makefiles pull in the same module, but specify different versions, it should error out, as there is probably a reason for the makefiles to be specific. Not specifying a version would be compatible with any other version.

It is a bit complicated to implement due to the recursive nature of drush make (say the master pulls in the lastest version of views and another module that has a makefile that specifies views in version 2.10).

dmitrig01’s picture

freephile’s picture

I too was experiencing spurious "Directory not empty" errors which would prevent me from downloading modules defined in my profile's make file.

The supplied patch did not apply for me cleanly, but when I manually edited drush_make.download.inc, it fixed the problem here. Thanks jp.stacey

--- drush_make.download.inc.orig	2010-11-21 15:25:50.000000000 -0800
+++ drush_make.download.inc	2011-01-14 14:00:35.046868916 -0800
@@ -2,6 +2,14 @@
 // $Id: drush_make.download.inc,v 1.1.2.99 2010/11/21 23:25:50 dmitrig01 Exp $
 
 function drush_make_download_factory($name, $download, $download_location) {
+  static $done;
+  $type = $download['type'];
+  $done[$name][] = $type;
+  if (count($done[$name]) > 1) {
+    drush_log(dt('Attempt to download project %name more than once (%types) prevented',
+      array('%name' => $name, '%types' => join(", ", $done[$name]))), 'warning');
+    return FALSE;
+  }
   $function = 'drush_make_download_' . $download['type'];
   if (function_exists($function)) {
     return $function($name, $download, $download_location);

I don't think that this issue should be marked 'closed' as a duplicate since the other issue does not (yet) have a solution and only speaks of a re-design of the workflow.

freephile’s picture

Version: 6.x-2.x-dev » 6.x-2.0-beta11
Status: Closed (duplicate) » Needs review

I cloned the git source and patched it https://github.com/freephile/drush_make/commit/edbf21150d86e65e79a71693a... similar to jp.stacey's patch. The basic difference is that I did not change the signature of drush_make_download_factory(). I do setup static variables to keep track of what we've already seen.

This doesn't fix everything (it doesn't address the issue of whether first-in or last-in project specs are more important). But at least it won't stop you from building your profile.

dman’s picture

I get this also.
Given the situation described in the OP,
I'm trying to build a very large site out of a number or re-usable 'features'.
To allow these features to auto-install themselves, a number of them come with their own .make files.
Sometimes, more than one feature decides it requires the same module. Natural enough.

BUT if the same module is defined in more than one makefile, the project is fetched twice, the 'directory already exists' error is thrown, and a copy of the offending duplicate is dropped in the root of my drupal install.
This is different from git death, but has proven quite messy.
I've noticed this mainly with standard d.o project downloads (no args) though I also run with --working-copy and get it then also.
I'm trying patch #7, hopefully that will at least abort nicer than the current way it drops its eggs.

boombatower’s picture

I have exactly the same problem...just like #9 where a large site with a number of independent components depend on the same things.

The supplied patch definitely does not seem like the proper solution. Seems to make the end download correct, but still produces error messages along the way.

dman’s picture

I found the most horrible thing that was happening was if a *theme* was being downloaded a second time (edge case I know) then the contents of the theme directory, all the files individually, landed at drupalroot. This actually over-wrote or conflicted with the operation of the site altogether.

DrushMakeProject() will complain about "directory not empty" BUT the calling function doesn't pay attention to that error (a return of FALSE) and carrys on with the wrong parameters.

I needed to ensure that if directory existed ... then further processing should stop.

Index: drush_make.project.inc
===================================================================
RCS file: /cvs/drupal-contrib/contributions/modules/drush_make/Attic/drush_make.project.inc,v
retrieving revision 1.1.2.53
diff -u -p -r1.1.2.53 drush_make.project.inc
--- drush_make.project.inc      12 Nov 2010 05:31:59 -0000      1.1.2.53
+++ drush_make.project.inc      2 Feb 2011 04:40:02 -0000
@@ -23,7 +23,10 @@ class DrushMakeProject {
       return FALSE;
     }
     $download_location = $this->findDownloadLocation();
-    if (drush_make_download_factory($this->name, $this->download, $download_location) === FALSE) {
+    if (empty($download_location)) {
+      return FALSE;
+    }
+    if (drush_make_download_factory($this->name, $this->download, $download_location, $this) === FALSE) {
       return FALSE;
     }
     if (!$this->addLockfile($download_location)) {
boombatower’s picture

What I ended up need since I have the module awssdk which also downloads a library of the same name was the following fix.

if ((count($done[$name]) > 1 && $name != 'awssdk') || (count($done[$name]) > 2 && $name == 'awssdk')) {

So we need to make sure that wherever this ends up being check that the project[type] is also considered.

boombatower’s picture

Seems like what is needed is a robust way of handling this. Perhaps the following ruleset:

1) download project (first means detected)
2) come across another entry for same project
3) ignore the download information, but attempt to apply patches or any other post processing, except use first locations directory location

or do we want to simply completely ignore duplicate references to a project?

boombatower’s picture

Reading through code I'm not sure why things are as they are.

$download_location = $this->findDownloadLocation();

next line

drush_make_download_factory()

which also returns the location and is ignored...

You run drush make --version and it attempts to build site...yikes

boombatower’s picture

I am actually a bit unnerved after reading through this code. It actually always "fails" to make EVERY SINGLE project since recurse never returns true...which also means it never prints an error even though the code feels that it failed...conditional statements assigning variables that never get used........

boombatower’s picture

Assigned: Unassigned » boombatower
FileSize
3.65 KB

This cleans up only the necessary bits to get this to work.

I used the singleton pattern to keep track of project instances so we can easily determine if that have been "made" already. Something that can be done in the future would be to perform a sort of diff if the project has already been made...aka only apply patches or what not as described above.

Either way this fixes the problem, fixes the inconsistency of the recurse method, and the made property works as it should.

The notice that I print if the project has already been made never seems to display, but that seems like a general drush issue as it doesn't seem to display notices.

boombatower’s picture

Title: "Directory not empty": recursive makefiles can cause conflicts (and conflicting git checkouts fail horribly) » Recursive makefiles can cause conflicts
boombatower’s picture

Version: 6.x-2.0-beta11 » 6.x-2.x-dev
Xen’s picture

Seems that my work in #1016924: Dealing with different versions of the same module in recursive makefiles is getting superceeded.

Like the singleton idea, but it needs to consider versions too. If 2 makefiles in a recursive makefile setup requires views in specific and differing versions, the make should fail. Different patch-sets should give a warning, as we'll assume that a patch doesn't change the patchee so much as to break it for other users, but the user might want to know.

We're trying to do the same thing as dman, and use recursive makefiles to allow subsystems to install their own dependencies, as some of the subsystems are optional.

boombatower’s picture

That and more can be done with the singleton approach since we have all the instance data from the previous entry. So lets get this committed, and open up a followup issue to add a bunch of extra check for things like other versions and patches and what not.

boombatower’s picture

Anyone up for a review, lets get this in and then start making the improvements mentioned in the last few comments?

My openSUSE packages apply this patch against "upstream" if anyone wants easy management.

Xen’s picture

Status: Needs review » Reviewed & tested by the community

I've tested it against the sandbox I used for testing my own patch. 6.x-2.x-dev fails when trying to fetch views again, with this patch it succeeds. With -v it prints "Attempt to build project views more then once prevented."

The patch looks tight to me.

boombatower’s picture

FileSize
3.88 KB

Updated patch for offsets.

zzolo’s picture

This patched worked for me. Thanks.

izmeez’s picture

subscribing

attiks’s picture

Status: Reviewed & tested by the community » Needs work

It appears to be working for modules, but not for libraries.

test_947158.make:

; This file was auto-generated by drush_make
core = 7.x

api = 2
projects[drupal][version] = "7.0"

projects[geofield][version] = "1.x-dev"

; The following are included in the make file of geofield
; http://drupalcode.org/project/geofield.git/blob/refs/heads/7.x-1.x:/geofield.make
projects[libraries][version] = "1.0"
libraries[geoPHP][download][type] = "get"
libraries[geoPHP][download][url] = "http://github.com/downloads/phayes/geoPHP/geoPHP.tar.gz"
libraries[geoPHP][directory_name] = "geoPHP"
libraries[geoPHP][type] = "library"

output:

drush make test_947158.make
Make new site in the current directory? (y/n): y
Project information for drupal retrieved.                                                     [ok]
Project information for geofield retrieved.                                                   [ok]
Project information for libraries retrieved.                                                  [ok]
drupal downloaded from http://ftp.drupal.org/files/projects/drupal-7.0.tar.gz.                [ok]
geofield downloaded from http://ftp.drupal.org/files/projects/geofield-7.x-1.x-dev.tar.gz.    [ok]
Found makefile: geofield.make                                                                 [ok]
Project information for libraries retrieved.                                                  [ok]
libraries downloaded from http://ftp.drupal.org/files/projects/libraries-7.x-1.0.tar.gz.      [ok]
geophp downloaded from https://github.com/downloads/phayes/geoPHP/geophp.tar.gz.              [ok]
Directory not empty: /tmp/drush_make_tmp_1304106711/__build__/sites/all/libraries/geoPHP      [error]
geoPHP downloaded from http://github.com/downloads/phayes/geoPHP/geoPHP.tar.gz.               [ok]
attiks’s picture

verbose output

drush make ~/makefiles/test_947158.make -v
Load alias @self                                                                                 [notice]
Load alias @server_master                                                                        [notice]
Loading mysql driver for the db service                                                          [notice]
Loading apache driver for the http service                                                       [notice]
Make new site in the current directory? (y/n): y
Executing: mkdir '/tmp/drush_make_tmp_1304106917'
Executing: mkdir '/tmp/drush_make_tmp_1304106917/__download__'
Executing: ls '/tmp/drush_make_tmp_1304106917/__download__'
Calling chdir(/tmp/drush_make_tmp_1304106917/__download__)
Executing: curl -LOD '/tmp/drush_make_tmp_1304106917/__header__' 'http://updates.drupal.org/release-history/drupal/7.x'
Calling chdir(/var/aegir/platforms/vino-v/vinov20110429)
Executing: ls '/tmp/drush_make_tmp_1304106917/__download__'
Executing: mv '/tmp/drush_make_tmp_1304106917/__download__/7.x' '/tmp/drush_make_tmp_1304106917/7.x'
Executing: rm -f '/tmp/drush_make_tmp_1304106917/__header__'
Executing: rm -rf '/tmp/drush_make_tmp_1304106917/__download__'
Calling unlink(/tmp/drush_make_tmp_1304106917/7.x)
Project information for drupal retrieved.                                                     [ok]
Executing: mkdir '/tmp/drush_make_tmp_1304106917/__download__'
Executing: ls '/tmp/drush_make_tmp_1304106917/__download__'
Calling chdir(/tmp/drush_make_tmp_1304106917/__download__)
Executing: curl -LOD '/tmp/drush_make_tmp_1304106917/__header__' 'http://updates.drupal.org/release-histo                                ry/geofield/7.x'
Calling chdir(/var/aegir/platforms/vino-v/vinov20110429)
Executing: ls '/tmp/drush_make_tmp_1304106917/__download__'
Executing: mv '/tmp/drush_make_tmp_1304106917/__download__/7.x' '/tmp/drush_make_tmp_1304106917/7.x'
Executing: rm -f '/tmp/drush_make_tmp_1304106917/__header__'
Executing: rm -rf '/tmp/drush_make_tmp_1304106917/__download__'
Calling unlink(/tmp/drush_make_tmp_1304106917/7.x)
Project information for geofield retrieved.                                                   [ok]
Executing: mkdir '/tmp/drush_make_tmp_1304106917/__download__'
Executing: ls '/tmp/drush_make_tmp_1304106917/__download__'
Calling chdir(/tmp/drush_make_tmp_1304106917/__download__)
Executing: curl -LOD '/tmp/drush_make_tmp_1304106917/__header__' 'http://updates.drupal.org/release-histo                                ry/libraries/7.x'
Calling chdir(/var/aegir/platforms/vino-v/vinov20110429)
Executing: ls '/tmp/drush_make_tmp_1304106917/__download__'
Executing: mv '/tmp/drush_make_tmp_1304106917/__download__/7.x' '/tmp/drush_make_tmp_1304106917/7.x'
Executing: rm -f '/tmp/drush_make_tmp_1304106917/__header__'
Executing: rm -rf '/tmp/drush_make_tmp_1304106917/__download__'
Calling unlink(/tmp/drush_make_tmp_1304106917/7.x)
Project information for libraries retrieved.                                                  [ok]
Executing: mkdir '/tmp/drush_make_tmp_1304106917/__build__'
Executing: mkdir '/tmp/drush_make_tmp_1304106917/__download__'
Executing: ls '/tmp/drush_make_tmp_1304106917/__download__'
Calling chdir(/tmp/drush_make_tmp_1304106917/__download__)
Executing: curl -LOD '/tmp/drush_make_tmp_1304106917/__header__' 'http://ftp.drupal.org/files/projects/drupal-7.0.tar.gz'
Calling chdir(/var/aegir/platforms/vino-v/vinov20110429)
Executing: ls '/tmp/drush_make_tmp_1304106917/__download__'
Executing: mv '/tmp/drush_make_tmp_1304106917/__download__/drupal-7.0.tar.gz' '/tmp/drush_make_tmp_1304106917/drupal-7.0.ta              r.gz.tar.gz'
Executing: rm -f '/tmp/drush_make_tmp_1304106917/__header__'
Executing: rm -rf '/tmp/drush_make_tmp_1304106917/__download__'
drupal downloaded from http://ftp.drupal.org/files/projects/drupal-7.0.tar.gz.                [ok]
Executing: gzip --list '/tmp/drush_make_tmp_1304106917/drupal-7.0.tar.gz.tar.gz'
Executing: gzip -d '/tmp/drush_make_tmp_1304106917/drupal-7.0.tar.gz.tar.gz'
Executing: mkdir '/tmp/drush_make_tmp_1304106917/__unzip__'
Executing: tar -x -C '/tmp/drush_make_tmp_1304106917/__unzip__' -f '/tmp/drush_make_tmp_1304106917/drupal-7.0.tar.gz.tar'
Executing: ls '/tmp/drush_make_tmp_1304106917/__unzip__'
Executing: mv '/tmp/drush_make_tmp_1304106917/__unzip__/drupal-7.0' '/tmp/drush_make_tmp_1304106917/__unzip__/__build__'
Executing: cp -Rf '/tmp/drush_make_tmp_1304106917/__unzip__/__build__' '/tmp/drush_make_tmp_1304106917'
Executing: rm -rf '/tmp/drush_make_tmp_1304106917/__unzip__'
Executing: rm '/tmp/drush_make_tmp_1304106917/drupal-7.0.tar.gz.tar'
Executing: mkdir '/tmp/drush_make_tmp_1304106917/__build__/sites/all/modules/geofield'
Executing: mkdir '/tmp/drush_make_tmp_1304106917/__download__'
Executing: ls '/tmp/drush_make_tmp_1304106917/__download__'
Calling chdir(/tmp/drush_make_tmp_1304106917/__download__)
Executing: curl -LOD '/tmp/drush_make_tmp_1304106917/__header__' 'http://ftp.drupal.org/files/projects/geofield-7.x-1.x-dev.tar.gz'
Calling chdir(/var/aegir/platforms/vino-v/vinov20110429)
Executing: ls '/tmp/drush_make_tmp_1304106917/__download__'
Executing: mv '/tmp/drush_make_tmp_1304106917/__download__/geofield-7.x-1.x-dev.tar.gz' '/tmp/drush_make_tmp_1304106917/geofield-7.x-1.x-dev.tar.gz.tar.gz'
Executing: rm -f '/tmp/drush_make_tmp_1304106917/__header__'
Executing: rm -rf '/tmp/drush_make_tmp_1304106917/__download__'
geofield downloaded from http://ftp.drupal.org/files/projects/geofield-7.x-1.x-dev.tar.gz.    [ok]
Executing: gzip --list '/tmp/drush_make_tmp_1304106917/geofield-7.x-1.x-dev.tar.gz.tar.gz'
Executing: gzip -d '/tmp/drush_make_tmp_1304106917/geofield-7.x-1.x-dev.tar.gz.tar.gz'
Executing: mkdir '/tmp/drush_make_tmp_1304106917/__unzip__'
Executing: tar -x -C '/tmp/drush_make_tmp_1304106917/__unzip__' -f '/tmp/drush_make_tmp_1304106917/geofield-7.x-1.x-dev.tar.gz.tar'
Executing: ls '/tmp/drush_make_tmp_1304106917/__unzip__'
Executing: cp -Rf '/tmp/drush_make_tmp_1304106917/__unzip__/geofield' '/tmp/drush_make_tmp_1304106917/__build__/sites/all/modules'
Executing: rm -rf '/tmp/drush_make_tmp_1304106917/__unzip__'
Executing: rm '/tmp/drush_make_tmp_1304106917/geofield-7.x-1.x-dev.tar.gz.tar'
Found makefile: geofield.make                                                                 [ok]
Executing: mkdir '/tmp/drush_make_tmp_1304106917/__download__'
Executing: ls '/tmp/drush_make_tmp_1304106917/__download__'
Calling chdir(/tmp/drush_make_tmp_1304106917/__download__)
Executing: curl -LOD '/tmp/drush_make_tmp_1304106917/__header__' 'http://updates.drupal.org/release-history/libraries/7.x'
Calling chdir(/var/aegir/platforms/vino-v/vinov20110429)
Executing: ls '/tmp/drush_make_tmp_1304106917/__download__'
Executing: mv '/tmp/drush_make_tmp_1304106917/__download__/7.x' '/tmp/drush_make_tmp_1304106917/7.x'
Executing: rm -f '/tmp/drush_make_tmp_1304106917/__header__'
Executing: rm -rf '/tmp/drush_make_tmp_1304106917/__download__'
Calling unlink(/tmp/drush_make_tmp_1304106917/7.x)
Project information for libraries retrieved.                                                  [ok]
Executing: mkdir '/tmp/drush_make_tmp_1304106917/__build__/sites/all/modules/libraries'
Executing: mkdir '/tmp/drush_make_tmp_1304106917/__download__'
Executing: ls '/tmp/drush_make_tmp_1304106917/__download__'
Calling chdir(/tmp/drush_make_tmp_1304106917/__download__)
Executing: curl -LOD '/tmp/drush_make_tmp_1304106917/__header__' 'http://ftp.drupal.org/files/projects/libraries-7.x-1.0.tar.gz'
Calling chdir(/var/aegir/platforms/vino-v/vinov20110429)
Executing: ls '/tmp/drush_make_tmp_1304106917/__download__'
Executing: mv '/tmp/drush_make_tmp_1304106917/__download__/libraries-7.x-1.0.tar.gz' '/tmp/drush_make_tmp_1304106917/libraries-7.x-1.0.tar.gz.tar.gz'
Executing: rm -f '/tmp/drush_make_tmp_1304106917/__header__'
Executing: rm -rf '/tmp/drush_make_tmp_1304106917/__download__'
libraries downloaded from http://ftp.drupal.org/files/projects/libraries-7.x-1.0.tar.gz.      [ok]
Executing: gzip --list '/tmp/drush_make_tmp_1304106917/libraries-7.x-1.0.tar.gz.tar.gz'
Executing: gzip -d '/tmp/drush_make_tmp_1304106917/libraries-7.x-1.0.tar.gz.tar.gz'
Executing: mkdir '/tmp/drush_make_tmp_1304106917/__unzip__'
Executing: tar -x -C '/tmp/drush_make_tmp_1304106917/__unzip__' -f '/tmp/drush_make_tmp_1304106917/libraries-7.x-1.0.tar.gz.tar'
Executing: ls '/tmp/drush_make_tmp_1304106917/__unzip__'
Executing: cp -Rf '/tmp/drush_make_tmp_1304106917/__unzip__/libraries' '/tmp/drush_make_tmp_1304106917/__build__/sites/all/modules'
Executing: rm -rf '/tmp/drush_make_tmp_1304106917/__unzip__'
Executing: rm '/tmp/drush_make_tmp_1304106917/libraries-7.x-1.0.tar.gz.tar'
Executing: mkdir '/tmp/drush_make_tmp_1304106917/__build__/sites/all/libraries'
Executing: mkdir '/tmp/drush_make_tmp_1304106917/__build__/sites/all/libraries/geoPHP'
Executing: mkdir '/tmp/drush_make_tmp_1304106917/__download__'
Executing: ls '/tmp/drush_make_tmp_1304106917/__download__'
Calling chdir(/tmp/drush_make_tmp_1304106917/__download__)
Executing: curl -LOD '/tmp/drush_make_tmp_1304106917/__header__' 'https://github.com/downloads/phayes/geoPHP/geophp.tar.gz'
Calling chdir(/var/aegir/platforms/vino-v/vinov20110429)
Executing: ls '/tmp/drush_make_tmp_1304106917/__download__'
Executing: mv '/tmp/drush_make_tmp_1304106917/__download__/geophp.tar.gz' '/tmp/drush_make_tmp_1304106917/geoPHP.tar.gz'
Executing: rm -f '/tmp/drush_make_tmp_1304106917/__header__'
Executing: rm -rf '/tmp/drush_make_tmp_1304106917/__download__'
geophp downloaded from https://github.com/downloads/phayes/geoPHP/geophp.tar.gz.              [ok]
Executing: gzip --list '/tmp/drush_make_tmp_1304106917/geoPHP.tar.gz'
Executing: gzip -d '/tmp/drush_make_tmp_1304106917/geoPHP.tar.gz'
Executing: mkdir '/tmp/drush_make_tmp_1304106917/__unzip__'
Executing: tar -x -C '/tmp/drush_make_tmp_1304106917/__unzip__' -f '/tmp/drush_make_tmp_1304106917/geoPHP.tar'
Executing: ls '/tmp/drush_make_tmp_1304106917/__unzip__'
Executing: cp -Rf '/tmp/drush_make_tmp_1304106917/__unzip__/geoPHP' '/tmp/drush_make_tmp_1304106917/__build__/sites/all/libraries'
Executing: rm -rf '/tmp/drush_make_tmp_1304106917/__unzip__'
Executing: rm '/tmp/drush_make_tmp_1304106917/geoPHP.tar'
Attempt to build project libraries more then once prevented.                                     [notice]
Directory not empty: /tmp/drush_make_tmp_1304106917/__build__/sites/all/libraries/geoPHP      [error]
Executing: mkdir '/tmp/drush_make_tmp_1304106917/__download__'
Executing: ls '/tmp/drush_make_tmp_1304106917/__download__'
Calling chdir(/tmp/drush_make_tmp_1304106917/__download__)
Executing: curl -LOD '/tmp/drush_make_tmp_1304106917/__header__' 'http://github.com/downloads/phayes/geoPHP/geoPHP.tar.gz'
Calling chdir(/var/aegir/platforms/vino-v/vinov20110429)
Executing: ls '/tmp/drush_make_tmp_1304106917/__download__'
Executing: mv '/tmp/drush_make_tmp_1304106917/__download__/geoPHP.tar.gz' '/tmp/drush_make_tmp_1304106917/geoPHP.tar.gz'
Executing: rm -f '/tmp/drush_make_tmp_1304106917/__header__'
Executing: rm -rf '/tmp/drush_make_tmp_1304106917/__download__'
geoPHP downloaded from http://github.com/downloads/phayes/geoPHP/geoPHP.tar.gz.               [ok]
Executing: gzip --list '/tmp/drush_make_tmp_1304106917/geoPHP.tar.gz'
Executing: gzip -d '/tmp/drush_make_tmp_1304106917/geoPHP.tar.gz'
Executing: mkdir '/tmp/drush_make_tmp_1304106917/__unzip__'
Executing: tar -x -C '/tmp/drush_make_tmp_1304106917/__unzip__' -f '/tmp/drush_make_tmp_1304106917/geoPHP.tar'
Executing: ls '/tmp/drush_make_tmp_1304106917/__unzip__'
Executing: mv '/tmp/drush_make_tmp_1304106917/__unzip__/geoPHP' '/tmp/drush_make_tmp_1304106917/__unzip__/'
Executing: cp -Rf '/tmp/drush_make_tmp_1304106917/__unzip__/' ''
Executing: rm -rf '/tmp/drush_make_tmp_1304106917/__unzip__'
Executing: rm '/tmp/drush_make_tmp_1304106917/geoPHP.tar'
Command dispatch complete                                                                        [notice]
attiks’s picture

I double checked with the following make file, only libraries declared twice, not the geophp lib and it's working

; This file was auto-generated by drush_make
core = 7.x

api = 2
projects[drupal][version] = "7.0"

projects[geofield][version] = "1.x-dev"

; The following are included in the make file of geofield
projects[libraries][version] = "1.0"
boombatower’s picture

Yea, I'm patching...looks like since libraries have different defaults there was one other location that needed changing.

boombatower’s picture

Status: Needs work » Needs review
FileSize
6.33 KB

The following was needed:

diff --git a/drush_make.drush.inc b/drush_make.drush.inc
index 84d7674..e2b2ac2 100644
--- a/drush_make.drush.inc
+++ b/drush_make.drush.inc
@@ -259,7 +259,7 @@ function drush_make_libraries($contrib_destination, $info, $build_path) {
     if ($ignore_checksums) {
       unset($library['download']['md5']);
     }
-    $class = new DrushMakeProject_Library($library);
+    $class = DrushMakeProject::getInstance('library', $library);
     $class->make();
   }
 }

I also went ahead and made the constructors private since they should never be called directly. Please double check and then RTBC.

boombatower’s picture

Ran a grep to be sure I got them all.

grep -nR DrushMakeProject .
./drush_make.project.inc:6:class DrushMakeProject {
./drush_make.project.inc:16:   * @see DrushMakeProject::getInstance()
./drush_make.project.inc:42:      $class = 'DrushMakeProject_' . $type;
./drush_make.project.inc:282:class DrushMakeProject_Core extends DrushMakeProject {
./drush_make.project.inc:302:class DrushMakeProject_Library extends DrushMakeProject {
./drush_make.project.inc:319:class DrushMakeProject_Module extends DrushMakeProject {
./drush_make.project.inc:326:class DrushMakeProject_Profile extends DrushMakeProject {
./drush_make.project.inc:337:class DrushMakeProject_Theme extends DrushMakeProject {                                                            
./drush_make.project.inc:344:class DrushMakeProject_Translation extends DrushMakeProject {                                                      
./drush_make.drush.inc:217:      if ($instance = DrushMakeProject::getInstance($project['type'], $project)) {
./drush_make.drush.inc:262:    $class = DrushMakeProject::getInstance('library', $library);
attiks’s picture

Status: Needs review » Needs work

Error: Call to private DrushMakeProject_Core::__construct() from context 'DrushMakeProject' in
/var/aegir/.drush/drush_make/drush_make.project.inc, line 43

attiks’s picture

I changed them to protected and it's working

attiks’s picture

Remark: In my test make file I used geoPHP as key, the geofield makefile uses geophp as key. The side effect is that it's downloaded twice.

Can we lowercase all keys? Or add this to the readme?

boombatower’s picture

Woops, yea they should be protected.

boombatower’s picture

Status: Needs work » Needs review
FileSize
6.34 KB
boombatower’s picture

#34: That is a separate issue, as currently drush make is case sensitive as unix systems tend to be anyway. You can open a new issue for that if you want to pursue it.

Thanks for your reviews of this patch.

attiks’s picture

Status: Needs review » Reviewed & tested by the community

Working as expected, thanks

attiks’s picture

#37: issue created #1143102: Keys are case sensitive, but i think it's a bug

Leeteq’s picture

Subscribing.

zoo33’s picture

Yes, the last patch fixed the problem for me too.

mikl’s picture

Please merge this :)

jox’s picture

I was trying drush make the first time and ran into this problem. The patch in #36 fixed it for me.

dman’s picture

#36 is good for me - fixed a recent new aegir deploy I had.
+1

boombatower’s picture

Status: Reviewed & tested by the community » Fixed

Committed. (got access now)

Status: Fixed » Closed (fixed)

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

igor.ro’s picture

do not work for me.

This is make file

projects[features][version] = "1.0-rc3"
projects[features][subdir] = "contrib"

projects[solution_core][type] = "profile"
projects[solution_core][download][type] = "git"
projects[solution_core][download][url] = "git://github.com/goruha/DrupalSolutionCore7.git"
projects[solution_core][download][branch] = "develop"

igor.ro’s picture

Status: Closed (fixed) » Needs work
boombatower’s picture

Status: Needs work » Closed (fixed)

Are you using drush 5.x which includes drush_make? If so please file a bug there and if necessary link to this.

Xen’s picture

Recursion has general brokeness in 5.x: #1621030: Regression for recursive makefiles.