Sync package composer repository on install#3276
Conversation
| $json_manipulator->addConfigSetting( 'secure-http', true ); | ||
|
|
||
| // If the composer file does not contain the current package index url, refresh the repository definition. | ||
| if ( false === strpos( $composer_backup, self::PACKAGE_INDEX_URL ) ) { |
There was a problem hiding this comment.
This is a bit too blunt.
We should specifically check the repository definition, and only replace http://wp-cli.org/package-index/
|
@aaemnnosttv Can you add a functional test case for this? |
|
I was thinking about that, but I'm not sure what the best way to handle that would be. I would need to modify the packages |
You could modify the composer.json with |
|
Hey Daniel, see what you think about the test I added. It's a little more verbose with the scripting in the scenario, but it works. You have any ideas as to how to clean that up, or you think it's ok? |
| """ | ||
| When I run `rm -rf /tmp/wp-cli-package-install-test/ && mkdir /tmp/wp-cli-package-install-test/ && mv composer.json /tmp/wp-cli-package-install-test/` | ||
| Then the /tmp/wp-cli-package-install-test/composer.json file should exist | ||
| When I run `WP_CLI_PACKAGES_DIR=/tmp/wp-cli-package-install-test/ wp --info` |
There was a problem hiding this comment.
Instead of depending on /tmp/, can't we just put WP_CLI_PACKAGES_DIR in a directory relative to the test directory? This way, we don't have to clean up the temp directory at the end of the test run either.
You're headed in the right direction! I've left some comments. |
|
Not sure why this failed...
Retry? |
Do the tests pass locally? |
|
All of the assertions of the test are passing, but it's just hanging there at the end, which is what happened on Travis too. Really strange.. any idea why it would be locked up like that? I've tried it a bunch of different ways but it keeps getting stuck at the end and I have to
Changing the package directory to a sub directory doesn't change anything either. You seen anything like that before? |
|
Have you verified the installation process completes successfully? |
| """ | ||
| WP-CLI packages dir: . | ||
| """ | ||
| When I run `WP_CLI_PACKAGES_DIR=. wp package install runcommand/hook` |
There was a problem hiding this comment.
Can we include an assertion that the "Updating package index repository URL" string is a part of the output?
- removed the subdirectory for the packages/composer.json which caused the test to hang indefinitely. - setup a local dummy package to “install” instead of the real one making the test a bit faster as well.
|
Success! |
|
Anything more here before we can merge this one? |
Right now, older installs of WP-CLI error when trying to install a new package due to the package index url not being updated from http to https. This fails when composer goes to query the package index over http, since
secure_httpis being set to true (I believe this is also the default now). Some users may not know how to correct this manually, and they shouldn't really have to.This PR performs a simple check when preparing to install a package. If the current package index isn't found in the composer config file, then it will refresh the definition for the package index repository.
fixes #3275