At the moment, the recurse function looks like this :
function recurse($path, $directory) {
$makefiles = drush_scan_directory($this->path . '/' . $directory, '/\.make$/');
if (!empty($makefiles)) {
$build_path = $this->base_install_path;
foreach ($makefiles as $file) {
$info = $this->queue->addItem('drush_make_parse_info_file', $file->filename);
$valid = $this->queue->addItem('drush_make_validate_info_file', array(), $info);
$this->queue->addItem('drush_make_add_projects', array(TRUE, trim($build_path, '/'), $this->tmp_path, $this->base_path), array($valid));
$this->queue->addItem('drush_make_add_libraries', array(trim($build_path, '/'), $this->tmp_path, $this->base_path), array($valid));
}
}
}
What this means is that when it downloads an install profile and a module, it will execute ALL the makefiles found in the entire directory tree.
In my mind there should only be one makefile automatically assumed, namely : $projectpath/$project.make
Even though this isn't an issue currently, I ship my drush backend module 'provision' with a makefile that can create the front end, including downloading drupal from a drush command.
This makefile is specifically not called provision.make, but with this code if drush_make had to fetch provision (which is irrelevant at the moment because it isn't a module or hosted on d.o), it would try and fetch an entire drupal directory and store it inside the system.
Another use (other than shipping with a distro makefile), is the option of shipping with light / full makefiles. So if there are optional, but recommended modules you can ship the rules to fetch those alongside your install profile.
Plus, i'm kind of hesitant of shipping with this since the possibility of conflicting makefiles hasn't been explored yet, and this might trigger errors.
This is a pretty simple change to that function, but i wanted to make my intention to change this documented so we can discuss the implications.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | drush_make_singular_recurse4.diff | 3.02 KB | adrian |
| #10 | drush_make_singular_recurse3.diff | 3.02 KB | adrian |
| #8 | drush_make_singular_recurse2.diff | 3.24 KB | adrian |
| #2 | drush_make_singular_recurse.diff | 1.69 KB | adrian |
Comments
Comment #1
hunmonk commentedi fully support this change, and i've already made similar changes to the drupal.org packaging script to have it only look for one .make file.
tagging as a deployment blocker, since we want this in the first official release of drush_make before it hits d.o...
Comment #2
adrian commentedand here's an (untested) patch.
i haven't tested this with the subdirectory or directory_name settings.
Comment #3
adrian commentedfixing tag and setting status
Comment #4
dww+100.
Good ol'
make, which this is modeled on (or at least named after), always assumes a single Makefile per directory. It's got a magic name, and you can override that with a command-line arg if you want it to use a different make file. We don't have to go that far, I like project_name.make by default. But yeah, it should assume a single makefile per directory, not search for every possible file ending in .make and executing all of them.In fact,
makedoesn't automatically recurse at all. If you want it to build stuff in subdirectories, the parent directory makefile includes targets for building those subdirectories. I'd be in favor of these semantics for drush_make, too, but I suspect I'll be in the minority on this one and quickly out-voted/shouted-down. ;)But, that aside, definitely in favor of 1-makefile-per-directory and having a default search path for the one makefile we're going to try to use in each dir. Straw-man search order proposal:
1) [project_name].make
2) [current_directory_name].make
3) drush.make
?
Comment #5
adrian commentedbumping this up to critical :
http://drupal.org/node/644254
because d.o supports an optional separate makefile , in the current iteration drush_make would always execute both of them in projects that use that feature.
this means they are likely downloading the same files, and thus conflict with each other.
Comment #6
hunmonk commentedi support the search order detailed by dww, it makes sense to me, and should be easy to implement.
Comment #7
dww(hunmonk cross posted -- restoring the priority)
Also, site note to vertice:
If you just type:
#644254: Add option to write out makefile with specific version infoyou automatically get:
#644254: Add option to write out makefile with specific version info
which is a lot nicer for people reading the issue than:
http://drupal.org/node/644254
;)
Comment #8
adrian commentedHere is a new version of the patch that works with the subdirectory stuff, by using $this->name to find the makefile.
I don't think the multiple optional names for the makefile are relevant, as I do not see the use case for this situation.
I have also added a log message to show that it is fetching stuff from a new makefile.
I was a bit taken back by the duplication in the file. the recurse function is duplicated in both DrushMakeProject and DrushMakeProject_Profile, when the only difference between them is the definition of the $build_path variable.
would it not make sense to use a private function to determine $this->buildpath() instead of duplicating ~10 lines of code where it can get out of synch.
Comment #9
dmitrig01 commentedI agree about the basepath
Comment #10
adrian commentedhere's a new patch.
Comment #11
adrian commentednew patch with base_install_path a variable, not a function.
Comment #12
dmitrig01 commentedbam. thanks