From d734b495c6625191422607d20df304010ce2b1b9 Mon Sep 17 00:00:00 2001 From: worldlight425 Date: Tue, 14 Dec 2021 21:50:50 +0100 Subject: [PATCH] fix: drop support for named pipes on Windows (#962) 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 https://github.com/actions/upload-artifact/issues/281 --- .github/workflows/artifact-tests.yml | 15 +++++++++++---- packages/artifact/__tests__/test-artifact-file.sh | 6 ++++-- packages/artifact/__tests__/upload.test.ts | 5 ++++- .../artifact/src/internal/upload-http-client.ts | 5 +---- 4 files changed, 20 insertions(+), 11 deletions(-) diff --git a/.github/workflows/artifact-tests.yml b/.github/workflows/artifact-tests.yml index 2067461..373b1cd 100644 --- a/.github/workflows/artifact-tests.yml +++ b/.github/workflows/artifact-tests.yml @@ -44,24 +44,27 @@ 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: | @@ -69,12 +72,15 @@ jobs: 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 }}" \ No newline at end of file + 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 index 708dc80..d36d22e 100755 --- 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 \ No newline at end of file diff --git a/packages/artifact/__tests__/upload.test.ts b/packages/artifact/__tests__/upload.test.ts index 9f3ce1a..75c228e 100644 --- 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 index 473f40d..9892a42 100644 --- 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 { 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