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

fix: Downgrade Docker image for WordPress #8427

Merged
merged 20 commits into from
Aug 3, 2018

Conversation

tofumatt
Copy link
Member

@tofumatt tofumatt commented Aug 3, 2018

Description

I've forced our Docker image to be WordPress 4.9.7. If anything, it's possible we want to stick to pinning versions; this sort of upset to our CI/dev environments because of upstream changes is not good.

Fixes #8418.

How has this been tested?

Run ./bin/setup-local-env.sh; it worked. Run npm run test-e2e; they work.

Testing instructions

  1. Run ./bin/setup-local-env.sh; it should output the "Gutenberg is ready" text.
  2. Run npm run test-e2e; the tests should run.

@tofumatt tofumatt requested a review from a team August 3, 2018 09:26
@tofumatt tofumatt changed the title fix: Downgrade Docker image for WordPress to fix #8418 fix: Downgrade Docker image for WordPress Aug 3, 2018
@sarahmonster
Copy link
Member

I broke it!

STATUS: Installing WordPress...
Success: WordPress installed successfully.
Updating to version 4.9.8 (en_US)...
Downloading update from https://downloads.wordpress.org/release/wordpress-4.9.8-no-content.zip...
Unpacking the update...
Error: Could not create directory.

Also, you may want to fix that CI build that's failing. 😜 🙃

@tofumatt
Copy link
Member Author

tofumatt commented Aug 3, 2018

BLARG, I'm so dumb. I had docker-compose run --rm $CLI core update >/dev/null commented out locally so it worked for me. Hmmm... maybe something is just up with 4.9.8? 🤔

@tofumatt
Copy link
Member Author

tofumatt commented Aug 3, 2018

I'm wondering if the permissions issue is related to 0b36845... testing now. If so I'll add a comment back to it and maybe it won't get deleted by me thinking it's not needed 😓

@notnownikki
Copy link
Member

I found an issue switching to php5.2, so deleted the cached build and now there's an issue building it.

Expect more 80s/90s lyrics themed debug commit messages from me. A lot more.

@aduth
Copy link
Member

aduth commented Aug 3, 2018

I'm wondering if the permissions issue is related to 0b36845...

But we no longer assign the -u flag in master?

@tofumatt
Copy link
Member Author

tofumatt commented Aug 3, 2018

But we no longer assign the -u flag in master?

Sorry, I wasn't clear. That was my point; maybe not assigning it is the issue.

And, in fact, if I supply it the update script totally works. I then run into an issue with the PHPUnit Docker:

STATUS: Attempting to connect to WordPress.....
STATUS: Installing WordPress...
Success: WordPress installed successfully.
Success: WordPress is up to date.
STATUS: Activating Gutenberg...
Plugin 'gutenberg' activated.
Success: Activated 1 of 1 plugins.
STATUS: Installing PHPUnit test scaffolding...
env: can't execute 'bash -x -v': No such file or directory

@notnownikki
Copy link
Member

env: can't execute 'bash -x -v': No such file or directory was me getting debug wrong. That's fixed now.

@@ -1,4 +1,4 @@
#!/bin/bash
#!/bin/bash -e -x
Copy link
Member Author

Choose a reason for hiding this comment

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

Can we use set for this instead? It breaks locally so makes things hard to test locally 🤷‍♂️

Copy link
Member

Choose a reason for hiding this comment

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

gone now, we're back to building and using php5.2 with the latest commit

@tofumatt
Copy link
Member Author

tofumatt commented Aug 3, 2018

Looks like the same test is still failing, even for 4.9.7: https://travis-ci.org/WordPress/gutenberg/jobs/411763137#L1106

😢

@notnownikki
Copy link
Member

Ok, I can reproduce this locally now, there's something going on with the docker path that's different to the non-docker path we use when we switch php versions.

@notnownikki
Copy link
Member

one of the images that runs phpunit has 4.9.8 on it.


nikki@tardis:~/code/test-4.9.7-g$ docker-compose run --rm wordpress_phpunit cat /tmp/wordpress/wp-includes/version.php
<?php
/**
 * The WordPress version string
 *
 * @global string $wp_version
 */
$wp_version = '4.9.8';

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

Honestly, I don't really understand the changes here, but I guess since we're trying to fix the e2e tests environment, the most important validation is that the tests pass. So I'm approving

@notnownikki notnownikki merged commit 0c12b15 into master Aug 3, 2018
@youknowriad youknowriad deleted the fix/8418-docker-image-writable branch August 3, 2018 16:44
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.

5 participants