We are not checking the version of MySQL and PostgreSQL servers anymore (nor we do for SQLite)

chx and crell and I discussed, bumping priority to critical.

this is for sure untested and probably doesn't work, and also is probably the wrong approach and only implemented for mysql, but here's something

Yeah, not the right approach at all. :-)

Here's something more robust and tested, probably closer to working.

upping min sqlite

From IRC:

<dmitrig01>  Crell: the documentation for the function says " Run database tasks and tests to see if Drupal can run on the database."
<dmitrig01> Crell: this is technically a test to see if drupal can run on the database

This looks good. Committed to CVS HEAD! :)

Should't we move the constants DB version to the correspondent drivers?

Very nicely done. (Minor nitpick, i dislike the return NULL function body but that's a stable of DBTNG and does not belong here.)

I'm not sure about removing the constants (which have been around at least since drupal 5) this late in the development process. I would recommend keeping the constants here but mark them as deprecated instead.
(At least some consistency among the constants is required, DRUPAL_MINIMUM_SQLITE is left in by the patch in #11, but the other two are removed.)

Points 2 to 4 of #11 remain valid though.

A modified patch is attached.

What.Ever.

I would recommend keeping the constants here but mark them as deprecated instead.

Why? These constants should be internal to the installation process, anyway. bootstrap.inc has no business saying what the db layer supports in terms of versions. Let's go with Damien's patch in #16.

I think the constants should be removed. It's cleaner to keep it as a property of the database.

Only in case of a reroll:
+ * @TODO: consider upping to 3.6.8 in Drupal 8 to get SAVEPOINT support.
"@TODO:" -> "@todo"

Done, @tstoeckler.

The only thing blocking this is the discussion on whether the constants belong in the db layer or in the bootstrap.inc. It seems that everyone agrees about this except for Stevel, for the sake of backwards compatibility. However, I can't think of any realistic use case for using depending these constants from an outside perspective, and these also really don't belong in bootstrap.inc (they're part of the database layer, not bootstrapping). This issue is RTBC and 5/6 of the issue participants agree that the constants should be removed.

After discussing with Crell, the default version number should be 0 and not NULL, so we can carry out a real version check (comparing the version to 0).

Updated the docs too. I think it's good to go, setting to CNR for final review.

Returning 0 doesn't make sense as a version (it is not even a string, like the comment says), and seems instead just for the sake of saving lines of code. Doing a version compare with 0 is pointless :) Let's do #20, please.

version_compare() is a perfectly appropriate tool here; it's for just this sort of comparison. There's lots of weirdness in the way version strings could hypothetically be defined and version_compare() is a good-faith attempt to handle all of them. If a given driver has to do something totally different it can override the method and do whatever it needs.

We also have no need for the constants anymore, I agree.

Assuming the testbot likes #24, so do I.

version_compare() is only useful when you have two version strings to compare. I don't see why we should invoke it if we already know it will return -1. This is a minor issue though, it's just annoying that we are confusing integers with strings here. I still vote for #20.

Let's not get in a deadlock over such things. However we have a perfectly good patch in #20 that I am re-uploading. There is simply no logic in this statement (from #22): "so we can carry out a real version check (comparing the version to 0)."

Absolutely agree with #29

I think this approach (#20/#29) makes more sense than comparing against 0 (which is not a string anyway), but it doesn't seem like a huge problem either way.

Nitpick on documentation:

+   *   A version string. If not NULL, it will be checked against the version
+   *   reported by the Database engine using version_compare().

"Database engine" should be "database engine", I think, unless you're referring to the class or something.

From a logic standpoint I like the solution in #29. We are not saying there is a minimum version of 0 in cases where there isn't a version. Instead, if there is a version then we test to make sure the minimum is met. If no minimum is supplied we do not check. This makes logical sense.

Why are we not using the optional 3rd paramater in version_compare? Wouldn't it make more sense to use version_compare(Database::getConnection()->version(), $this->minimumVersion(), '<'). In this case it returns TRUE or FALSE depending on the operator.

@mfer, makes no difference in functionality, personally I don't see why version_compare() even has the $operator argument, because it doesn't do anything you can't do with standard PHP operators.

re #34: yep, but maybe it's a minor improve since you are using C comparison, so attaching the patch that do mfer suggestion.

Looks like 6 of us agree on NULL instead of 0, RTBC then.

Committed #35 to HEAD.

A-w-e-s-o-m-e! Beta3 comin' up.