Skip to content

Commit

Permalink
Enforce the use of yarn for package management (#11368)
Browse files Browse the repository at this point in the history
  • Loading branch information
rsimha committed Sep 21, 2017
1 parent ee2dea7 commit 4710863
Show file tree
Hide file tree
Showing 7 changed files with 140 additions and 54 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@ typings.json
build-system/runner/TESTS-TestSuites.xml
/test/manual/amp-ad.adtech.html
test/coverage
package-lock.json
60 changes: 60 additions & 0 deletions build-system/check-package-manager.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/**
* Copyright 2017 The AMP HTML Authors. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS-IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
'use strict';

const setupInstructionsUrl = 'https://github.com/ampproject/amphtml/blob/master/contributing/getting-started-quick.md#one-time-setup';

// Color formatting may not yet be available via gulp-util.
function red(text) {return '\x1b[31m' + text + '\x1b[0m';}
function cyan(text) {return '\x1b[36m' + text + '\x1b[0m';}
function green(text) {return '\x1b[32m' + text + '\x1b[0m';}
function yellow(text) {return '\x1b[33m' + text + '\x1b[0m';}

/**
* @fileoverview Makes sure that packages are being installed via yarn
*/
function main() {
// Yarn is already used by default on Travis, so there is nothing more to do.
if (process.env.TRAVIS) {
return 0;
}

// If npm is being run, print a message and cause 'npm install' to fail.
if (process.env.npm_execpath.indexOf('yarn') === -1) {
console/*OK*/.log(red(
'*** The AMP project uses yarn for package management ***'), '\n');
console/*OK*/.log(yellow('To install all packages:'));
console/*OK*/.log(cyan('$'), 'yarn', '\n');
console/*OK*/.log(yellow(
'To install a new package (and update package.json and yarn.lock):'));
console/*OK*/.log(cyan('$'),
'yarn add --dev --exact [package_name@version]', '\n');
console/*OK*/.log(yellow('To upgrade a package:'));
console/*OK*/.log(cyan('$'),
'yarn upgrade --exact [package_name@version]', '\n');
console/*OK*/.log(yellow('To remove a package:'));
console/*OK*/.log(cyan('$'), 'yarn remove [package_name]', '\n');
console/*OK*/.log(yellow('For detailed instructions, see'),
cyan(setupInstructionsUrl), '\n');
return 1;
}

// If yarn is being run, proceed with the install.
console/*OK*/.log(green('Detected yarn. Installing packages...'));
return 0;
}

process.exit(main());
26 changes: 22 additions & 4 deletions build-system/exec.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,12 @@ exports.execOrDie = function(cmd) {
};

/**
* Executes the provided command, returning its stdout.
* Executes the provided command, returning the process object.
* This will throw an exception if something goes wrong.
* @param {string} cmd
* @return {!Array<string>}
* @return {!Object}
*/
exports.getStdout = function(cmd) {
function getOutput(cmd) {
const p = spawnProcess(
cmd,
{
Expand All @@ -73,5 +73,23 @@ exports.getStdout = function(cmd) {
'stdio': 'pipe',
'encoding': 'utf-8',
});
return p.stdout;
return p;
}

/**
* Executes the provided command, returning its stdout.
* @param {string} cmd
* @return {string}
*/
exports.getStdout = function(cmd) {
return getOutput(cmd).stdout;
};

/**
* Executes the provided command, returning its stderr.
* @param {string} cmd
* @return {string}
*/
exports.getStderr = function(cmd) {
return getOutput(cmd).stderr;
};
70 changes: 38 additions & 32 deletions build-system/pr-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@
const atob = require('atob');
const execOrDie = require('./exec').execOrDie;
const getStdout = require('./exec').getStdout;
const getStderr = require('./exec').getStderr;
const path = require('path');
const util = require('gulp-util');

const gulp = 'node_modules/gulp/bin/gulp.js';
const fileLogPrefix = util.colors.yellow.bold('pr-check.js:');

/**
Expand Down Expand Up @@ -244,53 +244,53 @@ function determineBuildTargets(filePaths) {

const command = {
testBuildSystem: function() {
timedExecOrDie(`${gulp} ava`);
timedExecOrDie('gulp ava');
},
testDocumentLinks: function() {
timedExecOrDie(`${gulp} check-links`);
timedExecOrDie('gulp check-links');
},
cleanBuild: function() {
timedExecOrDie(`${gulp} clean`);
timedExecOrDie('gulp clean');
},
runLintCheck: function() {
timedExecOrDie(`${gulp} lint`);
timedExecOrDie('gulp lint');
},
runJsonCheck: function() {
timedExecOrDie(`${gulp} json-syntax`);
timedExecOrDie('gulp json-syntax');
},
buildCss: function() {
timedExecOrDie(`${gulp} css`);
timedExecOrDie('gulp css');
},
buildRuntime: function() {
timedExecOrDie(`${gulp} build`);
timedExecOrDie('gulp build');
},
buildRuntimeMinified: function() {
timedExecOrDie(`${gulp} dist --fortesting`);
timedExecOrDie('gulp dist --fortesting');
},
runDepAndTypeChecks: function() {
timedExecOrDie(`${gulp} dep-check`);
timedExecOrDie(`${gulp} check-types`);
timedExecOrDie('gulp dep-check');
timedExecOrDie('gulp check-types');
},
runUnitTests: function() {
// Unit tests with Travis' default chromium
timedExecOrDie(`${gulp} test --unit --nobuild`);
timedExecOrDie('gulp test --unit --nobuild');
// All unit tests with an old chrome (best we can do right now to pass tests
// and not start relying on new features).
// Disabled because it regressed. Better to run the other saucelabs tests.
// timedExecOrDie(
// `${gulp} test --nobuild --saucelabs --oldchrome --compiled`);
// `gulp test --nobuild --saucelabs --oldchrome --compiled`);
},
runIntegrationTests: function(compiled) {
// Integration tests with all saucelabs browsers
let cmd = `${gulp} test --nobuild --saucelabs --integration`;
let cmd = 'gulp test --nobuild --saucelabs --integration';
if (compiled) {
cmd += ' --compiled';
}
timedExecOrDie(cmd);
},
runVisualDiffTests: function(opt_mode) {
process.env['PERCY_TOKEN'] = atob(process.env.PERCY_TOKEN_ENCODED);
let cmd = `${gulp} visual-diff`;
let cmd = 'gulp visual-diff';
if (opt_mode === 'skip') {
cmd += ' --skip';
} else if (opt_mode === 'master') {
Expand All @@ -299,16 +299,16 @@ const command = {
timedExecOrDie(cmd);
},
verifyVisualDiffTests: function() {
timedExecOrDie(`${gulp} visual-diff --verify`);
timedExecOrDie('gulp visual-diff --verify');
},
runPresubmitTests: function() {
timedExecOrDie(`${gulp} presubmit`);
timedExecOrDie('gulp presubmit');
},
buildValidatorWebUI: function() {
timedExecOrDie(`${gulp} validator-webui`);
timedExecOrDie('gulp validator-webui');
},
buildValidator: function() {
timedExecOrDie(`${gulp} validator`);
timedExecOrDie('gulp validator');
},
};

Expand Down Expand Up @@ -372,19 +372,25 @@ function main() {
process.exit(1);
}

// Make sure changes to package.json also update yarn.lock.
if (files.indexOf('package.json') != -1 && files.indexOf('yarn.lock') == -1) {
console.error(fileLogPrefix, util.colors.red('ERROR:'),
'Updates to', util.colors.cyan('package.json'),
'must be accompanied by a corresponding update to',
util.colors.cyan('yarn.lock'));
console.error(fileLogPrefix, util.colors.yellow('NOTE:'),
'To update', util.colors.cyan('yarn.lock'), 'after changing',
util.colors.cyan('package.json') + ',', 'run',
'"' + util.colors.cyan('yarn install') + '"',
'and include the change to', util.colors.cyan('yarn.lock'),
'in your PR.');
process.exit(1);
// Make sure package.json and yarn.lock are in sync.
if (files.indexOf('package.json') != -1 || files.indexOf('yarn.lock') != -1) {
const yarnIntegrityCheck = getStderr('yarn check --integrity').trim();
if (yarnIntegrityCheck.includes('error')) {
console.error(fileLogPrefix, util.colors.red('ERROR:'),
'Found the following', util.colors.cyan('yarn'), 'errors:\n' +
util.colors.cyan(yarnIntegrityCheck));
console.error(fileLogPrefix, util.colors.red('ERROR:'),
'Updates to', util.colors.cyan('package.json'),
'must be accompanied by a corresponding update to',
util.colors.cyan('yarn.lock'));
console.error(fileLogPrefix, util.colors.yellow('NOTE:'),
'To update', util.colors.cyan('yarn.lock'), 'after changing',
util.colors.cyan('package.json') + ',', 'run',
'"' + util.colors.cyan('yarn install') + '"',
'and include the updated', util.colors.cyan('yarn.lock'),
'in your PR.');
process.exit(1);
}
}

console.log(
Expand Down
19 changes: 8 additions & 11 deletions contributing/getting-started-e2e.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,19 +155,14 @@ amphtml uses Node.js, the Yarn package manager and the Gulp build system to buil

* Install [NodeJS](https://nodejs.org/) version >= 6 (which includes npm)

* Install [Yarn](https://yarnpkg.com/) version >= 1.0.2, follow the instructions on the website or install it with npm:
```
npm install -g yarn@latest
```

The preceding command might require elevated privileges using `sudo` on some platforms.
* Install [Yarn](https://yarnpkg.com/) version >= 1.0.2 (instructions [here](https://yarnpkg.com/en/docs/install), this may require elevated privileges using `sudo` on some platforms)

* In your local repository directory (e.g. `~/src/ampproject/amphtml`), install the packages that AMP uses by running
```
yarn
```

You should see a progress indicator and some messages scrolling by. You may see some warnings about optional dependencies that are generally safe to ignore.
You should see a progress indicator and some messages scrolling by. You may see some warnings about optional dependencies, which are generally safe to ignore.

* For some local testing we refer to fake local URLs in order to simulate referencing third party URLs. This requires extra setup so your browser will know that these URLs actually point to your local server.

Expand Down Expand Up @@ -278,7 +273,9 @@ The common workflow for making changes to files in Git is:
* edit some files using your favorite editor
* if you edited `package.json`, run `yarn install` to generate an updated `yarn.lock` file
* if your code requires a new dependency, run `yarn add --dev --exact [packagename]`, which will automatically update `package.json` and `yarn.lock`
* if you manually edited `package.json`, run `yarn install` to install the dependency and generate an updated `yarn.lock` file
* tell Git that you care about these changes by _staging_ them using the `git add` command
Expand Down Expand Up @@ -344,7 +341,7 @@ If the tests have failed you will need to determine whether the failure is relat

If the failing test looks completely unrelated to your change, it *might* be due to bad code/tests that have made it into the amphtml repository. You can check the latest [amphtml test run on Travis](https://travis-ci.org/ampproject/amphtml/builds). If it's green (meaning the tests pass) then it's more likely the failure is a problem with your change. If it's red, you can click through to see if the failing tests are the same as the ones you see locally.
Fixing the tests will depend heavily on the change you are making and what tests are failing. If you need help to fix them you can ask on the GitHub issue you're working on or reach out to the community as described in [How to get help](#how-to-get-help).
Fixing the tests will depend heavily on the change you are making and what tests are failing. If you need help to fix them you can ask on the GitHub issue you're working on or reach out to the community as described in [How to get help](#how-to-get-help).

## Adding tests for your change

Expand Down Expand Up @@ -408,7 +405,7 @@ Note that you *can* edit files in your branch directly on GitHub using the web U
In order for your changes to become part of the amphtml repository, you will need to get your code reviewed by one of the [core committers](https://github.com/ampproject/amphtml/blob/master/GOVERNANCE.md) via a Pull Request (PR). In fact you won't actually merge your code into the amphtml repository directly; once a core committer approves it he or she will handle the merge for you.

Once your code is ready for a review, go to [https://github.com/ampproject/amphtml](https://github.com/ampproject/amphtml) and click on the "Compare & pull request" button on the "recently pushed branches" banner. If that banner isn't visible, go to your GitHub fork at
`https://github.com/<username>/amphtml`, use the Branch dropdown to select the branch that contains the changes you want reviewed and press the "New pull request" button.
`https://github.com/<username>/amphtml`, use the Branch dropdown to select the branch that contains the changes you want reviewed and press the "New pull request" button.
On the "Open a pull request" page, you will see dropdowns at the top indicating the proposed merge. It will look something like:
Expand Down Expand Up @@ -489,7 +486,7 @@ If you're looking for ideas on your next contribution feel free to reach out to
This end-to-end guide provided enough details to get a basic understanding of a typical workflow for contributing code to the AMP Project. If you find yourself wanting to know more there are a lot of resources available. Here are a few:
* The ["Creating your first AMP Component" codelab](https://codelabs.developers.google.com/codelabs/creating-your-first-amp-component/index.html) provides step-by-step instructions for a common type of code contribution to the AMP Project. Even if your project involves modifying an existing AMP component this codelab will give you an overview of how AMP components work.
* The ["Creating your first AMP Component" codelab](https://codelabs.developers.google.com/codelabs/creating-your-first-amp-component/index.html) provides step-by-step instructions for a common type of code contribution to the AMP Project. Even if your project involves modifying an existing AMP component this codelab will give you an overview of how AMP components work.
* GitHub has a lot of helpful introductory material, including:
* a [Hello World tutorial](https://guides.github.com/activities/hello-world/) that's a bit less in depth than this guide, but it covers things like creating a new repository and merging in code after a pull request
* the [Git cheat sheet](https://services.github.com/on-demand/downloads/github-git-cheat-sheet.pdf) from GitHub provides a quick reference to some common commands, including many we didn't cover in this guide (such as [diff](https://www.git-tower.com/learn/git/ebook/en/command-line/advanced-topics/diffs) and [log](https://git-scm.com/book/en/v2/Git-Basics-Viewing-the-Commit-History))
Expand Down
11 changes: 7 additions & 4 deletions contributing/getting-started-quick.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ This Quick Start guide is the TL;DR version of the longer [end-to-end guide](get

* [Install and set up Git](https://help.github.com/articles/set-up-git/); in the "Authenticating" step of that page use SSH instead of HTTPS

* Install [NodeJS](https://nodejs.org/) version >= 6
* Install [NodeJS](https://nodejs.org/) version >= 6 (which includes npm)

* Install [Yarn](https://yarnpkg.com/) version >= 1.0.2, follow the instructions on the website or install it with npm: `npm install -g yarn@latest` (this command might require elevated privileges using `sudo` on some platforms)
* Install [Yarn](https://yarnpkg.com/) version >= 1.0.2 (instructions [here](https://yarnpkg.com/en/docs/install), this may require elevated privileges using `sudo` on some platforms)

* Install Gulp by running `yarn global add gulp` (as before, this command might require `sudo`)
* Install Gulp by running `yarn global add gulp` (this may require elevated privileges using `sudo` on some platforms)

* Add this line to your hosts file (`/etc/hosts` on Mac or Linux, `%SystemRoot%\System32\drivers\etc\hosts` on Windows):

Expand All @@ -48,12 +48,14 @@ This Quick Start guide is the TL;DR version of the longer [end-to-end guide](get
* Go to the branch: `git checkout <branch name>`

# Build AMP & run a local server

* Make sure you have the latest packages (after you pull): `yarn`
* Start the server: `gulp`
* Access your server at [http://localhost:8000](http://localhost:8000)
* Access your sample pages at [http://localhost:8000/examples](http://localhost:8000/examples)

# Test AMP

* Run the tests: `gulp test`
* Run the tests in a specified set of files: `gulp test --files=<filename>`
* Add the `--watch` flag to any `gulp test` command to automatically re-run the tests when a file changes
Expand All @@ -62,7 +64,8 @@ This Quick Start guide is the TL;DR version of the longer [end-to-end guide](get
# Create commits to contain your changes

* Edit files in your favorite editor
* If you edited `package.json`, run `yarn install` to generate an updated `yarn.lock` file
* if your code requires a new dependency, run `yarn add --dev --exact [packagename]`, which will automatically update `package.json` and `yarn.lock`
* if you manually edited `package.json`, run `yarn install` to install the dependency and generate an updated `yarn.lock` file
* Add each file you change: `git add <file>`
* Create a commit: `git commit -m "<your commit message>"`
* Instead of `add`ing each file individually you can use the `-a` flag on the commit instead
Expand Down
7 changes: 4 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,16 @@
"url": "https://github.com/ampproject/amphtml.git"
},
"scripts": {
"test": "gulp test && ava",
"ava": "ava",
"test": "gulp test && gulp ava",
"ava": "gulp ava",
"lint": "gulp lint",
"build": "gulp build",
"dist": "gulp dist",
"heroku-postbuild": "npm run test-env-var && gulp clean && gulp build --fortesting && gulp dist --fortesting && npm run prepend-amp && npm run prepend-v0",
"test-env-var": "if [ -z \"${AMP_TESTING_HOST}\" ]; then echo \"ERROR: Please set the 'AMP_TESTING_HOST' environment variable. If you are in heroku this can be set by executing 'heroku config:set AMP_TESTING_HOST=my-heroku-subdomain.herokuapp.com' on the command line.\"; exit 1; fi",
"prepend-amp": "gulp prepend-global --prod node_modules/AMP_CONFIG.json --local --target dist/amp.js && gulp prepend-global --prod node_modules/AMP_CONFIG.json --local --target dist.3p/current/integration.js",
"prepend-v0": "gulp prepend-global --prod node_modules/AMP_CONFIG.json --local --target dist/v0.js && gulp prepend-global --prod node_modules/AMP_CONFIG.json --local --target dist.3p/current-min/f.js"
"prepend-v0": "gulp prepend-global --prod node_modules/AMP_CONFIG.json --local --target dist/v0.js && gulp prepend-global --prod node_modules/AMP_CONFIG.json --local --target dist.3p/current-min/f.js",
"preinstall": "node build-system/check-package-manager.js"
},
"dependencies": {
"babel-polyfill": "6.26.0",
Expand Down

0 comments on commit 4710863

Please sign in to comment.