Calling `set` inside of tasks with closure

[UlrichThomasGabor]

• Deployer version: 7.0.0-rc3
• Deployment OS: macOS

Overwriting variables in tasks does not seem to work.

I'd like to build a custom-deploy task (similar to the existing push), but I need to call deploy:shared and deploy:writable. As I am not really building a release, the symlink release does not exist and thus the two tasks fail.

I wanted to correct the variable release_path in a prepare task so that I can re-use the existing tasks, but whatever configuration I assign the new closure to, it is dropped for the next task, see

<?php

namespace Deployer;

use Deployer\Task\Context;

require_once 'recipe/common.php';

host('staging')
    ->setHostname('staging')
    ->set('labels', ['stage' => 'staging'])
    ->set('deploy_path', '/var/www/staging.example.de/htdocs')
;

task('example', [
    'prepare',
    'do',
]);

Deployer::get()->config->offsetUnset('release_path');

task('prepare', function () {
    $releasePathFn = function () {
        $link = run("readlink " . get('release_or_current_path'));
        return substr($link, 0, 1) === '/' ? $link : get('deploy_path') . '/' . $link;
    };
    Deployer::get()->config->set('release_path', $releasePathFn);
    Context::get()->getConfig()->set('release_path', $releasePathFn);
    currentHost()->config()->set('release_path', $releasePathFn);
});

task('do', function () {
    echo 'Current release_path: ' . get('release_path') . "\n";


    $releasePathFn = function () {
        $link = run("readlink " . get('release_or_current_path'));
        return substr($link, 0, 1) === '/' ? $link : get('deploy_path') . '/' . $link;
    };
    Deployer::get()->config->set('release_path', $releasePathFn);
    Context::get()->getConfig()->set('release_path', $releasePathFn);
    currentHost()->config()->set('release_path', $releasePathFn);
    echo 'Current release_path: ' . get('release_path') . "\n";
});

and the output

./vendor/bin/dep example staging
task prepare                                                                                                                                                                                                                                                                  
  ⠙ 
task deploy:shared
task do
  ⠹ 
Current release_path: 
Current release_path: /var/www/staging.example.de/htdocs/releases/43

Already the first call to release_path should work in my opinion.

If the do task is invoked directly from the prepare task, it works:

<?php
task('prepare', function () {
    $releasePathFn = function () {
        $link = run("readlink " . get('release_or_current_path'));
        return substr($link, 0, 1) === '/' ? $link : get('deploy_path') . '/' . $link;
    };
    Deployer::get()->config->set('release_path', $releasePathFn);
    Context::get()->getConfig()->set('release_path', $releasePathFn);
    currentHost()->config()->set('release_path', $releasePathFn);
    invoke('do');
});

and the output

./vendor/bin/dep example staging
task prepare                                                                                                                                                                                                                                                                  
  ⠦ 
Current release_path: /var/www/staging.example.de/htdocs/releases/43
Current release_path: /var/www/staging.example.de/htdocs/releases/43

I am not sure what is going on. It seems all configuration objects are the same (at least spl_object_id says so), but nevertheless, changes do not persist switching of tasks. I was not able to determine if this is the expected behavior or not...

I guess it has something to do with the building of the tasks here and the constructed chain of closures: https://github.com/deployphp/deployer/blob/master/src/Importer/Importer.php#L121-L230 But I am not sure what is going on there and with the extract and the goto this is not really easy to follow...

Possible mitigations coming to my mind:

1. Fix it, so that my example works, or
2. prevent calling set inside of tasks to avoid silent faults, if this is not intended to work anyway (are there like task-local variables?), or
3. if tasks should be independent from each other, why are they not, when they are called directly with invoke?

If one can clarify what SHOULD happen and/or how to fix one or the other issue, I am happy to provide a pull request.

[antonmedv]

Hi,

You can modify host config via set() call, but only to a JSON seraizable value. Not closures.

Reason for this is what tasks executed in separate php process, and updated values have to be updated. It’s done via http request to the master process. Obviously you can’t send closure.

I think Deployer should emit an error in this case saying what only JSON types are allowed.

[UlrichThomasGabor]

Hm, but the original value was also a closure? https://github.com/deployphp/deployer/blob/e3e1dace98b2cde30bb515281fbd9be123f2775f/recipe/deploy/release.php#L58-L66 And even if I set it on the main Deployer object, it is not propagated down?

[antonmedv]

It’s possible to set a closure to configs. But not inside tasks.

[UlrichThomasGabor]

And how can we decide inside set if this is still early stage and we are capable of accepting a closure, or later stage where we should throw a warning/exception when something non-serializable is given as parameter?

[antonmedv]

I didn’t get the question.

[UlrichThomasGabor]

If it's sometimes (outside of tasks) ok to pass a closure to set, but sometimes it is not (inside of tasks), how to decide in set when to throw a warning if the given value is not serializable?

[antonmedv]

It's not possible to set up closure inside tasks for technical reasons.

Please, provide code example and we can think on how to rewrite it without using set/closure pair.,

[UlrichThomasGabor]

I already made it work. Question is, what to do so nobody else drops into this hole. You said

I think Deployer should emit an error in this case saying what only JSON types are allowed.

so how to achieve that?

[antonmedv]

There should be a check in set(): if Context::has() is true -> show a warning.

[UlrichThomasGabor]

Isn't it possible to just evaluate a given closure in set() if Context::has() is true and assign the result of the closure to the configuration instead? May be better than a warning and works (at least in my case) as expected

[antonmedv]

Let me think about it.