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

Populate $_SERVER with $_ENV #571

Merged
merged 1 commit into from Apr 10, 2019

Conversation

Projects
None yet
4 participants
@nicolas-grekas
Copy link
Member

nicolas-grekas commented Apr 10, 2019

Q A
License MIT

Reverts #540, fixes #532 (comment)

@symfony-flex-server
Copy link
Contributor

symfony-flex-server bot left a comment

Pull request passes validation.

@lyrixx

lyrixx approved these changes Apr 10, 2019

@Nyholm

Nyholm approved these changes Apr 10, 2019

Copy link
Member

Nyholm left a comment

Thank you

@symfony-flex-server symfony-flex-server bot merged commit 2b3e7e2 into symfony:master Apr 10, 2019

1 check passed

continuous-integration/symfony/pr Done
Details

symfony-flex-server bot added a commit that referenced this pull request Apr 10, 2019

@nicolas-grekas nicolas-grekas deleted the nicolas-grekas:env-load branch Apr 10, 2019

@MisatoTremor

This comment has been minimized.

Copy link
Contributor

MisatoTremor commented Apr 10, 2019

While you are right about thread safety this again introduces the problem that a system behaves different after calling composer dump-env prod.

The better solution would be to remove putenv from Dotenv, too. But that's not how it currently is.

@nicolas-grekas

This comment has been minimized.

Copy link
Member Author

nicolas-grekas commented Apr 10, 2019

The better solution would be to remove putenv from Dotenv, too. But that's not how it currently is.

I agree with that but indeed it would be quite a BC break.
Alternatively, you should ensure to always use $_SERVER and never getenv(). It's broken anyway.

@MisatoTremor

This comment has been minimized.

Copy link
Contributor

MisatoTremor commented Apr 10, 2019

Yep, unfortunately it is in the Symfony doc prior to 4.2, so it might be out in the wild.
Should therefor mainly apply to the 3.3 version of the receipe ... whiiich I coincidentally overlooked in my PR :-\

fabpot added a commit to symfony/symfony that referenced this pull request Apr 10, 2019

feature #31062 [Dotenv] Deprecate useage of "putenv" (Nyholm)
This PR was squashed before being merged into the 4.3-dev branch (closes #31062).

Discussion
----------

[Dotenv] Deprecate useage of "putenv"

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

From discussions on symfony/recipes#571, I think it is a good idea to make people opt-in to using `putenv`.

In Symfony 5.0 we will just change the value of the constructor. As an alternative, we could decide we want to remove `putenv` in Symfony 5.0. If so, I would also deprecate `$usePutenv=true`.

Commits
-------

8e45fc0 [Dotenv] Deprecate useage of \"putenv\"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.