There have been several discussions regarding moving Drush Make into Drush core. This will be the issue to track progress towards that goal.

The primary reasons for this move are to

  • Better support make, and keep it in sync with drush
  • Take advantage of Drush's native caching of downloads and git references

The 2.x branch of Drush Make has been merged into commands/make, in a new branch in Drush.

The next steps will be

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

moshe weitzman’s picture

Looks like a terrific plan to me. If for any reason you want to work in a branch in the drush repo, thats ok by me.

I don't see any planned changes to the file format so existing make files will continue to work.

jonhattan’s picture

#1290750: PMTNG part 1 is a key issue for this. Once it is committed I'll go for the first of those two tasks:

1. Drush's new release_info engine can replace drush_make_updatexml($project):

drush_make.drush.inc:function drush_make_updatexml($project) {
drush_make.drush.inc:function drush_make_update_xml_download($project) {
drush_make.drush.inc:function _drush_make_update_xml_best_version($major, $releases) {
drush_make.generate.inc:function _drush_generate_makefile_check_updatexml($name, $type, $status_url = '') {

2. Drush_make's download classes can be the foundation of Drush's new package fetcher engine:

drush_make.project.inc:class DrushMakeProject {
drush_make.project.inc:class DrushMakeProject_Core extends DrushMakeProject {
drush_make.project.inc:class DrushMakeProject_Library extends DrushMakeProject {
drush_make.project.inc:class DrushMakeProject_Module extends DrushMakeProject {
drush_make.project.inc:class DrushMakeProject_Profile extends DrushMakeProject {
drush_make.project.inc:class DrushMakeProject_Theme extends DrushMakeProject {
drush_make.project.inc:class DrushMakeProject_Translation extends DrushMakeProject {
nielsonm’s picture

Subscribe

greg.1.anderson’s picture

@Nielsonm: use the big green "Follow" button!

I'll take a look at the sandbox when I have a chance. I have an issue in the drush make queue that I'd like to work in here: #1206340: introduce an options array in the root level of the makefile.

ergonlogic’s picture

Any chance this could get backported to Drush 4?

greg.1.anderson’s picture

I think that would be theoretically possible, but someone would need to do the work. I wonder if it would be worth doing, though; by the time we are done with this project, I would imagine that we'll have drush-5 at a point where we'll be ready to release drush-5 stable. Then there would be less reason for anyone to stay on drush-4.

jhedstrom’s picture

Issue summary: View changes

Changing sandbox project to point at the one that uses Drush Make 2.x as a starting point.

jhedstrom’s picture

I've created a new sandbox project that uses the 2.x branch as a starting point instead of 3.x. The reasoning here is that the 3.x branch contains a lot of advanced .ini parsing logic that isn't needed in Drush.

clemens.tolboom’s picture

@jhedstrom: it would be great when using a drush issue branch as moshe suggest in #2 as switching branch is a snap for me to occasionally test it.

clemens.tolboom’s picture

Issue summary: View changes

Typo in url.

jhedstrom’s picture

@clemens.tolboom this work has now been merged into a branch ('make') in the main Drush repo. I've updated this issue's summary to reflect that.

jhedstrom’s picture

Issue summary: View changes

Updated issue summary.

jhedstrom’s picture

Component: PM (dl, en, up ...) » Make
Status: Active » Needs review

Changing component, and status so people can start reviewing and testing.

moshe weitzman’s picture

Would be great if jonhattan could look into #2 since PMTNG landed.

Not that we are now using a 'make' branch in drush core project instead of the sandbox mentioned in #7.

helmo’s picture

Nice work, it works as expected for me.

I also rerolled a patch for the two remaining issues in #1216398: [meta] Translation support in a platform, please have a look.

moshe weitzman’s picture

dww’s picture

This is all very exciting, thanks to everyone who's making this a reality!

However, I'm a bit concerned about the stability of the Drupal.org distribution packaging system with these changes underway. I'm in the middle of finally trying to finish off the stuff at the Distribution Packaging community initiative page. Quite a few of the issues on there involve changes to the drupal.org-specific command handler for drush make (which is the cornerstone of the d.o distro packaging system, as I'm sure y'all are aware). I'm not sure where I should put those issues or the code. I had been planning to just continue working directly in the drush_make Git repo and issue queue, but that seems counter-productive at this point. But, I can't just wait indefinitely for drush core to completely replace drush_make, since I need to get the distro stuff deployed ASAP. Help! ;)

Thanks,
-Derek

jhedstrom’s picture

dww, at this point the d.o-specific functionality in make hasn't changed at all in the drush branch--the tests for that functionality do still need to be updated to to phpunit though (#1325438: Convert the make tests to use drush's PHPUnit framework). #1267228: Drush Make should use Drush core's native download abilities concurrently may get in at this point, but since RC1 is fast-approaching, any drastic refactoring will wait until drush 6, as I understand it.

dww’s picture

Good to know, thanks. However, my point is that we're going to want to be doing some "drastic" changes to the d.o drush stuff over the next couple of weeks, and I'm not sure where that code should live. Should we split the drupalorg drush commands out of drush make (and therefore drush core) and put them into the drupalorg project? Or is it going to be possible to get stuff quickly into drush core if it's just in the drupalorg-specific contrib commands/extensions? I'd rather the code continued to live with the rest of drush make, but I don't want to paint ourselves into a corner if we want/need changes on a different time scale than the core drush release cycle.

moshe weitzman’s picture

Why do you want this validation stuff in make itself? I think the best place is in drupalorg project, personally

dww’s picture

The only reasons it's historically been part of drush make:

A) To make it more likely to stay in sync with changes in drush make. If the code lives in a totally other universe, it's more likely that people will make sweeping changes to drush make and forget about (and therefore break) the drupalorg validation stuff. By keeping it all in the same git repo, grep can still be your friend.

B) So that anyone with a copy of drush make can just add --drupalorg to do the verify/convert stuff without having to download a whole separate drush extension.

Those aren't necessarily compelling reasons. It might be that "at the mercy of drush core release schedule" is a more powerful con that surpasses both of the above pros. But, that's why I'm asking. Really, this code hasn't changed much in 2+ years. And it's not likely to change much more in the future, either. Basically, over the next month there's going to be another round of development, then it's probably going to just coast along for quite a while. So long as there's a sane solution for the next month, I'd rather keep the code "next to" drush make.

Thanks,
-Derek

jhedstrom’s picture

After talking this through with Derek, it was decided that the drupal.org-specific functionality should moved to a separate project, to allow that code to develop at the pace of drupal.org.

dww’s picture

After IRC chat with msonnabaum and conversations in person with jhedstrom and mikey_p here at the mini PDX distro packaging sprint, we decided it was best all around to move the drupal.org-specific drush make commands into a separate project:

http://drupal.org/project/drupalorg_drush

jhedstrom is pushing code there as I write this. Just wanted to let everyone know the conclusion of that discussion. ;)

So, yeah, once that's moved out, it should be a lot easier for us d.o maintainers to do whatever we need at our own pace, and it should be hopefully easier for the drush maintainers to handle drush make.

The ONLY lingering concern is the likelihood that y'all are going to change drush make in ways that break the d.o packaging system. ;) Originally, when dmitrig01 was pushing for using drush make as the basis for the disto packaging stuff, we agreed to work together on this point and he said he wouldn't break stuff in a stable series, blah blah. ;) It'd be nice to have a similar agreement here as this moves forward. Not sure if this is the best issue to have that discussion or what.

Thanks!
-Derek

moshe weitzman’s picture

We'll keep that same promise. I hardly want to be the guy who broke distros.

dww’s picture

Yeah right, you can leave that title for me. ;) Anyway, thanks moshe!

jonhattan’s picture

Category: feature » task
FileSize
16.6 KB
21.51 KB

First item of #2 here.

Attached both the format-patch version and a plain diff. The patch is for the make branch. I'm not sure how to proceed,.. perhaps committing to that branch and then cherry pick from master?

jonhattan’s picture

My comment above is related to the fact that there're changes in some files apart of drushmake ones.

moshe weitzman’s picture

Awesome work! Your suggestion to commit this to the make branch sounds good to me. Lets do that as soon as all the test suite is passing (both regular tests and make tests). As soon as thats committed, I'm ready to merge the make branch into master. Anyone who has the time and skills to do that is welcome to do it. After that merge, we have two big tasks remaining (ideally done in next 2 weeks):

  1. Finish #2 from #2
  2. Improve error handling for the make code. It does stuff like drush_die(), and I think it is generally optimistic about successful network operations
jhedstrom’s picture

Status: Needs review » Needs work

The patch in #24 seems to have broken git checkouts.

From the git.make file in the tests/makefiles directory:

; Test that a specific tag can be pulled.
projects[tao][type] = theme
projects[tao][download][type] = git
projects[tao][download][tag] = 6.x-3.2

; Test that a branch can be pulled. We use a super-old "stale" branch in the
; Aegir project that we expect not to change.
projects[hostmaster][type] = profile
projects[hostmaster][download][type] = git
projects[hostmaster][download][branch] = 5.x

; Test that a specific revision can be pulled. Note that provision is not
; actually a module.
projects[provision][type] = module
projects[provision][download][type] = git
projects[provision][download][revision] = 23ccccd074b0c5c92df7c3a2a298907250525421

; Test a non-Drupal.org repository.
projects[geocode][type] = "module"
projects[geocode][download][type] = "git"
projects[geocode][download][url] = "git@github.com:phayes/geocode"
projects[geocode][download][revision] = "281c70f86"

Pre-patch, this performed a git checkouts, post-patch it's grabbing tarballs.

jhedstrom’s picture

Adding a conditional around setting the project download array fixes the git test:

      if (!isset($project['download'])) {
        $project['download'] =array(
          'type' => 'get',
          'url'  => $release['download_link'],
          'md5'  => $release['mdhash'],
        );
      }

but now includes (include.make) are failing.

jonhattan’s picture

Assigned: jhedstrom » moshe weitzman
Status: Needs work » Patch (to be ported)

Committed #24 with a fix to the git checkouts error.

One git clone within git.make is still failing but I think it is not related to this patch:

git clone 'git@github.com:phayes/geocode' /tmp/make_tmp_1323462355_4ee26ed313b6e/__git__/geocode
Cloning into /tmp/make_tmp_1323462355_4ee26ed313b6e/__git__/geocode...
Permission denied (publickey).
fatal: The remote end hung up unexpectedly

One of the next steps: #1267228: Drush Make should use Drush core's native download abilities concurrently

greg.1.anderson’s picture

We aren't going to backport drush make to drush-4.x core, are we?

jonhattan’s picture

Well, it is really a Branch To Be Merged per #26 (make -> master)

jhedstrom’s picture

Assigned: moshe weitzman » jhedstrom
Status: Patch (to be ported) » Needs work

Tests are still failing for me--I'll work on getting those to pass.

jhedstrom’s picture

Assigned: jhedstrom » moshe weitzman
Status: Needs work » Patch (to be ported)

All make tests are now passing. I'm seeing 12 failures when running the entire test suite, but I don't think those are related, since I get the same 12 failures when running on the master branch. I'd say this is ready to be merged.

jhedstrom’s picture

Assigned: moshe weitzman » Unassigned
Status: Patch (to be ported) » Fixed

Mark just merged this into master. Marking fixed!

greg.1.anderson’s picture

Status: Fixed » Needs work

Congratulations -- great accomplishment!

I am seeing two failures post-commit, the first of which was also present pre-commit (only one failure for me, not 12, at the time of #33).

Here's the pre-existing failure:

1) completeCase::testComplete
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'dl --: --gitsubmoduleaddparams'
+'dl --: --no-cache'

Here's the new one:

2) makeMakefileCase::testMakeMakefile
Unexpected exit code: /usr/bin/drush --nocolor make /home/ga/local/drush/tests/makefiles/git.make --no-core --test --md5=print
Failed asserting that 1 matches expected 0.

drush/tests/drush_testcase.inc:238
drush/tests/drush_testcase.inc:302
drush/tests/makeTest.php:21
jhedstrom’s picture

I just pushed up a fix to the test git.make file that changes the github url from a git-readonly style to https, which should fix the breakages described above.

Also, I'm in the process of finishing up #1325438: Convert the make tests to use drush's PHPUnit framework, which should make it easier to detect failures (moves each make file build into its own test).

dww’s picture

Something's also broken with the handling of the drush-make-update-default-url option. That seems to have vanished entirely, in fact. ;) This is breaking drupalorg_drush since it's assuming this option exists when creating the URL to fetch release history XML:

      'location'       => drush_get_option('make-update-default-url'),

and more importantly:

function drupalorg_drush_updatexml($project) {
  if ($filename = _make_download_file(drush_get_option('make-update-default-url') . '/' . $project['name'] . '/' . $project['core'])) {
    $release_history = simplexml_load_string(file_get_contents($filename));
    drush_op('unlink', $filename);
  }
  ...

Obviously that could be fixed in drupalorg_drush directly in a separate issue, but it seems like a red flag that this option used to exist when drush make was its own thing, and is now gone.

jhedstrom’s picture

Status: Needs work » Needs review

That option was apparently removed here. I don't see any harm in allowing that to be an option though, so I've re-added the make-update-default-url option.

jonhattan, or greg.1.anderson, could you see if the make tests are now passing for you?

greg.1.anderson’s picture

completeCase::testComplete and makeMakefileCase::testMakeBzr still failing for me on master pulled just now.

greg.1.anderson’s picture

Status: Needs review » Needs work

makeMakefileCase::testMakeBzr was failing because bzr is not installed on my system. I added a check to silently skip this test if bzr is not available. This is not ideal. Perhaps we could log here -- except log messages are not printed unless in --verbose or --debug mode. Perhaps this should be changed.

The next make test that is failing for me is the md5 fail test.

jonhattan’s picture

As now make is based on release_info engine the better is to fully integrate --source option instead of preserving make-update-default-url.

Could work on this in a couple of days.

Steven Jones’s picture

Drush make used to say:

Retrieved project information for ...

before downloading the modules in the make file.
Seems that the version in drush 5 doesn't any more. This isn't a problem, but it does look like make has stopped when processing a large makefile, when it hasn't.

anarcat’s picture

Shouldn't we close this issue now that drush is merged, and instead open new issues for those that crop up?

Otherwise we'll end up discussing nothing and everything here which could be very confusing. For example, I have moved this issue #581348: have drush make use the pm primitives here...

greg.1.anderson’s picture

Status: Needs work » Fixed

I re-opened because not all of the tests were passing, but I agree that #43 is more sensible. Let's s track remaining issues separately.

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

Anonymous’s picture

Issue summary: View changes

Updated issue summary.