search

acquia / acquia-sdk-php

The Acquia SDK for PHP allows developers to build applications on top of Acquia services.
MIT License
25 stars 20 forks source link

Swap out getenv() calls for $_ENV #57

Closed jacobbednarz closed 10 years ago

jacobbednarz commented 10 years ago

After setting up the Acquia SDK for our use our SSO, I have found a few errors popping up around calls to PHP's getenv() for the Acquia environment variables which end up not being available and they fall back to 'local' in most cases.

Consider the following:

$ drush php-eval "var_dump(getenv('AH_SITE_ENVIRONMENT'))"
bool(false)

$ drush php-eval "var_dump(\$_ENV['AH_SITE_ENVIRONMENT'])"
string(3) "dev"

Happy to provide you with the patch that we have rolled for our installation if you're happy to take this.

cpliakas commented 10 years ago

Hi @jacobbednarz.

Thanks for raising the issue. I am all for changing the logic to use something that is more reliable, especially when used with Drush. I would love to get a pull request with your change.

With that being said I am definitely curious why getenv() doesn't work with Drush, but that is a separate issue that vetted elsewhere.

Thanks in advance for the contribution! Chris

jacobbednarz commented 10 years ago

With that being said I am definitely curious why getenv() doesn't work with Drush, but that is a separate issue that vetted elsewhere.

I'm not sure if it's just isolated to drush as we also experienced this under standalone PHP. I believe the cause is the way C library routine getenv() is accessing the defined variables. Similar to the incompatibility between setting environment variables at the system level versus having them set at runtime by Apache, where the later (set by Apache) can only be accessed using apache-getenv() because it's created by a spawned process and doesn't hold the same access properties.

The PHP documentation for getenv() claims the only difference is that $_ENV performs a case sensitive search whereas getenv() uses case insensitive searches, however I am not convinced so I might have a further look into the underlying libraries to properly debug this one and provide a solid explanation on it.

Anyway, #58 should make this a little more versatile and fix the issues we encountered.

cpliakas commented 10 years ago

Thanks for the detailed information, definitely some food for thought here. As long as we can get it to work consistently which is seems $_ENV does, it probably saves some hours digging through low-level C code to find the root of the issue.

Thanks!