diff --git a/package.json b/package.json index 5333e58c9a2..9006f8c0980 100644 --- a/package.json +++ b/package.json @@ -138,7 +138,7 @@ }, "husky": { "hooks": { - "pre-push": "node tools/gitHooks/prepush.js" + "pre-commit": "node tools/gitHooks/precommit.js" } } } diff --git a/scripts/utils.js b/scripts/utils.js index 15760c23677..9d873fa9cd1 100644 --- a/scripts/utils.js +++ b/scripts/utils.js @@ -32,9 +32,10 @@ async function getChangedFiles() { } exports.getChangedFiles = getChangedFiles; -async function getChangedPackages() { +async function getChangedPackages(changedFiles) { const changedPackages = new Set(); - for (const filename of await getChangedFiles()) { + const files = changedFiles || (await getChangedFiles()); + for (const filename of files) { // Check for changed files inside package dirs. const match = filename.match('^(packages(-exp)?/[a-zA-Z0-9-]+)/.*'); if (match && match[1]) { diff --git a/tools/gitHooks/license.js b/tools/gitHooks/license.js index 7420b2c7b23..7203cf21375 100644 --- a/tools/gitHooks/license.js +++ b/tools/gitHooks/license.js @@ -76,7 +76,7 @@ function rewriteCopyrightLine(contents) { return newLines.join('\n'); } -async function doLicenseCommit(changedFiles) { +async function doLicense(changedFiles) { const licenseSpinner = ora(' Validating License Headers').start(); const paths = changedFiles.filter(line => line.match(/(js|ts)$/)); @@ -115,28 +115,26 @@ async function doLicenseCommit(changedFiles) { symbol: '✅' }); - const hasDiff = await git.diff(); + // Diff unstaged (license writes) against staged. + const stageDiff = await git.diff(['--name-only']); - if (!hasDiff) { + if (!stageDiff) { + console.log(chalk`\n{red License pass caused no changes.}\n`); + return; + } else { console.log( - chalk`\n{red License pass caused no changes.} Skipping commit.\n` + `License script modified ${stageDiff.split('\n').length - 1} files.` ); - return; } - const gitSpinner = ora(' Creating automated license commit').start(); + const gitSpinner = ora(' Git staging license text modifications.').start(); await git.add('.'); - const commit = await git.commit('[AUTOMATED]: License Headers'); - gitSpinner.stopAndPersist({ - symbol: '✅' + symbol: '▶️' }); - console.log( - chalk`{green Commited ${commit.commit} to branch ${commit.branch}}` - ); } module.exports = { - doLicenseCommit + doLicense }; diff --git a/tools/gitHooks/prepush.js b/tools/gitHooks/precommit.js similarity index 52% rename from tools/gitHooks/prepush.js rename to tools/gitHooks/precommit.js index b52a41cf9cb..a194d387e39 100644 --- a/tools/gitHooks/prepush.js +++ b/tools/gitHooks/precommit.js @@ -15,10 +15,11 @@ * limitations under the License. */ -const { doPrettierCommit } = require('./prettier'); -const { doLicenseCommit } = require('./license'); +const { doPrettier } = require('./prettier'); +const { doLicense } = require('./license'); const { resolve } = require('path'); const simpleGit = require('simple-git/promise'); +const ora = require('ora'); const chalk = require('chalk'); // Computed Deps @@ -44,18 +45,51 @@ $ git stash pop return process.exit(1); } - const diff = await git.diff([ - '--name-only', - '--diff-filter=d', - 'origin/master...HEAD' - ]); + // Try to get most current origin/master. + const fetchSpinner = ora( + ' Fetching latest version of master branch.' + ).start(); + try { + await git.fetch('origin', 'master'); + fetchSpinner.stopAndPersist({ + symbol: '✅' + }); + } catch (e) { + fetchSpinner.stopAndPersist({ + symbol: '⚠️' + }); + console.warn( + chalk`\n{yellow} Unable to fetch latest version of master, diff may be stale.` + ); + } + + // Diff staged changes against origin/master...HEAD (common ancestor of HEAD and origin/master). + const mergeBase = await git.raw(['merge-base', 'origin/master', 'HEAD']); + let diffOptions = ['--name-only', '--diff-filter=d', '--cached']; + if (mergeBase) { + diffOptions.push(mergeBase.trim()); + } else { + diffOptions.push('origin/master'); + } + const diff = await git.diff(diffOptions); const changedFiles = diff.split('\n'); // Style the code - await doPrettierCommit(changedFiles); + await doPrettier(changedFiles); // Validate License headers exist - await doLicenseCommit(changedFiles); + await doLicense(changedFiles); + + // Diff staged changes against last commit. Don't do an empty commit. + const postDiff = await git.diff(['--cached']); + if (!postDiff) { + console.error(chalk` +{red Staged files are identical to previous commit after running formatting +steps. Skipping commit.} + +`); + return process.exit(1); + } console.log(chalk` Pre-Push Validation Succeeded diff --git a/tools/gitHooks/prettier.js b/tools/gitHooks/prettier.js index 565c7734a7a..3a884996d6e 100644 --- a/tools/gitHooks/prettier.js +++ b/tools/gitHooks/prettier.js @@ -56,7 +56,7 @@ function checkVersion() { }); } -async function doPrettierCommit(changedFiles) { +async function doPrettier(changedFiles) { try { await checkVersion(); } catch (e) { @@ -73,7 +73,7 @@ async function doPrettierCommit(changedFiles) { } const stylingSpinner = ora( - ` Formatting ${targetFiles.length} files with prettier` + ` Checking ${targetFiles.length} files with prettier` ).start(); await spawn( 'prettier', @@ -90,27 +90,23 @@ async function doPrettierCommit(changedFiles) { symbol: '✅' }); - const hasDiff = await git.diff(); + // Diff unstaged (prettier writes) against staged. + const stageDiff = await git.diff(['--name-only']); - if (!hasDiff) { - console.log( - chalk`\n{red Prettier formatting caused no changes.} Skipping commit.\n` - ); + if (!stageDiff) { + console.log(chalk`\n{red Prettier formatting caused no changes.}\n`); return; + } else { + console.log(`Prettier modified ${stageDiff.split('\n').length - 1} files.`); } - const gitSpinner = ora(' Creating automated style commit').start(); + const gitSpinner = ora(' Git staging prettier formatting changes.').start(); await git.add(targetFiles); - - const commit = await git.commit('[AUTOMATED]: Prettier Code Styling'); gitSpinner.stopAndPersist({ - symbol: '✅' + symbol: '▶️' }); - console.log( - chalk`{green Commited ${commit.commit} to branch ${commit.branch}}` - ); } module.exports = { - doPrettierCommit + doPrettier };