feat: guard Deployer singleton before Host construction#4189
feat: guard Deployer singleton before Host construction#4189antonmedv merged 3 commits intodeployphp:masterfrom
Conversation
|
Additional info/motivation of this PR: I came across the issue when implementing setDefaultSelector as thin typed wrapper for set('default_selector', ...) (which will be a subsequent PR). |
src/Host/Host.php
Outdated
| $parent = null; | ||
| if (Deployer::get()) { | ||
| if (Deployer::hasInstance()) { | ||
| $parent = Deployer::get()->config; |
There was a problem hiding this comment.
What a use case where we create a Host instance before creating Deployer?
There was a problem hiding this comment.
Maybe we should throw an error here instead?
There was a problem hiding this comment.
What a use case where we create a Host instance before creating Deployer?
I ran into an error when trying to implement a thin typed wrapper for set('default_selector', ...), the wip implementation is here:
master...m1rm:deployer:feat/add-setDefaultSelector-function
maybe I also took a wrong turn, I'd be fine with throwing an error if that is more appropriate
There was a problem hiding this comment.
I think so. Can't imagine why host without deployer may be needed.
Also please check phpstan errors.
…('Deployer is not initialized.') when the singleton is missing, so the constructor now always uses the deployer config as the parent and relies on that throw (no separate if (!Deployer::hasInstance()) block needed
…t due to refactoring
Bug fix #…?Background
Host::__constructusedDeployer::get()to decide whether to inherit the global recipe config. With a typed, uninitialized static$instance, PHP 8+ throws when get() reads the property before new Deployer(...) has run. That can break HostTest (and any code path that builds a Host without a bootstrapped singleton).Solution
Deployer
Host
Always wire the parent Configuration from Deployer::get()->config. If Deployer is not initialized, get() throws — there is no “host without deployer” path anymore (as discussed in review).
Tests
Tooling
Behavior / compatibility
New
new Host(...) without a prior Deployer now always fails fast with that same initialization error instead of silently using a Configuration with no deployer parent.