Requiring php 5.3.5 for Drush is premature

wait what? node_save() has problems with PHP 5.3.5? in which drupal version? and if so why weren't the requirements updated? reminder: Drupal 7 depends on PHP 5.2.5 or later.

https://drupal.org/requirements/php

The above page also mentions that Drupal 6 contrib modules are mostly unusable with PHP 5.4 (which I can confirm, at least views is really annoying), and I suspect Drupal 7 has its own issues with it. Often PHP 5.4 is the solution to fix this drush dependency: it's the only upgrade path available in Debian for example. It's also the version available in Fedora...

So what, exactly, makes Drush incompatible with PHP 5.3.3? and why the heck do we force a version comparison instead of letting stuff break? a warning could be sufficient...

and here's a patch for ya.

If the test suite passes, I'm OK with drush6 moving back to 5.2.

If we're moving back to 5.2, some changes in 8dca558da0fd2eb91552d3fb554365840e48b4af must be reverted. Also 31b9bb0ba6c0f10d36fdd2baa82a6f29e9360dde

In general git log -S"5.3" should be reviewed.

I believe that the point in #4 was to continue support Drupal 6 and Debian Squeeze. Even if 5.2 seems like a bit of a stretch, it seems clear that we could lower the requirement at least that far (5.3.3), no?

Also, the original issue suggested:

add a check for 5.3.5 when we are about to bootstrap Drupal. We could detect D8 via its filesystem layout, since we probably can't get its exact version until after we bootstrap.

I certainly like the idea of de-coupling Drush's required PHP version from Drupal's. Would this imply that we'd move the Drupal version-specific check into _drush_bootstrap_drupal_root()? In that case, perhaps drush_drupal_major_version() shouldn't depend on bootstrapping Drupal at all, but should rather just check for the existence of core/includes/bootstrap.inc (et. al.).

What's the point with 5.3.3, Drupal 6 and Squeeze? dotdeb.org provides php 5.3.26 for squeeze.

btw, drush_drupal_major_version() only needs the path to drupal root. No further bootstrap.

Some people (including me and many others) do not run Dotdeb packages. They compile PHP differently and the upgrade path to and from dotdeb is not exactly trivial.

The point of D6, PHP 5.3.3 and squeeze is that this is what squeeze is. It's a stable and controled environment that will only feature security fixes. Who knows what went into dotdeb's version of 5.3.26?

Using your logic, we might as well say that people can easily compile PHP themselves and could run PHP 6.x on Debian potato without problems. Heck, we should just rewrite Drush in Ruby while we're here, that would make for fun version dependencies.

I find it amazing that we go through all those crazy hoops to support Windows but we can't support Debian properly or Ubuntu LTS 10.04.

As for the check itself: i am not proposing to remove the check, I am proposing to make it a warning. If drush will fail with older PHP versions, why don't we let if fail? How do we know it will really fail? Maybe some people did some hackery to work around their issues in their PHP compilation (it could very well be the case for squeeze's 5.3.3).

This reminds me of problems with checking if we have write access before writing - you should just try to write and rollback properly. See also http://en.wikipedia.org/wiki/Time_of_check_to_time_of_use

8dca558da0fd2eb91552d3fb554365840e48b4af requires 5.2.1, so that's okay.

31b9bb0ba6c0f10d36fdd2baa82a6f29e9360dde is less clear - it says there's inconsistencies in mime type detection in 5.2, but doesn't say which version exactly.

the run-server stuff needs 5.3, but that's okay. i looked at git log -S 5.3 until 2 years back and that's all i found.

so i really think that requiring a higher 5.3 version is premature. 5.3.0 should work for everything that was mentionned here, and some 5.2 release may just work for most of drush.

it would be a damn shame if drush would just lock people away because they are running an older PHP version. at least allow them to *start* drush and they get *something*, even if with reduced functionality. worse case, a kill switch for lower PHP versions should be opt-out with an option, say --yes-i-know-i-am-an-old-fart that would override the version check...

otherwise we are just leaving people out.

i hate it when i need to throw away perfectly working devices after 5 years. software should be the same, and we should strive to avoid obsolescence, not encourage it.

I tested the latest HEAD (d543b36faf6) against PHP 5.3.3, and while there were 2 errors, I don't believe either were related to this:

There were 2 failures:

1) drushScriptCase::testPhpOptionsTest
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'text/drush'
+'text/html'

/usr/share/drush/tests/drushScriptTest.php:25

2) quickDrupalCase::testQuickDrupal
Unexpected exit code: /usr/bin/drush --root=/tmp/drush-sandbox/qd-devel --nocolor core-quick-drupal --yes --no-server --makefile=/usr/share/drush/tests/makefiles/qd-devel.make
Failed asserting that 1 matches expected 0.

/usr/share/drush/tests/drush_testcase.inc:349
/usr/share/drush/tests/drush_testcase.inc:416
/usr/share/drush/tests/quickDrupalTest.php:42
/usr/share/drush/tests/quickDrupalTest.php:53

FAILURES!
Tests: 109, Assertions: 1000, Failures: 2, Skipped: 6.

I've attached a patch that reduces the minimum PHP version to 5.3.3, as well as introducing a new global option, "--no-min-php-error", to allow the error to be ignored.

Re. testPhpOptionsTest, I had /usr/bin/drush linked directly to drush.php, and so was bypassing the shell script. That test passes now.

fwiw, I'd be fine with just rolling back to 5.3.3, as I don't have a 5.2 environment to test the rollbacks mentioned in #9.

As for an '--no-min-php-error' option, it's just a nice-to-have, imo. We could avoid documenting it in the help output by doing something like:

  if ($key = array_search('--no-min-php-error', $GLOBALS['argv'])) {
    unset($GLOBALS['argv'][$key]);
  }
  if (version_compare(phpversion(), DRUSH_MINIMUM_PHP) < 0 && $key === FALSE) {
    return drush_set_error('DRUSH_REQUIREMENTS_ERROR', dt('Your command line PHP installation is too old. Drush requires at least PHP !version.', array('!version' => DRUSH_MINIMUM_PHP)));
  }

Here's a patch that lowers the minimum PHP to 5.3.3 and skips the error in an environment variable (DRUSH_NO_MIN_PHP) is set.

I don't think we should allow earlier than 5.2.0, though, even with an environment variable set.

It seems to me that if someone sets this env var, they'd have to be aware that they are taking their chances, and we should probably let them.

Committed. Thanks for sticking with this.

If anyone has an idea about where to document the new Env Variable, let us know or write a docs patch. Thanks.

thanks!

How about documenting it in examples/example.bashrc?

#25 looks good.

Committed.