fix(cli): use more explicit amplify pragma in .gitignore (aws-amplify…

…#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 aws-amplify#7250 * test(cli): add coverage of legacy case * chore: address nits * chore: s/removeAmplifyIgnore/rebuildAmplifyIgnore
akshbhu · Aug 15, 2021 · dab7672 · dab7672
1 parent e6f2ca8
commit dab7672
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 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
@@ -9,7 +9,9 @@ const fsMock = fs as jest.Mocked<typeof fs>;
 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
@@ -2,20 +2,24 @@ 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 {
     fs.writeFileSync(gitIgnoreFilePath, getGitIgnoreAppendString().trim());
   }
 }
 
-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;
 }