fix: drop support for named pipes on Windows (actions#962)

zregvart · web-flow · commit 37f5a8521950 · 2021-12-14T15:50:50.000-05:00
Seems that folk are having issues with uploading 0-byte files from Windows agents. This effectively removes the support for Windows for uploading from named files that, due to `isFIFO` returning `false` on Windows for named pipes created using MSYS2's `mkfifo` command, resorted to checking if the file size is 0 - a common trait of named pipes. See actions/upload-artifact#281
diff --git a/.github/workflows/artifact-tests.yml b/.github/workflows/artifact-tests.yml
@@ -44,37 +44,43 @@ jobs:
         npm ci
         npm run tsc
       working-directory: packages/artifact
-    
+
     - name: Set artifact file contents
       shell: bash
       run: |
         echo "non-gzip-artifact-content=hello" >> $GITHUB_ENV
         echo "gzip-artifact-content=Some large amount of text that has a compression ratio that is greater than 100%. If greater than 100%, gzip is used to upload the file" >> $GITHUB_ENV
+        echo "empty-artifact-content=_EMPTY_" >> $GITHUB_ENV
 
     - name: Create files that will be uploaded
       run: |
-        mkdir artifact-path 
+        mkdir artifact-path
         echo ${{ env.non-gzip-artifact-content }} > artifact-path/world.txt
         echo ${{ env.gzip-artifact-content }} > artifact-path/gzip.txt
+        touch artifact-path/empty.txt
 
     # We're using node -e to call the functions directly available in the @actions/artifact package
     - name: Upload artifacts using uploadArtifact()
       run: |
         node -e "Promise.resolve(require('./packages/artifact/lib/artifact-client').create().uploadArtifact('my-artifact-1',['artifact-path/world.txt'], '${{ github.workspace }}'))"
         node -e "Promise.resolve(require('./packages/artifact/lib/artifact-client').create().uploadArtifact('my-artifact-2',['artifact-path/gzip.txt'], '${{ github.workspace }}'))"
+        node -e "Promise.resolve(require('./packages/artifact/lib/artifact-client').create().uploadArtifact('my-artifact-3',['artifact-path/empty.txt'], '${{ github.workspace }}'))"
 
     - name: Download artifacts using downloadArtifact()
       run: |
         mkdir artifact-1-directory
         node -e "Promise.resolve(require('./packages/artifact/lib/artifact-client').create().downloadArtifact('my-artifact-1','artifact-1-directory'))"
         mkdir artifact-2-directory
         node -e "Promise.resolve(require('./packages/artifact/lib/artifact-client').create().downloadArtifact('my-artifact-2','artifact-2-directory'))"
-    
+        mkdir artifact-3-directory
+        node -e "Promise.resolve(require('./packages/artifact/lib/artifact-client').create().downloadArtifact('my-artifact-3','artifact-3-directory'))"
+
     - name: Verify downloadArtifact()
       shell: bash
       run: |
         packages/artifact/__tests__/test-artifact-file.sh "artifact-1-directory/artifact-path/world.txt" "${{ env.non-gzip-artifact-content }}"
         packages/artifact/__tests__/test-artifact-file.sh "artifact-2-directory/artifact-path/gzip.txt" "${{ env.gzip-artifact-content }}"
+        packages/artifact/__tests__/test-artifact-file.sh "artifact-3-directory/artifact-path/empty.txt" "${{ env.empty-artifact-content }}"
 
     - name: Download artifacts using downloadAllArtifacts()
       run: |
@@ -85,4 +91,5 @@ jobs:
       shell: bash
       run: |
         packages/artifact/__tests__/test-artifact-file.sh "multi-artifact-directory/my-artifact-1/artifact-path/world.txt" "${{ env.non-gzip-artifact-content }}"
-        packages/artifact/__tests__/test-artifact-file.sh "multi-artifact-directory/my-artifact-2/artifact-path/gzip.txt" "${{ env.gzip-artifact-content }}"
+        packages/artifact/__tests__/test-artifact-file.sh "multi-artifact-directory/my-artifact-2/artifact-path/gzip.txt" "${{ env.gzip-artifact-content }}"
+        packages/artifact/__tests__/test-artifact-file.sh "multi-artifact-directory/my-artifact-3/artifact-path/empty.txt" "${{ env.empty-artifact-content }}"
diff --git a/packages/artifact/__tests__/test-artifact-file.sh b/packages/artifact/__tests__/test-artifact-file.sh
@@ -18,8 +18,10 @@ if [ ! -f "$path" ]; then
     exit 1
 fi
 
-actualContent=$(cat $path)
-if [ "$actualContent" != "$expectedContent" ];then
+actualContent=$(cat "$path")
+if [ "$expectedContent" == "_EMPTY_" ] && [ ! -s "$path" ]; then
+    exit 0
+elif  [ "$actualContent" != "$expectedContent" ]; then
     echo "File contents are not correct, expected $expectedContent, received $actualContent"
     exit 1
 fi
diff --git a/packages/artifact/__tests__/upload.test.ts b/packages/artifact/__tests__/upload.test.ts
@@ -181,7 +181,10 @@ describe('Upload Tests', () => {
   function hasMkfifo(): boolean {
     try {
       // make sure we drain the stdout
-      return execSync('which mkfifo').toString().length > 0
+      return (
+        process.platform !== 'win32' &&
+        execSync('which mkfifo').toString().length > 0
+      )
     } catch (e) {
       return false
     }
diff --git a/packages/artifact/src/internal/upload-http-client.ts b/packages/artifact/src/internal/upload-http-client.ts
@@ -221,10 +221,7 @@ export class UploadHttpClient {
   ): Promise<UploadFileResult> {
     const fileStat: fs.Stats = await stat(parameters.file)
     const totalFileSize = fileStat.size
-    // on Windows with mkfifo from MSYS2 stats.isFIFO returns false, so we check if running on Windows node and
-    // if the file has size of 0 to compensate
-    const isFIFO =
-      fileStat.isFIFO() || (process.platform === 'win32' && totalFileSize === 0)
+    const isFIFO = fileStat.isFIFO()
     let offset = 0
     let isUploadSuccessful = true
     let failedChunkSizes = 0
