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

Permit defining abspath before WP-CLI is loaded #69

Closed
janw-me opened this Issue Nov 28, 2017 · 7 comments

Comments

3 participants
@janw-me

janw-me commented Nov 28, 2017

If ABSPATH is defined before wp-cli loads it will throw a notice.

Example.
wp-cli.yaml

require:
     - abspath.php

abspath.php

<?php
if ( ! defined( 'ABSPATH' ) ) {
    define( 'ABSPATH', dirname( __FILE__ ) . '/pubic_html/wp/' );
}

Now if you run any wp-cli it will throw a notice that ABSPATH is already defined.

When would you want this?
In short symlinks, if you include WordPress in a symlink.

WordPress tries to define the ABSPATH it uses wp-load.php and if it's in a symlink it will define the path baed on the actual location. Not the symlink path.
That will give a problem if the wp-config.php is one directory above wp-load.php

The file structure of this with my example to help visualize this.

  • ./public_html/index.php < the webroot
  • ./public_html/wp-config.php
  • ./public_html/wp < symlink to /opt/wordpress/
  • ./public_html/wp/wp-load.php < here the ABSPATH is based on

ABSPATH will be /opt/wordpress/, but you would want it to be ./public_html/wp/ so wp-load will look for ./public_html/wp-config.php

The fix.
Just add a check if ABSPATH is already defined. I have the fix ready in my fork

@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Nov 28, 2017

Member

Previously:

While it seems easy enough to make this change, WP-CLI has historically only officially supported WordPress install configurations that you can also achieve through the web interface. Introducing this check implies we support every potential installation configuration, which is a rabbit hole I'm not keen to enter.

From wp-cli/wp-cli#4054 (comment)

Member

danielbachhuber commented Nov 28, 2017

Previously:

While it seems easy enough to make this change, WP-CLI has historically only officially supported WordPress install configurations that you can also achieve through the web interface. Introducing this check implies we support every potential installation configuration, which is a rabbit hole I'm not keen to enter.

From wp-cli/wp-cli#4054 (comment)

@janw-me

This comment has been minimized.

Show comment
Hide comment
@janw-me

janw-me Nov 28, 2017

This kind of setup can be achieved through the web interface. If you check wp-load.php:20 This check is the first thing in the file.

So when for example I go to the wp-login the first thing it does is include wp-load.php. Which tries to define the ABSPATH.

But where is this alternative ABSPATH defined for the web interface you ask.
I let the webserver do an auto-prepend-file of the abspath.php

I have this setup running for a week in test mode and in the web interface I haven't encountered problems.
I wouldn't say I've tested every little thing. But the things I have tested include. uploads, rest api, wp-admin, login, crud of posts/pages/taxonomies, installing plugin's, theme's.

Are there specific things that come to mind that worry you with this setup?
Or might there be extra checks that might help to prevent problems/warn users.

Maybe add an extra debug message when ABSPATH is defined in this kind of way.

janw-me commented Nov 28, 2017

This kind of setup can be achieved through the web interface. If you check wp-load.php:20 This check is the first thing in the file.

So when for example I go to the wp-login the first thing it does is include wp-load.php. Which tries to define the ABSPATH.

But where is this alternative ABSPATH defined for the web interface you ask.
I let the webserver do an auto-prepend-file of the abspath.php

I have this setup running for a week in test mode and in the web interface I haven't encountered problems.
I wouldn't say I've tested every little thing. But the things I have tested include. uploads, rest api, wp-admin, login, crud of posts/pages/taxonomies, installing plugin's, theme's.

Are there specific things that come to mind that worry you with this setup?
Or might there be extra checks that might help to prevent problems/warn users.

Maybe add an extra debug message when ABSPATH is defined in this kind of way.

@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Nov 28, 2017

Member

So when for example I go to the wp-login the first thing it does is include wp-load.php. Which tries to define the ABSPATH.

Oh, huh:

image

Maybe WordPress does support ABSPATH defined before PHP execution: WordPress/WordPress@4074bcc

https://core.trac.wordpress.org/ticket/26592

Are there specific things that come to mind that worry you with this setup?

More so: unintended consequences. If we permit this and it causes problems X, Y and Z down then line, then I'd rather not let this happen in the first place.

But, given WordPress supports it, I'm open to it. Want to submit a pull request with functional tests?

Member

danielbachhuber commented Nov 28, 2017

So when for example I go to the wp-login the first thing it does is include wp-load.php. Which tries to define the ABSPATH.

Oh, huh:

image

Maybe WordPress does support ABSPATH defined before PHP execution: WordPress/WordPress@4074bcc

https://core.trac.wordpress.org/ticket/26592

Are there specific things that come to mind that worry you with this setup?

More so: unintended consequences. If we permit this and it causes problems X, Y and Z down then line, then I'd rather not let this happen in the first place.

But, given WordPress supports it, I'm open to it. Want to submit a pull request with functional tests?

@janw-me

This comment has been minimized.

Show comment
Hide comment
@janw-me

janw-me Nov 29, 2017

Functional tests. Yes, this is new to me. I'll look into it. I'll expect between the holidays at last.

janw-me commented Nov 29, 2017

Functional tests. Yes, this is new to me. I'll look into it. I'll expect between the holidays at last.

@schlessera

This comment has been minimized.

Show comment
Hide comment
@schlessera

schlessera Nov 29, 2017

Member

Just let us know if you happen to get stuck with the tests.

Member

schlessera commented Nov 29, 2017

Just let us know if you happen to get stuck with the tests.

@danielbachhuber danielbachhuber changed the title from ABSPATH isn't checked if it is defined to Permit defining abspath before WP-CLI is loaded Dec 4, 2017

@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Dec 8, 2017

Member

Looking into this briefly, we'll also need to accommodate --path=<path>. Because --path=<path> is a way of defining the path to WordPress at runtime, we'll have somewhat ambiguous behavior if we also support definition of ABSPATH at runtime.

Member

danielbachhuber commented Dec 8, 2017

Looking into this briefly, we'll also need to accommodate --path=<path>. Because --path=<path> is a way of defining the path to WordPress at runtime, we'll have somewhat ambiguous behavior if we also support definition of ABSPATH at runtime.

@schlessera

This comment has been minimized.

Show comment
Hide comment
@schlessera

schlessera Mar 31, 2018

Member

Implemented via wp-cli/wp-cli#4743

Member

schlessera commented Mar 31, 2018

Implemented via wp-cli/wp-cli#4743

@schlessera schlessera closed this Mar 31, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment