Move prepush scripts to precommit (#3026)

Author: hsubox76

package.json

@@ -138,7 +138,7 @@
   },
   "husky": {
     "hooks": {
-      "pre-push": "node tools/gitHooks/prepush.js"
+      "pre-commit": "node tools/gitHooks/precommit.js"
     }
   }
 }

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]) {

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
 };

tools/gitHooks/prepush.js renamed to 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

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
 };