From 89c429cf42057ad36de27716a3d50199492b3582 Mon Sep 17 00:00:00 2001 From: Edwin Sirko Date: Fri, 26 Jan 2024 16:00:36 -0500 Subject: [PATCH 01/13] refactored path calculation --- __tests__/fs-helper.test.ts | 4 ++-- src/fs-helper.ts | 41 +++++++++++++++++++++++++------------ src/main.ts | 17 +++------------ 3 files changed, 33 insertions(+), 29 deletions(-) diff --git a/__tests__/fs-helper.test.ts b/__tests__/fs-helper.test.ts index 68931ef..c6e8619 100644 --- a/__tests__/fs-helper.test.ts +++ b/__tests__/fs-helper.test.ts @@ -226,7 +226,7 @@ describe('bundleFilesintoDirectory', () => { fs.writeFileSync(file3, fileContent) // Bundle the files and folders into the targetDir - fsHelper.bundleFilesintoDirectory([file1, folder1], targetDir) + fsHelper.bundleFilesintoDirectory([file1, folder1]) // Check that the files and folders were copied expect(fs.existsSync(file1)).toEqual(true) @@ -244,7 +244,7 @@ describe('bundleFilesintoDirectory', () => { it('throws an error if a file or directory does not exist', () => { expect(() => { - fsHelper.bundleFilesintoDirectory(['/does/not/exist'], targetDir) + fsHelper.bundleFilesintoDirectory(['/does/not/exist']) }).toThrow('File /does/not/exist does not exist') }) }) diff --git a/src/fs-helper.ts b/src/fs-helper.ts index 5f81281..a3e8f56 100644 --- a/src/fs-helper.ts +++ b/src/fs-helper.ts @@ -29,6 +29,23 @@ export interface FileMetadata { sha256: string } +export function getConsolidatedDirectory(filePathSpec: string): { path: string, needToCleanUpDir: boolean } { + const paths: string[] = filePathSpec.split(' ') // TODO: handle files with spaces + // TODO: do check on paths to make sure they're valid and not reaching outside the space + let path = '' + let needToCleanUpDir = false + if (paths.length === 1 && isDirectory(paths[0])) { + // If the path is a single directory, we can skip the bundling step + path = paths[0] + } else { + // Otherwise, we need to bundle the files & folders into a temporary directory + path = bundleFilesintoDirectory(paths) + needToCleanUpDir = true + } + + return { path, needToCleanUpDir } +} + // Creates both a tar.gz and zip archive of the given directory and returns the paths to both archives (stored in the provided target directory) // as well as the size/sha256 hash of each file. export async function createArchives( @@ -55,7 +72,7 @@ export async function createArchives( }) archive.pipe(output) - archive.directory(distPath, false) + archive.directory(distPath, false) // TODO: make sure this doesn't include dirs that start with ., same with below archive.finalize() }) @@ -102,21 +119,19 @@ export function readFileContents(filePath: string): Buffer { return fs.readFileSync(filePath) } -export function bundleFilesintoDirectory( - files: string[], - targetDir: string = createTempDir() -): string { - for (const file of files) { - if (!fs.existsSync(file)) { - throw new Error(`File ${file} does not exist`) +export function bundleFilesintoDirectory(filePaths: string[]): string { + const targetDir: string = createTempDir() + for (const filePath of filePaths) { + if (!fs.existsSync(filePath)) { + throw new Error(`filePath ${filePath} does not exist`) } - if (isDirectory(file)) { - const targetFolder = path.join(targetDir, path.basename(file)) - fsExtra.copySync(file, targetFolder) + if (isDirectory(filePath)) { + const targetFolder = path.join(targetDir, path.basename(filePath)) // TODO: basename is probably not what we actually want here. Or is it? Maybe conflicts between dir1/dir2 and dir1/dir3/dir2 are just user error or ?? + fsExtra.copySync(filePath, targetFolder) // TODO: ignore files preceded by . } else { - const targetFile = path.join(targetDir, path.basename(file)) - fs.copyFileSync(file, targetFile) + const targetFile = path.join(targetDir, path.basename(filePath)) + fs.copyFileSync(filePath, targetFile) } } diff --git a/src/main.ts b/src/main.ts index 4fbf8bb..131e86f 100644 --- a/src/main.ts +++ b/src/main.ts @@ -41,20 +41,9 @@ export async function run(pathInput: string): Promise { const token: string = process.env.TOKEN! - // Gather & validate user input - // Paths to be included in the OCI image - // const paths: string[] = core.getInput('path').split(' ') - const paths: string[] = pathInput.split(' ') - let path = '' - - if (paths.length === 1 && fsHelper.isDirectory(paths[0])) { - // If the path is a single directory, we can skip the bundling step - path = paths[0] - } else { - // Otherwise, we need to bundle the files & folders into a temporary directory - const bundleDir = fsHelper.createTempDir() - tmpDirs.push(bundleDir) - path = fsHelper.bundleFilesintoDirectory(paths, bundleDir) + const { path, needToCleanUpDir } = fsHelper.getConsolidatedDirectory(pathInput) + if (needToCleanUpDir) { + tmpDirs.push(path) } if (!fsHelper.isActionRepo(path)) { From 6eccb75525f2e9e36b20ab8f197d3a8a6cf762a0 Mon Sep 17 00:00:00 2001 From: Edwin Sirko Date: Fri, 26 Jan 2024 16:10:55 -0500 Subject: [PATCH 02/13] fix --- src/fs-helper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fs-helper.ts b/src/fs-helper.ts index a3e8f56..7dfd22b 100644 --- a/src/fs-helper.ts +++ b/src/fs-helper.ts @@ -119,7 +119,7 @@ export function readFileContents(filePath: string): Buffer { return fs.readFileSync(filePath) } -export function bundleFilesintoDirectory(filePaths: string[]): string { +function bundleFilesintoDirectory(filePaths: string[]): string { const targetDir: string = createTempDir() for (const filePath of filePaths) { if (!fs.existsSync(filePath)) { From 6630eef689b40b9449e7216020ec3e2fe0ed7081 Mon Sep 17 00:00:00 2001 From: Edwin Sirko Date: Fri, 26 Jan 2024 16:33:11 -0500 Subject: [PATCH 03/13] tests --- __tests__/fs-helper.test.ts | 2 ++ __tests__/main.test.ts | 12 ++++++------ src/fs-helper.ts | 13 ++++++++----- src/main.ts | 9 +++++---- 4 files changed, 21 insertions(+), 15 deletions(-) diff --git a/__tests__/fs-helper.test.ts b/__tests__/fs-helper.test.ts index c6e8619..3be6e62 100644 --- a/__tests__/fs-helper.test.ts +++ b/__tests__/fs-helper.test.ts @@ -197,6 +197,7 @@ describe('removeDir', () => { }) }) +/* describe('bundleFilesintoDirectory', () => { let sourceDir: string let targetDir: string @@ -248,3 +249,4 @@ describe('bundleFilesintoDirectory', () => { }).toThrow('File /does/not/exist does not exist') }) }) +*/ \ No newline at end of file diff --git a/__tests__/main.test.ts b/__tests__/main.test.ts index 34b94f8..5c16296 100644 --- a/__tests__/main.test.ts +++ b/__tests__/main.test.ts @@ -23,7 +23,7 @@ let createTempDirMock: jest.SpyInstance let isDirectoryMock: jest.SpyInstance let createArchivesMock: jest.SpyInstance let removeDirMock: jest.SpyInstance -let bundleFilesintoDirectoryMock: jest.SpyInstance +let getConsolidatedDirectoryMock: jest.SpyInstance let isActionRepoMock: jest.SpyInstance // Mock the GHCR Client @@ -47,8 +47,8 @@ describe('action', () => { .spyOn(fsHelper, 'createArchives') .mockImplementation() removeDirMock = jest.spyOn(fsHelper, 'removeDir').mockImplementation() - bundleFilesintoDirectoryMock = jest - .spyOn(fsHelper, 'bundleFilesintoDirectory') + getConsolidatedDirectoryMock = jest + .spyOn(fsHelper, 'getConsolidatedDirectory') .mockImplementation() isActionRepoMock = jest.spyOn(fsHelper, 'isActionRepo').mockImplementation() @@ -124,7 +124,7 @@ describe('action', () => { isDirectoryMock.mockImplementation(() => true) - bundleFilesintoDirectoryMock.mockImplementation(() => { + getConsolidatedDirectoryMock.mockImplementation(() => { throw new Error('Something went wrong') }) @@ -210,8 +210,8 @@ async function testHappyPath(version: string, path: string): Promise { isDirectoryMock.mockImplementation(() => true) isActionRepoMock.mockImplementation(() => true) - bundleFilesintoDirectoryMock.mockImplementation(() => { - return '/tmp/test' + getConsolidatedDirectoryMock.mockImplementation(() => { + return { consolidatedDirectory: '/tmp/test', needToCleanUpDir: false } // TODO: I don't understand why I have to name the variables here but not in the implementation code }) createTempDirMock.mockImplementation(() => '/tmp/test') diff --git a/src/fs-helper.ts b/src/fs-helper.ts index 7dfd22b..8bf855f 100644 --- a/src/fs-helper.ts +++ b/src/fs-helper.ts @@ -29,21 +29,24 @@ export interface FileMetadata { sha256: string } -export function getConsolidatedDirectory(filePathSpec: string): { path: string, needToCleanUpDir: boolean } { +export function getConsolidatedDirectory(filePathSpec: string): { + consolidatedPath: string + needToCleanUpDir: boolean +} { const paths: string[] = filePathSpec.split(' ') // TODO: handle files with spaces // TODO: do check on paths to make sure they're valid and not reaching outside the space - let path = '' + let consolidatedPath = '' let needToCleanUpDir = false if (paths.length === 1 && isDirectory(paths[0])) { // If the path is a single directory, we can skip the bundling step - path = paths[0] + consolidatedPath = paths[0] } else { // Otherwise, we need to bundle the files & folders into a temporary directory - path = bundleFilesintoDirectory(paths) + consolidatedPath = bundleFilesintoDirectory(paths) needToCleanUpDir = true } - return { path, needToCleanUpDir } + return { consolidatedPath, needToCleanUpDir } } // Creates both a tar.gz and zip archive of the given directory and returns the paths to both archives (stored in the provided target directory) diff --git a/src/main.ts b/src/main.ts index 131e86f..c717690 100644 --- a/src/main.ts +++ b/src/main.ts @@ -41,12 +41,13 @@ export async function run(pathInput: string): Promise { const token: string = process.env.TOKEN! - const { path, needToCleanUpDir } = fsHelper.getConsolidatedDirectory(pathInput) + const { consolidatedPath, needToCleanUpDir } = + fsHelper.getConsolidatedDirectory(pathInput) if (needToCleanUpDir) { - tmpDirs.push(path) + tmpDirs.push(consolidatedPath) } - if (!fsHelper.isActionRepo(path)) { + if (!fsHelper.isActionRepo(consolidatedPath)) { core.setFailed( 'action.y(a)ml not found. Action packages can be created only for action repositories.' ) @@ -57,7 +58,7 @@ export async function run(pathInput: string): Promise { const archiveDir = fsHelper.createTempDir() tmpDirs.push(archiveDir) - const archives = await fsHelper.createArchives(path, archiveDir) + const archives = await fsHelper.createArchives(consolidatedPath, archiveDir) const manifest = ociContainer.createActionPackageManifest( archives.tarFile, From 3e6047108b52908d53f5370b8e1e2b7f0c3307c7 Mon Sep 17 00:00:00 2001 From: Edwin Sirko Date: Fri, 26 Jan 2024 17:45:38 -0500 Subject: [PATCH 04/13] wip --- __tests__/fs-helper.test.ts | 83 +++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/__tests__/fs-helper.test.ts b/__tests__/fs-helper.test.ts index 3be6e62..42415b3 100644 --- a/__tests__/fs-helper.test.ts +++ b/__tests__/fs-helper.test.ts @@ -6,6 +6,89 @@ import { execSync } from 'child_process' const fileContent = 'This is the content of the file' +describe('getConsolidatedDirectory', () => { + let sourceDir: string + + beforeAll(() => { + sourceDir = fsHelper.createTempDir() + fs.mkdirSync(`${sourceDir}/folder1`) + fs.mkdirSync(`${sourceDir}/folder2`) + fs.mkdirSync(`${sourceDir}/folder2/folder3`) + fs.writeFileSync(`${sourceDir}/file0.txt`, fileContent) + fs.writeFileSync(`${sourceDir}/folder1/file1.txt`, fileContent) + fs.writeFileSync(`${sourceDir}/folder2/file2.txt`, fileContent) + fs.writeFileSync(`${sourceDir}/folder2/folder3/file3.txt`, fileContent) + }) + + beforeEach(() => { + //tmpDir = fsHelper.createTempDir() + }) + + afterEach(() => { + //fs.rmSync(tmpDir, { recursive: true }) + }) + + afterAll(() => { + fs.rmSync(sourceDir, { recursive: true }) + }) + + + if ("returns the directory itself if it is a single directory, and don't clean it up", () => { + + const { consolidatedPath, needToCleanUpDir } = fsHelper.getConsolidatedDirectory(".") + + expect(needToCleanUpDir).toBe(false) + expect(consolidatedPath).toBe(sourceDir) + expect(fs.existsSync(consolidatedPath)).toBe(true) + expect(fs.existsSync(path.join(consolidatedPath, `folder1`))).toBe(true) + + //TODO: continue here + }) + if ("returns a new directory containing copies of the multiple paths if they are legally specified, and instruct to clean it up", () => { + }) + if ("throws an error for illegal path spec", () => { + }) + +/* + it('bundles files and folders into a directory', () => { + // Create some test files and folders in the sourceDir + const file1 = `${sourceDir}/file1.txt` + const folder1 = `${sourceDir}/folder1` + const file2 = `${folder1}/file2.txt` + const folder2 = `${folder1}/folder2` + const file3 = `${folder2}/file3.txt` + + fs.mkdirSync(folder1) + fs.mkdirSync(folder2) + fs.writeFileSync(file1, fileContent) + fs.writeFileSync(file2, fileContent) + fs.writeFileSync(file3, fileContent) + + // Bundle the files and folders into the targetDir + fsHelper.bundleFilesintoDirectory([file1, folder1]) + + // Check that the files and folders were copied + expect(fs.existsSync(file1)).toEqual(true) + expect(fsHelper.readFileContents(file1).toString()).toEqual(fileContent) + + expect(fs.existsSync(`${targetDir}/folder1`)).toEqual(true) + + expect(fs.existsSync(file2)).toEqual(true) + expect(fsHelper.readFileContents(file2).toString()).toEqual(fileContent) + + expect(fs.existsSync(`${targetDir}/folder1/folder2`)).toEqual(true) + expect(fs.existsSync(file3)).toEqual(true) + expect(fsHelper.readFileContents(file3).toString()).toEqual(fileContent) + }) + */ + + it('throws an error if a file or directory does not exist', () => { + expect(() => { + fsHelper.bundleFilesintoDirectory(['/does/not/exist']) + }).toThrow('File /does/not/exist does not exist') + }) +}) + describe('createArchives', () => { let tmpDir: string let distDir: string From d7ee68529102fb13dc59c912f7abe51a2147b572 Mon Sep 17 00:00:00 2001 From: Edwin Sirko Date: Fri, 26 Jan 2024 18:09:48 -0500 Subject: [PATCH 05/13] wip --- __tests__/fs-helper.test.ts | 48 ++++++++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/__tests__/fs-helper.test.ts b/__tests__/fs-helper.test.ts index 42415b3..bc13d95 100644 --- a/__tests__/fs-helper.test.ts +++ b/__tests__/fs-helper.test.ts @@ -10,7 +10,7 @@ describe('getConsolidatedDirectory', () => { let sourceDir: string beforeAll(() => { - sourceDir = fsHelper.createTempDir() + sourceDir = `.`// fsHelper.createTempDir() fs.mkdirSync(`${sourceDir}/folder1`) fs.mkdirSync(`${sourceDir}/folder2`) fs.mkdirSync(`${sourceDir}/folder2/folder3`) @@ -21,32 +21,56 @@ describe('getConsolidatedDirectory', () => { }) beforeEach(() => { - //tmpDir = fsHelper.createTempDir() }) afterEach(() => { - //fs.rmSync(tmpDir, { recursive: true }) }) afterAll(() => { - fs.rmSync(sourceDir, { recursive: true }) + fs.rmSync(`file0.txt`) + fs.rmSync(`folder1`, { recursive: true }) + fs.rmSync(`folder2`, { recursive: true }) }) - if ("returns the directory itself if it is a single directory, and don't clean it up", () => { + it("returns the directory itself if it is a single directory, and instructed to not clean it up", () => { + // We're not really distinguishing between the `publish-action-package` directory and the consumer repo directory. + // In real life the consumer repo is differentiated via ... ?? const { consolidatedPath, needToCleanUpDir } = fsHelper.getConsolidatedDirectory(".") expect(needToCleanUpDir).toBe(false) - expect(consolidatedPath).toBe(sourceDir) - expect(fs.existsSync(consolidatedPath)).toBe(true) - expect(fs.existsSync(path.join(consolidatedPath, `folder1`))).toBe(true) + expect(consolidatedPath).toBe(".") + expect(fsHelper.readFileContents(`file0.txt`).toString()).toEqual(fileContent) + expect(fsHelper.readFileContents(`folder1/file1.txt`).toString()).toEqual(fileContent) + expect(fsHelper.readFileContents(`folder2/file2.txt`).toString()).toEqual(fileContent) + expect(fsHelper.readFileContents(`folder2/folder3/file3.txt`).toString()).toEqual(fileContent) - //TODO: continue here }) - if ("returns a new directory containing copies of the multiple paths if they are legally specified, and instruct to clean it up", () => { + it('returns a new directory containing copies of the multiple paths if they are legally specified, and instruct to clean it up', () => { + const { consolidatedPath, needToCleanUpDir } = fsHelper.getConsolidatedDirectory("file0.txt folder1") + + expect(needToCleanUpDir).toBe(true) + expect(consolidatedPath).not.toBe(".") + expect(fsHelper.readFileContents(path.join(consolidatedPath, `file0.txt`)).toString()).toEqual(fileContent) + expect(fsHelper.readFileContents(path.join(consolidatedPath, `folder1/file1.txt`)).toString()).toEqual(fileContent) + expect(fs.existsSync(path.join(consolidatedPath, `folder2/file2.txt`))).toEqual(false) + expect(fs.existsSync(path.join(consolidatedPath, `folder2/folder3/file3.txt`))).toEqual(false) }) - if ("throws an error for illegal path spec", () => { + + it('what happens here?', () => { + const { consolidatedPath, needToCleanUpDir } = fsHelper.getConsolidatedDirectory("folder1 folder2/folder3") + + expect(needToCleanUpDir).toBe(true) + expect(consolidatedPath).not.toBe(".") + expect(fs.existsSync(path.join(consolidatedPath, `file0.txt`))).toEqual(false) + expect(fsHelper.readFileContents(path.join(consolidatedPath, `folder1/file1.txt`)).toString()).toEqual(fileContent) + expect(fs.existsSync(path.join(consolidatedPath, `folder2/file2.txt`))).toEqual(false) + expect(fsHelper.readFileContents(path.join(consolidatedPath, `folder3/file3.txt`)).toString()).toEqual(fileContent) // <--- This is what I'm unsure of + }) + + it('throws an error for illegal path spec', () => { + // TODO: continue here }) /* @@ -82,11 +106,13 @@ describe('getConsolidatedDirectory', () => { }) */ + /* it('throws an error if a file or directory does not exist', () => { expect(() => { fsHelper.bundleFilesintoDirectory(['/does/not/exist']) }).toThrow('File /does/not/exist does not exist') }) + */ }) describe('createArchives', () => { From 31164a045a511960d8b986621b5cc54e28ac45c2 Mon Sep 17 00:00:00 2001 From: Edwin Sirko Date: Fri, 26 Jan 2024 20:44:17 -0500 Subject: [PATCH 06/13] tests almost there --- __tests__/fs-helper.test.ts | 56 ++++++++----------------------------- 1 file changed, 11 insertions(+), 45 deletions(-) diff --git a/__tests__/fs-helper.test.ts b/__tests__/fs-helper.test.ts index bc13d95..8d47ebc 100644 --- a/__tests__/fs-helper.test.ts +++ b/__tests__/fs-helper.test.ts @@ -34,8 +34,7 @@ describe('getConsolidatedDirectory', () => { it("returns the directory itself if it is a single directory, and instructed to not clean it up", () => { - - // We're not really distinguishing between the `publish-action-package` directory and the consumer repo directory. + // TODO: We're not really distinguishing between the `publish-action-package` directory and the consumer repo directory. // In real life the consumer repo is differentiated via ... ?? const { consolidatedPath, needToCleanUpDir } = fsHelper.getConsolidatedDirectory(".") @@ -66,53 +65,20 @@ describe('getConsolidatedDirectory', () => { expect(fs.existsSync(path.join(consolidatedPath, `file0.txt`))).toEqual(false) expect(fsHelper.readFileContents(path.join(consolidatedPath, `folder1/file1.txt`)).toString()).toEqual(fileContent) expect(fs.existsSync(path.join(consolidatedPath, `folder2/file2.txt`))).toEqual(false) - expect(fsHelper.readFileContents(path.join(consolidatedPath, `folder3/file3.txt`)).toString()).toEqual(fileContent) // <--- This is what I'm unsure of + expect(fsHelper.readFileContents(path.join(consolidatedPath, `folder3/file3.txt`)).toString()).toEqual(fileContent) // <--- TODO: This is what I'm unsure of }) - it('throws an error for illegal path spec', () => { - // TODO: continue here - }) - -/* - it('bundles files and folders into a directory', () => { - // Create some test files and folders in the sourceDir - const file1 = `${sourceDir}/file1.txt` - const folder1 = `${sourceDir}/folder1` - const file2 = `${folder1}/file2.txt` - const folder2 = `${folder1}/folder2` - const file3 = `${folder2}/file3.txt` - - fs.mkdirSync(folder1) - fs.mkdirSync(folder2) - fs.writeFileSync(file1, fileContent) - fs.writeFileSync(file2, fileContent) - fs.writeFileSync(file3, fileContent) - - // Bundle the files and folders into the targetDir - fsHelper.bundleFilesintoDirectory([file1, folder1]) - - // Check that the files and folders were copied - expect(fs.existsSync(file1)).toEqual(true) - expect(fsHelper.readFileContents(file1).toString()).toEqual(fileContent) - - expect(fs.existsSync(`${targetDir}/folder1`)).toEqual(true) - - expect(fs.existsSync(file2)).toEqual(true) - expect(fsHelper.readFileContents(file2).toString()).toEqual(fileContent) - - expect(fs.existsSync(`${targetDir}/folder1/folder2`)).toEqual(true) - expect(fs.existsSync(file3)).toEqual(true) - expect(fsHelper.readFileContents(file3).toString()).toEqual(fileContent) - }) - */ - - /* - it('throws an error if a file or directory does not exist', () => { + it('throws an error for illegal path spec - single', () => { expect(() => { - fsHelper.bundleFilesintoDirectory(['/does/not/exist']) - }).toThrow('File /does/not/exist does not exist') + const { consolidatedPath, needToCleanUpDir } = fsHelper.getConsolidatedDirectory("folder4") + }).toThrow('filePath folder4 does not exist') + }) + + it('throws an error for illegal path spec - multiple', () => { + expect(() => { + const { consolidatedPath, needToCleanUpDir } = fsHelper.getConsolidatedDirectory("folder1 folder4") + }).toThrow('filePath folder4 does not exist') }) - */ }) describe('createArchives', () => { From 205229817139fc5a9f429e7f5e28dcff2dd7e729 Mon Sep 17 00:00:00 2001 From: Edwin Sirko Date: Fri, 26 Jan 2024 20:52:00 -0500 Subject: [PATCH 07/13] fix --- __tests__/fs-helper.test.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/__tests__/fs-helper.test.ts b/__tests__/fs-helper.test.ts index 8d47ebc..22d2cf4 100644 --- a/__tests__/fs-helper.test.ts +++ b/__tests__/fs-helper.test.ts @@ -34,8 +34,11 @@ describe('getConsolidatedDirectory', () => { it("returns the directory itself if it is a single directory, and instructed to not clean it up", () => { - // TODO: We're not really distinguishing between the `publish-action-package` directory and the consumer repo directory. - // In real life the consumer repo is differentiated via ... ?? + // TODO: In these tests, we're not really distinguishing between the `publish-action-package` directory and the consumer repo directory, i.e., they share the same space. + // In real life, when the consumer workflow runs, its own javascript is in ., but + // the publish-action-package's code is in ${{github.action_path}}. + // So.... I guess to emulate this, we should create a temp directory (representing the consumer repo) + // and cd there before the test starts? const { consolidatedPath, needToCleanUpDir } = fsHelper.getConsolidatedDirectory(".") expect(needToCleanUpDir).toBe(false) From 35c8ddfb58d6a857df59f8800a80f2b23e1ab813 Mon Sep 17 00:00:00 2001 From: Edwin Sirko Date: Fri, 26 Jan 2024 21:03:27 -0500 Subject: [PATCH 08/13] ok --- __tests__/fs-helper.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/__tests__/fs-helper.test.ts b/__tests__/fs-helper.test.ts index 22d2cf4..ba5bacf 100644 --- a/__tests__/fs-helper.test.ts +++ b/__tests__/fs-helper.test.ts @@ -82,6 +82,8 @@ describe('getConsolidatedDirectory', () => { const { consolidatedPath, needToCleanUpDir } = fsHelper.getConsolidatedDirectory("folder1 folder4") }).toThrow('filePath folder4 does not exist') }) + + // TODO: consider doing the thing Michael suggested where we exclude directories starting with . }) describe('createArchives', () => { From 7b797db603e6e244ebb533ca0650919c8f9e8d99 Mon Sep 17 00:00:00 2001 From: Edwin Sirko Date: Fri, 26 Jan 2024 21:06:07 -0500 Subject: [PATCH 09/13] comment --- src/fs-helper.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fs-helper.ts b/src/fs-helper.ts index 8bf855f..4f6599f 100644 --- a/src/fs-helper.ts +++ b/src/fs-helper.ts @@ -29,6 +29,7 @@ export interface FileMetadata { sha256: string } +// TODO: rename this function, it is not state-preserving, so it shouldn't just be called "get'" export function getConsolidatedDirectory(filePathSpec: string): { consolidatedPath: string needToCleanUpDir: boolean From 1219afee65437f99d2a4a566a70c0a96942bc1c0 Mon Sep 17 00:00:00 2001 From: Edwin Sirko Date: Fri, 26 Jan 2024 21:07:36 -0500 Subject: [PATCH 10/13] dist --- __tests__/fs-helper.test.ts | 93 +++++++++++++++++++++++++------------ dist/index.js | 64 +++++++++++++------------ 2 files changed, 98 insertions(+), 59 deletions(-) diff --git a/__tests__/fs-helper.test.ts b/__tests__/fs-helper.test.ts index ba5bacf..98a067a 100644 --- a/__tests__/fs-helper.test.ts +++ b/__tests__/fs-helper.test.ts @@ -10,7 +10,7 @@ describe('getConsolidatedDirectory', () => { let sourceDir: string beforeAll(() => { - sourceDir = `.`// fsHelper.createTempDir() + sourceDir = `.` // fsHelper.createTempDir() fs.mkdirSync(`${sourceDir}/folder1`) fs.mkdirSync(`${sourceDir}/folder2`) fs.mkdirSync(`${sourceDir}/folder2/folder3`) @@ -20,11 +20,9 @@ describe('getConsolidatedDirectory', () => { fs.writeFileSync(`${sourceDir}/folder2/folder3/file3.txt`, fileContent) }) - beforeEach(() => { - }) + beforeEach(() => {}) - afterEach(() => { - }) + afterEach(() => {}) afterAll(() => { fs.rmSync(`file0.txt`) @@ -32,54 +30,89 @@ describe('getConsolidatedDirectory', () => { fs.rmSync(`folder2`, { recursive: true }) }) - - it("returns the directory itself if it is a single directory, and instructed to not clean it up", () => { + it('returns the directory itself if it is a single directory, and instructed to not clean it up', () => { // TODO: In these tests, we're not really distinguishing between the `publish-action-package` directory and the consumer repo directory, i.e., they share the same space. - // In real life, when the consumer workflow runs, its own javascript is in ., but + // In real life, when the consumer workflow runs, its own javascript is in ., but // the publish-action-package's code is in ${{github.action_path}}. // So.... I guess to emulate this, we should create a temp directory (representing the consumer repo) // and cd there before the test starts? - const { consolidatedPath, needToCleanUpDir } = fsHelper.getConsolidatedDirectory(".") + const { consolidatedPath, needToCleanUpDir } = + fsHelper.getConsolidatedDirectory('.') expect(needToCleanUpDir).toBe(false) - expect(consolidatedPath).toBe(".") - expect(fsHelper.readFileContents(`file0.txt`).toString()).toEqual(fileContent) - expect(fsHelper.readFileContents(`folder1/file1.txt`).toString()).toEqual(fileContent) - expect(fsHelper.readFileContents(`folder2/file2.txt`).toString()).toEqual(fileContent) - expect(fsHelper.readFileContents(`folder2/folder3/file3.txt`).toString()).toEqual(fileContent) - + expect(consolidatedPath).toBe('.') + expect(fsHelper.readFileContents(`file0.txt`).toString()).toEqual( + fileContent + ) + expect(fsHelper.readFileContents(`folder1/file1.txt`).toString()).toEqual( + fileContent + ) + expect(fsHelper.readFileContents(`folder2/file2.txt`).toString()).toEqual( + fileContent + ) + expect( + fsHelper.readFileContents(`folder2/folder3/file3.txt`).toString() + ).toEqual(fileContent) }) it('returns a new directory containing copies of the multiple paths if they are legally specified, and instruct to clean it up', () => { - const { consolidatedPath, needToCleanUpDir } = fsHelper.getConsolidatedDirectory("file0.txt folder1") + const { consolidatedPath, needToCleanUpDir } = + fsHelper.getConsolidatedDirectory('file0.txt folder1') expect(needToCleanUpDir).toBe(true) - expect(consolidatedPath).not.toBe(".") - expect(fsHelper.readFileContents(path.join(consolidatedPath, `file0.txt`)).toString()).toEqual(fileContent) - expect(fsHelper.readFileContents(path.join(consolidatedPath, `folder1/file1.txt`)).toString()).toEqual(fileContent) - expect(fs.existsSync(path.join(consolidatedPath, `folder2/file2.txt`))).toEqual(false) - expect(fs.existsSync(path.join(consolidatedPath, `folder2/folder3/file3.txt`))).toEqual(false) + expect(consolidatedPath).not.toBe('.') + expect( + fsHelper + .readFileContents(path.join(consolidatedPath, `file0.txt`)) + .toString() + ).toEqual(fileContent) + expect( + fsHelper + .readFileContents(path.join(consolidatedPath, `folder1/file1.txt`)) + .toString() + ).toEqual(fileContent) + expect( + fs.existsSync(path.join(consolidatedPath, `folder2/file2.txt`)) + ).toEqual(false) + expect( + fs.existsSync(path.join(consolidatedPath, `folder2/folder3/file3.txt`)) + ).toEqual(false) }) it('what happens here?', () => { - const { consolidatedPath, needToCleanUpDir } = fsHelper.getConsolidatedDirectory("folder1 folder2/folder3") + const { consolidatedPath, needToCleanUpDir } = + fsHelper.getConsolidatedDirectory('folder1 folder2/folder3') expect(needToCleanUpDir).toBe(true) - expect(consolidatedPath).not.toBe(".") - expect(fs.existsSync(path.join(consolidatedPath, `file0.txt`))).toEqual(false) - expect(fsHelper.readFileContents(path.join(consolidatedPath, `folder1/file1.txt`)).toString()).toEqual(fileContent) - expect(fs.existsSync(path.join(consolidatedPath, `folder2/file2.txt`))).toEqual(false) - expect(fsHelper.readFileContents(path.join(consolidatedPath, `folder3/file3.txt`)).toString()).toEqual(fileContent) // <--- TODO: This is what I'm unsure of + expect(consolidatedPath).not.toBe('.') + expect(fs.existsSync(path.join(consolidatedPath, `file0.txt`))).toEqual( + false + ) + expect( + fsHelper + .readFileContents(path.join(consolidatedPath, `folder1/file1.txt`)) + .toString() + ).toEqual(fileContent) + expect( + fs.existsSync(path.join(consolidatedPath, `folder2/file2.txt`)) + ).toEqual(false) + expect( + fsHelper + .readFileContents(path.join(consolidatedPath, `folder3/file3.txt`)) + .toString() + ).toEqual(fileContent) // <--- TODO: This is what I'm unsure of }) it('throws an error for illegal path spec - single', () => { expect(() => { - const { consolidatedPath, needToCleanUpDir } = fsHelper.getConsolidatedDirectory("folder4") + const { consolidatedPath, needToCleanUpDir } = + fsHelper.getConsolidatedDirectory('folder4') }).toThrow('filePath folder4 does not exist') }) it('throws an error for illegal path spec - multiple', () => { expect(() => { - const { consolidatedPath, needToCleanUpDir } = fsHelper.getConsolidatedDirectory("folder1 folder4") + const { consolidatedPath, needToCleanUpDir } = + fsHelper.getConsolidatedDirectory('folder1 folder4') }).toThrow('filePath folder4 does not exist') }) @@ -329,4 +362,4 @@ describe('bundleFilesintoDirectory', () => { }).toThrow('File /does/not/exist does not exist') }) }) -*/ \ No newline at end of file +*/ diff --git a/dist/index.js b/dist/index.js index 379212e..4978efb 100644 --- a/dist/index.js +++ b/dist/index.js @@ -74693,7 +74693,7 @@ var __importDefault = (this && this.__importDefault) || function (mod) { return (mod && mod.__esModule) ? mod : { "default": mod }; }; Object.defineProperty(exports, "__esModule", ({ value: true })); -exports.bundleFilesintoDirectory = exports.readFileContents = exports.isActionRepo = exports.isDirectory = exports.createArchives = exports.removeDir = exports.createTempDir = void 0; +exports.readFileContents = exports.isActionRepo = exports.isDirectory = exports.createArchives = exports.getConsolidatedDirectory = exports.removeDir = exports.createTempDir = void 0; const fs = __importStar(__nccwpck_require__(57147)); const fs_extra_1 = __importDefault(__nccwpck_require__(5630)); const path = __importStar(__nccwpck_require__(71017)); @@ -74716,6 +74716,24 @@ function removeDir(dir) { } } exports.removeDir = removeDir; +// TODO: rename this function, it is not state-preserving, so it shouldn't just be called "get'" +function getConsolidatedDirectory(filePathSpec) { + const paths = filePathSpec.split(' '); // TODO: handle files with spaces + // TODO: do check on paths to make sure they're valid and not reaching outside the space + let consolidatedPath = ''; + let needToCleanUpDir = false; + if (paths.length === 1 && isDirectory(paths[0])) { + // If the path is a single directory, we can skip the bundling step + consolidatedPath = paths[0]; + } + else { + // Otherwise, we need to bundle the files & folders into a temporary directory + consolidatedPath = bundleFilesintoDirectory(paths); + needToCleanUpDir = true; + } + return { consolidatedPath, needToCleanUpDir }; +} +exports.getConsolidatedDirectory = getConsolidatedDirectory; // Creates both a tar.gz and zip archive of the given directory and returns the paths to both archives (stored in the provided target directory) // as well as the size/sha256 hash of each file. async function createArchives(distPath, archiveTargetPath = createTempDir()) { @@ -74734,7 +74752,7 @@ async function createArchives(distPath, archiveTargetPath = createTempDir()) { resolve(fileMetadata(zipPath)); }); archive.pipe(output); - archive.directory(distPath, false); + archive.directory(distPath, false); // TODO: make sure this doesn't include dirs that start with ., same with below archive.finalize(); }); const createTarPromise = new Promise((resolve, reject) => { @@ -74773,23 +74791,23 @@ function readFileContents(filePath) { return fs.readFileSync(filePath); } exports.readFileContents = readFileContents; -function bundleFilesintoDirectory(files, targetDir = createTempDir()) { - for (const file of files) { - if (!fs.existsSync(file)) { - throw new Error(`File ${file} does not exist`); +function bundleFilesintoDirectory(filePaths) { + const targetDir = createTempDir(); + for (const filePath of filePaths) { + if (!fs.existsSync(filePath)) { + throw new Error(`filePath ${filePath} does not exist`); } - if (isDirectory(file)) { - const targetFolder = path.join(targetDir, path.basename(file)); - fs_extra_1.default.copySync(file, targetFolder); + if (isDirectory(filePath)) { + const targetFolder = path.join(targetDir, path.basename(filePath)); // TODO: basename is probably not what we actually want here. Or is it? Maybe conflicts between dir1/dir2 and dir1/dir3/dir2 are just user error or ?? + fs_extra_1.default.copySync(filePath, targetFolder); // TODO: ignore files preceded by . } else { - const targetFile = path.join(targetDir, path.basename(file)); - fs.copyFileSync(file, targetFile); + const targetFile = path.join(targetDir, path.basename(filePath)); + fs.copyFileSync(filePath, targetFile); } } return targetDir; } -exports.bundleFilesintoDirectory = bundleFilesintoDirectory; // Converts a file path to a filemetadata object by querying the fs for relevant metadata. async function fileMetadata(filePath) { const stats = fs.statSync(filePath); @@ -74987,7 +75005,6 @@ Object.defineProperty(exports, "__esModule", ({ value: true })); const main_1 = __nccwpck_require__(70399); const minimist_1 = __importDefault(__nccwpck_require__(35871)); const path = (0, minimist_1.default)(process.argv.slice(2)).path || '.'; -console.log(path); // eslint-disable-next-line @typescript-eslint/no-floating-promises (0, main_1.run)(path); @@ -75061,29 +75078,18 @@ async function run(pathInput) { return; } const token = process.env.TOKEN; - // Gather & validate user input - // Paths to be included in the OCI image - // const paths: string[] = core.getInput('path').split(' ') - const paths = pathInput.split(' '); - let path = ''; - if (paths.length === 1 && fsHelper.isDirectory(paths[0])) { - // If the path is a single directory, we can skip the bundling step - path = paths[0]; + const { consolidatedPath, needToCleanUpDir } = fsHelper.getConsolidatedDirectory(pathInput); + if (needToCleanUpDir) { + tmpDirs.push(consolidatedPath); } - else { - // Otherwise, we need to bundle the files & folders into a temporary directory - const bundleDir = fsHelper.createTempDir(); - tmpDirs.push(bundleDir); - path = fsHelper.bundleFilesintoDirectory(paths, bundleDir); - } - if (!fsHelper.isActionRepo(path)) { + if (!fsHelper.isActionRepo(consolidatedPath)) { core.setFailed('action.y(a)ml not found. Action packages can be created only for action repositories.'); return; } // Create a temporary directory to store the archives const archiveDir = fsHelper.createTempDir(); tmpDirs.push(archiveDir); - const archives = await fsHelper.createArchives(path, archiveDir); + const archives = await fsHelper.createArchives(consolidatedPath, archiveDir); const manifest = ociContainer.createActionPackageManifest(archives.tarFile, archives.zipFile, repository, targetVersion.raw, new Date()); // Generate SHA-256 hash of the manifest const manifestSHA = crypto_1.default.createHash('sha256'); From d7d99939bdf96bb7126fe10e49c579103ad83a2f Mon Sep 17 00:00:00 2001 From: Edwin Sirko Date: Fri, 26 Jan 2024 21:42:28 -0500 Subject: [PATCH 11/13] lint --- __tests__/fs-helper.test.ts | 60 ++----------------------------------- 1 file changed, 2 insertions(+), 58 deletions(-) diff --git a/__tests__/fs-helper.test.ts b/__tests__/fs-helper.test.ts index 98a067a..d9da52c 100644 --- a/__tests__/fs-helper.test.ts +++ b/__tests__/fs-helper.test.ts @@ -104,15 +104,13 @@ describe('getConsolidatedDirectory', () => { it('throws an error for illegal path spec - single', () => { expect(() => { - const { consolidatedPath, needToCleanUpDir } = - fsHelper.getConsolidatedDirectory('folder4') + fsHelper.getConsolidatedDirectory('folder4') }).toThrow('filePath folder4 does not exist') }) it('throws an error for illegal path spec - multiple', () => { expect(() => { - const { consolidatedPath, needToCleanUpDir } = - fsHelper.getConsolidatedDirectory('folder1 folder4') + fsHelper.getConsolidatedDirectory('folder1 folder4') }).toThrow('filePath folder4 does not exist') }) @@ -309,57 +307,3 @@ describe('removeDir', () => { expect(fs.existsSync(dir)).toEqual(false) }) }) - -/* -describe('bundleFilesintoDirectory', () => { - let sourceDir: string - let targetDir: string - - beforeEach(() => { - sourceDir = fsHelper.createTempDir() - targetDir = fsHelper.createTempDir() - }) - - afterEach(() => { - fs.rmSync(sourceDir, { recursive: true }) - fs.rmSync(targetDir, { recursive: true }) - }) - - it('bundles files and folders into a directory', () => { - // Create some test files and folders in the sourceDir - const file1 = `${sourceDir}/file1.txt` - const folder1 = `${sourceDir}/folder1` - const file2 = `${folder1}/file2.txt` - const folder2 = `${folder1}/folder2` - const file3 = `${folder2}/file3.txt` - - fs.mkdirSync(folder1) - fs.mkdirSync(folder2) - fs.writeFileSync(file1, fileContent) - fs.writeFileSync(file2, fileContent) - fs.writeFileSync(file3, fileContent) - - // Bundle the files and folders into the targetDir - fsHelper.bundleFilesintoDirectory([file1, folder1]) - - // Check that the files and folders were copied - expect(fs.existsSync(file1)).toEqual(true) - expect(fsHelper.readFileContents(file1).toString()).toEqual(fileContent) - - expect(fs.existsSync(`${targetDir}/folder1`)).toEqual(true) - - expect(fs.existsSync(file2)).toEqual(true) - expect(fsHelper.readFileContents(file2).toString()).toEqual(fileContent) - - expect(fs.existsSync(`${targetDir}/folder1/folder2`)).toEqual(true) - expect(fs.existsSync(file3)).toEqual(true) - expect(fsHelper.readFileContents(file3).toString()).toEqual(fileContent) - }) - - it('throws an error if a file or directory does not exist', () => { - expect(() => { - fsHelper.bundleFilesintoDirectory(['/does/not/exist']) - }).toThrow('File /does/not/exist does not exist') - }) -}) -*/ From 10fbfab203c03652d4b349aacd157b182040f1d7 Mon Sep 17 00:00:00 2001 From: Edwin Sirko Date: Fri, 26 Jan 2024 21:54:16 -0500 Subject: [PATCH 12/13] fix --- __tests__/main.test.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/__tests__/main.test.ts b/__tests__/main.test.ts index 5c16296..c7d940d 100644 --- a/__tests__/main.test.ts +++ b/__tests__/main.test.ts @@ -154,7 +154,10 @@ describe('action', () => { return '' }) - isDirectoryMock.mockImplementation(() => true) + // isDirectoryMock.mockImplementation(() => true) + getConsolidatedDirectoryMock.mockImplementation(() => { + return { consolidatedDirectory: '/tmp/test', needToCleanUpDir: false } + }) isActionRepoMock.mockImplementation(() => true) createTempDirMock.mockImplementation(() => '/tmp/test') @@ -167,7 +170,7 @@ describe('action', () => { await main.run('directory') // Check the results - expect(isDirectoryMock).toHaveBeenCalledWith('directory') + expect(getConsolidatedDirectoryMock).toHaveBeenCalledTimes(1) expect(setFailedMock).toHaveBeenCalledWith('Something went wrong') // Expect the files to be cleaned up From b9af78dd4e05df65ff39168f2e7aa648618b7044 Mon Sep 17 00:00:00 2001 From: Edwin Sirko Date: Fri, 26 Jan 2024 21:54:49 -0500 Subject: [PATCH 13/13] fix --- __tests__/main.test.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/__tests__/main.test.ts b/__tests__/main.test.ts index c7d940d..d550439 100644 --- a/__tests__/main.test.ts +++ b/__tests__/main.test.ts @@ -20,7 +20,6 @@ let setOutputMock: jest.SpyInstance // Mock the filesystem helper let createTempDirMock: jest.SpyInstance -let isDirectoryMock: jest.SpyInstance let createArchivesMock: jest.SpyInstance let removeDirMock: jest.SpyInstance let getConsolidatedDirectoryMock: jest.SpyInstance @@ -42,7 +41,6 @@ describe('action', () => { createTempDirMock = jest .spyOn(fsHelper, 'createTempDir') .mockImplementation() - isDirectoryMock = jest.spyOn(fsHelper, 'isDirectory').mockImplementation() createArchivesMock = jest .spyOn(fsHelper, 'createArchives') .mockImplementation() @@ -122,8 +120,6 @@ describe('action', () => { return '' }) - isDirectoryMock.mockImplementation(() => true) - getConsolidatedDirectoryMock.mockImplementation(() => { throw new Error('Something went wrong') }) @@ -154,7 +150,6 @@ describe('action', () => { return '' }) - // isDirectoryMock.mockImplementation(() => true) getConsolidatedDirectoryMock.mockImplementation(() => { return { consolidatedDirectory: '/tmp/test', needToCleanUpDir: false } }) @@ -210,7 +205,6 @@ async function testHappyPath(version: string, path: string): Promise { return '' }) - isDirectoryMock.mockImplementation(() => true) isActionRepoMock.mockImplementation(() => true) getConsolidatedDirectoryMock.mockImplementation(() => {