Infer commit hash from change file introduction commit (#282)

.vscode/settings.json

@@ -9,6 +9,7 @@
   "typescript.tsdk": "node_modules/typescript/lib",
   "search.exclude": {
     "**/node_modules": true,
-    "**/lib": true
+    "**/lib": true,
+    "docs": true
   }
 }

change/beachball-2020-03-21-04-55-51-infer-commit.json

@@ -0,0 +1,8 @@
+{
+  "type": "minor",
+  "comment": "Delay inferring commit hash until changelog generation (and remove commit from changefiles)",
+  "packageName": "beachball",
+  "email": "elcraig@microsoft.com",
+  "dependentChangeType": "patch",
+  "date": "2020-03-21T11:55:51.028Z"
+}

packages/beachball/src/__e2e__/bump.test.ts

@@ -75,7 +75,6 @@ describe('version bumping', () => {
         'pkg-1': {
           type: 'minor',
           comment: 'test',
-          commit: 'test',
           date: new Date('2019-01-01'),
           email: 'test@test.com',
           packageName: 'pkg-1',
@@ -163,7 +162,6 @@ describe('version bumping', () => {
         'pkg-1': {
           type: 'minor',
           comment: 'test',
-          commit: 'test',
           date: new Date('2019-01-01'),
           email: 'test@test.com',
           packageName: 'pkg-1',
@@ -233,7 +231,6 @@ describe('version bumping', () => {
         'pkg-1': {
           type: 'minor',
           comment: 'test',
-          commit: 'test',
           date: new Date('2019-01-01'),
           email: 'test@test.com',
           packageName: 'pkg-1',
@@ -322,7 +319,6 @@ describe('version bumping', () => {
         commonlib: {
           type: 'minor',
           comment: 'test',
-          commit: 'test',
           date: new Date('2019-01-01'),
           email: 'test@test.com',
           packageName: 'commonlib',
@@ -363,7 +359,6 @@ describe('version bumping', () => {
         foo: {
           type: 'minor',
           comment: 'test',
-          commit: 'test',
           date: new Date('2019-01-01'),
           email: 'test@test.com',
           packageName: 'foo',
@@ -395,7 +390,6 @@ describe('version bumping', () => {
         bar: {
           type: 'patch',
           comment: 'test',
-          commit: 'test',
           date: new Date('2019-01-01'),
           email: 'test@test.com',
           packageName: 'bar',

packages/beachball/src/__e2e__/changelog.test.ts

@@ -14,6 +14,7 @@ import { selectAll } from 'unist-util-select';
 import { writeChangeFiles } from '../changefile/writeChangeFiles';
 import { readChangeFiles } from '../changefile/readChangeFiles';
 import { BeachballOptions } from '../types/BeachballOptions';
+import { ChangeFileInfo } from '../types/ChangeInfo';
 
 const readFileAsync = promisify(fs.readFile);
 
@@ -23,27 +24,77 @@ function parseMarkdown(markdown: string) {
     .parse(markdown);
 }
 
-describe('validation', () => {
+describe('changelog generation', () => {
   let repositoryFactory: RepositoryFactory;
+  let repository: Repository;
+
   beforeAll(async () => {
     repositoryFactory = new RepositoryFactory();
     await repositoryFactory.create();
   });
 
-  describe('writeChangelog', () => {
-    let repository: Repository;
+  beforeEach(async () => {
+    repository = await repositoryFactory.cloneRepository();
+  });
 
-    beforeEach(async () => {
-      repository = await repositoryFactory.cloneRepository();
-    });
-
-    it('generate changelog is a valid markdown file', async () => {
+  describe('readChangelog', () => {
+    it('adds actual commit hash', async () => {
+      await repository.commitChange('foo');
+      writeChangeFiles(
+        {
+          foo: {
+            comment: 'comment 1',
+            date: new Date('Thu Aug 22 2019 14:20:40 GMT-0700 (Pacific Daylight Time)'),
+            email: 'test@testtestme.com',
+            packageName: 'foo',
+            type: 'patch',
+            dependentChangeType: 'patch',
+          },
+        },
+        repository.rootPath
+      );
+
+      const currentHash = await repository.getCurrentHash();
+      const changeSet = readChangeFiles({ path: repository.rootPath } as BeachballOptions);
+      const changes = [...changeSet.values()];
+      expect(changes).toHaveLength(1);
+      expect(changes[0].commit).toBe(currentHash);
+    });
+
+    it('uses hash of original commit', async () => {
+      const changeInfo: ChangeFileInfo = {
+        comment: 'comment 1',
+        date: new Date('Thu Aug 22 2019 14:20:40 GMT-0700 (Pacific Daylight Time)'),
+        email: 'test@testtestme.com',
+        packageName: 'foo',
+        type: 'patch',
+        dependentChangeType: 'patch',
+      };
+
+      await repository.commitChange('foo');
+      const changeFilePaths = writeChangeFiles({ foo: changeInfo }, repository.rootPath);
+      const changeFilePath = path.relative(repository.rootPath, changeFilePaths[0]);
+      const changeFileAddedHash = await repository.getCurrentHash();
+
+      // change the change file
+      await repository.commitChange(changeFilePath, JSON.stringify({ ...changeInfo, comment: 'comment 2' }, null, 2));
+      await repository.commitChange(changeFilePath, JSON.stringify({ ...changeInfo, comment: 'comment 3' }, null, 2));
+
+      // keeps original hash
+      const changeSet = readChangeFiles({ path: repository.rootPath } as BeachballOptions);
+      const changes = [...changeSet.values()];
+      expect(changes).toHaveLength(1);
+      expect(changes[0].commit).toBe(changeFileAddedHash);
+    });
+  });
+
+  describe('writeChangelog', () => {
+    it('generates changelog that is a valid markdown file', async () => {
       await repository.commitChange('foo');
       writeChangeFiles(
         {
           foo: {
             comment: 'comment 1',
-            commit: 'sha1-1',
             date: new Date('Thu Aug 22 2019 14:20:40 GMT-0700 (Pacific Daylight Time)'),
             email: 'test@testtestme.com',
             packageName: 'foo',
@@ -59,7 +110,6 @@ describe('validation', () => {
         {
           foo: {
             comment: 'comment 2',
-            commit: 'sha1-2',
             date: new Date('Thu Aug 22 2019 14:20:40 GMT-0700 (Pacific Daylight Time)'),
             email: 'test@testtestme.com',
             packageName: 'foo',

packages/beachball/src/__e2e__/publishE2E.test.ts

@@ -32,7 +32,6 @@ describe('publish command (e2e)', () => {
         foo: {
           type: 'minor',
           comment: 'test',
-          commit: 'test',
           date: new Date('2019-01-01'),
           email: 'test@test.com',
           packageName: 'foo',
@@ -93,7 +92,6 @@ describe('publish command (e2e)', () => {
         foo: {
           type: 'minor',
           comment: 'test',
-          commit: 'test',
           date: new Date('2019-01-01'),
           email: 'test@test.com',
           packageName: 'foo',
@@ -108,7 +106,6 @@ describe('publish command (e2e)', () => {
         bar: {
           type: 'minor',
           comment: 'test',
-          commit: 'test',
           date: new Date('2019-01-01'),
           email: 'test@test.com',
           packageName: 'bar',

packages/beachball/src/__e2e__/publishGit.test.ts

@@ -7,7 +7,7 @@ import { writeChangeFiles } from '../changefile/writeChangeFiles';
 import { git, gitFailFast } from '../git';
 import { gatherBumpInfo } from '../bump/gatherBumpInfo';
 import { BeachballOptions } from '../types/BeachballOptions';
-import { ChangeInfo } from '../types/ChangeInfo';
+import { ChangeFileInfo } from '../types/ChangeInfo';
 
 describe('publish command (git)', () => {
   let repositoryFactory: RepositoryFactory;
@@ -29,7 +29,6 @@ describe('publish command (git)', () => {
         foo: {
           type: 'minor',
           comment: 'test',
-          commit: 'test',
           date: new Date('2019-01-01'),
           email: 'test@test.com',
           packageName: 'foo',
@@ -79,7 +78,6 @@ describe('publish command (git)', () => {
         foo: {
           type: 'minor',
           comment: 'test',
-          commit: 'test',
           date: new Date('2019-01-01'),
           email: 'test@test.com',
           packageName: 'foo',
@@ -129,7 +127,6 @@ describe('publish command (git)', () => {
         foo2: {
           type: 'minor',
           comment: 'test',
-          commit: 'test',
           date: new Date('2019-01-01'),
           email: 'test@test.com',
           packageName: 'foo2',
@@ -150,7 +147,7 @@ describe('publish command (git)', () => {
     expect(fs.existsSync(newChangePath)).toBeTruthy();
     const changeFiles = fs.readdirSync(newChangePath);
     expect(changeFiles.length).toBe(1);
-    const changeFileContent: ChangeInfo = JSON.parse(
+    const changeFileContent: ChangeFileInfo = JSON.parse(
       fs.readFileSync(path.join(newChangePath, changeFiles[0]), 'utf-8')
     );
     expect(changeFileContent.packageName).toBe('foo2');

packages/beachball/src/__e2e__/publishRegistry.test.ts

@@ -31,7 +31,6 @@ describe('publish command (registry)', () => {
         foo: {
           type: 'minor',
           comment: 'test',
-          commit: 'test',
           date: new Date('2019-01-01'),
           email: 'test@test.com',
           packageName: 'foo',
@@ -110,7 +109,6 @@ describe('publish command (registry)', () => {
         foopkg: {
           type: 'minor',
           comment: 'test',
-          commit: 'test',
           date: new Date('2019-01-01'),
           email: 'test@test.com',
           packageName: 'foopkg',
@@ -184,7 +182,6 @@ describe('publish command (registry)', () => {
         badname: {
           type: 'minor',
           comment: 'test',
-          commit: 'test',
           date: new Date('2019-01-01'),
           email: 'test@test.com',
           packageName: 'badname',

packages/beachball/src/changefile/getChangedPackages.ts

@@ -1,4 +1,4 @@
-import { ChangeInfo } from '../types/ChangeInfo';
+import { ChangeFileInfo } from '../types/ChangeInfo';
 import { findPackageRoot, getChangePath } from '../paths';
 import { getChanges, getStagedChanges, git, fetchRemote, parseRemoteBranch } from '../git';
 import fs from 'fs';
@@ -75,7 +75,7 @@ export function getChangedPackages(options: BeachballOptions) {
   // Loop through the change files, building up a set of packages that we can skip
   changeFiles.forEach(file => {
     try {
-      const changeInfo: ChangeInfo = JSON.parse(fs.readFileSync(file, 'utf-8'));
+      const changeInfo: ChangeFileInfo = JSON.parse(fs.readFileSync(file, 'utf-8'));
       changeFilePackageSet.add(changeInfo.packageName);
     } catch (e) {
       console.warn(`Invalid change file encountered: ${file}`);

packages/beachball/src/changefile/getPackageChangeTypes.ts

@@ -1,4 +1,4 @@
-import { ChangeInfo, ChangeSet, ChangeType } from '../types/ChangeInfo';
+import { ChangeFileInfo, ChangeSet, ChangeType } from '../types/ChangeInfo';
 
 const SortedChangeTypes: ChangeType[] = ['none', 'prerelease', 'patch', 'minor', 'major'];
 
@@ -18,7 +18,7 @@ const ChangeTypeWeights = SortedChangeTypes.reduce((weights, changeType, index)
 
 export function getPackageChangeTypes(changeSet: ChangeSet) {
   const changePerPackage: {
-    [pkgName: string]: ChangeInfo['type'];
+    [pkgName: string]: ChangeFileInfo['type'];
   } = {};
 
   for (let [_, change] of changeSet) {

packages/beachball/src/changefile/promptForChange.ts

@@ -1,4 +1,4 @@
-import { ChangeInfo, ChangeType } from '../types/ChangeInfo';
+import { ChangeFileInfo, ChangeType } from '../types/ChangeInfo';
 import { getChangedPackages } from './getChangedPackages';
 import { getRecentCommitMessages, getUserEmail, getCurrentHash } from '../git';
 import prompts from 'prompts';
@@ -10,14 +10,13 @@ import { PackageGroups, PackageInfos } from '../types/PackageInfo';
 
 /**
  * Uses `prompts` package to prompt for change type and description, fills in git user.email, scope, and the commit hash
- * @param cwd
  */
 export async function promptForChange(options: BeachballOptions) {
   const { branch, path: cwd, package: specificPackage } = options;
 
   const changedPackages = specificPackage ? [specificPackage] : getChangedPackages(options);
   const recentMessages = getRecentCommitMessages(branch, cwd) || [];
-  const packageChangeInfo: { [pkgname: string]: ChangeInfo } = {};
+  const packageChangeInfo: { [pkgname: string]: ChangeFileInfo } = {};
 
   const packageInfos = getPackageInfos(cwd);
   const packageGroups = getPackageGroups(packageInfos, options.path, options.groups);
@@ -86,7 +85,6 @@ export async function promptForChange(options: BeachballOptions) {
       ...response,
       packageName: pkg,
       email: getUserEmail(cwd) || 'email not defined',
-      commit: getCurrentHash(cwd) || 'hash not available',
       dependentChangeType: response.type === 'none' ? 'none' : 'patch',
       date: new Date(),
     };

packages/beachball/src/changefile/readChangeFiles.ts

@@ -1,11 +1,12 @@
-import { ChangeSet } from '../types/ChangeInfo';
+import { ChangeSet, ChangeInfo } from '../types/ChangeInfo';
 import { getChangePath } from '../paths';
 import fs from 'fs-extra';
 import path from 'path';
 import { BeachballOptions } from '../types/BeachballOptions';
 import { getScopedPackages } from '../monorepo/getScopedPackages';
+import { getFileAddedHash } from '../git';
 
-export function readChangeFiles(options: BeachballOptions) {
+export function readChangeFiles(options: BeachballOptions): ChangeSet {
   const { path: cwd } = options;
   const scopedPackages = getScopedPackages(options);
   const changeSet: ChangeSet = new Map();
@@ -16,10 +17,15 @@ export function readChangeFiles(options: BeachballOptions) {
   const changeFiles = fs.readdirSync(changePath);
   changeFiles.forEach(changeFile => {
     try {
-      const packageJson = JSON.parse(fs.readFileSync(path.join(changePath, changeFile)).toString());
-      const packageName = packageJson.packageName;
+      const changeInfo: ChangeInfo = {
+        ...fs.readJSONSync(path.join(changePath, changeFile)),
+        // Add the commit hash where the file was actually first introduced
+        commit: getFileAddedHash(changePath, cwd) || '',
+      };
+
+      const packageName = changeInfo.packageName;
       if (scopedPackages.includes(packageName)) {
-        changeSet.set(changeFile, packageJson);
+        changeSet.set(changeFile, changeInfo);
       } else {
         console.log(`Skipping reading change file for out-of-scope package ${packageName}`);
       }

packages/beachball/src/changefile/writeChangeFiles.ts

@@ -1,48 +1,56 @@
-import { ChangeInfo } from '../types/ChangeInfo';
+import { ChangeFileInfo } from '../types/ChangeInfo';
 import { getChangePath } from '../paths';
 import { getBranchName, stageAndCommit } from '../git';
 import fs from 'fs-extra';
 import path from 'path';
 import { getTimeStamp } from './getTimeStamp';
+
 /**
  * Loops through the `changes` and writes out a list of change files
- * @param changes
- * @param cwd
+ * @returns List of changefile paths, mainly for testing purposes.
  */
 export function writeChangeFiles(
   changes: {
-    [pkgname: string]: ChangeInfo;
+    [pkgname: string]: ChangeFileInfo;
   },
   cwd: string
-) {
+): string[] {
   if (Object.keys(changes).length === 0) {
-    return;
+    return [];
   }
+
   const changePath = getChangePath(cwd);
   const branchName = getBranchName(cwd);
   if (changePath && !fs.existsSync(changePath)) {
     fs.mkdirpSync(changePath);
   }
+
   if (changes && branchName && changePath) {
-    const changeFiles: string[] = [];
-    Object.keys(changes).forEach(pkgName => {
+    const changeFiles = Object.keys(changes).map(pkgName => {
       const suffix = branchName.replace(/[\/\\]/g, '-');
       const prefix = pkgName.replace(/[^a-zA-Z0-9@]/g, '-');
       const fileName = `${prefix}-${getTimeStamp()}-${suffix}.json`;
       let changeFile = path.join(changePath, fileName);
+
       if (fs.existsSync(changeFile)) {
         const nextFileName = `${prefix}-${getTimeStamp()}-${suffix}-${Math.random()
           .toString(36)
           .substr(2, 9)}.json`;
         changeFile = path.join(changePath, nextFileName);
       }
+
       const change = changes[pkgName];
       fs.writeFileSync(changeFile, JSON.stringify(change, null, 2));
-      changeFiles.push(changeFile);
+      return changeFile;
     });
+
     stageAndCommit(changeFiles, 'Change files', cwd);
+
     console.log(`git committed these change files:
 ${changeFiles.map(f => ` - ${f}`).join('\n')}
 `);
+    return changeFiles;
   }
+
+  return [];
 }

packages/beachball/src/fixtures/exec.ts

@@ -23,10 +23,11 @@ export function exec(command: string): Promise<PsResult> {
   });
 }
 
-export async function runCommands(commands: string[]): Promise<void> {
+export async function runCommands(commands: string[]): Promise<PsResult[]> {
+  const results: PsResult[] = [];
   for (let i = 0; i < commands.length; i++) {
     try {
-      await exec(commands[i]);
+      results.push(await exec(commands[i]));
     } catch (e) {
       console.error('runCommands failed:');
       console.error(e.stdout);
@@ -36,11 +37,16 @@ export async function runCommands(commands: string[]): Promise<void> {
       throw e;
     }
   }
+  return results;
 }
 
-export async function runInDirectory(targetDirectory: string, commands: string[]) {
+/**
+ * @returns The results of the commands run
+ */
+export async function runInDirectory(targetDirectory: string, commands: string[]): Promise<PsResult[]> {
   const originalDirectory = process.cwd();
   process.chdir(targetDirectory);
-  await runCommands(commands);
+  const results = await runCommands(commands);
   process.chdir(originalDirectory);
+  return results;
 }

packages/beachball/src/fixtures/repository.ts

@@ -76,7 +76,7 @@ export class Repository {
     this.origin = path;
   }
 
-  async commitChange(newFilename: string, content?: string) {
+  async commitChange(newFilename: string, content?: string): Promise<void> {
     if (!this.root) {
       throw new Error('Must initialize before cloning');
     }
@@ -90,6 +90,15 @@ export class Repository {
     await runInDirectory(this.root.name, [`git add ${newFilename}`, `git commit -m '${newFilename}'`]);
   }
 
+  async getCurrentHash(): Promise<string> {
+    if (!this.root) {
+      throw new Error('Must initialize before getting head');
+    }
+
+    const result = await runInDirectory(this.root.name, ['git rev-parse HEAD']);
+    return result[0].stdout.trim();
+  }
+
   async branch(branchName: string) {
     if (!this.root) {
       throw new Error('Must initialize before cloning');
@@ -105,6 +114,10 @@ export class Repository {
     await runInDirectory(this.root.name, [`git push ${remote} ${branch}`]);
   }
 
+  /**
+   * Clean up created repo. This isn't necessary to call manually in most cases because `tmp` will automatically
+   * remove created directories on program exit (assuming `tmp.setGracefulCleanup()` is still called somewhere).
+   */
   async cleanUp() {
     if (!this.root) {
       throw new Error('Must initialize before clean up');

packages/beachball/src/fixtures/tmpdir.ts

@@ -1,5 +1,8 @@
 import * as tmp from 'tmp';
 
+// Clean up created directories when the program exits
+tmp.setGracefulCleanup();
+
 export type DirResult = tmp.DirResult;
 
 export async function tmpdir(options: tmp.DirOptions): Promise<tmp.DirResult> {

packages/beachball/src/git/index.ts

@@ -6,8 +6,6 @@ import gitUrlParse from 'git-url-parse';
 
 /**
  * Runs git command - use this for read only commands
- * @param args
- * @param options
  */
 export function git(args: string[], options?: { cwd: string }) {
   const results = spawnSync('git', args, options);
@@ -29,8 +27,6 @@ export function git(args: string[], options?: { cwd: string }) {
 
 /**
  * Runs git command - use this for commands that makes changes to the file system
- * @param args
- * @param options
  */
 export function gitFailFast(args: string[], options?: { cwd: string }) {
   const gitResult = git(args, options);
@@ -202,6 +198,22 @@ export function getCurrentHash(cwd: string) {
   return null;
 }
 
+/**
+ * Get the commit hash in which the file was first added.
+ */
+export function getFileAddedHash(filename: string, cwd: string) {
+  const results = git(['rev-list', 'HEAD', filename], { cwd });
+
+  if (results.success) {
+    return results.stdout
+      .trim()
+      .split('\n')
+      .slice(-1)[0];
+  }
+
+  return undefined;
+}
+
 export function stageAndCommit(patterns: string[], message: string, cwd: string) {
   try {
     patterns.forEach(pattern => {

packages/beachball/src/types/ChangeInfo.ts

@@ -1,13 +1,22 @@
 export type ChangeType = 'prerelease' | 'patch' | 'minor' | 'major' | 'none';
 
-export interface ChangeInfo {
+/**
+ * Info saved in each change file.
+ */
+export interface ChangeFileInfo {
   type: ChangeType;
   comment: string;
   packageName: string;
   email: string;
-  commit: string;
   date: Date;
   dependentChangeType?: ChangeType;
 }
 
+/**
+ * Info saved in each change file, plus the commit hash.
+ */
+export interface ChangeInfo extends ChangeFileInfo {
+  commit: string;
+}
+
 export type ChangeSet = Map<string, ChangeInfo>;