diff --git a/packages/cli/changelog/@unreleased/pr-55.v2.yml b/packages/cli/changelog/@unreleased/pr-55.v2.yml new file mode 100644 index 000000000..069fa65a0 --- /dev/null +++ b/packages/cli/changelog/@unreleased/pr-55.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: Address minor issues + links: + - https://github.com/palantir/osdk-ts/pull/55 diff --git a/packages/cli/package.json b/packages/cli/package.json index b918c6731..b09762b99 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -38,12 +38,14 @@ "consola": "^3.2.3", "find-up": "^6.3.0", "open": "^9.1.0", + "semver": "^7.6.0", "yargs": "^17.7.2" }, "devDependencies": { "@types/archiver": "^6.0.0", "@types/ngeohash": "^0.6.4", "@types/node": "^18.0.0", + "@types/semver": "^7.5.7", "@types/yargs": "^17.0.29", "ts-expect": "^1.3.0", "typescript": "^5.2.2" @@ -54,7 +56,7 @@ "imports": { "#net": "./src/net/index.mts" }, - "keywords": [ ], + "keywords": [], "bin": { "osdk": "./bin/osdk.mjs" }, diff --git a/packages/cli/src/commands/site/version/list/versionListCommand.mts b/packages/cli/src/commands/site/version/list/versionListCommand.mts index c9af62966..e8c4edaf8 100644 --- a/packages/cli/src/commands/site/version/list/versionListCommand.mts +++ b/packages/cli/src/commands/site/version/list/versionListCommand.mts @@ -55,7 +55,10 @@ export default async function versionListCommand( } consola.success("Found versions:"); - for (const version of versions) { + + const semver = await import("semver"); + const sortedVersions = semver.rsort(versions.filter(v => semver.valid(v))); + for (const version of sortedVersions) { consola.log( ` - ${version}${ deployedVersion diff --git a/packages/cli/src/util/autoVersion.test.ts b/packages/cli/src/util/autoVersion.test.ts index 6b70c427e..a330e0e86 100644 --- a/packages/cli/src/util/autoVersion.test.ts +++ b/packages/cli/src/util/autoVersion.test.ts @@ -14,70 +14,84 @@ * limitations under the License. */ -import { execSync } from "node:child_process"; +import { exec } from "node:child_process"; +import { promisify } from "node:util"; import { describe, expect, it, vi } from "vitest"; import { autoVersion } from "./autoVersion.js"; vi.mock("node:child_process"); +const execAsync = promisify(exec); describe("autoVersion", () => { - const execSyncMock = vi.mocked(execSync); + const execMock = vi.mocked(execAsync); + const execReturnValue = (out: string) => ({ stdout: out, stderr: "" }); it("should return a valid SemVer version from git describe", async () => { const validGitVersion = "1.2.3"; - execSyncMock.mockReturnValue(validGitVersion); + execMock.mockResolvedValue(execReturnValue(validGitVersion)); const version = await autoVersion(); expect(version).toBe("1.2.3"); - - expect(execSyncMock).toHaveBeenCalledWith( - "git describe --tags --first-parent --dirty", - { encoding: "utf8" }, - ); }); it("should replace default prefix v from git describe output", async () => { const validGitVersion = "v1.2.3"; - execSyncMock.mockReturnValue(validGitVersion); - const version = await autoVersion(); + execMock.mockResolvedValue(execReturnValue(validGitVersion)); + const version = await autoVersion(); expect(version).toBe("1.2.3"); - expect(execSyncMock).toHaveBeenCalledWith( - "git describe --tags --first-parent --dirty", - { encoding: "utf8" }, - ); }); it("should replace the prefix from the found git tag", async () => { const validGitVersion = "@package@1.2.3"; - execSyncMock.mockReturnValue(validGitVersion); + execMock.mockResolvedValue(execReturnValue(validGitVersion)); const version = await autoVersion("@package@"); - expect(version).toBe("1.2.3"); - expect(execSyncMock).toHaveBeenCalledWith( - "git describe --tags --first-parent --dirty --match=\"@package@*\"", - { encoding: "utf8" }, - ); }); it("should only replace the prefix if found at the start of the tag only", async () => { const validGitVersion = "1.2.3-package"; - execSyncMock.mockReturnValue(validGitVersion); + execMock.mockResolvedValue(execReturnValue(validGitVersion)); const version = await autoVersion("-package"); - expect(version).toBe("1.2.3-package"); - expect(execSyncMock).toHaveBeenCalledWith( - "git describe --tags --first-parent --dirty --match=\"-package*\"", - { encoding: "utf8" }, - ); }); it("should throw an error if git describe returns a non-SemVer string", async () => { const nonSemVerGitVersion = "not-semver"; - execSyncMock.mockReturnValue(nonSemVerGitVersion); + execMock.mockResolvedValue(execReturnValue(nonSemVerGitVersion)); + + await expect(autoVersion()).rejects.toThrowError(); + }); - await expect(autoVersion()).rejects.toThrow(); + it("should throw an error if git isn't found", async () => { + execMock.mockImplementation(() => { + throw new Error("Command not found"); + }); + + await expect(autoVersion()).rejects.toThrowError( + "Unable to determine the version using git-describe as git is not installed", + ); + }); + + it("should throw an error if the current directory is not a git repository", async () => { + execMock.mockImplementation(() => { + throw new Error("fatal: not a git repository"); + }); + + await expect(autoVersion()).rejects.toThrowError( + "Unable to determine the version using git-describe as the current directory is not a git repository", + ); + }); + + it("should throw an error if no git tags are found", async () => { + execMock.mockImplementation(() => { + throw new Error("fatal: no names found, cannot describe anything."); + }); + + await expect(autoVersion()).rejects.toThrowError( + "Unable to determine the version using git-describe as no matching tags were found.", + ); }); }); diff --git a/packages/cli/src/util/autoVersion.ts b/packages/cli/src/util/autoVersion.ts index 0a65ec012..1cb5197ae 100644 --- a/packages/cli/src/util/autoVersion.ts +++ b/packages/cli/src/util/autoVersion.ts @@ -14,7 +14,8 @@ * limitations under the License. */ -import { execSync } from "node:child_process"; +import { exec } from "node:child_process"; +import { promisify } from "node:util"; import { ExitProcessError } from "../ExitProcessError.js"; import { isValidSemver } from "./isValidSemver.js"; @@ -29,26 +30,71 @@ export async function autoVersion(tagPrefix: string = ""): Promise { const [matchPrefix, prefixRegex] = tagPrefix !== "" ? [tagPrefix, new RegExp(`^${tagPrefix}`)] : [undefined, new RegExp(`^v?`)]; + + const gitVersion = await gitDescribe(matchPrefix); + const version = gitVersion.trim().replace(prefixRegex, ""); + if (!isValidSemver(version)) { + throw new ExitProcessError( + 2, + `The version string ${version} is not SemVer compliant.`, + ); + } + + return version; +} + +async function gitDescribe(matchPrefix: string | undefined): Promise { + let gitVersion; try { - const gitVersion = execSync( + const execAsync = promisify(exec); + const { stdout } = await execAsync( `git describe --tags --first-parent --dirty${ matchPrefix != null ? ` --match="${matchPrefix}*"` : "" }`, { encoding: "utf8" }, ); - const version = gitVersion.trim().replace(prefixRegex, ""); - if (!isValidSemver(version)) { - throw new ExitProcessError( - 2, - `The version string ${version} is not SemVer compliant.`, - ); + gitVersion = stdout; + } catch (error: any) { + if (error instanceof Error) { + const errorMessage: string = error.message.toLowerCase(); + + if ( + errorMessage.includes("not recognized") + || errorMessage.includes("command not found") + || errorMessage.includes("no such file or directory") + ) { + throw new ExitProcessError( + 2, + `Unable to determine the version using git-describe as git is not installed or found in the PATH.\nPlease correctly configure git or supply a --version option.`, + ); + } + + if ( + errorMessage.includes("fatal: not a git repository") + ) { + throw new ExitProcessError( + 2, + `Unable to determine the version using git-describe as the current directory is not a git repository.\nPlease run the command in a git respository or supply a --version option.`, + ); + } + + if ( + errorMessage.includes( + "fatal: no names found, cannot describe anything.", + ) + ) { + throw new ExitProcessError( + 2, + `Unable to determine the version using git-describe as no matching tags were found.\nPlease tag a matching version or supply a --version option.`, + ); + } } - return version; - } catch (error) { throw new ExitProcessError( 2, - `Unable to determine the version automatically. Please supply a --version argument. ${error}`, + `Unable to determine the version automatically: ${error}.\nPlease supply a --version option.`, ); } + + return gitVersion; } diff --git a/packages/cli/src/util/config.test.ts b/packages/cli/src/util/config.test.ts index 5f8dcbaf8..7d73bd9e3 100644 --- a/packages/cli/src/util/config.test.ts +++ b/packages/cli/src/util/config.test.ts @@ -16,14 +16,19 @@ import { findUp } from "find-up"; import { promises as fsPromises } from "node:fs"; -import { describe, expect, it, vi } from "vitest"; +import { extname } from "node:path"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { loadFoundryConfig } from "./config.js"; vi.mock("find-up"); vi.mock("node:fs"); +vi.mock("node:path"); describe("loadFoundryConfig", () => { - vi.mocked(findUp).mockResolvedValue("/path/foundry.config.json"); + beforeEach(() => { + vi.mocked(findUp).mockResolvedValue("/path/foundry.config.json"); + vi.mocked(extname).mockReturnValue(".json"); + }); it("should load and parse the configuration file correctly", async () => { const correctConfig = { @@ -149,4 +154,11 @@ describe("loadFoundryConfig", () => { vi.mocked(findUp).mockResolvedValue(undefined); await expect(loadFoundryConfig()).resolves.toBeUndefined(); }); + + it("should throw if config file extension isn't supported", async () => { + vi.mocked(extname).mockResolvedValue(".yaml"); + await expect(loadFoundryConfig()).rejects.toThrow( + "Unsupported file extension:", + ); + }); }); diff --git a/packages/cli/src/util/config.ts b/packages/cli/src/util/config.ts index 51451d068..5669a60c3 100644 --- a/packages/cli/src/util/config.ts +++ b/packages/cli/src/util/config.ts @@ -16,6 +16,7 @@ import type { JSONSchemaType } from "ajv"; import { promises as fsPromises } from "node:fs"; +import { extname } from "node:path"; import { ExitProcessError } from "../ExitProcessError.js"; export interface GitDescribeAutoVersionConfig { @@ -94,11 +95,11 @@ export async function loadFoundryConfig(): Promise< let foundryConfig: FoundryConfig; try { const fileContent = await fsPromises.readFile(configFilePath, "utf-8"); - foundryConfig = JSON.parse(fileContent); - } catch { + foundryConfig = parseConfigFile(fileContent, configFilePath); + } catch (error) { throw new ExitProcessError( 2, - `Couldn't read or parse config file ${configFilePath}`, + `Couldn't read or parse config file ${configFilePath}. Error: ${error}`, ); } @@ -116,3 +117,19 @@ export async function loadFoundryConfig(): Promise< return undefined; } + +function parseConfigFile( + fileContent: string, + configFilePath: string, +): FoundryConfig { + const extension = extname(configFilePath); + switch (extension) { + case ".json": + return JSON.parse(fileContent); + default: + throw new ExitProcessError( + 2, + `Unsupported file extension: ${extension} for config file.`, + ); + } +} diff --git a/packages/cli/src/util/isValidSemver.ts b/packages/cli/src/util/isValidSemver.ts index be670a867..9946353ec 100644 --- a/packages/cli/src/util/isValidSemver.ts +++ b/packages/cli/src/util/isValidSemver.ts @@ -14,10 +14,8 @@ * limitations under the License. */ -// https://semver.org/#is-there-a-suggested-regular-expression-regex-to-check-a-semver-string -const semver = - /^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$/; +import { valid } from "semver"; export function isValidSemver(semverString: string): boolean { - return semver.test(semverString); + return valid(semverString) != null; } diff --git a/packages/cli/src/util/token.ts b/packages/cli/src/util/token.ts index 50817f1e7..10a20596f 100644 --- a/packages/cli/src/util/token.ts +++ b/packages/cli/src/util/token.ts @@ -74,10 +74,10 @@ interface LoadedToken { token: string; } /** - * Synchronously reads a JWT Auth Token from a file. + * Reads a JWT Auth Token from a file. * @param filePath The path to the token file. * @returns The token as a string. - * @throws An error if the file cannot be read or if the file does not contain a valid JWT. + * @throws An error if the file cannot be read. */ export async function loadTokenFile(filePath: string): Promise { let token: string; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ca1818e2b..3d3afdb63 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -323,6 +323,9 @@ importers: open: specifier: ^9.1.0 version: 9.1.0 + semver: + specifier: ^7.6.0 + version: 7.6.0 yargs: specifier: ^17.7.2 version: 17.7.2 @@ -336,6 +339,9 @@ importers: '@types/node': specifier: ^18.0.0 version: 18.17.15 + '@types/semver': + specifier: ^7.5.7 + version: 7.5.7 '@types/yargs': specifier: ^17.0.29 version: 17.0.29 @@ -1064,7 +1070,7 @@ packages: outdent: 0.5.0 prettier: 2.8.8 resolve-from: 5.0.0 - semver: 7.5.4 + semver: 7.6.0 dev: true /@changesets/assemble-release-plan@5.2.4: @@ -1075,7 +1081,7 @@ packages: '@changesets/get-dependents-graph': 1.3.6 '@changesets/types': 5.2.1 '@manypkg/get-packages': 1.1.3 - semver: 7.5.4 + semver: 7.6.0 dev: true /@changesets/changelog-git@0.1.14: @@ -1148,7 +1154,7 @@ packages: '@manypkg/get-packages': 1.1.3 chalk: 2.4.2 fs-extra: 7.0.1 - semver: 7.5.4 + semver: 7.6.0 dev: true /@changesets/get-release-plan@3.0.17: @@ -2343,6 +2349,10 @@ packages: /@types/semver@7.5.1: resolution: {integrity: sha512-cJRQXpObxfNKkFAZbJl2yjWtJCqELQIdShsogr1d2MilP8dKD9TE/nEKHkJgUNHdGKCQaf9HbIynuV2csLGVLg==} + /@types/semver@7.5.7: + resolution: {integrity: sha512-/wdoPq1QqkSj9/QOeKkFquEuPzQbHTWAMPH/PaUMB+JuR31lXhlWXRZ52IpfDYVlDOUBvX09uBrPwxGT1hjNBg==} + dev: true + /@types/set-cookie-parser@2.4.7: resolution: {integrity: sha512-+ge/loa0oTozxip6zmhRIk8Z/boU51wl9Q6QdLZcokIGMzY5lFXYy/x7Htj2HTC6/KZP1hUbZ1ekx8DYXICvWg==} dependencies: @@ -2549,7 +2559,7 @@ packages: debug: 4.3.4 globby: 11.1.0 is-glob: 4.0.3 - semver: 7.5.4 + semver: 7.6.0 ts-api-utils: 1.0.3(typescript@5.2.2) typescript: 5.2.2 transitivePeerDependencies: @@ -2570,7 +2580,7 @@ packages: debug: 4.3.4 globby: 11.1.0 is-glob: 4.0.3 - semver: 7.5.4 + semver: 7.6.0 ts-api-utils: 1.0.3(typescript@5.2.2) typescript: 5.2.2 transitivePeerDependencies: @@ -2590,7 +2600,7 @@ packages: '@typescript-eslint/types': 6.11.0 '@typescript-eslint/typescript-estree': 6.11.0(typescript@5.2.2) eslint: 8.53.0 - semver: 7.5.4 + semver: 7.6.0 transitivePeerDependencies: - supports-color - typescript @@ -2609,7 +2619,7 @@ packages: '@typescript-eslint/types': 6.13.1 '@typescript-eslint/typescript-estree': 6.13.1(typescript@5.2.2) eslint: 8.49.0 - semver: 7.5.4 + semver: 7.6.0 transitivePeerDependencies: - supports-color - typescript @@ -5440,7 +5450,7 @@ packages: dependencies: hosted-git-info: 4.1.0 is-core-module: 2.13.0 - semver: 7.5.4 + semver: 7.6.0 validate-npm-package-license: 3.0.4 dev: true @@ -6281,6 +6291,13 @@ packages: dependencies: lru-cache: 6.0.0 + /semver@7.6.0: + resolution: {integrity: sha512-EnwXhrlwXMk9gKu5/flx5sv/an57AkRplG3hTK68W7FRDN+k+OWBj65M7719OkA82XLBxrcX0KSHj+X5COhOVg==} + engines: {node: '>=10'} + hasBin: true + dependencies: + lru-cache: 6.0.0 + /set-blocking@2.0.0: resolution: {integrity: sha512-KiKBS8AnWGEyLzofFfmvKwpdPzqiy16LvQfK3yv/fVH7Bj13/wl3JSR1J+rfgRE9q7xUJK4qvgS8raSOeLUehw==} dev: true