From dab767228ab94d38daa6c5d76260ea31f94eafdc Mon Sep 17 00:00:00 2001 From: John Corser Date: Tue, 20 Jul 2021 14:15:59 -0400 Subject: [PATCH] fix(cli): use more explicit amplify pragma in .gitignore (#7568) * fix(cli): use more explicit amplify pragma in .gitignore Previously, we used the #amplify pragma to show which block of .gitignore was managed by amplify. This causes confusion for customers who think it is safe to add or remove lines this section of their gitignore. The cli will blitz any changes to this section on amplify init or amplify pull. By using a more explicit pragma, customers will realize they should not touch this section of the gitignore. fix #7250 * test(cli): add coverage of legacy case * chore: address nits * chore: s/removeAmplifyIgnore/rebuildAmplifyIgnore --- .../amplify-helpers/git-manager.test.ts | 27 +++++++++++++++---- .../extensions/amplify-helpers/git-manager.ts | 16 ++++++----- 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/packages/amplify-cli/src/__tests__/extensions/amplify-helpers/git-manager.test.ts b/packages/amplify-cli/src/__tests__/extensions/amplify-helpers/git-manager.test.ts index 6615bb929e5..ca489e83dbd 100644 --- a/packages/amplify-cli/src/__tests__/extensions/amplify-helpers/git-manager.test.ts +++ b/packages/amplify-cli/src/__tests__/extensions/amplify-helpers/git-manager.test.ts @@ -9,7 +9,9 @@ const fsMock = fs as jest.Mocked; fsMock.readFileSync.mockImplementation(() => 'test'); fsMock.writeFileSync.mockImplementation(); -const amplifyMark = '#amplify'; +const amplifyMark = '#amplify-do-not-edit-begin'; +const amplifyEndMark = '#amplify-do-not-edit-end'; +const deprecatedAmplifyMark = '#amplify'; const ignoreList = [ 'amplify/\\#current-cloud-backend', @@ -32,23 +34,38 @@ const ignoreList = [ '.secret-*', ]; -const toAppend = `${os.EOL + os.EOL + amplifyMark + os.EOL}${ignoreList.join(os.EOL)}`; - +const toAppend = `${os.EOL + os.EOL + amplifyMark + os.EOL}${ignoreList.join(os.EOL)}${os.EOL + amplifyEndMark + os.EOL}`; +const legacyToAppend = `${os.EOL + os.EOL + deprecatedAmplifyMark + os.EOL}${ignoreList.join(os.EOL)}`; describe('git-manager', () => { beforeEach(() => { fsMock.writeFileSync.mockClear(); + fsMock.readFileSync.mockClear(); + fsMock.appendFileSync.mockClear(); + fsMock.existsSync.mockClear(); }); const gitIgnoreFilePath = 'testPath'; test('appends files to an existing .gitignore', () => { fsMock.existsSync.mockImplementation(() => true); + fsMock.readFileSync.mockImplementation(() => 'amplify/'); insertAmplifyIgnore(gitIgnoreFilePath); - expect(fsMock.appendFileSync.mock.calls[0][0]).toEqual('testPath'); + expect(fsMock.writeFileSync.mock.calls[0][0]).toEqual(gitIgnoreFilePath); + expect(fsMock.writeFileSync.mock.calls[0][1]).toEqual('amplify/'); + expect(fsMock.appendFileSync.mock.calls[0][0]).toEqual(gitIgnoreFilePath); expect(fsMock.appendFileSync.mock.calls[0][1]).toEqual(toAppend); }); test('create a new .gitignore', () => { fsMock.existsSync.mockImplementation(() => false); insertAmplifyIgnore(gitIgnoreFilePath); - expect(fsMock.writeFileSync.mock.calls[0][0]).toEqual('testPath'); + expect(fsMock.writeFileSync.mock.calls[0][0]).toEqual(gitIgnoreFilePath); expect(fsMock.writeFileSync.mock.calls[0][1]).toEqual(toAppend.trim()); }); + test('legacy .gitignore files reformat themselves when accessed by a newer version of the CLI', () => { + fsMock.existsSync.mockImplementation(() => true); + fsMock.readFileSync.mockImplementation(() => 'amplify/' + os.EOL + legacyToAppend.trim()); + insertAmplifyIgnore(gitIgnoreFilePath); + expect(fsMock.writeFileSync.mock.calls[0][0]).toEqual(gitIgnoreFilePath); + expect(fsMock.writeFileSync.mock.calls[0][1]).toEqual('amplify/'); + expect(fsMock.appendFileSync.mock.calls[0][0]).toEqual(gitIgnoreFilePath); + expect(fsMock.appendFileSync.mock.calls[0][1]).toEqual(toAppend); + }); }); diff --git a/packages/amplify-cli/src/extensions/amplify-helpers/git-manager.ts b/packages/amplify-cli/src/extensions/amplify-helpers/git-manager.ts index af803131f3e..b868a37cc52 100644 --- a/packages/amplify-cli/src/extensions/amplify-helpers/git-manager.ts +++ b/packages/amplify-cli/src/extensions/amplify-helpers/git-manager.ts @@ -2,12 +2,16 @@ import * as fs from 'fs-extra'; import * as os from 'os'; import { LocalLogDirectory } from 'amplify-cli-logger'; -const amplifyMark = '#amplify'; +const amplifyMark = '#amplify-do-not-edit-begin'; +const amplifyEndMark = '#amplify-do-not-edit-end'; +const deprecatedAmplifyMark = '#amplify'; const amplifyMarkRegExp = new RegExp(`^${amplifyMark}`); +const amplifyEndMarkRegExp = new RegExp(`^${amplifyEndMark}`); +const deprecatedAmplifyMarkRegExp = new RegExp(`^${deprecatedAmplifyMark}`); export function insertAmplifyIgnore(gitIgnoreFilePath: string): void { if (fs.existsSync(gitIgnoreFilePath)) { - removeAmplifyIgnore(gitIgnoreFilePath); + rebuildAmplifyIgnore(gitIgnoreFilePath); fs.appendFileSync(gitIgnoreFilePath, getGitIgnoreAppendString()); } else { @@ -15,7 +19,7 @@ export function insertAmplifyIgnore(gitIgnoreFilePath: string): void { } } -function removeAmplifyIgnore(gitIgnoreFilePath: string): void { +function rebuildAmplifyIgnore(gitIgnoreFilePath: string): void { if (fs.existsSync(gitIgnoreFilePath)) { let newGitIgnoreString = ''; const gitIgnoreStringArray = fs.readFileSync(gitIgnoreFilePath, 'utf8').split(os.EOL); @@ -26,10 +30,10 @@ function removeAmplifyIgnore(gitIgnoreFilePath: string): void { const newLine = gitIgnoreStringArray[i].trim(); if (isInRemoval) { - if (newLine.length === 0) { + if (amplifyEndMarkRegExp.test(newLine) || newLine.length === 0) { isInRemoval = false; } - } else if (amplifyMarkRegExp.test(newLine)) { + } else if (amplifyMarkRegExp.test(newLine) || deprecatedAmplifyMarkRegExp.test(newLine)) { isInRemoval = true; } else { newGitIgnoreString += newLine + os.EOL; @@ -64,7 +68,7 @@ function getGitIgnoreAppendString() { '.secret-*', ]; - const toAppend = `${os.EOL + os.EOL + amplifyMark + os.EOL}${ignoreList.join(os.EOL)}`; + const toAppend = `${os.EOL + os.EOL + amplifyMark + os.EOL}${ignoreList.join(os.EOL)}${os.EOL + amplifyEndMark + os.EOL}`; return toAppend; }