Add a more reliable build step #3183

schlessera · 2019-09-04T17:05:51Z

This PR adapts the build process to be more reliable in terms of:

code it bases the build on
dependencies it pulls in via Composer

The following changes are included:

Adds a shell script to clean up the current folder and then trigger a build. The shell script is based on the one from Gutenberg: https://github.com/WordPress/gutenberg/blob/master/bin/build-plugin-zip.sh
Adapts the Gruntfile.js file to fetch optimized Composer dependencies within the build folder. Pruning the vendor folder at the filesystem level only will always produce issues with the meta information that Composer has assembled, like the autoloaders that assume all dependencies to still be around. Therefore, we do an actual composer install --no-dev -o within the build folder.
As we are currently patching a dependency via cweagans/composer-patches, we need to add an additional composer remove, as that package needs to be a production requirement to work as expected. As Composer is unaware of the fact that something was patched, it will not undo the patch at this point.
Retrieves the Sabberworm patch directly from the GitHub PR. This removes the reliance on a local folder and its exact location, so we needn't copy the patch over to the build folder nor hack the patcher into using a smarter relative path. The file was locked at the specific single commit of the current PR, so that additional pushes will not automatically be included with our build.

Note: The build process is spread out across a shell script, the composer.json file, the package.json file and the gruntfile.js. It might make sense to simplify this in a future step to stick with one technology only.

See #1840

bin/build-plugin-zip.sh

westonruter · 2019-09-04T19:41:38Z

bin/build-plugin-zip.sh

+
+# Run the build.
+status "Installing dependencies... 📦"
+composer install


Is this needed because grunt.task.run( 'shell:composer_install' ); is being done during build?

Yes, it is needed right now because things are a bit too intertwined. The grunt task readme calls the a tool from the xwp/wp-dev-lib PHP development dependency:

https://github.com/ampproject/amp-wp/blob/1.2.2/Gruntfile.js#L26-L27

I didn't want to rewrite half of the build pipeline in this PR. That's one of the things the Note at the end of the PR message is referring to.
But let me know if you think I should do a bigger change in one go.

composer.json

westonruter · 2019-09-05T21:17:49Z

@schlessera Could you rebase the commits to fix the merge conflict and undo the accidental inclusion of the build directory removed in 2e6fde1?

This remove the reliance on a local folder and its exact location, so we needn't copy the patch over to the `build` folder nor hack the patcher into using a smarter relative path.

…uild folder Pruning the `vendor` folder at the filesystem level only will always produce issues with the meta information that Composer has assembled, like the autoloaders that assume all dependencies to still be around.

The shell script is based on the one frmo Gutenberg: https://github.com/WordPress/gutenberg/blob/master/bin/build-plugin-zip.sh

Instead of cleaning up the current folder, we create a fresh temporary folder and clone files into that one to start the build.

westonruter · 2019-09-08T06:31:55Z

Gruntfile.js

-			paths.push( 'vendor/composer/**' );
-			paths.push( 'vendor/sabberworm/php-css-parser/lib/**' );
-			paths.push( 'vendor/fasterimage/fasterimage/src/**' );
-			paths.push( 'vendor/willwashburn/stream/src/**' );


There are a lot of files now being included which should not be part of the build (e.g. the test files):

A wp-content/plugins/amp/vendor/fasterimage/fasterimage/.editorconfig A wp-content/plugins/amp/vendor/fasterimage/fasterimage/.gitignore A wp-content/plugins/amp/vendor/fasterimage/fasterimage/CHANGELOG.md A wp-content/plugins/amp/vendor/fasterimage/fasterimage/LICENSE A wp-content/plugins/amp/vendor/fasterimage/fasterimage/README.md A wp-content/plugins/amp/vendor/fasterimage/fasterimage/circle.yml A wp-content/plugins/amp/vendor/fasterimage/fasterimage/composer.json A wp-content/plugins/amp/vendor/fasterimage/fasterimage/phpunit.xml A wp-content/plugins/amp/vendor/fasterimage/fasterimage/tests/FasterImageTest.php A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/.gitignore A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/.travis.yml A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/CHANGELOG.md A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/Doxyfile A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/PATCHES.txt A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/README.md A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/composer.json A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/composer.lock A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/phpunit.xml A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/Sabberworm/CSS/CSSList/AtRuleBlockListTest.php A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/Sabberworm/CSS/CSSList/DocumentTest.php A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/Sabberworm/CSS/OutputFormatTest.php A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/Sabberworm/CSS/ParserTest.php A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/Sabberworm/CSS/RuleSet/DeclarationBlockTest.php A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/Sabberworm/CSS/RuleSet/LenientParsingTest.php A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/bootstrap.php A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/-calc-no-space-around-minus.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/-charset-after-rule.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/-charset-in-block.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/-empty-grid-linename.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/-empty.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/-end-token-2.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/-end-token.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/-fault-tolerance.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/-tobedone.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/1readme.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/2readme.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/atrules.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/calc-nested.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/calc.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/case-insensitivity.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/colortest.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/comments.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/create-shorthands.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/docuwiki.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/empty-grid-linename.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/expand-shorthands.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/functions.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/grid-linename.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/hex-alpha.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/ie-hacks.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/ie.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/important.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/inner-color.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/line-numbers.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/missing-property-value.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/ms-filter.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/namespaces.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/nested.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/slashed.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/specificity.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/trailing-whitespace.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/unicode-range.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/unicode.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/unmatched_braces.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/unopened-close-brackets.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/url.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/values.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/webkit.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/files/whitespace.css A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/phpunit.xml A wp-content/plugins/amp/vendor/sabberworm/php-css-parser/tests/quickdump.php A wp-content/plugins/amp/vendor/willwashburn/stream/.gitignore A wp-content/plugins/amp/vendor/willwashburn/stream/.travis.yml A wp-content/plugins/amp/vendor/willwashburn/stream/LICENSE A wp-content/plugins/amp/vendor/willwashburn/stream/README.md A wp-content/plugins/amp/vendor/willwashburn/stream/composer.json A wp-content/plugins/amp/vendor/willwashburn/stream/phpunit.xml A wp-content/plugins/amp/vendor/willwashburn/stream/tests/StreamTests.php

Can these be excluded? The files add to the ZIP size and they are unused in production.

Done in 2a86f5d. This should make it easier to maintain going forward, though it's surely not a good as it can be.

westonruter · 2019-09-09T23:21:37Z

Gruntfile.js

@@ -95,17 +134,24 @@ module.exports = function( grunt ) {
 			const versionAppend = new Date().toISOString().replace( /\.\d+/, '' ).replace( /-|:/g, '' ) + '-' + commitHash;

 			const paths = lsOutput.trim().split( /\n/ ).filter( function( file ) {
-				return ! /^(blocks|\.|bin|([^/]+)+\.(md|json|xml)|Gruntfile\.js|tests|wp-assets|readme\.md|composer\..*|patches|webpack.*|assets\/images\/stories-editor\/.*\.svg|assets\/src|assets\/css\/src|docker-compose\.yml|.*\.config\.js|codecov\.yml|example\.env)/.test( file );


I'm very glad this mess of a regex is now gone 😄

schlessera · 2019-09-16T20:23:14Z

Screencast: https://youtu.be/KZutho_Ldog

googlebot added the cla: yes Signed the Google CLA label Sep 4, 2019

schlessera added the Infrastructure Changes impacting testing infrastructure or build tooling label Sep 4, 2019

schlessera requested review from westonruter, kienstra and swissspidy September 4, 2019 17:06

westonruter reviewed Sep 4, 2019

View reviewed changes

schlessera added 5 commits September 7, 2019 15:01

Retrieve Sabberworm patch directly from the GitHub PR

e951317

This remove the reliance on a local folder and its exact location, so we needn't copy the patch over to the `build` folder nor hack the patcher into using a smarter relative path.

Add shell script to clean up the current folder and then trigger a build

7034b52

The shell script is based on the one frmo Gutenberg: https://github.com/WordPress/gutenberg/blob/master/bin/build-plugin-zip.sh

Run build in a temporary folder

bcf6e4f

Instead of cleaning up the current folder, we create a fresh temporary folder and clone files into that one to start the build.

Refresh Composer lock file

c2ea84d

schlessera force-pushed the add/1840-reliable-builds branch from 2e6fde1 to c2ea84d Compare September 7, 2019 13:03

schlessera requested review from westonruter and removed request for swissspidy September 7, 2019 13:04

westonruter requested changes Sep 8, 2019

View reviewed changes

westonruter added this to the v1.3 milestone Sep 8, 2019

Exclude development files from production build ZIPs

2a86f5d

westonruter reviewed Sep 9, 2019

View reviewed changes

westonruter approved these changes Sep 9, 2019

View reviewed changes

westonruter mentioned this pull request Sep 9, 2019

Reliable builds and deploys #1840

Closed

westonruter added the [Size] M label Sep 9, 2019

westonruter merged commit 24fb61a into develop Sep 9, 2019

westonruter deleted the add/1840-reliable-builds branch September 9, 2019 23:47

schlessera added the Has Screencast label Sep 16, 2019

schlessera mentioned this pull request Sep 19, 2019

Decouple build process from packaging process #3302

Closed

westonruter mentioned this pull request Apr 16, 2020

Integrate Composer into production build process #1836

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a more reliable build step #3183

Add a more reliable build step #3183

schlessera commented Sep 4, 2019 •

edited by westonruter

Loading

westonruter Sep 4, 2019

schlessera Sep 5, 2019 •

edited

Loading

westonruter commented Sep 5, 2019

westonruter Sep 8, 2019

westonruter Sep 9, 2019

westonruter Sep 9, 2019 •

edited

Loading

schlessera commented Sep 16, 2019

Add a more reliable build step #3183

Add a more reliable build step #3183

Conversation

schlessera commented Sep 4, 2019 • edited by westonruter Loading

westonruter Sep 4, 2019

Choose a reason for hiding this comment

schlessera Sep 5, 2019 • edited Loading

Choose a reason for hiding this comment

westonruter commented Sep 5, 2019

westonruter Sep 8, 2019

Choose a reason for hiding this comment

westonruter Sep 9, 2019

Choose a reason for hiding this comment

westonruter Sep 9, 2019 • edited Loading

Choose a reason for hiding this comment

schlessera commented Sep 16, 2019

schlessera commented Sep 4, 2019 •

edited by westonruter

Loading

schlessera Sep 5, 2019 •

edited

Loading

westonruter Sep 9, 2019 •

edited

Loading