Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Retain fs stats in artifacts zip #1609

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Retain fs stats in artifacts zip #1609

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 31 additions & 15 deletions packages/artifact/__tests__/upload-artifact.test.ts
View file
Open in desktop
Expand Up @@ -8,6 +8,7 @@ import * as blobUpload from '../src/internal/upload/blob-upload'
import {uploadArtifact} from '../src/internal/upload/upload-artifact'
import {noopLogs} from './common'
import {FilesNotFoundError} from '../src/internal/shared/errors'
import {Stats} from 'fs'

describe('upload-artifact', () => {
beforeEach(() => {
Expand All @@ -28,15 +29,18 @@ describe('upload-artifact', () => {
.mockReturnValue([
{
sourcePath: '/home/user/files/plz-upload/file1.txt',
destinationPath: 'file1.txt'
destinationPath: 'file1.txt',
stats: new Stats()
},
{
sourcePath: '/home/user/files/plz-upload/file2.txt',
destinationPath: 'file2.txt'
destinationPath: 'file2.txt',
stats: new Stats()
},
{
sourcePath: '/home/user/files/plz-upload/dir/file3.txt',
destinationPath: 'dir/file3.txt'
destinationPath: 'dir/file3.txt',
stats: new Stats()
}
])

Expand Down Expand Up @@ -136,15 +140,18 @@ describe('upload-artifact', () => {
.mockReturnValue([
{
sourcePath: '/home/user/files/plz-upload/file1.txt',
destinationPath: 'file1.txt'
destinationPath: 'file1.txt',
stats: new Stats()
},
{
sourcePath: '/home/user/files/plz-upload/file2.txt',
destinationPath: 'file2.txt'
destinationPath: 'file2.txt',
stats: new Stats()
},
{
sourcePath: '/home/user/files/plz-upload/dir/file3.txt',
destinationPath: 'dir/file3.txt'
destinationPath: 'dir/file3.txt',
stats: new Stats()
}
])

Expand Down Expand Up @@ -175,15 +182,18 @@ describe('upload-artifact', () => {
.mockReturnValue([
{
sourcePath: '/home/user/files/plz-upload/file1.txt',
destinationPath: 'file1.txt'
destinationPath: 'file1.txt',
stats: Stats.prototype
},
{
sourcePath: '/home/user/files/plz-upload/file2.txt',
destinationPath: 'file2.txt'
destinationPath: 'file2.txt',
stats: Stats.prototype
},
{
sourcePath: '/home/user/files/plz-upload/dir/file3.txt',
destinationPath: 'dir/file3.txt'
destinationPath: 'dir/file3.txt',
stats: Stats.prototype
}
])

Expand Down Expand Up @@ -230,15 +240,18 @@ describe('upload-artifact', () => {
.mockReturnValue([
{
sourcePath: '/home/user/files/plz-upload/file1.txt',
destinationPath: 'file1.txt'
destinationPath: 'file1.txt',
stats: new Stats()
},
{
sourcePath: '/home/user/files/plz-upload/file2.txt',
destinationPath: 'file2.txt'
destinationPath: 'file2.txt',
stats: new Stats()
},
{
sourcePath: '/home/user/files/plz-upload/dir/file3.txt',
destinationPath: 'dir/file3.txt'
destinationPath: 'dir/file3.txt',
stats: new Stats()
}
])

Expand Down Expand Up @@ -293,15 +306,18 @@ describe('upload-artifact', () => {
.mockReturnValue([
{
sourcePath: '/home/user/files/plz-upload/file1.txt',
destinationPath: 'file1.txt'
destinationPath: 'file1.txt',
stats: new Stats()
},
{
sourcePath: '/home/user/files/plz-upload/file2.txt',
destinationPath: 'file2.txt'
destinationPath: 'file2.txt',
stats: new Stats()
},
{
sourcePath: '/home/user/files/plz-upload/dir/file3.txt',
destinationPath: 'dir/file3.txt'
destinationPath: 'dir/file3.txt',
stats: new Stats()
}
])

Expand Down
4 changes: 2 additions & 2 deletions packages/artifact/__tests__/upload-zip-specification.test.ts
View file
Open in desktop
Expand Up @@ -287,14 +287,14 @@ describe('Search', () => {
expect(specifications.length).toEqual(2)
const absolutePaths = specifications.map(item => item.sourcePath)
expect(absolutePaths).toContain(goodItem1Path)
expect(absolutePaths).toContain(null)
expect(absolutePaths).toContain(folderEPath)

for (const specification of specifications) {
if (specification.sourcePath === goodItem1Path) {
expect(specification.destinationPath).toEqual(
path.join('/folder-a', 'folder-b', 'folder-c', 'good-item1.txt')
)
} else if (specification.sourcePath === null) {
} else if (specification.sourcePath === folderEPath) {
expect(specification.destinationPath).toEqual(
path.join('/folder-a', 'folder-b', 'folder-e')
)
Expand Down
54 changes: 24 additions & 30 deletions packages/artifact/src/internal/upload/upload-zip-specification.ts
View file
Open in desktop
Expand Up @@ -5,14 +5,19 @@ import {validateFilePath} from './path-and-artifact-name-validation'

export interface UploadZipSpecification {
/**
* An absolute source path that points to a file that will be added to a zip. Null if creating a new directory
* An absolute source path that points to a file that will be added to a zip.
*/
sourcePath: string | null
sourcePath: string

/**
* The destination path in a zip for a file
*/
destinationPath: string

/**
* Information about the file on disk
*/
stats: fs.Stats
}

/**
Expand Down Expand Up @@ -75,37 +80,26 @@ export function getUploadZipSpecification(
- file3.txt
*/
for (let file of filesToZip) {
if (!fs.existsSync(file)) {
throw new Error(`File ${file} does not exist`)
const stats = fs.statSync(file, {throwIfNoEntry: false})
if (!stats) throw new Error(`File ${file} does not exist`)
// Normalize and resolve, this allows for either absolute or relative paths to be used
file = normalize(file)
file = resolve(file)
if (!file.startsWith(rootDirectory)) {
throw new Error(
`The rootDirectory: ${rootDirectory} is not a parent directory of the file: ${file}`
)
}
if (!fs.statSync(file).isDirectory()) {
// Normalize and resolve, this allows for either absolute or relative paths to be used
file = normalize(file)
file = resolve(file)
if (!file.startsWith(rootDirectory)) {
throw new Error(
`The rootDirectory: ${rootDirectory} is not a parent directory of the file: ${file}`
)
}

// Check for forbidden characters in file paths that may cause ambiguous behavior if downloaded on different file systems
const uploadPath = file.replace(rootDirectory, '')
validateFilePath(uploadPath)
// Check for forbidden characters in file paths that may cause ambiguous behavior if downloaded on different file systems
const destinationPath = file.replace(rootDirectory, '')
validateFilePath(destinationPath)

specification.push({
sourcePath: file,
destinationPath: uploadPath
})
} else {
// Empty directory
const directoryPath = file.replace(rootDirectory, '')
validateFilePath(directoryPath)

specification.push({
sourcePath: null,
destinationPath: directoryPath
})
}
specification.push({
sourcePath: file,
destinationPath,
stats
})
}
return specification
}
10 changes: 7 additions & 3 deletions packages/artifact/src/internal/upload/zip.ts
View file
Open in desktop
Expand Up @@ -42,14 +42,18 @@ export async function createZipUploadStream(
zip.on('end', zipEndCallback)

for (const file of uploadSpecification) {
if (file.sourcePath !== null) {
if (!file.stats.isDirectory()) {
// Add a normal file to the zip
zip.append(createReadStream(file.sourcePath), {
name: file.destinationPath
name: file.destinationPath,
stats: file.stats
})
} else {
// Add a directory to the zip
zip.append('', {name: file.destinationPath})
zip.append('', {
name: file.destinationPath,
stats: file.stats
})
}
}

Expand Down