Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Dotenv] dont use getenv() to read SYMFONY_DOTENV_VARS #29131

Merged
merged 1 commit into from Nov 8, 2018

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

Since using it is not thread safe, let's not rely on it (already done in other places.)

@ro0NL
Copy link
Contributor

ro0NL commented Nov 8, 2018

see also AboutCommand::getDotenvVars()

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Nov 8, 2018

the commands don't care about thread safety: they never run in an IIS thread.

@nicolas-grekas nicolas-grekas merged commit 21a909a into symfony:3.4 Nov 8, 2018
nicolas-grekas added a commit that referenced this pull request Nov 8, 2018
…colas-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[Dotenv] dont use getenv() to read SYMFONY_DOTENV_VARS

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Since using it is not thread safe, let's not rely on it (already done in other places.)

Commits
-------

21a909a [Dotenv] dont use getenv() to read SYMFONY_DOTENV_VARS
@nicolas-grekas nicolas-grekas deleted the dotenv-up branch November 8, 2018 21:51
@fabpot fabpot mentioned this pull request Nov 16, 2018
This was referenced Nov 26, 2018
fabpot added a commit that referenced this pull request Aug 30, 2019
…(brentybh)

This PR was squashed before being merged into the 4.3 branch (closes #33357).

Discussion
----------

[FrameworkBundle] Fix about command not showing .env vars

Before this fix, `bin/console about` can't properly show `Environment (.env)` section because:
`SYMFONY_DOTENV_VARS` which stores all keys of Dotenv-set env vars, is being fetched via `getenv()`.

However, in Symfony Dotenv 4.3, usage of `putenv()` is deprecated:
symfony/dotenv@d2fa94d#diff-a6967492da82dce9ba93bcba3eee0334
so we can get env vars via `getenv()` only when `new Dotenv(true)`.
In the default shipped `config/bootstrap.php`, there is `new Dotenv(false)` set.

(Maybe related #29131)

| Q             | A
| ------------- | ---
| Branch?       | 4.3 for bug fixes <!-- see below -->
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| License       | MIT

<!--
Replace this notice by a short README for your feature/bugfix. This will help people
understand your PR and can be used as a start for the documentation.

Additionally (see https://symfony.com/roadmap):
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against branch 4.4.
 - Legacy code removals go to the master branch.
-->

Commits
-------

f48f19d [FrameworkBundle] Fix about command not showing .env vars
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants