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

Add a more reliable build step #3183

Merged
merged 6 commits into from
Sep 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 53 additions & 6 deletions Gruntfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,42 @@
module.exports = function( grunt ) {
'use strict';
require( 'dotenv' ).config();

// Root paths to include in the plugin build ZIP when running `npm run build`.
const productionIncludedRootFiles = [
'LICENSE',
'amp.php',
'assets',
'back-compat',
'includes',
'readme.txt',
'templates',
'vendor',
];

// These patterns paths will be excluded from among the above directory.
const productionExcludedPathPatterns = [
/.*\/src\/.*/,
/.*images\/stories-editor\/.*\.svg/,
];

// These will be removed from the vendor directory after installing but prior to creating a ZIP.
// ⚠️ Warning: These paths are passed straight to rm command in the shell, without any escaping.
const productionVendorExcludedFilePatterns = [
'composer.*',
'vendor/*/*/.editorconfig',
'vendor/*/*/.gitignore',
'vendor/*/*/composer.*',
'vendor/*/*/Doxyfile',
'vendor/*/*/LICENSE',
'vendor/*/*/phpunit.*',
'vendor/*/*/*.md',
'vendor/*/*/*.txt',
'vendor/*/*/*.yml',
'vendor/*/*/.*.yml',
'vendor/*/*/tests',
];

grunt.initConfig( {

pkg: grunt.file.readJSON( 'package.json' ),
Expand Down Expand Up @@ -32,6 +68,9 @@ module.exports = function( grunt ) {
verify_matching_versions: {
command: 'php bin/verify-version-consistency.php',
},
composer_install: {
command: 'if [ ! -e build ]; then echo "Run grunt build first."; exit 1; fi; cd build; composer install --no-dev -o && composer remove cweagans/composer-patches --update-no-dev -o && rm -r ' + productionVendorExcludedFilePatterns.join( ' ' ),
},
create_build_zip: {
command: 'if [ ! -e build ]; then echo "Run grunt build first."; exit 1; fi; if [ -e amp.zip ]; then rm amp.zip; fi; cd build; zip -r ../amp.zip .; cd ..; echo; echo "ZIP of build: $(pwd)/amp.zip"',
},
Expand Down Expand Up @@ -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 );
Copy link
Member

@westonruter westonruter Sep 9, 2019

Choose a reason for hiding this comment

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

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

const topSegment = file.replace( /\/.*/, '' );
if ( ! productionIncludedRootFiles.includes( topSegment ) ) {
return false;
}

for ( const productionExcludedPathPattern of productionExcludedPathPatterns ) {
if ( productionExcludedPathPattern.test( file ) ) {
return false;
}
}

return true;
} );

paths.push( 'vendor/autoload.php' );
paths.push( 'composer.*' ); // Copy in order to be able to do run composer_install.
paths.push( 'assets/js/*.js' );
paths.push( 'assets/js/*.deps.json' );
paths.push( 'assets/css/*.css' );
paths.push( 'vendor/composer/**' );
paths.push( 'vendor/sabberworm/php-css-parser/lib/**' );
paths.push( 'vendor/fasterimage/fasterimage/src/**' );
paths.push( 'vendor/willwashburn/stream/src/**' );
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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


grunt.config.set( 'copy', {
build: {
Expand Down Expand Up @@ -138,6 +184,7 @@ module.exports = function( grunt ) {
} );
grunt.task.run( 'readme' );
grunt.task.run( 'copy' );
grunt.task.run( 'shell:composer_install' );

done();
}
Expand Down
56 changes: 56 additions & 0 deletions bin/build-plugin-zip.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
#!/bin/bash

# Exit if any command fails.
set -e

# Change to the expected directory.
cd "$(dirname "$0")"
cd ..
PLUGIN_DIR=$(pwd)

# Enable nicer messaging for build status.
BLUE_BOLD='\033[1;34m';
GREEN_BOLD='\033[1;32m';
RED_BOLD='\033[1;31m';
YELLOW_BOLD='\033[1;33m';
COLOR_RESET='\033[0m';
error () {
echo -e "\n${RED_BOLD}$1${COLOR_RESET}\n"
}
status () {
echo -e "\n${BLUE_BOLD}$1${COLOR_RESET}\n"
}
success () {
echo -e "\n${GREEN_BOLD}$1${COLOR_RESET}\n"
}
warning () {
echo -e "\n${YELLOW_BOLD}$1${COLOR_RESET}\n"
}

status "Time to release AMP ⚡️"

status "Setting up a fresh build environment in a temporary folder. ✨"

# Create a fresh temporary folder in a way that works across platforms.
BUILD_DIR=$(mktemp -d 2>/dev/null || mktemp -d -t 'amp-production-build')

# Do a local clone to move the current files across.
git clone -l . "$BUILD_DIR"
cd "$BUILD_DIR"

# Run the build.
status "Installing dependencies... 📦"
composer install
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Collaborator Author

@schlessera schlessera Sep 5, 2019

Choose a reason for hiding this comment

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

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.

PUPPETEER_SKIP_CHROMIUM_DOWNLOAD=true npm install

status "Generating build... ⚙️"
npm run build

status "Copying the ZIP file back over... ↩️"
rm -f "$PLUGIN_DIR/amp.zip"
cp "$BUILD_DIR/amp.zip" "$PLUGIN_DIR/amp.zip"

status "Removing the temporary folder again... 🗑️"
rm -rf "$BUILD_DIR"

success "You've built AMP! 🎉 \nThe ZIP file can be found in the following location:\n$PLUGIN_DIR/amp.zip"
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
"extra": {
"patches": {
"sabberworm/php-css-parser": {
"PHP-CSS-Parser: Fix parsing CSS selectors which contain commas <https://github.com/sabberworm/PHP-CSS-Parser/pull/138>": "patches/php-css-parser-mods.diff"
"PHP-CSS-Parser: Fix parsing CSS selectors which contain commas <https://github.com/sabberworm/PHP-CSS-Parser/pull/138>": "https://github.com/sabberworm/PHP-CSS-Parser/commit/fa139f65c5b098ae652c970b25e6eb03fc495eb4.diff"
schlessera marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand Down
22 changes: 19 additions & 3 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

109 changes: 0 additions & 109 deletions patches/php-css-parser-mods.diff

This file was deleted.