Config system silently ignores Only: blocks checking for non-live environment

[oddnoc]

Given the following YAML file in mysite/_config/:

---
Only:
  environment: dev

---
SSViewer:
  source_file_comments: 1

After dev/build, Config::inst()->get('SSViewer', 'source_file_comments') still returns false even though the site is in dev mode.

[mateusz]

The 'Only' and 'Except' conditional logic is rather broken beyond usefulness. I've had a quick poke here: https://gist.github.com/mateusz/074356785f0231d4f75d , this fixes some of the issues, but the two TODOs need to be checked and the whole thing given a unit test (or at least a good manual test) to be accepted into the core.

Would you like to have a go at fixing this? :-)

[oddnoc]

When you do a dev/build, I've discovered that when ConfigManifest#matchesVariantRules() is called, the system has not yet set the environment from _ss_environment.php, and the mode defaults to live. Therefore, no exclusion based on test or dev mode can possibly succeed. By the time DevelopmentAdmin#build() is called, the environment is correct.

[chillu]

@hafriedlander (the author of the Config API) isn't around for comments. If we receive a well tested and documented patch, it might find its way into 3.1, but for now I'm unsetting the milestone since its not critical enough to hold up that release (this bug has been around since 3.0.0 presumably).

[mateusz]

@oddnoc oh, that's true - the config system runs before ConfigureFromEnv.php is included, so Director::environment_type is not set yet. We could however use the more explicit variant which is already available: Except: SS_ENVIRONMENT_TYPE: 'dev'

I think though it will still not work properly because the conditional logic is broken.

I have cleaned up my previous gist a bit more here https://github.com/mateusz/sapphire/tree/fixes-to-conf , but didn't have a chance to write any tests yet - they would need to go into ConfigManifestTest.php.

Interested in teaming up? :-)

[ajshort]

I rewrote the config system stuff ages ago to actually work in my GSOC branch, but it never got merged. It still might be useful as a reference, I'm not sure.

https://github.com/ajshort/sapphire/blob/composer/src/SilverStripe/Framework/Manifest/ConfigManifest.php https://github.com/ajshort/sapphire/blob/composer/tests/SilverStripe/Framework/Tests/Manifest/ConfigManifestTest.php

[oddnoc]

Hi guys, it's great to see this getting some attention. I've just arrived in France and my permanent Internet connection has not been set up yet, but I'd be glad to help out once I'm not working intermittently out of cafés :)