This patch adds an includes syntax for makefiles. It allows one makefile to include one or more external makefiles (either local or remote) and treat them as if their contents were part of the original makefile.

Basically the goal of the patch is to make this work:

includes[example] = "example.make"
includes[remote] = "http://www.example.com/remote.make"
CommentFileSizeAuthor
#13 820992-13_includes.patch9.07 KBalex_b
#12 820992_includes_tests_v3.patch9.04 KByhahn
#11 820992_includes_tests_v2.patch9 KByhahn
#10 820992_includes_tests.patch9 KBdmitrig01
#9 820992_includes_v2.patch7.52 KByhahn
drush_make_includes.patch7.27 KByhahn
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kepford’s picture

kepford CreditAttribution: kepford commented

Great idea! +1

brentratliff’s picture

brentratliff CreditAttribution: brentratliff commented

Didn't try the patch yet, but great functionality. Would definitely use it.

dmitrig01’s picture

dmitrig01 CreditAttribution: dmitrig01 commented

w00t! +1 subscribe!!!!!

Grayside’s picture

Grayside CreditAttribution: Grayside commented

I used the following makefiles:
http://github.com/grayside/oa_fserver/raw/master/oa_fserver.make

projects[] = admin_menu
projects[] = devel
projects[] = dhtml_menu
includes[example] = "../drush_make/EXAMPLE.make"
includes[oa_fserver] = "http://github.com/grayside/oa_fserver/raw/master/oa_fserver.make"

Result:

Project admin_menu defined twice (remove the first projects[] = admin_menu).                                                                                             [error]
An error occurred at function : drush_drush_make_make

Specifying the admin_menu to 3.x-alpha4 (just to be something different) seemed to work, although the EXAMPLE.make Tao could not be cloned from it's Git repository.

yhahn’s picture

yhahn CreditAttribution: yhahn commented

I'm having no problem cloning Tao in the EXAMPLE.make makefile in several environments... check that you can do a git clone of it manually?

On the error you're getting about a project being defined twice, this is expected behavior and I think we'll need to discuss separately if this is something we want to change. Projects that are added using the lazy projects[] = foo syntax can't be overridden cleanly by the including file unless we make some serious changes to the makefile validation code. Any other project syntax should work and allow overrides as part of the way info file parsing works, e.g. projects[foo][version] = 1.0.

Basically, this is a syntactical include and if the merging of makefiles wouldn't have been valid as a single file then they're not going to work split up across includes.

Grayside’s picture

Grayside CreditAttribution: Grayside commented

Then the patch worked as expected ;)

dasjo’s picture

dasjo
he/him
Primary language German
Location Zurich
CreditAttribution: dasjo commented

You could change the "defined twice" error to a warning where later definitions will overwrite the previous ones as this might be a common usecase

patricksettle’s picture

patricksettle CreditAttribution: patricksettle commented

Great idea!

Would like it if the the includes (and patches) could be pulled in via version control. Besides the versioning control it would provide, it would also add a layer of security by allowing users to pull form private repositories.

yhahn’s picture

yhahn CreditAttribution: yhahn commented
FileSize
820992_includes_v2.patch7.52 KB

New patch fixes problem where recursive makefile builds were not properly handling includes.

dmitrig01’s picture

dmitrig01 CreditAttribution: dmitrig01 commented
Status: Needs review » Needs work
FileSize
820992_includes_tests.patch9 KB

Not testable :-( here is as far as i got

yhahn’s picture

yhahn CreditAttribution: yhahn commented
FileSize
820992_includes_tests_v2.patch9 KB

Good catch. Updated to include relative to the including file, not the build base path.

Test now passes for me from both any directory and tests directory.

yhahn’s picture

yhahn CreditAttribution: yhahn commented
Status: Needs work » Needs review
FileSize
820992_includes_tests_v3.patch9.04 KB

Slight fix, drush_make_valid_url() should look for the include file as an absolute, not relative, URL.

alex_b’s picture

alex_b CreditAttribution: alex_b commented
FileSize
820992-13_includes.patch9.07 KB

When recursing through directories, $build_path would be passed on to drush_make_include_info_files() which is not enough to find the makefile to include; the full path to the makefile containing an include is necessary. The result is includes not being found if they are contained in a non top-level makefile.

// In DrushMakeProject::recurse():

// Broken:
$include = $this->queue->addItem('drush_make_include_info_files', array($build_path), array($info, $data));

// Fixed:
$include = $this->queue->addItem('drush_make_include_info_files', array(dirname($makefile)), array($info, $data));

This patch fixes the issue above and also adds an error message if an include file could not be found.

dmitrig01’s picture

dmitrig01 CreditAttribution: dmitrig01 commented

oh, that's a really interesting problem -- good find. can we have a test for this?

dmitrig01’s picture

dmitrig01 CreditAttribution: dmitrig01 commented
Status: Needs review » Fixed

decided it's not really necessary ;-).

Thanks for the code, I committed it.

Status: Fixed » Closed (fixed)

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

greg.harvey’s picture

greg.harvey
Primary language English
CreditAttribution: greg.harvey commented

Wait, what? So drush make supports includes? And I was about to ask for that. COOL!! =D