drush -y @sites pm-list --pipe --status=enabled

results in:

Array
(
)

Removing --pipe from the command gives the expected results.
Run time for the command with --pipe and without is about the same, making me think the command is running correctly either way.

CommentFileSizeAuthor
#6 pipe-with-invoke.patch3.24 KBgreg.1.anderson

Comments

jonhattan’s picture

is @sites a valid alias?

moshe weitzman’s picture

yes it is. it is a multiple value alias where each value is a subdir in the sites directory of the current root. that is, all the multi-sites in a single install.

greg.1.anderson’s picture

Assigned: Unassigned » greg.1.anderson

I thought this was a duplicate, but I couldn't find any other issue, so I guess this is the first place this is documented.

--pipe does not work correctly with any command involving a list of sites, @sites being one such example. The "execute multiple" code does not merge together the pipe output when it's done.

clemens.tolboom’s picture

Component: Code » PM (dl, en, up ...)

Not sure whether this is related but I get a similar result when invoking drush @example.d6 pml --pipe where the site is on a remote server.

Invoking the same command on the server give the expected list.

Tested this with 4.5 on the server

clemens.tolboom’s picture

Digging deeper I found that the $proc['output'] is a __string__ not json so $values is never going to be an array. Thus parsing this results in the drush_set_error flow.

function _drush_backend_invoke($cmd, $data = null, $integrate = TRUE) {
...
    if ($proc['output']) {
      $values = drush_backend_parse_output($proc['output'], $integrate);
      if (is_array($values)) {
        return $values;
      }
      else {
        return drush_set_error('DRUSH_FRAMEWORK_ERROR', dt("The command could not be executed successfully (returned: !return, code: %code)", array("!return" => $proc['output'], "%code" =>  $proc['code'])));
      }
    }

Unfortunately the error is send to oblivion somehow. I guess due to the --pipe construct regarding the invoking drush.

The code from into function drush_pm_list seems clean to me apart from the last lines

 if (isset($pipe)) {
    // Newline-delimited list for use by other scripts. Set the --pipe option.
    drush_print_pipe($pipe);
  }
  // Set the result for backend invoke
  drush_backend_set_result($extension_info);

There is no backend result for $pipe ... or should it be json-ed?

greg.1.anderson’s picture

Status: Active » Needs review
StatusFileSize
new3.24 KB

The thing that is tricky about using --pipe with backend invoke is that --pipe mode causes drush to very quickly (in DRUSH_BOOTSTRAP_DRUSH) suppress all output via ob_start(). Beyond this one problem, drush is actually handling and propagating the pipe information correctly.

Solving this problem per #3 would actually be pretty involved, as you would have to migrate data from the backend result into drush_print_pipe, only so that it could be pulled out and printed again later. Although this may sound reasonable, there are a lot of different code paths that would have to be managed with different special-case conditionals.

This patch takes a different approach. The drush bootstrap is reorganized so that the output suppression that happens in backend and pipe modes is postponed until after remote command execution / multiple site alias execution modes are tested for. The code paths for "normal" commands and backend commands still runs all of the same bootstrap steps in the same order, although the output suppression routine is now called from a different place rather than being inlined in DRUSH_BOOTSTRAP_DRUSH. In remote command execution / multiple site alias execution modes, output suppression never happens at all.

Finally, this patch forces "interactive" mode when --pipe is specified. While this solution is not very sophisticated, it does produce the correct output, and all of the test cases still pass.

moshe weitzman’s picture

Version: All-versions-3.0 »
Component: PM (dl, en, up ...) » Base system (internal API)
Status: Needs review » Reviewed & tested by the community

Fix looks reasonable to me.

Should we add any more tests for --backend or --pipe? Even if we stub them out as TODO that would be helpful.

greg.1.anderson’s picture

Status: Reviewed & tested by the community » Fixed

Committed to master. No tests yet, but added some notes to COVERAGE.txt about tests still needed.

clemens.tolboom’s picture

Thanks ... it works for drush @remote.d5 pml --no-core --status=enabled [edit]remote = drush-4.5 which is great :)[/edit]

I don't quite copy the

Finally, this patch forces "interactive" mode when --pipe is specified.

Does that mean bash script will fail to run?

greg.1.anderson’s picture

No, that was a note on an internal implementation detail. You can ignore it. Someday, backend invoke's "interactive mode" will be rewritten; this will not affect --pipe.

clemens.tolboom’s picture

Tnx for the quick fix and replay #10

Status: Fixed » Closed (fixed)

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