[Dotenv] DebugCommand must respect dotenv_path runtime configuration #47901

alamirault · 2022-10-18T21:03:39Z

Q	A
Branch?	5.4
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets	Fix #47880
License	MIT
Doc PR	symfony/symfony-docs#...

Attempt to fix #47880. DebugCommand must respect dotenv_path when it's defined in composer.json. When it is defined, output will show relative path ✓ sub/path/.env.local.php

I took example on DotenvDumpCommand

symfony/src/Symfony/Component/Dotenv/Command/DotenvDumpCommand.php

Lines 61 to 69 in 889f4d6

    
           $config = []; 
        
           if (is_file($projectDir = $this->projectDir)) { 
        
               $config = ['dotenv_path' => basename($projectDir)]; 
        
               $projectDir = \dirname($projectDir); 
        
           } 
        
           $composerFile = $projectDir.'/composer.json'; 
        
           $config += (is_file($composerFile) ? json_decode(file_get_contents($composerFile), true) : [])['extra']['runtime'] ?? []; 
        
           $dotenvPath = $projectDir.'/'.($config['dotenv_path'] ?? '.env');

alamirault · 2022-10-29T13:11:40Z

@carsonbot find me a reviewer please

carsonbot · 2022-10-29T13:12:32Z

@rmikalkenas could maybe review this PR?

rmikalkenas · 2022-11-08T10:09:40Z

LGTM!

GromNaN

Thank you for taking care of this bug fix. Reading the directory from the config is too restrictive as the name of the file can be modified too.

The initial issue is that the file name .env is hardcoded in getEnvFiles. By using the config to generate the list of files, the dir path don't have to be passed separately from the file name and the code can be simplified.

(please ask if my comment is not clear)

src/Symfony/Component/Dotenv/Command/DebugCommand.php

src/Symfony/Component/Dotenv/Tests/Command/Fixtures/Scenario4/composer.json

alamirault · 2022-11-17T20:44:04Z

Thank you @GromNaN, your explanations are very clear :)

I updated PR, I think is prettier now

src/Symfony/Component/Dotenv/Command/DebugCommand.php

GromNaN

Thanks for the update. That's a lot more lisible and correct.

The dotenv_path can be configured in several places. We cannot cover all the cases and guess when the Dotenv class is instantiated in the code with a custom path.

It would be possible if we added a new variable SYMFONY_DOTENV_PATH initialized by Dotenv (like SYMFONY_DOTENV_VARS), to store the configured path. But that kind of change cannot be done in a minor version (as a bugfix) and then it will have to target 6.3 as a new feature.

src/Symfony/Component/Dotenv/Command/DebugCommand.php

GromNaN · 2022-11-18T00:31:30Z

src/Symfony/Component/Dotenv/Command/DebugCommand.php

+    {
+        $projectDir = is_file($projectDir = $this->projectDirectory) ? basename($projectDir) : $projectDir;
+
+        $composerFile = $projectDir.'/composer.json';


I see the dotenv_path can be configured for Symfony Runtime in APP_RUNTIME_OPTIONS env var. We can look at it too, but I don't know what is the priority.

symfony/src/Symfony/Component/Runtime/SymfonyRuntime.php

Lines 36 to 39 in ac88f9a

* In addition to the options managed by GenericRuntime, it accepts the following options:

* - "env" to define the name of the environment the app runs in;

* - "disable_dotenv" to disable looking for .env files;

* - "dotenv_path" to define the path of dot-env files - defaults to ".env";

symfony/src/Symfony/Component/Runtime/Internal/autoload_runtime.template

Lines 15 to 16 in ac88f9a

$runtime = $_SERVER['APP_RUNTIME'] ?? $_ENV['APP_RUNTIME'] ?? %runtime_class%;

$runtime = new $runtime(($_SERVER['APP_RUNTIME_OPTIONS'] ?? $_ENV['APP_RUNTIME_OPTIONS'] ?? []) + %runtime_options%);

GromNaN

Let's apply the bug fix like this and work on improving the feature in the new PR.

Co-authored-by: Jérôme Tamarelle <jerome@tamarelle.net>

nicolas-grekas

I think we should not read from composer.json.
Instead, @GromNaN puts us on the correct path to me: we should only read from $_ENV['APP_RUNTIME_CONFIG'], and we should do this change as a sidekick:

--- a/src/Symfony/Component/Runtime/Internal/autoload_runtime.template
+++ b/src/Symfony/Component/Runtime/Internal/autoload_runtime.template
@@ -19,7 +19,7 @@ if (!is_object($app)) {
 }
 
 $runtime = $_SERVER['APP_RUNTIME'] ?? $_ENV['APP_RUNTIME'] ?? %runtime_class%;
-$runtime = new $runtime(($_SERVER['APP_RUNTIME_OPTIONS'] ?? $_ENV['APP_RUNTIME_OPTIONS'] ?? []) + %runtime_options%);
+$runtime = new $runtime($_ENV['APP_RUNTIME_OPTIONS'] = ($_SERVER['APP_RUNTIME_OPTIONS'] ?? $_ENV['APP_RUNTIME_OPTIONS'] ?? []) + %runtime_options%);

GromNaN · 2022-12-13T15:22:33Z

@nicolas-grekas symfony/dotenv is not always used with symfony/runtime. That's why I want to add a new variable SYMFONY_DOTENV_PATH that is initialized by Dotenv::bootEnv() with the path argument it received. This is more agnostic.

nicolas-grekas · 2022-12-13T15:29:46Z

@GromNaN I missed that part. I'm not sure we should do this. Dotenv should remain clear of such concerns to me. The runtime is a better place - and would expose all options, not just a subset.

GromNaN · 2022-12-13T15:41:43Z

This command already rely on SYMFONY_DOTENV_VARS.

To make things cleaner, the path have to be injected into the command constructor instead of being discovered from env vars.
That way, if someone builds a symfony/console + symfony/dotenv application, they can explicitly set the dotenv path if needed.
We can use env var processors to extract the path (something like %env(key:dotenv_path:json:APP_RUNTIME_OPTIONS)%) when the command is registered in the container.

nicolas-grekas · 2022-12-15T11:07:12Z

Constructor injection FTW I agree.
Works for you @alamirault?
I'm moving to the 6.3 milestone as this turned into an improvement.

alamirault · 2022-12-18T18:16:21Z

@nicolas-grekas I will change to constructor injection. However which service will populate APP_RUNTIME_OPTIONS based on composer.json ?
SymfonyRuntime construct use composer extra but I think is never set in env APP_RUNTIME_OPTIONS, should we add a putenv somewhere ?

GromNaN · 2023-11-17T20:31:24Z

I haven't found how the dotenv_path could be injected into the command constructor as it is not known by the container.

I'm still convinced that there should be a SYMFONY_DOTENV_FILE variable containing the path to the .env file that has been parsed. It could even have the ordered list of parsed files, which would eliminate the need to reimplement the logic in getEnvFiles.

Symfony Runtime is optional. If people wants to call DotEnv explicitly with an other name, "the old way" on an other file, the command would work.

(new Dotenv())->bootEnv(dirname(__DIR__).'/custom/app.env');

GromNaN · 2023-11-17T22:39:21Z

I reworked this PR for 7.1, adding an env var #52638

nicolas-grekas · 2023-11-20T09:00:50Z

Closing in favor of #52638, thanks for submitting.

…:dotenv` for custom `.env` path (GromNaN) This PR was merged into the 7.1 branch. Discussion ---------- [Dotenv] Add `SYMFONY_DOTENV_PATH`, consumed by `debug:dotenv` for custom `.env` path | Q | A | ------------- | --- | Branch? | 7.1 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | Fix #47880 | License | MIT Continuation of #47901 Introduce `SYMFONY_DOTENV_PATH` set by DotEnv class and read by the `debug:dotenv` command to contextualize the debug info with the file that was actually parsed. The custom path can be set in many ways: - Doing a call to `(new Dotenv())->bootEnv(dirname(__DIR__).'my/custom/path/to/.env');` - In `composer.json`: `"extra": { "runtime": { "dotenv_path": "my/custom/path/to/.env" }` - With the env var: `$_SERVER['APP_RUNTIME_OPTIONS'] = ['dotenv_path' => 'my/custom/path/to/.env'];` The dotenv file can be outside of the `project_dir`. Commits ------- 52b6416 DotEnv debug command aware of custom dotenv_path

carsonbot added Status: Needs Review Bug Dotenv labels Oct 18, 2022

carsonbot added this to the 5.4 milestone Oct 18, 2022

[Dotenv] DebugCommand must respect dotenv_path runtime configuration

a56b24d

GromNaN requested changes Nov 15, 2022

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels Nov 15, 2022

alamirault commented Nov 17, 2022

View reviewed changes

src/Symfony/Component/Dotenv/Command/DebugCommand.php Outdated Show resolved Hide resolved

alamirault requested a review from GromNaN November 17, 2022 20:49

carsonbot added Status: Needs Review and removed Status: Needs Work labels Nov 17, 2022

Be more flexible with a better approach

2fefa74

GromNaN reviewed Nov 18, 2022

View reviewed changes

GromNaN approved these changes Nov 18, 2022

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Nov 18, 2022

Update src/Symfony/Component/Dotenv/Command/DebugCommand.php

20508b3

Co-authored-by: Jérôme Tamarelle <jerome@tamarelle.net>

nicolas-grekas requested changes Dec 13, 2022

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Reviewed labels Dec 13, 2022

nicolas-grekas modified the milestones: 5.4, 6.3 Dec 15, 2022

alamirault mentioned this pull request May 3, 2023

debug:dotenv command does not respect dotenv_path runtime configuration value #47880

Closed

nicolas-grekas modified the milestones: 6.3, 6.4 May 23, 2023

nicolas-grekas modified the milestones: 6.4, 7.1 Nov 15, 2023

GromNaN mentioned this pull request Nov 17, 2023

[Dotenv] Add SYMFONY_DOTENV_PATH, consumed by debug:dotenv for custom .env path #52638

Merged

nicolas-grekas closed this Nov 20, 2023

	$config = [];
	if (is_file($projectDir = $this->projectDir)) {
	$config = ['dotenv_path' => basename($projectDir)];
	$projectDir = \dirname($projectDir);
	}

	$composerFile = $projectDir.'/composer.json';
	$config += (is_file($composerFile) ? json_decode(file_get_contents($composerFile), true) : [])['extra']['runtime'] ?? [];
	$dotenvPath = $projectDir.'/'.($config['dotenv_path'] ?? '.env');

	* In addition to the options managed by GenericRuntime, it accepts the following options:
	* - "env" to define the name of the environment the app runs in;
	* - "disable_dotenv" to disable looking for .env files;
	* - "dotenv_path" to define the path of dot-env files - defaults to ".env";

	$runtime = $_SERVER['APP_RUNTIME'] ?? $_ENV['APP_RUNTIME'] ?? %runtime_class%;
	$runtime = new $runtime(($_SERVER['APP_RUNTIME_OPTIONS'] ?? $_ENV['APP_RUNTIME_OPTIONS'] ?? []) + %runtime_options%);

Uh oh!

[Dotenv] DebugCommand must respect dotenv_path runtime configuration #47901

[Dotenv] DebugCommand must respect dotenv_path runtime configuration #47901

Uh oh!

Conversation

alamirault commented Oct 18, 2022

Uh oh!

alamirault commented Oct 29, 2022

Uh oh!

carsonbot commented Oct 29, 2022

Uh oh!

rmikalkenas commented Nov 8, 2022

Uh oh!

GromNaN left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alamirault commented Nov 17, 2022

Uh oh!

Uh oh!

GromNaN left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

GromNaN Nov 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GromNaN left a comment

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GromNaN commented Dec 13, 2022

Uh oh!

nicolas-grekas commented Dec 13, 2022

Uh oh!

GromNaN commented Dec 13, 2022

Uh oh!

nicolas-grekas commented Dec 15, 2022

Uh oh!

alamirault commented Dec 18, 2022

Uh oh!

GromNaN commented Nov 17, 2023

Uh oh!

GromNaN commented Nov 17, 2023

Uh oh!

nicolas-grekas commented Nov 20, 2023

Uh oh!

Uh oh!

GromNaN left a comment •

edited

Loading

GromNaN Nov 18, 2022 •

edited

Loading

nicolas-grekas left a comment •

edited

Loading