Skip to content

Commit

Permalink
fix: drop support for named pipes on Windows (#962)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
worldlight425 committed Dec 14, 2021
1 parent 7e314e2 commit d734b49
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 11 deletions.
15 changes: 11 additions & 4 deletions .github/workflows/artifact-tests.yml
Expand Up @@ -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: |
Expand All @@ -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 }}"
6 changes: 4 additions & 2 deletions packages/artifact/__tests__/test-artifact-file.sh
Expand Up @@ -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
5 changes: 4 additions & 1 deletion packages/artifact/__tests__/upload.test.ts
Expand Up @@ -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
}
Expand Down
5 changes: 1 addition & 4 deletions packages/artifact/src/internal/upload-http-client.ts
Expand Up @@ -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
Expand Down

0 comments on commit d734b49

Please sign in to comment.