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

Incompatibility with a WP Composer installation #13497

Closed
LeoColomb opened this issue Sep 19, 2019 · 14 comments · Fixed by #14900
Closed

Incompatibility with a WP Composer installation #13497

LeoColomb opened this issue Sep 19, 2019 · 14 comments · Fixed by #14900
Labels
[Focus] Jetpack DNA [Pri] Normal [Type] Bug When a feature is broken and / or not performing as intended

Comments

@LeoColomb
Copy link
Contributor

LeoColomb commented Sep 19, 2019

Context

Output

PHP Fatal error:  Uncaught Error: Call to undefined function add_action() in /var/www/example.com/vendor/automattic/jetpack-compat/functions.php:23
Stack trace:
#0 /var/www/example.com/vendor/composer/autoload_real.php(66): require()
#1 /var/www/example.com/vendor/composer/autoload_real.php(56): composerRequire9e2e0155ae2296865e0a62cb9575b86b('009de6aaa0d497e...', '/var/www/...')
#2 /var/www/example.com/vendor/autoload.php(7): ComposerAutoloaderInit9e2e0155ae2296865e0a62cb9575b86b::getLoader()
#3 phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php(1198) : eval()'d code(6): require_once('/var/www/stagin...')
#4 phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php(1198): eval()
#5 phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php(1159): WP_CLI\Runner->load_wordpress()
#6 phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Bootstrap/LaunchRunner in /var/www/example.com/vendor/automattic/jetpack-compat/functions.php on line 23

Notes

Seems very related to the jetpack-compat package.

@LeoColomb LeoColomb changed the title Incompatibility with WP CLI and composer Incompatibility with WP CLI and Composer install Sep 19, 2019
@jeherve jeherve added [Focus] Jetpack DNA [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Sep 20, 2019
@jeherve
Copy link
Member

jeherve commented Sep 20, 2019

Could you tell me more about your setup? add_action should be available since it's added by WordPress. Is WordPress loaded properly in your instance?

@LeoColomb
Copy link
Contributor Author

LeoColomb commented Sep 20, 2019

Most simple setup:

  1. Download the latest Roots Bedrock stack.
  2. Simplify the composer (and delete the lock file)
{
  "repositories": [
    {
      "type": "composer",
      "url": "https://wpackagist.org"
    }
  ],
  "require": {
    "php": ">=7.1",
    "automattic/jetpack": "^7.0",
    "composer/installers": "^1.7",
    "vlucas/phpdotenv": "^3.4.0",
    "oscarotero/env": "^1.2.0",
    "roots/wordpress": "5.2.3",
    "roots/wp-config": "1.0.0"
  },
  "extra": {
    "installer-paths": {
      "web/app/plugins/{$name}/": ["type:wordpress-plugin"]
    },
    "wordpress-install-dir": "web/wp"
  }
}
  1. Run the following Dockerfile at the root of the Bedrock path.
FROM composer:latest as vendor
COPY composer.* ./
COPY web ./web
RUN composer install \
    --ignore-platform-reqs \
    --no-interaction

FROM wordpress:cli
COPY . .
COPY --from=vendor /app/vendor ./vendor
COPY --from=vendor /app/web ./web
RUN wp plugin list
docker build .
Sending build context to Docker daemon  50.18kB                                                                         Step 1/9 : FROM composer:latest as vendor
 ---> 26c32bc78314
Step 2/9 : COPY composer.* ./
 ---> Using cache
 ---> 7b8eea13f888
Step 3/9 : COPY web ./web
 ---> Using cache
 ---> 6433a9a8d710
Step 4/9 : RUN composer install     --ignore-platform-reqs     --no-interaction
 ---> Running in 2f9442c55867
Loading composer repositories with package information
Updating dependencies (including require-dev)
Package operations: 18 installs, 0 updates, 0 removals
  - Installing composer/installers (v1.7.0): Downloading (100%)
  - Installing roots/wordpress-core-installer (1.1.0): Downloading (100%)
  - Installing automattic/jetpack-autoloader (v1.3.1): Downloading (100%)
  - Installing roots/wordpress (5.2.3): Downloading (100%)
  - Installing automattic/jetpack-compat (v1.0.2): Downloading (100%)
  - Installing automattic/jetpack-constants (v1.1.1): Downloading (100%)
  - Installing automattic/jetpack-options (v1.1.0): Downloading (100%)
  - Installing automattic/jetpack-tracking (v1.0.1): Downloading (100%)
  - Installing automattic/jetpack-status (v1.0.1): Downloading (100%)
  - Installing automattic/jetpack-roles (v1.0.1): Downloading (100%)
  - Installing automattic/jetpack-connection (v1.0.1): Downloading (100%)
  - Installing automattic/jetpack-sync (v1.0.1): Downloading (100%)
  - Installing automattic/jetpack-assets (v1.0.0): Downloading (100%)
  - Installing automattic/jetpack-logo (v1.1.1): Downloading (100%)
  - Installing automattic/jetpack-jitm (v1.0.0): Downloading (100%)
  - Installing automattic/jetpack-error (v1.0.1): Downloading (100%)
  - Installing automattic/jetpack-abtest (v1.0.0): Downloading (100%)
  - Installing automattic/jetpack (7.7.1): Downloading (100%)
Writing lock file
Generating autoload files
Generated /app/vendor/composer/autoload_classmap_package.php
Generated /app/vendor/autoload_packages.php
Removing intermediate container 2f9442c55867
 ---> 93ad0f5b16e1
Step 5/9 : FROM wordpress:cli
 ---> 660faee1fafd
Step 6/9 : COPY . .
 ---> 20e5efd5928d
Step 7/9 : COPY --from=vendor /app/vendor ./vendor
 ---> 228879a81600
Step 8/9 : COPY --from=vendor /app/web ./web
 ---> 2553e5997781
Step 9/9 : RUN wp plugin list
 ---> Running in eba26fa94311
[20-Sep-2019 14:00:04 UTC] PHP Fatal error:  Uncaught Error: Call to undefined function add_action() in /var/www/html/vendor/automattic/jetpack-compat/functions.php:23
Stack trace:
#0 /var/www/html/vendor/composer/autoload_real.php(66): require()
#1 /var/www/html/vendor/composer/autoload_real.php(56): composerRequire61340a303421db65a19e95c2ee686b1f('009de6aaa0d497e...', '/var/www/html/v...')
#2 /var/www/html/vendor/autoload.php(7): ComposerAutoloaderInit61340a303421db65a19e95c2ee686b1f::getLoader()
#3 phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php(1198) : eval()'d code(6): require_once('/var/www/html/v...')
#4 phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php(1198): eval()
#5 phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php(1159): WP_CLI\Runner->load_wordpress()
#6 phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Bootstrap/LaunchRunner.php(23): WP_CLI\Runner->start()      #7 phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/bootstrap.php(74): WP_CLI\Bootstrap\LaunchRunner->process(Object(WP_CL in /var/www/html/vendor/automattic/jetpack-compat/functions.php on line 23
The command '/bin/sh -c wp plugin list' returned a non-zero code: 255

@jeherve
Copy link
Member

jeherve commented Sep 20, 2019

Thanks. You're most likely running into this issue:
https://discourse.roots.io/t/bedrock-composer-autoloader-and-wordpress/4343

Ideally you wouldn't be copying the Jetpack plugin into /var/www/html/vendor/ with other dependencies, since it's a plugin. It should really be sourced from /web/app/plugins/, with the other plugins.

Bedrock seems to support this out of the box, thanks to the wordpress-plugin type that you can find here:

"type": "wordpress-plugin",

That is why the Jetpack plugin properly gets copied to the /web/app/plugins/ directory when you run composer install instead of using your custom Dockerfile.

Unfortunately in your set up we don't get to that step, as it fails before WordPress can be installed, as all of Jetpack's packages got autoloaded from the root of your project instead (/var/www/html/vendor/autoload.php) of being autoloaded from Jetpack itself here:

require $jetpack_autoloader;

Could you try to move plugins out of the root's vendor directory when you initialize your setup, so they can only be found as plugins in the plugins directory?

@LeoColomb
Copy link
Contributor Author

LeoColomb commented Sep 20, 2019

Thanks for your reply.

You're most likely running into this issue:
discourse.roots.io/t/bedrock-composer-autoloader-and-wordpress/4343

That's a 4 yo question, I'm speaking about a recent issue raised directly with the 7.5 release of Jetpack.
I deploy "my" stack about 2/3 times a day, automatically, at each dependency update, and it was working very well before 7.5 (when more packages were added to Jetpack' composer file).

That is why the Jetpack plugin properly gets copied to the /web/app/plugins/ directory when you run composer install instead of using your custom Dockerfile.

I'm not sure to follow you... I do run a composer install.
I write a Dockerfile to let anyone replicates with ease, but that's exactly the same, it do run composer install.

Could you try to move plugins out of the root's vendor directory when you initialize your setup, so they can only be found as plugins in the plugins directory?

That is already done by the composer/installers package. When building the Dockerfile, jetpack is in the plugin directory.

@jeherve
Copy link
Member

jeherve commented Sep 20, 2019

When building the Dockerfile, jetpack is in the plugin directory.

Right, but all Jetpack packages (those were added in Jetpack 7.5) can also be found in the root's vendor directory, and they are loaded by the autoloader there instead of only being loaded by the autoloader in the Jetpack plugin, once WordPress has already been loaded.

That is why you run into that 4 year-old issue I linked to earlier: you end up using add_action() before WordPress gets loaded.

That is already done by the composer/installers package. When building the Dockerfile, jetpack is in the plugin directory.

Excellent. That would mean I am wrong; it's not the Jetpack plugin that's the problem, but the other packages required by the Jetpack plugin.

Maybe since those are not WordPress plugins, they do not get moved?

When building the Dockerfile, jetpack is in the plugin directory.

Do you see a vendor directory with all packages inside the Jetpack plugin there?

@LeoColomb
Copy link
Contributor Author

@jeherve Thanks for your answer.

Do you see a vendor directory with all packages inside the Jetpack plugin there?

Yes, I do:

$ find /var/www/html/web/app/plugins/jetpack/vendor/ -maxdepth 2 -type d -exec ls -ld "{}" \;
drwxr-xr-x    4 root     root          4096 Sep 21 09:03 /var/www/html/web/app/plugins/jetpack/vendor/
drwxr-xr-x   15 root     root          4096 Sep 21 09:03 /var/www/html/web/app/plugins/jetpack/vendor/automattic
drwxr-xr-x    4 root     root          4096 Sep 21 09:03 /var/www/html/web/app/plugins/jetpack/vendor/automattic/jetpack-compat
drwxr-xr-x    3 root     root          4096 Sep 21 09:03 /var/www/html/web/app/plugins/jetpack/vendor/automattic/jetpack-options
drwxr-xr-x    3 root     root          4096 Sep 21 09:03 /var/www/html/web/app/plugins/jetpack/vendor/automattic/jetpack-assets
drwxr-xr-x    3 root     root          4096 Sep 21 09:03 /var/www/html/web/app/plugins/jetpack/vendor/automattic/jetpack-roles
drwxr-xr-x    4 root     root          4096 Sep 21 09:03 /var/www/html/web/app/plugins/jetpack/vendor/automattic/jetpack-tracking
drwxr-xr-x    3 root     root          4096 Sep 21 09:03 /var/www/html/web/app/plugins/jetpack/vendor/automattic/jetpack-status
drwxr-xr-x    3 root     root          4096 Sep 21 09:03 /var/www/html/web/app/plugins/jetpack/vendor/automattic/jetpack-sync
drwxr-xr-x    3 root     root          4096 Sep 21 09:03 /var/www/html/web/app/plugins/jetpack/vendor/automattic/jetpack-error
drwxr-xr-x    3 root     root          4096 Sep 21 09:03 /var/www/html/web/app/plugins/jetpack/vendor/automattic/jetpack-abtest
drwxr-xr-x    3 root     root          4096 Sep 21 09:03 /var/www/html/web/app/plugins/jetpack/vendor/automattic/jetpack-constants
drwxr-xr-x    4 root     root          4096 Sep 21 09:03 /var/www/html/web/app/plugins/jetpack/vendor/automattic/jetpack-jitm
drwxr-xr-x    3 root     root          4096 Sep 21 09:03 /var/www/html/web/app/plugins/jetpack/vendor/automattic/jetpack-logo
drwxr-xr-x    4 root     root          4096 Sep 21 09:03 /var/www/html/web/app/plugins/jetpack/vendor/automattic/jetpack-connection
drwxr-xr-x    2 root     root          4096 Sep 21 09:03 /var/www/html/web/app/plugins/jetpack/vendor/composer

I think the main issue is declaring a autoload.files in the composer file, since this asks composer to explicitly require the files.

https://github.com/Automattic/jetpack-compat/blob/a9af3164b4dfdcaa6b2209f14de129b199db19e4/composer.json#L13

@LeoColomb
Copy link
Contributor Author

LeoColomb commented Sep 21, 2019

Do you see a vendor directory with all packages inside the Jetpack plugin there?

BTW note these files are present here because Jetpack GitHub releases are made on built branches where vendor files are present. They are packed in the downloaded release zip file.

In a standard way to use composer, vendor files are kept outside of the parent package.

@LeoColomb LeoColomb changed the title Incompatibility with WP CLI and Composer install Incompatibility with a WP Composer installation Sep 21, 2019
@jeherve
Copy link
Member

jeherve commented Sep 23, 2019

I think the main issue is declaring a autoload.files in the composer file, since this asks composer to explicitly require the files.

Right. That's to expected as the file is needed. The problem here is that those packages are typically only loaded after WordPress has been loaded. In your case that's not what happens, since the packages are loaded right away from the main autoloader of your app.

In a standard way to use composer, vendor files are kept outside of the parent package.

Right, since we are in a WordPress plugin things are a bit different from a more typical setup.

@jeherve jeherve added [Pri] Normal [Type] Bug When a feature is broken and / or not performing as intended and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Sep 23, 2019
@LeoColomb
Copy link
Contributor Author

LeoColomb commented Sep 23, 2019

@jeherve Thanks for your reply.

That's to expected as the file is needed.

For sure. But there are tons of ways to include necessary files. Also this file seems to load deprecated features, so are we sure we do want to load it in any case?

Right, since we are in a WordPress plugin things are a bit different

Well, not necessary! 🙂
I mean, you may use GitHub release assets to publish the fully build package for WordPress, or use WP SVN for release directly.
See, for eg https://github.com/10up/action-wordpress-plugin-deploy used by https://github.com/10up/restricted-site-access or https://github.com/LeoColomb/wp-auto-links
Before release, the script build everything needed and deploy to WP SVN for a standard WP plugin tag.

As you have already the script to "build", doing this should be quite simple.

That said this is a different issue, do you want I open another one to discuss about this?

@jeherve
Copy link
Member

jeherve commented Sep 23, 2019

are we sure we do want to load it in any case?

Yep, that file is needed for Jetpack to work properly.

you may use GitHub release assets to publish the fully build package for WordPress, or use WP SVN for release directly.

For sure. We've started playing with actions on our end as well, and may switch to using them in the future. Regardless, the final version of Jetpack that gets shipped to SVN will always ship with the packages defined in composer.json, so you'll always get /vendor package with your version of Jetpack, unless you were to require Jetpack master instead of requiring a stable version.

Back to that original issue, we'll see what we can do to address this, since #12923 and #12943 weren't enough to catch everything.

@LeoColomb
Copy link
Contributor Author

LeoColomb commented Sep 28, 2019

SVN will always ship with the packages defined in composer.json

Sure. 100% right.

so you'll always get /vendor

Shouldn't, since I get the Jetapck plugin/package from Packagist.

we'll see what we can do to address this

As long as you include the /vendor dir in GitHub release archives (used by Packagist), you can simply remove lines in the composer.json where Jetpack's composer packages are defined and drop the composer.lock.
Simple to implement, impacts no one.

@JPry
Copy link

JPry commented Dec 10, 2019

I bumped into this issue when trying to set up a site using WP Starter. I think the problem is that Jetpack's composer.json file is requiring all of the extensions. While the main jetpack repo is successfully copied into the wp-content directory instead of vendor, all of the dependencies are left in vendor and subject to the Composer autoloader.

jetpack/composer.json

Lines 14 to 28 in 3a0874e

"automattic/jetpack-abtest": "@dev",
"automattic/jetpack-assets": "@dev",
"automattic/jetpack-autoloader": "@dev",
"automattic/jetpack-backup": "@dev",
"automattic/jetpack-compat": "@dev",
"automattic/jetpack-connection": "@dev",
"automattic/jetpack-constants": "@dev",
"automattic/jetpack-error": "@dev",
"automattic/jetpack-jitm": "@dev",
"automattic/jetpack-logo": "@dev",
"automattic/jetpack-options": "@dev",
"automattic/jetpack-roles": "@dev",
"automattic/jetpack-sync": "@dev",
"automattic/jetpack-terms-of-service": "@dev",
"automattic/jetpack-tracking": "@dev"

This was the stack trace:

[10-Dec-2019 16:44:36 America/New_York] PHP Fatal error:  Uncaught Error: Call to undefined function add_action() in /Users/jpry/Sites/wpcomposer/content/vendor/automattic/jetpack-backup/actions.php:13
Stack trace:
#0 /Users/jpry/Sites/wpcomposer/content/vendor/composer/autoload_real.php(66): require()
#1 /Users/jpry/Sites/wpcomposer/content/vendor/composer/autoload_real.php(56): composerRequireada658bd793edd691f5fb0952e184cd8('d4eb94df91a7298...', '/Users/jpry/Sit...')
#2 /Users/jpry/Sites/wpcomposer/content/vendor/autoload.php(7): ComposerAutoloaderInitada658bd793edd691f5fb0952e184cd8::getLoader()
#3 /Users/jpry/Sites/wpcomposer/wp-config.php(16): require_once('/Users/jpry/Sit...')
#4 /Users/jpry/Sites/wpcomposer/wp/wp-load.php(42): require_once('/Users/jpry/Sit...')
#5 /Users/jpry/Sites/wpcomposer/wp/wp-blog-header.php(13): require_once('/Users/jpry/Sit...')
#6 /Users/jpry/Sites/wpcomposer/index.php(17): require('/Users/jpry/Sit...')
#7 /Users/jpry/.composer/vendor/weprovide/valet-plus/server.php(131): require('/Users/jpry/Sit...')
#8 {main}
  thrown in /Users/jpry/Sites/wpcomposer/content/vendor/automattic/jetpack-backup/actions.php on line 13

I can confirm that version 7.5 seems to have been the breaking point.

If it would be helpful to provide my project composer.json I can do that.

@LeoColomb
Copy link
Contributor Author

As long as you include the /vendor dir in GitHub release archives (used by Packagist), you can simply remove lines in the composer.json where Jetpack's composer packages are defined and drop the composer.lock.
Simple to implement, impacts no one.

@jeherve Is it worth opening a PR with my suggestion?

@jeherve
Copy link
Member

jeherve commented Mar 5, 2020

For sure. I think that's something that could be incorporated to the script we use to generate the built branches that are tagged and released on GitHub. This happens here:

This way, we could edit composer.json right after it's been used to install all packages. By then, and as you mentioned, we may not need the list of composer dependencies anymore.

jeherve pushed a commit that referenced this issue Mar 18, 2020
* Remove composer dependencies listing for built branches

Ref: #13497

* Improve removing composer packages listing script

Simplifying by using composer regexp capability

* Cleaning up the composer edition in the build script
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Jetpack DNA [Pri] Normal [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants