drush_drupal_version() includes /modules/system/system.module, but VERSION is defined in includes/bootstrap.inc in drupal7.

Comments

acrollet’s picture

acrollet commented

whoops, I jumped the gun a bit - this function works correctly after bootstrap, but not before - in my case, it's returning false when called from an implementation of hook_drush_command(). I don't know if this is considered a bug, or if I need to get the version number myself.

moshe weitzman’s picture

moshe weitzman commented

Looks like a bug to me. bootstrap.inc should be included also at drush_drupal_version()

greg.1.anderson’s picture

greg.1.anderson commented

Maybe #0 and #1 are both correct -- drush_drupal_version() is including modules/system/system.module so that the function will work before a full bootstrap, but includes/bootstrap.inc is loaded at bootstrap, so this error is only noticed prior to bootstrap.

Just a thought. I also think it might be appropriate to just include both system.module and bootstrap.inc in drush_drupal_version, as these files should exist in d5 - d7, and it's probably not a huge hit considering the use.

moshe weitzman’s picture

moshe weitzman commented
Version: » All-versions-3.x-dev
Status: Active » Patch (to be ported)

Fixed in HEAD in http://drupal.org/cvs?commit=384206. Marking for backport. I tested on D6 and D7 but we should test D5 before committing to drush3.

moshe weitzman’s picture

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

committed to 6--3

adrian’s picture

adrian commented

this severely breaks a whole class of drush commands, that rely on random api functions not bootstrapping all on their own.

#879314: drush_drupal_version bootstraps, breaking all commands that want to bootstrap on their own.

i believe this needs to be reverted.

At the very least remove the bootstrap_max.

adrian’s picture

adrian commented

and for what it's worth.

drupal version number is unreliable. it's only applicable when you are running it on a drupal directory, which is _not_ always the case.

moshe weitzman’s picture

moshe weitzman commented

OK, I reverted as per adrian's suggestion and added includes/bootstrap.inc to the searchpath in major_version. So Aegir should be back in action.

This revert did expose a problem when we show help for a single command. We were printing out the help the first chance we could, instead of bootstrapping to the command's minimum and then reloading the command. Before the fix, these two command behaved differently:

drush help sql-conf
drush @self help sql-conf

The first command did not show the --target option even though it should have. adrian fixed that up for us in http://drupal.org/cvs?commit=405176

Please test so we can tag 3.3 release.

Lastly, this clusterfuck should have been caught a while ago since it was committed to HEAD on June 24, 2010. I really hope that Aegir devs can start using drush HEAD in their testing. I know this was a special circumstance with the php.net CVS => SVN bug. But this is gonna happen again if Aegir focuses exclusively on stable drush releases.

Status: Fixed » Closed (fixed)

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