I should be able to use $_ENV

[mglaman]

The latest release prevents using globals. vlucas/phpdotenv encourages using $_ENV because it is thread safe. See https://github.com/vlucas/phpdotenv#putenv-and-getenv

[danepowell]

Putting aside thread safety for a moment, there are other reasons why superglobals including $_ENV are bad when distributing libraries and modules. Previous discussion on this:

• Usage of $_ENV should be forbidden #9
• DT-1193: Use getenv instead of $_ENV blt#4005
• Disallow superglobals slevomat/coding-standard#911

tl;dr: PHP has a config setting variables_order that varies between environments, affects how $_ENV can be accessed, and has a history of generating support tickets. The only way to prevent that as a package maintainer is to disallow usage of $_ENV.

To the point of thread safety, this wasn't something I was aware of and I agree it's concerning. But vlucas himself comments in the issue discussing thread safety that if this is an issue in production, you're holding it wrong. At least that's my interpretation.

In the end, I guess we're balancing a known problem (support tickets generated by inconsistent variables_order and $_ENV usage) against a theoretical one (thread safety).

[mglaman]

It's funny about that linked issue: If you don't load via createUnsafeImmutable, you cannot use getenv. When using createImmutable (the recommended call) it only populates $_ENV. That issue is from 2015, so maybe opinions changed? Either way, there seems to be a conflict of assumptions there.

See v4 to v5 https://github.com/vlucas/phpdotenv/blob/master/UPGRADING.md

The Dotenv\Dotenv::createImmutable and Dotenv\Dotenv::createMutable methods no longer call will result in getenv and putenv being called. One should instead use Dotenv\Dotenv::createUnsafeImmutable and Dotenv\Dotenv::createUnsafeMutable methods, if one really needs these functions.

You make a good point about variables_order. We hit that with upgrade_status somewhere.

[mglaman]

Also to note: this should probably stay because Drush doesn't pass context to child processes so getenv is required.

// Note: we cannot use createImmutable() as subprocess commands used by Drush
// are missing $_ENV contents, but work with `getenv.`
// @todo open Drush issue
// @link https://drupal.slack.com/archives/C62H9CWQM/p1654540748519809
$dotenv = \Dotenv\Dotenv::createUnsafeImmutable(__DIR__);
$dotenv->safeLoad();

So I guess this can be closed. Regardless what may be used outside of Drupal or elsewhere in PHP, getenv is required for Drush.

[danepowell]

Thanks for the Drush reference, I vaguely recall that being a problem.

I'm a little wigged out by the thread safety issue. But until somewhat demonstrates that actually causing problems, I don't see much choice but to let it go.

[danepowell]

I referenced this thread in the AcquiaPHP ruleset so we can avoid sharding or repeating this discussion: 08dd037

[danepowell]

Another reason to use putenv, it actually modifies the process environment, unlike $_ENV which despite its name is just one more global variable. And global variables are still bad.