Remove calls to drush_backend_invoke() (Call to undefined function drush_backend_invoke() in features.drush.inc line 214)

drush_backend_invoke was removed because this function allows command parameters to be included in the same string as the command, separated by spaces. This is very bad for Windows compatibility, when parameters might sometimes contain spaces themselves. Removing the whole function was harsh action, but it does allow us to quickly ferret out places that might still have this sort of problem.

To that end, calls that contain both commands and parameters, like this one:

      if (empty($project_info['diff']) && !drush_invoke_process('dl diff'))

Need to change to the new syntax:

      if (empty($project_info['diff']) && !drush_invoke_process('dl', 'diff'))

Or:

      if (empty($project_info['diff']) && !drush_invoke_process_args('dl', array('diff')))

I'm happy to roll a patch and test when I have time, but perhaps Mark will beat me to it. :)

P.S. the new call exists in drush-4 as well, so this change will not make features dependent on drush-5. Breaks for drush-3, though.

I should also point out that the 'process' functions differ from the 'invoke' functions in that the former by default do not show the command output, while the later do. To maintain the current behavior, where output is visible, set $integrate to TRUE. The way to do that in drush_invoke_process_args is as follows:

      if (empty($project_info['diff']) && !drush_invoke_process_args('dl', array('diff'), array('#integrate' => TRUE)))

Seems to me that 'process' functions are fine here, but I should have mentioned this difference right off the bat.

It's so nice to have a working patch waiting for you when you hit a bug.

Works for me. Thanks!

Patch works on 7.x-1.x-dev as well.

(deleted)

<3 git apply. Thx @msonnabaum & @greg.1.anderson

See, this is our conundrum. If #9 is committed, it breaks features for drush-4, but without #9, features will be broken for drush-5.

See discussion in #1205890: Add support for *.drush-5.inc, etc., to allow module developers to provide different code for different versions of drush.. We should decide what to do there before returning here with a recommendation for features.

c.f. #1245778: Make drush_invoke_process in drush-4.x forward-compatible with the same function in drush-5.x.

Re-roll of #9 against latest features-6.x-1.x branch.

#1245778: Make drush_invoke_process in drush-4.x forward-compatible with the same function in drush-5.x. has been committed, and drush-4.5 has been released. If #14 is committed, then features will work with drush-4.5 or later (including 5.x-dev), but will be broken for drush-4.4 and earlier. Without #14, features is broken with drush-5.x.

Simplistic, but to get both working, could the patch just do function_exists on backend invoke?

Yes, that's a good idea. I'll re-roll a new patch.

Actually, I am going back to recommending #14 should be committed after waiting a reasonable period of time for features clients to upgrade to drush 4.5. If drush_invoke_process did not exist prior to drush-4.5, then I would agree that using function_exists to put the new function into features if it did not exist in drush-4.x would be a good strategy. As it is, though, #16 would require keeping the features code compatible with drush-4.x, and adding backwards-compatible code if it does not exist in drush-5.x. At the drush code sprint, we decided not to maintain backwards compatibility for backend_invoke & c. in drush-5.x, so I must also conclude that it is not a good idea for each individual module to maintain backwards compatibility on its own.

with #14 and drush-5.x, fr works great but fra complains:

array_key_exists(): The second argument should be either an array or an object backend.inc:559

Will look at that.

I cannot reproduce #19, but it looks like the problem is more likely related to #663290: Better feedback with bad arguments or options (partially committed). Anyone having a similar problem should open a new issue in the drush queue, and include the output of drush fra --debug --show-invoke, as this code path may be affected by the drush commandfiles installed.

Actually, I think that #9 is broken in drush-4.5. Using the string shorthand for the site alias record is not supported in the current implementation. I'll probably fix this in drush.

Just for the record, i.e. for those that want (or need) to work with drush-4.5 (stable) on *nix *and* drush-5.x on Windows in parallel, here is an updated patch that works with both branches.

#23 is spot-on, but does not apply to HEAD of today's 6.x-1.x-dev branch. Here is one that does.

#24 does work with both drush-5.x and drush 4.5; however, on drush 4.5 the output from the backend calls is not shown. Drush 4.x works better if #integrate is set; this is not necessary on 5.x, though.

Updated patch using #integrate attached.

Did not mean to change status.

Tagging

Slightly obscure parameter, but it works, so RTBC :)

Works great for me. I'd say it's RTBC as well.

ditto

This is relevant to D7, right? Applies cleanly.

The patch in #25 does break Drush 4.4. Is this worth reverting to "needs work"?

If the features maintainers are not ready to drop support for drush 4.4 yet, then this issue should be set to "postponed", not "needs work". Support for drush 4.4 in features is not recommended per #18 above.

I recommend committing this patch to features, and document the dependency on drush 4.5 or later. It is my hope that it will not be postponed for too long, though, as drush 5 will be coming out soon (maybe by the end of the year). If drush 5 is released ahead of a stable release of features with this patch, then the latest stable drush would not work with the latest stable features. Avoiding that situation, in my mind, trumps concerns with backwards compatibility.

Needs feedback from the D7 maintainer.

The patch in #25 worked for me as well with Drush 5.x-dev.

Would it be possible to use drush_do_command_redispatch() instead, since that would be (in theory) version agnostic? Hate to derail a patch that is RTBC, but if this is a valid alternative I think it's worth exploring.

drush_do_command_redispatch is deprecated, and likely to be renamed _drush_do_command_redispatch in drush-5.

Back to RTBC.

In general I would be more than happy to drop Drush 4.4 support in favor if 4.5, however the major problem right now is that the most recent debian package for drush is still drush 4.4. Do we know who manages that and can we somehow get that upgraded to 4.5? (wishful thinking???)

I just dont want Features to break on what is the normal operation for getting Drupal/Drush up and running on Ubuntu, etc.

The Debian package is at 4.5 in squeeze-backports, wheezy and unstable:

http://packages.qa.debian.org/d/drush.html

Ubuntu also has 4.5 in the latest release, but hasn't backported to earlier releases:

https://launchpad.net/ubuntu/+source/drush

Brian Mercer's PPA is more up to date:

https://launchpad.net/~brianmercer/+archive/drush

Those interested in making Ubuntu move their bottoms in the right direction should open a bug like this one:

https://bugs.launchpad.net/ubuntu/+source/drush/+bug/755169

and provide proper testing.

This is now officially a FAQ.

To clarify, the patch in question is #25.

Are we saying that we have to wait until April when Ubuntu updates Drush for anyone to be able to use Features and Drush 5?

To add my voice to #41:

While the debian packages will remain available per #40, there is always a significant lag between drush releases and the availability of the same package in a stable debian release. The drush project page now recommends using pear as the recommended "normal" operation for installing drush.

Admittedly, many people are still going to be using apt-get today; however, with drush-5 stable slated to come out much sooner than April (end of calendar year being the current goal), it would be my hope that we would not have to wait that long for versions of drush and features that work with both the drush-4.x and 5.x current stable branches by the time drush-5 stable is available. Perhaps pear channel-discover pear.drush.org + pear install drush/drush might be an acceptable enough substitute for apt-get to move this along a little sooner?

I am not sure I understand the issue here, but I can certainly say that I will do my best to ship drush 5 with Debian once it's release.

It could also be possible to install drush 4 and 5 side by side by hardcoding /usr/share/drush4 as a search path instead of /usr/share/drush.

... but I do not understand why this is being discussed in the Features issue queue...

Explanation:

drush-5 will not work with features until this patch is committed. febbraro wants to hold off on committing this patch until drush-4.5 is the most recent debian package for drush (#39), because this patch breaks features for drush-4.4 and earlier.

Edit: So, in other words, the issue is not drush-5 in debian, but drush-4.5 in debian...

I see, so this is not about Drush 5.

But, again, Drush 4.5 *is* in Debian stable (squeeze, release a year and a half ago), through backports. Unless you're running lenny (released two and a half years ago), you should be able to run Drush 4.5 very easily. And even if you *are* running lenny, you can just install the package from squeeze-backports and it /should/ run fine.

See http://packages.qa.debian.org/d/drush.html for the different versions of Drush in Debian.

Now, I think what we are talking about here is Drush 4.5 in *Ubuntu*. Ubuntu Pangolin has 4.5 already. Oneiric and previous versions will need a backport or a sync request. I have already indicated how to followup on that process in #40.

See https://launchpad.net/ubuntu/+source/drush for the different versions of Drush in Ubuntu.

There is also a PPA with an up to date drush that anyone can use. You can also uninstall the drush package and install it through PEAR.

So I do not feel the issue here is the packaging of drush at all.

I'm ok with getting this patch committed so Features will run with Drush 5. It will have to wait until tomorrow though, I'm on empty right now. :)

I can put a notice on the project page for those that may need to upgrade to minimum Drush 4.5, assuming the pear install/upgrade mechanism is preferred.

So is #25 the RTBC patch for 7.x?

Yep.

Committed to 7.x, still need to update the project page docs.

http://drupalcode.org/project/features.git/commit/a098d96

tim, this one for you?

Awesome! Thanks drush guys, and Frank!

The patch applies cleanly to D6, but I'm going to test it on one of our production sites tomorrow.

FWIW, I have requested a backport of Drush to lucid, maverick and natty.

https://bugs.launchpad.net/lucid-backports/+bug/899655

Testers are welcome: just try out this package under one of those distributions (that is lucid, maverick or natty) and report how it works well.

http://packages.debian.org/squeeze-backports/all/drush/download

Sorry for the noise, but I figured that could be useful to people here.

Worked like a charm.
http://drupalcode.org/project/features.git/commit/e6df614

Excellent!

Just a quick note: this is broken with Drush-master again, but it is a Drush issue; nothing will need to change in Features. See: #1417984: drush_invoke_process no longer API compatible (e.g. with Features)

Setting status to 'Fixed' so that it is easier to find, should someone be looking in the Features queue, while allowing the system to just put it back to 'Closed' in two weeks if there's no further activity. Should be fixed in Drush by then; feel free to adjust the status here if skeptical. :)

It seems I was mistaken about this; the function works correctly.