Config\Factory can read from include_path #4574

Merged
merged 1 commit into from Jun 28, 2013

Conversation

Projects
None yet
4 participants
Contributor

bacinsky commented Jun 2, 2013

Added possibility to load config files from the include path.

Member

DASPRiD commented Jun 6, 2013

Why exactly would you include configuration from the include path. More like, why would you not know where the config files are located? This is kinda the same as the translation loaders loading from the include_path – there I couldn't really understand the true use case either, especially not since the include_path is not meant for non-PHP files.

Contributor

bacinsky commented Jun 6, 2013

It's because whole application is in PHAR, so I can have relative config_static_paths. chdir does not work on PHAR, otherwise no problem. Same about translations.

And from my point of view it's a feature, because doesn't matter the file path is relative to the file system or within an include path of PHAR. You can PHAR entire application and it works.

Owner

ezimuel commented Jun 6, 2013

@bacinsky I don't think this feature is needed because it's focused on a specific use case that actually can be manageable in a different way. You can manage relative path in a PHAR project but you have to refer to the path of the PHAR file. For more example on that you can check this post: http://mangstacular.blogspot.it/2011/06/php-relative-paths-and-phar-archives.html

ezimuel closed this Jun 6, 2013

Contributor

bacinsky commented Jun 6, 2013

The only problem in Config\Factory is that exception before include. It lies. But whatever...

Member

DASPRiD commented Jun 6, 2013

@ezimuel I guess we could close #4515 for the same reason?

Owner

ezimuel commented Jun 6, 2013

@bacinsky Ok, it seems that this issue is also related with #4515. I'm reopening it to spend more time on it and double check. @DASPRiD thanks for your advice.

ezimuel reopened this Jun 6, 2013

@weierophinney weierophinney commented on the diff Jun 28, 2013

library/Zend/Config/Factory.php
@@ -65,7 +65,14 @@ class Factory
*/
public static function fromFile($filename, $returnConfigObject = false)
{
- $pathinfo = pathinfo($filename);
+ $fromIncludePath = stream_resolve_include_path($filename);
@weierophinney

weierophinney Jun 28, 2013

Owner

I wouldn't do this unless !file_exists($filename), as stream_resolve_include_path() can be quite expensive.

@weierophinney

weierophinney Jun 28, 2013

Owner

I made this change on merge.

@weierophinney weierophinney added a commit that referenced this pull request Jun 28, 2013

@weierophinney weierophinney [#4574] Only run stream_resolve_include_path when necessary
- i.e., when `file_exists()` fails
- Raise an exception if it still cannot be found
b789bc8

@weierophinney weierophinney added a commit that referenced this pull request Jun 28, 2013

@weierophinney weierophinney Merge branch 'hotfix/4574' into develop
Close #4574
b538e52

weierophinney merged commit 5c204ed into zendframework:develop Jun 28, 2013

1 check passed

default The Travis CI build passed
Details

bacinsky deleted the bacinsky:feature/config-factory-read-from-include-path branch Jun 28, 2013

@weierophinney weierophinney added a commit to zendframework/zend-config that referenced this pull request May 15, 2015

@weierophinney weierophinney Merge pull request zendframework/zendframework#4574 from bacinsky/fea…
…ture/config-factory-read-from-include-path

Config\Factory can read from include_path
632d223

@weierophinney weierophinney added a commit to zendframework/zend-config that referenced this pull request May 15, 2015

@weierophinney weierophinney [zendframework/zendframework#4574] Only run stream_resolve_include_pa…
…th when necessary

- i.e., when `file_exists()` fails
- Raise an exception if it still cannot be found
5012106

@weierophinney weierophinney added a commit to zendframework/zend-config that referenced this pull request May 15, 2015

@weierophinney weierophinney Merge branch 'hotfix/4574' into develop a5e31f0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment