drush dl failing: "Executing: bzr root"..... hangs it

The reason is here:

function drush_pm_include_version_control($directory = '.') {
  ...
  // Find the first valid engine in the list, checking signatures if needed.
  $engine = FALSE;
  while (!$engine && count($version_controls)) {
    $version_control = array_shift($version_controls);
    if (isset($version_control_engines[$version_control])) {
      if (!empty($version_control_engines[$version_control]['signature'])) {
        if (drush_shell_exec($version_control_engines[$version_control]['signature'], $directory)) {
          $engine = $version_control;
        }

In other words, drush will always use drush_shell_exec to test to see if a given location is under version control. This seems a little dubious to me; as pointed out in #0, these tools might get hung up doing network requests, or prompt for user input -- heavens, they shouldn't! -- but what a problem we have when they do.

Perhaps in addition to [$version_control]['signature'] we should also have [$version_control]['marker_file'] which must be empty, or must contain at least one file name that exists at the current location (e.g. 'CVS', '.svn', etc.) before we call drush_shell_exec on the signature item. That would be faster and safer.

Have I missed any cases, or would that do the trick?

@greg - isn't that signature what recently got committed at #988468: vcs engines should declare their control directories/files? This issue may be a use case for those "reserved" dirs.

Yes, that crossed my mind; we could assume that there must be at least one reserved directory at a given location before the signature function is called. That would win for brevity. However, it might be necessary to allow some vcs to declare that it wants its signature function to be called even if there are no reserved files found at the location being tested. Is there any vcs that is like this? Not that we implement yet, so we could do it as suggested, and adjust when and if the feature is needed.

'signature' is there since a long time. It was not introduced with #988468.

Sadly with php < 5.3 we can't access a static method by referencing its class with a variable:

$class = 'drush_pm_version_control_' . $name;
$class::reserved_files();

so it's needed to instantiate an object just to check for reserved files.

I think that #4 is okay.

@hanoii, can you confirm that it works for you?

committed.

Comming from #988468: vcs engines should declare their control directories/files.

OP reported "If I execute bzr root manually, it asks for my password on the subversion repository I use for my own sites."

If no --version-control is provided, engines are tested in this order: bzr, svn, backup.

What happened to OP is that bzr saw .svn and tried to authenticate: user is using svn as version control and bzr is installed, including its svn plugin (bzr-svn package in debian).

With patch in #4 we fixed this in a wrong way, as it brokes bzr integration because bzr do not store a .bzr file in each subdirectory but in it's root directory.

So we need to prevent running `bzr root` if .svn is present. In a quick look to bzr documentation it seems there's no flag to ignore svn or detech bzr-svn is installed --the only way is to add .svn to .bzrignore file.

Easiest fix for now is to revert #4 and change the order of engines to backup, bzr, svn (they're checked in the reverse order). Attached.

Other fix is to check for .svn in bzr signature. For now signature is a command to drush_shell_exec(). I think of moving signature to a function implemented by each vc engines.

I just committed this. Looks like a very good fix.

I had to add one line that had gotten lost: drush_include_engine('version_control', $version_control);