Use the merge base between PR base and head to calculate changed files correctly

.github/workflows/test.yml

@@ -30,13 +30,15 @@ jobs:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v2
+    - name: Setup Committer Info
+      run: |
+        git config user.name 'Github Action' && \
+        git config user.email '[email protected]'
     # add a fake commit
     - name: Add Commit
       run: | 
         mkdir testzone && \
         touch testzone/touched.txt && \
-        git config user.name 'Github Action' && \
-        git config user.email '[email protected]' && \
         git add testzone/touched.txt && \
         git commit -m "add testzone/touched.txt"
     - uses: ./
@@ -49,4 +51,50 @@ jobs:
         head: 'HEAD'
     - name: Check Output
       run: '[[ "${{ steps.filter.outputs.test }}" = "true" ]]'
+  # test case of issue #7
+  test-3:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - name: Setup Committer Info
+        run: |
+          git config user.name 'Github Action' && \
+          git config user.email '[email protected]'
+      - name: Add Commit
+        id: add_commit
+        run: |
+          git fetch --depth=1 origin ${{ github.event.pull_request.base.sha }} && \
+          git checkout -b this_pr && \
+          mkdir testzone && \
+          touch testzone/added_by_this_pr.txt && \
+          git add testzone/added_by_this_pr.txt && \
+          git commit -m "add by this PR" && \
+          echo "::set-output name=head::$(git rev-parse HEAD)"
+      - name: Move Base
+        id: move_base
+        run: |
+          git fetch --depth=1 origin ${{ github.event.pull_request.base.sha }} && \
+          git checkout -b base ${{ github.event.pull_request.base.sha }} && \
+          mkdir testzone_1 && \
+          touch testzone_1/added_by_other_pr.txt && \
+          git add testzone_1/added_by_other_pr.txt  && \
+          git commit -m "another merged PR" && \
+          echo "::set-output name=base::$(git rev-parse HEAD)" && \
+          git checkout this_pr
+      - uses: ./
+        id: filter
+        with:
+          # for testing, set the base to moved base, simulating the situation that the base changed (probably another PR
+          # got merged into the master)
+          base: ${{ steps.move_base.outputs.base }}
+          head: ${{ steps.add_commit.outputs.head }}
+          filters: |
+            this_pr:
+              - testzone/*.txt
+            test:
+              - testzone_1/*.txt
+      - name: Check Output
+        # Although "base" has a additional commit modifying the file matching  "testzone_1/*.txt" (testzone_1/added_by_other_pr.txt),
+        # the file is already merged in the "base"
+        run: '[[ "${{ steps.filter.outputs.test }}" = "false" && "${{ steps.filter.outputs.this_pr }}" = "true" ]]'
 

dist/index.js

@@ -2047,18 +2047,14 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
 };
 Object.defineProperty(exports, "__esModule", { value: true });
 const exec_1 = __webpack_require__(986);
-function getChangedFiles(baseSha, headSha, cwd) {
+function execForStdOut(commandLine, args, cwd) {
     return __awaiter(this, void 0, void 0, function* () {
         return new Promise((resolve, reject) => {
             try {
-                exec_1.exec('git', ['diff', '--name-only', `${baseSha}..${headSha}`, '--'], {
+                exec_1.exec(commandLine, args, {
                     cwd,
                     listeners: {
-                        stdout: buffer => resolve(buffer
-                            .toString()
-                            .split('\n')
-                            .map(x => x.trim())
-                            .filter(x => x.length > 0))
+                        stdout: buffer => resolve(buffer.toString())
                     }
                 }).catch(reject);
             }
@@ -2068,6 +2064,20 @@ function getChangedFiles(baseSha, headSha, cwd) {
         });
     });
 }
+function getMergeBase(shaA, shaB, cwd) {
+    return __awaiter(this, void 0, void 0, function* () {
+        return execForStdOut('git', ['merge-base', shaA, shaB], cwd);
+    });
+}
+function getChangedFiles(baseSha, headSha, cwd) {
+    return __awaiter(this, void 0, void 0, function* () {
+        const mergeBase = (yield getMergeBase(baseSha, headSha, cwd)).trim();
+        return (yield execForStdOut('git', ['diff', '--name-only', `${mergeBase}..${headSha}`, '--'], cwd))
+            .split('\n')
+            .map(x => x.trim())
+            .filter(x => x.length > 0);
+    });
+}
 exports.getChangedFiles = getChangedFiles;
 function revParse(rev, cwd) {
     return __awaiter(this, void 0, void 0, function* () {

src/git.ts

@@ -1,30 +1,48 @@
 import {exec} from '@actions/exec'
 
-export async function getChangedFiles(
-  baseSha: string,
-  headSha: string,
+async function execForStdOut(
+  commandLine: string,
+  args?: string[],
   cwd?: string
-): Promise<string[]> {
+): Promise<string> {
   return new Promise((resolve, reject) => {
     try {
-      exec('git', ['diff', '--name-only', `${baseSha}..${headSha}`, '--'], {
+      exec(commandLine, args, {
         cwd,
         listeners: {
-          stdout: buffer =>
-            resolve(
-              buffer
-                .toString()
-                .split('\n')
-                .map(x => x.trim())
-                .filter(x => x.length > 0)
-            )
+          stdout: buffer => resolve(buffer.toString())
         }
       }).catch(reject)
     } catch (err) {
       reject(err)
     }
   })
 }
+async function getMergeBase(
+  shaA: string,
+  shaB: string,
+  cwd?: string
+): Promise<string> {
+  return execForStdOut('git', ['merge-base', shaA, shaB], cwd)
+}
+
+export async function getChangedFiles(
+  baseSha: string,
+  headSha: string,
+  cwd?: string
+): Promise<string[]> {
+  const mergeBase = (await getMergeBase(baseSha, headSha, cwd)).trim()
+  return (
+    await execForStdOut(
+      'git',
+      ['diff', '--name-only', `${mergeBase}..${headSha}`, '--'],
+      cwd
+    )
+  )
+    .split('\n')
+    .map(x => x.trim())
+    .filter(x => x.length > 0)
+}
 
 export async function revParse(rev: string, cwd?: string): Promise<string> {
   return new Promise((resolve, reject) => {