Major feature extension & Drush 3.x Compatibility

I like the concept. Assigning to Fox to review and commit, since CVS is a PITA for me.

Excellent feature! It's also working as I'd imagine it's intended, so I'll go ahead and RTBC it.

That's a huge patch really difficult to review.

I'm not clear on why the environment_allowed() function is useful. How is it more useful than function_exists()?

Also, rather than run environment_init(), why not just have a no argument environment-switch, or an "environment-validate" command that defaults to the settings.php file's configuration. Checking on every page load might be cheap, but those two bits don't seem necessary since it only needs to run once on the site, and whenever you update the configuration form a certain way.

Given that this patch does some thing that are needed, I will run with it. Once it's committed we can revisit issues in a more targeted way.

Okay, that is helpful clarification of environment_allowed().

I disagree with environment_init(). After thinking further about it, I feel that the efficiency gain of skipping a line in a deployment script is not as great as the risk of a corner case botch in a variables table leading immediately to changing your environment status.

Strongly enough that despite the fact that I am already rolling patches against your patch, I will mark this as needs work. Hopefully ceardach or hefox can put two cents in.

Looking at the code, I see there is a hook_environments() for declaring additional environments. Why does this pull on an environment_definitions variable that is nowhere else set?

Attached patch removes the init stuff and does some small bits of cleanup, the most daring of which is in admin.inc.

My intent is to get this committed so we have the expanded functionality as a baseline to proceed with more cool stuff in the next couple days. Ceardach has asked me to run with some of this stuff, so I will commit this tomorrow, hopefully after someone else RTBC's.

Re: environment_init()
I really don't like the idea of an automated environment change, in the same way I wouldn't like it if the updatedb ran automatically if it detected updates were needed. I wouldn't mind seeing this place a message in the Status page, and possibly even a drupal_set_message() reminder. Another reasonable possibility is micro-module that implements hook_init() as you have it.

From my perspective I'd prefer to get the other stuff committed ASAP as it's generally a big improvement. a tie-breaking vote would be great here.

Re: environment_states
But without a UI, what's the point of it?
I definitely see the potential for making a UI around environment states. I'm just unclear about the benefit of both a UI and a hook.

My paranoic side says I should push for an environment_enforced.module. Then the checkbox option would be on admin/build/modules. Is that close enough?

Patch #8 committed as is. Let's continue talking about the enforced state stuff.

The main purpose of the Environment module is not to validate that the current state is enforced, it's to transition between states when you need different site behaviors. My resistance is the same reason why automatic running of updatedb isn't even an option in Core--because if something goes wrong, you have something between a bad security breach, data corruption, and site failure.

I am not absolutely against it, and I understand the value of the functionality you describe. Puppet-like behavior certainly has a value, and one I have only started to think about in the context of Drupal since you have pointed it out.

What I'm trying to evaluate now is whether it is acceptable for it to be in a submodule, so the edge case that ruins someone's day has that slight extra defense around it.

Can we step back a moment and look at the root goal here? Are you trying to block someone changing the environment on a site controlled by settings.php? Because we could simply handle that with validation in the UI's related to environment switching.

We can also do something like this:

/**
 * Gets the current environment.
 *
 * @param $load
 *  (default: FALSE) If TRUE, loads the full environment definition.
 * @param $property
 *  (default: NULL) If set, returns the specified element from the loaded environment object.
 */
function environment_current($load = FALSE, $property = NULL) {
  if (!($current = variable_get('environment_override', NULL))) {
    $current = variable_get('environment', NULL)
  }
  $current = check_plain($current);
  if (!$load) {
    return $current;
  }
  $environment = environment_load($current);
  if (!empty($environment)) {
    return is_null($property) ? $environment : $environment[$property];
  }
  return is_null($property) ? NULL : t('None');
}

Now when we grab the current environment, settings.php always has a chance to override it.

So we have:
1. A hook_requirements() implementation that shows a warning when there is an environment state mismatch.
2. A warning that links to a confirmation page to run the switch.
3. An environment_enforcement submodule that simply automates this.

Let's go ahead and separate these out.