Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cd (change directory) and within (run commands within a directory and revert to the previous workingPath) #180

Merged
merged 1 commit into from
Feb 15, 2015

Conversation

tomzx
Copy link
Contributor

@tomzx tomzx commented Feb 14, 2015

Here's my take on the cd function. I've also added a within function which allows the user to provide a callback:

<?php

task('test', function() {
    cd('my_dir');
    within('{some_dir}', function() {
        run('some command');
    });
    // back in my_dir
});

After the within function is called, the environment is reverted to the previous working path.

This current implementation only support going to an absolute path. We may consider changing the implementation to support relative navigation if that is something people would want (I don't think that is the case though).

Reference: #177.

@@ -140,6 +140,24 @@ function after($it, $that)
$afterScenario->addAfter($scenario);
}

function cd($path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add PhpDoc here, please.

@tomzx
Copy link
Contributor Author

tomzx commented Feb 14, 2015

I've forced pushed the changes you've asked. Please take the time to review them.

Thanks!

antonmedv pushed a commit that referenced this pull request Feb 15, 2015
Add cd (change directory) and within (run commands within a directory and revert to the previous workingPath)
@antonmedv antonmedv merged commit 230eb68 into deployphp:master Feb 15, 2015
@antonmedv
Copy link
Member

Done 🍺

@antonmedv
Copy link
Member

Hmmm.

It's not possible to write like this:

cd('{release_path}');

only like this:

cd(env('release_path'));

@tomzx Can you fix it?

@tomzx
Copy link
Contributor Author

tomzx commented Feb 15, 2015

@Elfet Pretty sure that should work (at least it did when I wrote the functionality). I'll pull the latest changes and see if that still works.

Edit: Works fine for me:

task('test', function () {
    run('pwd');

    within('{deploy_path}/current', function() {
        run('pwd');
    });

    cd('{deploy_path}');
    run('pwd');
    cd('{deploy_path}/shared');
    run('pwd');

    within('{deploy_path}/current', function() {
        run('pwd');
    });

    run('pwd');
});
➤ Executing task test
⤷ on [production]
Run: cd /var/www/test-target && pwd
# /var/www/test-target
# 
Run: cd /var/www/test-target/current && pwd
# /var/www/test-target/current
# 
Run: cd /var/www/test-target && pwd
# /var/www/test-target
# 
Run: cd /var/www/test-target/shared && pwd
# /var/www/test-target/shared
# 
Run: cd /var/www/test-target/current && pwd
# /var/www/test-target/current
# 
Run: cd /var/www/test-target/shared && pwd
# /var/www/test-target/shared
# 
⤶ done on [production]
✔ OK

@antonmedv
Copy link
Member

Hmmm, can you look at https://github.com/deployphp/deployer/blob/master/recipe/common.php#L134
Not working there :(

@tomzx
Copy link
Contributor Author

tomzx commented Feb 22, 2015

I assume you're getting this error: PHP Fatal error: Maximum function nesting level of '100' reached, aborting!?

It has to do with the fact that resolving the environment variable release_path will generate a recursive call.

env('release_path', function () {
    return str_replace("\n", '', run("readlink {deploy_path}/release"));
});

cd('{release_path}') -> workingPath() -> run("readlink {deploy_path}/release") -(cycle)-> workingPath() -> run("readlink {deploy_path}/release").

Basically, it tries to resolve release_path by trying using itself in the process, which ends up generating a loop.

One possible way to fix this might be to make the release_path and current environment variables strings instead, as such:

/**
 * Return release path.
 */
env('release_path', '{deploy_path}/release');

/**
 * Return current release path.
 */
env('current', '{deploy_path}/current');

Basically, any variable that would be used in the working_path is a closure/callable, a run cycle will occur. Thus, a probably better fix would be to resolve directly the path given to cd when it is called. This way, the working_path value is always a string with no environment variable that needs to be resolved.

function cd($path)
{
    env('working_path', env()->parse($path));
}

@antonmedv
Copy link
Member

Yes.

22 ôåâð. 2015 ã., â 21:32, tomzx notifications@github.com íàïèñàë(à):

I assume you're getting this error: PHP Fatal error: Maximum function nesting level of '100' reached, aborting!?


Reply to this email directly or view it on GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants