core download, --path option ignored

The --path option of the command wp core download is not taken into consideration during the copy of the files

https://github.com/wp-cli/wp-cli/blob/master/php/commands/core.php#L20

    $docroot = $assoc_args['path'];

https://github.com/wp-cli/wp-cli/blob/master/php/commands/core.php#L40

    WP_CLI::launch( 'cp -r wordpress/* . && rm -rf wordpress' );

Should be

    WP_CLI::launch( 'cp -r wordpress/* ' . $docroot . ' && rm -rf wordpress' );

Furthermore, if the $docroot does not exist, try to create it ;)

Nice catch. Care to transform it into a pull request, so that you show up on the contributors page? :)

Unfortunately, I do not have the time to switch to my dev machine to do so

Well, it's worse than expected :p

The --path option is not even in the $assoc_args

I've put a var_dump($assoc_args) to see it's content

wp core download --path=wordpress351 --version=3.5.1
array(1) {
  ["version"]=>
  string(5) "3.5.1"
}

So, there is something wrong somewhere else :p

--path is a global parameter, which is handled earlier:

https://github.com/wp-cli/wp-cli/blob/master/php/WP_CLI/Runner.php#L225

So, the correct fix would be to use ABSPATH, instead of checking $assoc_args['path'].

That leads to another issue :

var_dump(ABSPATH);

wp core download --path=wordpress351 --version=3.5.1
string(13) "wordpress351/"

wp core download  --version=3.5.1
string(35) "/home/tests/wp-cli/test-core/"

Ok, so what's the issue?

w/o the --path option, ABSPATH is an absolute path - no problem with that

however, with the option, it's a relative one

Yeah, I guess it should always be absolute, since that's why it's called ABSPATH. :)

That's a bug in set_wp_root() then.

private static function set_wp_root( $config ) {
        if ( !empty( $config['path'] ) ) {
            define( 'ABSPATH', rtrim( $config['path'], '/' ) . '/' );
        } else {
            define( 'ABSPATH', getcwd() . '/' );
        }
    }

Should be something like

private static function set_wp_root( $config ) {
        $path = getcwd() . '/';

        if ( !empty( $config['path'] ) )
            $path .= rtrim( $config['path'], '/' ) . '/';

        define( 'ABSPATH', $path );
    }

However, as there is not test for the Runner, I do not want to break something :D

Actually, we have functional tests run via Behat, but somehow they don't catch this issue:

https://github.com/wp-cli/wp-cli/blob/master/features/bootstrap/FeatureContext.php#L176-186

@erwanlr

Going to handle this in #327. Thanks for reporting, @erwanlr.

I'm currently fixing it :p

Will send a pull request in a few minutes

Ref wp-cli#320, fixes the ABSPATH issue

Fixes wp-cli#320 --path option ignored in core download

Navigation Menu

Repository navigation

core download, --path option ignored #320

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

core download, --path option ignored #320

Description

Activity

scribu commented on Feb 25, 2013

erwanlr commented on Feb 25, 2013

erwanlr commented on Feb 26, 2013

scribu commented on Feb 26, 2013

erwanlr commented on Feb 26, 2013

scribu commented on Feb 26, 2013

erwanlr commented on Feb 26, 2013

scribu commented on Feb 26, 2013

erwanlr commented on Feb 26, 2013

scribu commented on Feb 26, 2013

scribu commented on Feb 26, 2013

erwanlr commented on Feb 26, 2013

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions