Function _pm_find_common_path() should be more failproof

Sounds like quite the edge case.

For reference, this is fb-6.x-2.x.

_pm_find_common_path() is a hack to workaround the project <-> extensions problem, and has been the source for epic fails (ej. removing sites/ folder) when projects doesn't follow the rules. So making it more fail proof is a win.

Ideally the function should log a debug message (ej: "Finding base path for project X") and also log an error message when it finds an anomaly as the one in OP. Note that the fail in the first call to drupal_get_path is by chance. The other call inside the while may also fail.

btw I wonder if generate-makefile may call this function for install profiles.

I'm seeing a similar error when trying to run 'drush pm-update' on a site built with the OpenPublic 7.x-1.0-beta5 installation profile. I've also seen it with older versions of drush 5.0-dev and other profiles.
Results of 'drush pm-update --debug'

Bootstrap to phase 0. [0.03 sec, 2.2 MB]                             [bootstrap]
Drush bootstrap phase : _drush_bootstrap_drush() [0.04 sec, 2.34 MB] [bootstrap]
Cache HIT cid:                                                           [debug]
5.0-dev-commandfiles-0-7d347c87fa85dcfb6344a12dd58c25e0 [0.08 sec,
2.35 MB]
Bootstrap to phase 0. [0.19 sec, 5.17 MB]                            [bootstrap]
Bootstrap to phase 6. [0.22 sec, 5.17 MB]                            [bootstrap]
Drush bootstrap phase : _drush_bootstrap_drupal_root() [0.23 sec,    [bootstrap]
5.18 MB]
Initialized Drupal 7.12 root directory at /var/www/drupal7 [0.3 sec,    [notice]
7.18 MB]
Drush bootstrap phase : _drush_bootstrap_drupal_site() [0.31 sec,    [bootstrap]
7.18 MB]
Initialized Drupal site default at sites/default [0.31 sec, 7.18 MB]    [notice]
Cache HIT cid:                                                           [debug]
5.0-dev-commandfiles-2-c32acd775c18a90c5b41d0fc9accf5ab [0.35 sec,
7.19 MB]
Drush bootstrap phase : _drush_bootstrap_drupal_configuration() [0.35[bootstrap]
sec, 7.18 MB]
Drush bootstrap phase : _drush_bootstrap_drupal_database() [0.37 sec,[bootstrap]
7.19 MB]
Successfully connected to the Drupal database. [0.37 sec, 7.19 MB]   [bootstrap]
Drush bootstrap phase : _drush_bootstrap_drupal_full() [0.39 sec,    [bootstrap]
7.69 MB]
Cache HIT cid:                                                           [debug]
5.0-dev-commandfiles-5-c386900c54ef895cb94f2f2b8acc691c [1.07 sec,
28.23 MB]
Drush bootstrap phase : _drush_bootstrap_drupal_login() [1.08 sec,   [bootstrap]
28.43 MB]
Successfully logged into Drupal as Anonymous (uid=0) [1.08 sec, 28.43[bootstrap]
MB]
Found command: pm-update (commandfile=pm) [1.08 sec, 28.43 MB]       [bootstrap]
Loading release_info engine. [1.29 sec, 29.63 MB]                       [notice]
Loading version_control engine. [1.3 sec, 29.74 MB]                     [notice]
Loading package_handler engine. [1.3 sec, 29.76 MB]                     [notice]
Executing: wget --version
Including /opt/drush/commands/pm/updatecode.pm.inc [1.36 sec, 29.78  [bootstrap]
MB]
Backend invoke: /usr/bin/php /opt/drush/drush.php --php=/usr/bin/php   [command]
--verbose --debug --root=/var/www/drupal7 --uri=http://default
batch-process 12 12 --backend=2 2>&1 [2.63 sec, 34.12 MB]
/usr/bin/php /opt/drush/drush.php --php=/usr/bin/php  --verbose         [notice]
--debug --root=/var/www/drupal7 --uri=http://default batch-process 12
12 --backend=2 2>&1 [2.63 sec, 34.13 MB]
Bootstrap to phase 0. [2.92 sec, 5.17 MB]                            [bootstrap]
Bootstrap to phase 6. [2.95 sec, 5.18 MB]                            [bootstrap]
Drush bootstrap phase : _drush_bootstrap_drupal_root() [2.95 sec,    [bootstrap]
5.18 MB]
Initialized Drupal 7.12 root directory at /var/www/drupal7 [3.01 sec,   [notice]
7.18 MB]
Drush bootstrap phase : _drush_bootstrap_drupal_site() [3.02 sec,    [bootstrap]
7.18 MB]
Initialized Drupal site default at sites/default [3.02 sec, 7.19 MB]    [notice]
Cache HIT cid:                                                           [debug]
5.0-dev-commandfiles-2-c32acd775c18a90c5b41d0fc9accf5ab [3.04 sec,
7.19 MB]
Drush bootstrap phase : _drush_bootstrap_drupal_configuration() [3.05[bootstrap]
sec, 7.19 MB]
Drush bootstrap phase : _drush_bootstrap_drupal_database() [3.06 sec,[bootstrap]
7.2 MB]
Successfully connected to the Drupal database. [3.06 sec, 7.2 MB]    [bootstrap]
Drush bootstrap phase : _drush_bootstrap_drupal_full() [3.07 sec,    [bootstrap]
7.69 MB]
Cache HIT cid:                                                           [debug]
5.0-dev-commandfiles-5-c386900c54ef895cb94f2f2b8acc691c [3.63 sec,
28.24 MB]
Drush bootstrap phase : _drush_bootstrap_drupal_login() [3.64 sec,   [bootstrap]
28.44 MB]
Successfully logged into Drupal as Anonymous (uid=0) [3.64 sec, 28.44[bootstrap]
MB]
Found command: batch-process (commandfile=core) [3.64 sec, 28.44 MB] [bootstrap]
Checking available update data ... [3.69 sec, 28.87 MB]              [ok]
Command dispatch complete [3.72 sec, 28.83 MB]                          [notice]
Peak memory usage was 29.48 MB [3.72 sec, 28.83 MB]                     [memory]
strpos(): Empty delimiter pm.drush.inc:1074 [4.08 sec, 34.29 MB]     [warning]
strpos(): Empty delimiter pm.drush.inc:1088 [4.08 sec, 34.29 MB]     [warning]
strpos(): Empty delimiter pm.drush.inc:1088 [4.08 sec, 34.29 MB]     [warning]
strpos(): Empty delimiter pm.drush.inc:1088 [4.08 sec, 34.29 MB]     [warning]
strpos(): Empty delimiter pm.drush.inc:1088 [4.08 sec, 34.29 MB]     [warning]
strpos(): Empty delimiter pm.drush.inc:1088 [4.08 sec, 34.29 MB]     [warning]

Drush version and status

# git show
commit 2c7c7c9ddee69b67d3c06001c75497aa8a0f9bb7
Author: Pol Dell'Aiera <Pol@47194.no-reply.drupal.org>
Date:   Fri Feb 24 16:47:44 2012 +0100

# drush status
 Drupal version         :  7.12                                                                                                               
 Site URI               :  http://default                                                                                                     
 Database driver        :  mysql                                                                                                              
 Database hostname      :  localhost                                                                                                          
 Database username      :  drupal7                                                                                                            
 Database name          :  drupal7                                                                                                            
 Database               :  Connected                                                                                                          
 Drupal bootstrap       :  Successful                                                                                                         
 Drupal user            :  Anonymous                                                                                                          
 Default theme          :  openomega                                                                                                          
 Administration theme   :  rubik                                                                                                              
 PHP configuration      :  /etc/php5/cli/php.ini                                                                                              
 Drush version          :  5.0-dev                                                                                                            
 Drush configuration    :                                                                                                                     
 Drush alias files      :  /etc/drush/publisher.alias.drushrc.php /etc/drush/developer.alias.drushrc.php /etc/drush/drupal7.alias.drushrc.php 
 Drupal root            :  /var/www/drupal7                                                                                                   
 Site path              :  sites/default                                                                                                      
 File directory path    :  sites/default/files     

Like the author suggests, it goes into an infinite loop, repeating until it runs out of memory. I checked and found that there were several .info files without accompanying .module files. Seems to be an assumption that is not always true.

Since the first error occurs at line 1074 of pm.drush.inc, I inserted a printf statement followed by a break. Looks like the culprit is the phase2_profile. I have no idea why it returns a null path.

path2 = profiles/openpublic/modules/contrib/captcha
 path = profiles/openpublic/modules/contrib/captcha/image_captcha
path2 = profiles/openpublic/modules/custom/phase2_profile
 path = 
path2 = profiles/openpublic/modules/contrib/conditional_styles
 path = profiles/openpublic/modules/contrib/conditional_styles/tests
path2 = profiles/openpublic/modules/contrib/context/context_layouts
 path = profiles/openpublic/modules/contrib/context/context_ui
path2 = profiles/openpublic/modules/contrib/ctools/stylizer
 path = profiles/openpublic/modules/contrib/ctools/views_content
path2 = profiles/openpublic/modules/contrib/date/date_tools
 path = profiles/openpublic/modules/contrib/date/date_views
path2 = profiles/openpublic/modules/contrib/delta/delta_color
 path = profiles/openpublic/modules/contrib/delta/delta_ui
path2 = profiles/openpublic/modules/contrib/entity/tests
 path = profiles/openpublic/modules/contrib/entity
path2 = profiles/openpublic/modules/contrib/features
 path = profiles/openpublic/modules/contrib/features/tests
path2 = profiles/openpublic/modules/contrib/references
 path = profiles/openpublic/modules/contrib/references/user_reference
path2 = profiles/openpublic/modules/contrib/nodequeue/addons/nodequeue_service
 path = profiles/openpublic/modules/contrib/nodequeue
path2 = profiles/openpublic/modules/contrib/panels/panels_mini
 path = profiles/openpublic/modules/contrib/panels/panels_node
path2 = profiles/openpublic/modules/contrib/password_policy
 path = profiles/openpublic/modules/contrib/password_policy/contrib/password_tab
path2 = profiles/openpublic/modules/contrib/token
 path = profiles/openpublic/modules/contrib/token/tests
path2 = profiles/openpublic/modules/contrib/views/tests
 path = profiles/openpublic/modules/contrib/views
path2 = profiles/openpublic/modules/contrib/wysiwyg
 path = profiles/openpublic/modules/contrib/wysiwyg/tests
path2 = profiles/openpublic/modules/contrib/xmlsitemap/xmlsitemap_taxonomy
 path = profiles/openpublic/modules/contrib/xmlsitemap/xmlsitemap_user
path2 = profiles/openpublic/themes/omega/starterkits/omega-html5
 path = profiles/openpublic/themes/omega/starterkits/omega-xhtml

The attached patch adds a test for the edge case where path is null. It also tests for the case where path2 is null and corrects the final option where the path is reduced to '', not '.'. I've tried to run the phpunit tests but they are hanging on git.make. I don't believe the failure is related to this patch because the same failure happens when the patch is not present.

Results of phpunit tests with those in git.make commented out.

Time: 12:00, Memory: 12.25Mb

There were 5 failures:

1) makeMakefileCase::testMakeGit
GIT integration - build md5 matches expected value: 4c80d78b50c89b5ba11a997bafec2b43
Failed asserting that 'd41d8cd98f00b204e9800998ecf8427e' contains "4c80d78b50c89b5ba11a997bafec2b43".

/opt/drush/tests/makeTest.php:30
/opt/drush/tests/makeTest.php:39

2) makeMakefileCase::testMakeTranslationsInside
Translation downloads inside makefile - build md5 matches expected value: 0566b12158e6fba7070b80714ea4019d
Failed asserting that 'Project token contains 3 modules: tokenSTARTER, token, token_actions.
fc812206098634dbfeffb30e299a650d' contains "0566b12158e6fba7070b80714ea4019d".

/opt/drush/tests/makeTest.php:30
/opt/drush/tests/makeTest.php:89

3) makeMakefileCase::testInfoFileWritingGit
Failed asserting that file "/tmp/drush-sandbox/home/.drush/cache/git/cck_signup-deb2799df60f886f30cfd756d4234b1e" exists.

/opt/drush/tests/makeTest.php:134

4) shellAliasesCase::testShellAliasSimpleReplacement
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-''@none''
+'@none'

/opt/drush/tests/shellAliasTest.php:117

5) shellAliasesCase::testShellAliasReplacementWithAlias
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-''@myalias' /path/to/drupal /srv/data/mypath'
+'@myalias /path/to/drupal /srv/data/mypath'

/opt/drush/tests/shellAliasTest.php:142

FAILURES!
Tests: 76, Assertions: 523, Failures: 5, Skipped: 1.

I don't think any of the five failures is related to this patch. Perhaps someone can run the tests to crosscheck my results.

Revised code for _pm_find_common_path()

      while($extension = array_pop($extensions)) {
        $path2 = drupal_get_path($project_type, $extension);
        // Option 1: same path.
        if ($path == $path2) {
          continue;
        }
        // Option 2a: null path.
        if ($path == '') {
          $path = $path2;
        }
        // Option 2b: null path2.
        if ($path2 == '') {
          $path2 = $path;
        }
        // Option 3: $path is a prefix of $path2.
        if (strpos($path2, $path) === 0) {
          continue;
        }
        // Option 4: $path2 is a prefix of $path.
        if (strpos($path, $path2) === 0) {
          $path = $path2;
          continue;
        }
        // Option 5: no one is a prefix of the other. Find the common
        // prefix by iteratively strip the rightmost piece of $path.
        // We will iterate until a prefix is found or path = '', that on the
        // other hand is a condition theorically impossible to reach.
        do {
          $path = dirname($path);
          if (strpos($path2, $path) === 0) {
            break;
          }
        } while ($path != '');
     }
  }

Renamed patch in #5 correctly (without the # sign). Sorry about that, I'm trying to learn.

Forgot to change status.

I'm working on this.

@dude4linux, the problem with openpublic was identified in #1303488: Drush fails on critical error with OpenPublic. See also #1300726: Openpublic Beta 3 throws a critical Drush error

I've refactored code that was relying in _pm_find_common_path() and added some checks as in the patch from @smokris, with logging. Those are the commits:

http://drupalcode.org/project/drush.git/commit/1bda735
http://drupalcode.org/project/drush.git/commit/6a661cb

Now the output for a case such as openpublic's one is:

$ drush php-eval "drush_get_projects()" 
Unknown path for spartan module.                                                                                  [warning]
Unknown path for openpublic_theme module.                                                                         [warning]
Unknown path for openomega module.                                                                                [warning]

I've applied the changes and I, indeed, get the "Unknown path" messages as listed.

But drush upc results in endless loop as described in http://drupal.org/node/1352102, which I assume is not related to this particular issue. I'm noting it here to document that the Drush 5.x branch as it currently stands will not work with OP. Thanks for the work here - it's good to see progress... thanks!

I'm getting the same error when running 'drush up' command on OpenPublish 7.x-1.0-alpha6 as detailed here: Multiple strpos(): Empty delimiter pm.drush.inc:1088 errors on 'drush up' command

Aplied patch on #8 and it worked normally.

I took a second look at my code in #7 and jonhattan's fix, git commit 6a661cb94ce4bf6e5b5d62a8458645112b7587d5. There are three edge cases that weren't properly handled by the original code.

1. The initial extension $path is null
2. An additional extension $path2 is null
3. There is no common path

My code in #7 and patch #8 work, but aren't entirely correct. The code for Option 2a & 2b should have been

        // Option 2a: null path.
        if ($path == '') {
          $path = $path2;
          continue;
        }
        // Option 2b: null path2.
        if ($path2 == '') {
          continue;
        }

jonhattan's fix handles the first two edge cases differently than mine, but correctly as well. The remaining difference is how the third case is handled. If there is no common path shared by the extensions, the final option will iteratively strip the rightmost part of $path.

      // Option 4: no one is a prefix of the other. Find the common
      // prefix by iteratively strip the rigthtmost piece of $path.
      // We will iterate until a prefix is found or path = '.', that on the
      // other hand is a condition theorically impossible to reach.
      do {
        $path = dirname($path);
        if (strpos($path2, $path) === 0) {
          break;
        }
      } while ($path != '.');

The flaw here is that when there is no leading '.' in $path, the while is never satisfied and it loops until a memory fault occurs. I presume there may be cases in which the $path has a leading '.' but those I have encountered do not, so the while should test for both '.' and '' to prevent the infinite loop. It might be a good idea to add a warning or error message when no common path is found. I've been seeing cases where modules get installed in the site root and this might be the cause.