From 81d84e1f387cfe2b82eb255f940906f2e30ffdc9 Mon Sep 17 00:00:00 2001 From: Ramon Brullo Date: Thu, 14 Dec 2023 17:48:37 +0100 Subject: [PATCH 01/18] refactor: function signatures Replace object parameters with separate parameters. This is mostly done for consistency, as most functions in the project use multiple parameters. Also it makes the function signature a lot shorter. --- src/cmd-add.ts | 26 +++++------- src/cmd-deps.ts | 24 +++++------ src/cmd-login.ts | 82 +++++++++++++----------------------- src/cmd-remove.ts | 2 +- src/cmd-search.ts | 2 +- src/cmd-view.ts | 2 +- src/registry-client.ts | 14 +++--- src/utils/env.ts | 2 +- test/test-env.ts | 37 +++++++--------- test/test-pkg-manifest-io.ts | 8 ++-- test/test-registry-client.ts | 10 +---- 11 files changed, 80 insertions(+), 129 deletions(-) diff --git a/src/cmd-add.ts b/src/cmd-add.ts index 24cf4119..c631fdb5 100644 --- a/src/cmd-add.ts +++ b/src/cmd-add.ts @@ -43,14 +43,12 @@ export const add = async function ( ): Promise { if (!Array.isArray(pkgs)) pkgs = [pkgs]; // parse env - const envOk = await parseEnv(options, { checkPath: true }); + const envOk = await parseEnv(options, true); if (!envOk) return 1; // add const results = []; for (const pkg of pkgs) - results.push( - await _add({ pkg, testables: options.test, force: options.force }) - ); + results.push(await _add(pkg, options.test, options.force)); const result: AddResult = { code: results.filter((x) => x.code != 0).length > 0 ? 1 : 0, dirty: results.filter((x) => x.dirty).length > 0, @@ -61,15 +59,11 @@ export const add = async function ( return result.code; }; -const _add = async function ({ - pkg, - testables, - force, -}: { - pkg: PackageReference; - testables?: boolean; - force?: boolean; -}): Promise { +const _add = async function ( + pkg: PackageReference, + testables?: boolean, + force?: boolean +): Promise { // dirty flag let dirty = false; // is upstream package flag @@ -160,11 +154,11 @@ const _add = async function ({ } // pkgsInScope if (!isUpstreamPackage) { - const [depsValid, depsInvalid] = await fetchPackageDependencies({ + const [depsValid, depsInvalid] = await fetchPackageDependencies( name, version, - deep: true, - }); + true + ); // add depsValid to pkgsInScope. depsValid .filter((x) => !x.upstream && !x.internal) diff --git a/src/cmd-deps.ts b/src/cmd-deps.ts index 660031ae..a7f9b1db 100644 --- a/src/cmd-deps.ts +++ b/src/cmd-deps.ts @@ -20,32 +20,28 @@ export const deps = async function ( options: DepsOptions ) { // parse env - const envOk = await parseEnv(options, { checkPath: false }); + const envOk = await parseEnv(options, false); if (!envOk) return 1; // parse name const [name, version] = splitPackageReference(pkg); // deps - await _deps({ name, version, deep: options.deep }); + await _deps(name, version, options.deep); return 0; }; -const _deps = async function ({ - name, - version, - deep, -}: { - name: DomainName; - version: VersionReference | undefined; - deep?: boolean; -}) { +const _deps = async function ( + name: DomainName, + version: VersionReference | undefined, + deep?: boolean +) { if (version !== undefined && isPackageUrl(version)) throw new Error("Cannot get dependencies for url-version"); - const [depsValid, depsInvalid] = await fetchPackageDependencies({ + const [depsValid, depsInvalid] = await fetchPackageDependencies( name, version, - deep, - }); + deep + ); depsValid .filter((x) => !x.self) .forEach((x) => diff --git a/src/cmd-login.ts b/src/cmd-login.ts index f0ff06fa..3f9a9ebd 100644 --- a/src/cmd-login.ts +++ b/src/cmd-login.ts @@ -30,7 +30,7 @@ export type LoginOptions = CmdOptions<{ export const login = async function (options: LoginOptions) { // parse env - const envOk = await parseEnv(options, { checkPath: false }); + const envOk = await parseEnv(options, false); if (!envOk) return 1; // query parameters if (!options.username) options.username = await promptUsername(); @@ -45,12 +45,12 @@ export const login = async function (options: LoginOptions) { _auth = encodeBasicAuth(options.username, options.password); } else { // npm login - const result = await npmLogin({ - username: options.username, - password: options.password, - email: options.email, - registry: options._global.registry as RegistryUrl, - }); + const result = await npmLogin( + options.username, + options.password, + options.email, + options._global.registry as RegistryUrl + ); if (result.code == 1) return result.code; if (!result.token) { log.error("auth", "can not find token from server response"); @@ -58,37 +58,29 @@ export const login = async function (options: LoginOptions) { } token = result.token; // write npm token - await writeNpmToken({ - registry: options._global.registry as RegistryUrl, - token: result.token, - }); + await writeNpmToken(options._global.registry as RegistryUrl, result.token); } // write unity token - await writeUnityToken({ + await writeUnityToken( _auth, - alwaysAuth: options.alwaysAuth || false, - basicAuth: options.basicAuth || false, - email: options.email, - registry: options._global.registry as RegistryUrl, - token, - }); + options.alwaysAuth || false, + options.basicAuth || false, + options.email, + options._global.registry as RegistryUrl, + token + ); }; /** * Return npm login token */ -const npmLogin = async function ({ - username, - password, - email, - registry, -}: { - username: string; - password: string; - email: string; - registry: RegistryUrl; -}) { +const npmLogin = async function ( + username: string, + password: string, + email: string, + registry: RegistryUrl +) { const client = getNpmClient(); try { const data = await client.adduser(registry, { @@ -120,15 +112,8 @@ const npmLogin = async function ({ /** * Write npm token to .npmrc - * @param {*} param0 */ -const writeNpmToken = async function ({ - registry, - token, -}: { - registry: RegistryUrl; - token: string; -}) { +const writeNpmToken = async function (registry: RegistryUrl, token: string) { const configPath = getNpmrcPath(); // read config let content = ""; @@ -193,21 +178,14 @@ export const generateNpmrcLines = function ( /** * Write npm token to Unity */ -const writeUnityToken = async function ({ - _auth, - alwaysAuth, - basicAuth, - email, - registry, - token, -}: { - _auth: Base64 | null; - alwaysAuth: boolean; - basicAuth: boolean; - email: string; - registry: RegistryUrl; - token: string | null; -}) { +const writeUnityToken = async function ( + _auth: Base64 | null, + alwaysAuth: boolean, + basicAuth: boolean, + email: string, + registry: RegistryUrl, + token: string | null +) { // Create config dir if necessary const configDir = await getUpmConfigDir(); // Read config file diff --git a/src/cmd-remove.ts b/src/cmd-remove.ts index ac24679e..2021ee7f 100644 --- a/src/cmd-remove.ts +++ b/src/cmd-remove.ts @@ -22,7 +22,7 @@ export const remove = async function ( ) { if (!Array.isArray(pkgs)) pkgs = [pkgs]; // parse env - const envOk = await parseEnv(options, { checkPath: true }); + const envOk = await parseEnv(options, true); if (!envOk) return 1; // remove const results = []; diff --git a/src/cmd-search.ts b/src/cmd-search.ts index 9890cab3..0960510b 100644 --- a/src/cmd-search.ts +++ b/src/cmd-search.ts @@ -115,7 +115,7 @@ const getTableRow = function (pkg: SearchedPkgInfo): TableRow { export async function search(keyword: string, options: SearchOptions) { // parse env - const envOk = await parseEnv(options, { checkPath: false }); + const envOk = await parseEnv(options, false); if (!envOk) return 1; const table = getTable(); // search endpoint diff --git a/src/cmd-view.ts b/src/cmd-view.ts index 9c7f7dd8..6b371975 100644 --- a/src/cmd-view.ts +++ b/src/cmd-view.ts @@ -19,7 +19,7 @@ export const view = async function ( options: ViewOptions ) { // parse env - const envOk = await parseEnv(options, { checkPath: false }); + const envOk = await parseEnv(options, false); if (!envOk) return 1; // parse name const [name, version] = splitPackageReference(pkg); diff --git a/src/registry-client.ts b/src/registry-client.ts index 5cbf9208..37f9e318 100644 --- a/src/registry-client.ts +++ b/src/registry-client.ts @@ -130,15 +130,11 @@ export const fetchPackageInfo = async function ( }, ... ] */ -export const fetchPackageDependencies = async function ({ - name, - version, - deep, -}: { - name: DomainName; - version: SemanticVersion | "latest" | undefined; - deep?: boolean; -}): Promise<[Dependency[], Dependency[]]> { +export const fetchPackageDependencies = async function ( + name: DomainName, + version: SemanticVersion | "latest" | undefined, + deep?: boolean +): Promise<[Dependency[], Dependency[]]> { log.verbose( "dependency", `fetch: ${packageReference(name, version)} deep=${deep}` diff --git a/src/utils/env.ts b/src/utils/env.ts index 46128ae1..9cb26cfe 100644 --- a/src/utils/env.ts +++ b/src/utils/env.ts @@ -50,7 +50,7 @@ export const env: Env = {}; // Parse env export const parseEnv = async function ( options: CmdOptions, - { checkPath }: { checkPath: unknown } + checkPath: boolean ) { // set defaults env.registry = registryUrl("https://package.openupm.com"); diff --git a/test/test-env.ts b/test/test-env.ts index 803d392e..9e4280b9 100644 --- a/test/test-env.ts +++ b/test/test-env.ts @@ -31,7 +31,7 @@ describe("env", function () { mockConsole.detach(); }); it("defaults", async function () { - (await parseEnv({ _global: {} }, { checkPath: false })).should.be.ok(); + (await parseEnv({ _global: {} }, false)).should.be.ok(); env.registry.should.equal("https://package.openupm.com"); env.upstream.should.be.ok(); env.upstreamRegistry.should.equal("https://packages.unity.com"); @@ -44,7 +44,7 @@ describe("env", function () { ( await parseEnv( { _global: { chdir: getWorkDir("test-openupm-cli") } }, - { checkPath: true } + true ) ).should.be.ok(); env.cwd.should.be.equal(getWorkDir("test-openupm-cli")); @@ -56,7 +56,7 @@ describe("env", function () { ( await parseEnv( { _global: { chdir: getWorkDir("path-not-exist") } }, - { checkPath: true } + true ) ).should.not.be.ok(); mockConsole @@ -67,7 +67,7 @@ describe("env", function () { ( await parseEnv( { _global: { chdir: getWorkDir("test-openupm-cli-no-manifest") } }, - { checkPath: true } + true ) ).should.not.be.ok(); mockConsole @@ -78,7 +78,7 @@ describe("env", function () { ( await parseEnv( { _global: { registry: "https://registry.npmjs.org" } }, - { checkPath: false } + false ) ).should.be.ok(); env.registry.should.be.equal("https://registry.npmjs.org"); @@ -88,7 +88,7 @@ describe("env", function () { ( await parseEnv( { _global: { registry: "https://registry.npmjs.org/" } }, - { checkPath: false } + false ) ).should.be.ok(); env.registry.should.be.equal("https://registry.npmjs.org"); @@ -102,7 +102,7 @@ describe("env", function () { registry: "https://registry.npmjs.org/some", }, }, - { checkPath: false } + false ) ).should.be.ok(); env.registry.should.be.equal("https://registry.npmjs.org/some"); @@ -116,7 +116,7 @@ describe("env", function () { registry: "https://registry.npmjs.org/some/", }, }, - { checkPath: false } + false ) ).should.be.ok(); env.registry.should.be.equal("https://registry.npmjs.org/some"); @@ -124,10 +124,7 @@ describe("env", function () { }); it("custom registry without http", async function () { ( - await parseEnv( - { _global: { registry: "registry.npmjs.org" } }, - { checkPath: false } - ) + await parseEnv({ _global: { registry: "registry.npmjs.org" } }, false) ).should.be.ok(); env.registry.should.be.equal("http://registry.npmjs.org"); env.namespace.should.be.equal("org.npmjs"); @@ -136,7 +133,7 @@ describe("env", function () { ( await parseEnv( { _global: { registry: "http://127.0.0.1:4873" } }, - { checkPath: false } + false ) ).should.be.ok(); env.registry.should.be.equal("http://127.0.0.1:4873"); @@ -148,31 +145,27 @@ describe("env", function () { { _global: { registry: "http://[1:2:3:4:5:6:7:8]:4873" }, }, - { checkPath: false } + false ) ).should.be.ok(); env.registry.should.be.equal("http://[1:2:3:4:5:6:7:8]:4873"); env.namespace.should.be.equal("1:2:3:4:5:6:7:8"); }); it("upstream", async function () { - ( - await parseEnv({ _global: { upstream: false } }, { checkPath: false }) - ).should.be.ok(); + (await parseEnv({ _global: { upstream: false } }, false)).should.be.ok(); env.upstream.should.not.be.ok(); }); it("editorVersion", async function () { ( await parseEnv( { _global: { chdir: getWorkDir("test-openupm-cli") } }, - { checkPath: true } + true ) ).should.be.ok(); should(env.editorVersion).be.equal("2019.2.13f1"); }); it("region cn", async function () { - ( - await parseEnv({ _global: { cn: true } }, { checkPath: false }) - ).should.be.ok(); + (await parseEnv({ _global: { cn: true } }, false)).should.be.ok(); env.registry.should.be.equal("https://package.openupm.cn"); env.upstreamRegistry.should.be.equal("https://packages.unity.cn"); env.region.should.be.equal("cn"); @@ -186,7 +179,7 @@ describe("env", function () { registry: "https://reg.custom-package.com", }, }, - { checkPath: false } + false ) ).should.be.ok(); env.registry.should.be.equal("https://reg.custom-package.com"); diff --git a/test/test-pkg-manifest-io.ts b/test/test-pkg-manifest-io.ts index 2f020b8c..cc35d69c 100644 --- a/test/test-pkg-manifest-io.ts +++ b/test/test-pkg-manifest-io.ts @@ -41,7 +41,7 @@ describe("pkg-manifest io", function () { ( await parseEnv( { _global: { chdir: getWorkDir("test-openupm-cli") } }, - { checkPath: true } + true ) ).should.be.ok(); const manifest = shouldHaveManifest(); @@ -51,7 +51,7 @@ describe("pkg-manifest io", function () { ( await parseEnv( { _global: { chdir: getWorkDir("path-not-exist") } }, - { checkPath: false } + false ) ).should.be.ok(); shouldHaveNoManifest(); @@ -61,7 +61,7 @@ describe("pkg-manifest io", function () { ( await parseEnv( { _global: { chdir: getWorkDir("test-openupm-cli-wrong-json") } }, - { checkPath: true } + true ) ).should.be.ok(); shouldHaveNoManifest(); @@ -71,7 +71,7 @@ describe("pkg-manifest io", function () { ( await parseEnv( { _global: { chdir: getWorkDir("test-openupm-cli") } }, - { checkPath: true } + true ) ).should.be.ok(); const manifest = shouldHaveManifest(); diff --git a/test/test-registry-client.ts b/test/test-registry-client.ts index da03aaaa..01c561f4 100644 --- a/test/test-registry-client.ts +++ b/test/test-registry-client.ts @@ -25,10 +25,7 @@ describe("registry-client", function () { }); it("simple", async function () { ( - await parseEnv( - { _global: { registry: exampleRegistryUrl } }, - { checkPath: false } - ) + await parseEnv({ _global: { registry: exampleRegistryUrl } }, false) ).should.be.ok(); const pkgInfoRemote = buildPackageInfo(packageA); registerRemotePkg(pkgInfoRemote); @@ -37,10 +34,7 @@ describe("registry-client", function () { }); it("404", async function () { ( - await parseEnv( - { _global: { registry: exampleRegistryUrl } }, - { checkPath: false } - ) + await parseEnv({ _global: { registry: exampleRegistryUrl } }, false) ).should.be.ok(); registerMissingPackage(packageA); From 7511a4bf4d1b0419b6d76777910504393db5b14a Mon Sep 17 00:00:00 2001 From: Ramon Brullo Date: Thu, 14 Dec 2023 17:59:34 +0100 Subject: [PATCH 02/18] refactor: move env usage up Remove env usage from pkg-manifest-io --- src/cmd-add.ts | 4 ++-- src/cmd-remove.ts | 4 ++-- src/utils/pkg-manifest-io.ts | 16 +++++++++------- test/manifest-assertions.ts | 5 +++-- test/test-pkg-manifest-io.ts | 4 ++-- 5 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/cmd-add.ts b/src/cmd-add.ts index c631fdb5..10b557eb 100644 --- a/src/cmd-add.ts +++ b/src/cmd-add.ts @@ -74,7 +74,7 @@ const _add = async function ( let version = split[1]; // load manifest - const manifest = loadManifest(); + const manifest = loadManifest(env.manifestPath); if (manifest === null) return { code: 1, dirty }; // packages that added to scope registry const pkgsInScope: DomainName[] = []; @@ -233,7 +233,7 @@ const _add = async function ( if (testables) addTestable(manifest, name); // save manifest if (dirty) { - if (!saveManifest(manifest)) return { code: 1, dirty }; + if (!saveManifest(env.manifestPath, manifest)) return { code: 1, dirty }; } return { code: 0, dirty }; }; diff --git a/src/cmd-remove.ts b/src/cmd-remove.ts index 2021ee7f..08007f77 100644 --- a/src/cmd-remove.ts +++ b/src/cmd-remove.ts @@ -52,7 +52,7 @@ const _remove = async function (pkg: PackageReference) { return { code: 1, dirty }; } // load manifest - const manifest = loadManifest(); + const manifest = loadManifest(env.manifestPath); if (manifest === null) return { code: 1, dirty }; // not found array const pkgsNotFound = []; @@ -75,7 +75,7 @@ const _remove = async function (pkg: PackageReference) { } // save manifest if (dirty) { - if (!saveManifest(manifest)) return { code: 1, dirty }; + if (!saveManifest(env.manifestPath, manifest)) return { code: 1, dirty }; } if (pkgsNotFound.length) { log.error("404", `package not found: ${pkgsNotFound.join(", ")}`); diff --git a/src/utils/pkg-manifest-io.ts b/src/utils/pkg-manifest-io.ts index 8941ab61..33d73ac4 100644 --- a/src/utils/pkg-manifest-io.ts +++ b/src/utils/pkg-manifest-io.ts @@ -1,15 +1,15 @@ import fs from "fs"; import { assertIsError } from "./error-type-guards"; import log from "../logger"; -import { env } from "./env"; import { PkgManifest } from "../types/pkg-manifest"; /** * Attempts to load the manifest from the path specified in env + * @param path The path where the manifest is located */ -export const loadManifest = function (): PkgManifest | null { +export const loadManifest = function (path: string): PkgManifest | null { try { - const text = fs.readFileSync(env.manifestPath, { encoding: "utf8" }); + const text = fs.readFileSync(path, { encoding: "utf8" }); return JSON.parse(text); } catch (err) { assertIsError(err); @@ -18,7 +18,7 @@ export const loadManifest = function (): PkgManifest | null { else { log.error( "manifest", - `failed to parse Packages/manifest.json at ${env.manifestPath}` + `failed to parse Packages/manifest.json at ${path}` ); log.error("manifest", err.message); } @@ -27,12 +27,14 @@ export const loadManifest = function (): PkgManifest | null { }; /** - * Save manifest json file to the path specified in env + * Save manifest json file to the path specified in enva + * @param path The path where the manifest is located + * @param data The manifest to save */ -export const saveManifest = function (data: PkgManifest) { +export const saveManifest = function (path: string, data: PkgManifest) { const json = JSON.stringify(data, null, 2); try { - fs.writeFileSync(env.manifestPath, json); + fs.writeFileSync(path, json); return true; } catch (err) { assertIsError(err); diff --git a/test/manifest-assertions.ts b/test/manifest-assertions.ts index b10794ba..7a2e1bd0 100644 --- a/test/manifest-assertions.ts +++ b/test/manifest-assertions.ts @@ -5,15 +5,16 @@ import { SemanticVersion } from "../src/types/semantic-version"; import { PackageUrl } from "../src/types/package-url"; import { hasScope } from "../src/types/scoped-registry"; import { PkgManifest } from "../src/types/pkg-manifest"; +import { env } from "../src/utils/env"; export function shouldHaveManifest(): PkgManifest { - const manifest = loadManifest(); + const manifest = loadManifest(env.manifestPath); should(manifest).not.be.null(); return manifest!; } export function shouldHaveNoManifest() { - const manifest = loadManifest(); + const manifest = loadManifest(env.manifestPath); should(manifest).be.null(); } diff --git a/test/test-pkg-manifest-io.ts b/test/test-pkg-manifest-io.ts index cc35d69c..e5662bb0 100644 --- a/test/test-pkg-manifest-io.ts +++ b/test/test-pkg-manifest-io.ts @@ -4,7 +4,7 @@ import "should"; import path from "path"; import { saveManifest } from "../src/utils/pkg-manifest-io"; import { describe } from "mocha"; -import { parseEnv } from "../src/utils/env"; +import { env, parseEnv } from "../src/utils/env"; import { createWorkDir, getWorkDir, removeWorkDir } from "./mock-work-dir"; import { shouldHaveManifest, @@ -77,7 +77,7 @@ describe("pkg-manifest io", function () { const manifest = shouldHaveManifest(); shouldNotHaveAnyDependencies(manifest); addDependency(manifest, domainName("some-pack"), semanticVersion("1.0.0")); - saveManifest(manifest).should.be.ok(); + saveManifest(env.manifestPath, manifest).should.be.ok(); const manifest2 = shouldHaveManifest(); manifest2.should.be.deepEqual(manifest); }); From 90fcd0544f5936ddf449c9a372461a2bb072c36e Mon Sep 17 00:00:00 2001 From: Ramon Brullo Date: Thu, 14 Dec 2023 17:59:51 +0100 Subject: [PATCH 03/18] refactor: move env usage up Remove env usage from upm-config-io --- src/cmd-login.ts | 4 ++-- src/utils/env.ts | 5 +++-- src/utils/upm-config-io.ts | 21 +++++++++++++-------- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/cmd-login.ts b/src/cmd-login.ts index 3f9a9ebd..e3cda625 100644 --- a/src/cmd-login.ts +++ b/src/cmd-login.ts @@ -8,7 +8,7 @@ import { loadUpmConfig, saveUpmConfig, } from "./utils/upm-config-io"; -import { parseEnv } from "./utils/env"; +import { env, parseEnv } from "./utils/env"; import { encodeBasicAuth } from "./types/upm-config"; import { Base64 } from "./types/base64"; import { RegistryUrl, removeTrailingSlash } from "./types/registry-url"; @@ -187,7 +187,7 @@ const writeUnityToken = async function ( token: string | null ) { // Create config dir if necessary - const configDir = await getUpmConfigDir(); + const configDir = await getUpmConfigDir(env.wsl, env.systemUser); // Read config file const config = (await loadUpmConfig(configDir)) || {}; if (!config.npmAuth) config.npmAuth = {}; diff --git a/src/utils/env.ts b/src/utils/env.ts index 9cb26cfe..a50c2ef3 100644 --- a/src/utils/env.ts +++ b/src/utils/env.ts @@ -1,6 +1,6 @@ import log from "../logger"; import chalk from "chalk"; -import { loadUpmConfig } from "./upm-config-io"; +import { getUpmConfigDir, loadUpmConfig } from "./upm-config-io"; import path from "path"; import fs from "fs"; import yaml from "yaml"; @@ -97,7 +97,8 @@ export const parseEnv = async function ( // auth if (options._global.systemUser) env.systemUser = true; if (options._global.wsl) env.wsl = true; - const upmConfig = await loadUpmConfig(); + const configDir = await getUpmConfigDir(env.wsl, env.systemUser); + const upmConfig = await loadUpmConfig(configDir); if (upmConfig) { env.npmAuth = upmConfig.npmAuth; if (env.npmAuth !== undefined) { diff --git a/src/utils/upm-config-io.ts b/src/utils/upm-config-io.ts index 49913f9f..4a13ea25 100644 --- a/src/utils/upm-config-io.ts +++ b/src/utils/upm-config-io.ts @@ -5,22 +5,26 @@ import fs from "fs"; import log from "../logger"; import isWsl from "is-wsl"; import execute from "./process"; -import { env } from "./env"; import { UPMConfig } from "../types/upm-config"; const configFileName = ".upmconfig.toml"; /** * Gets the path to directory in which the upm config is stored + * @param wsl Whether WSL should be treated as Windows + * @param systemUser Whether to authenticate as a Windows system-user */ -export const getUpmConfigDir = async (): Promise => { +export const getUpmConfigDir = async ( + wsl: boolean, + systemUser: boolean +): Promise => { let dirPath: string | undefined = ""; const systemUserSubPath = "Unity/config/ServiceAccounts"; - if (env.wsl) { + if (wsl) { if (!isWsl) { throw new Error("no WSL detected"); } - if (env.systemUser) { + if (systemUser) { const allUserProfilePath = await execute( 'wslpath "$(wslvar ALLUSERSPROFILE)"', { trim: true } @@ -35,7 +39,7 @@ export const getUpmConfigDir = async (): Promise => { dirPath = process.env.USERPROFILE ? process.env.USERPROFILE : process.env.HOME; - if (env.systemUser) { + if (systemUser) { if (!process.env.ALLUSERSPROFILE) { throw new Error("env ALLUSERSPROFILE is empty"); } @@ -49,11 +53,11 @@ export const getUpmConfigDir = async (): Promise => { /** * Attempts to load the upm config + * @param configDir The directory from which to load the config */ export const loadUpmConfig = async ( - configDir?: string + configDir: string ): Promise => { - if (configDir === undefined) configDir = await getUpmConfigDir(); const configPath = path.join(configDir, configFileName); if (fs.existsSync(configPath)) { const content = fs.readFileSync(configPath, "utf8"); @@ -66,9 +70,10 @@ export const loadUpmConfig = async ( /** * Save the upm config + * @param config The config to save + * @param configDir The directory in which to save the config */ export const saveUpmConfig = async (config: UPMConfig, configDir: string) => { - if (configDir === undefined) configDir = await getUpmConfigDir(); mkdirp.sync(configDir); const configPath = path.join(configDir, configFileName); const content = TOML.stringify(config); From 4a4695cabf36d40d18048b480e74fbebac7dd46c Mon Sep 17 00:00:00 2001 From: Ramon Brullo Date: Thu, 14 Dec 2023 18:08:38 +0100 Subject: [PATCH 04/18] refactor: move up env usage Remove env from fetchPackageInfo --- src/cmd-add.ts | 13 +++++++++++-- src/cmd-view.ts | 13 +++++++++++-- src/registry-client.ts | 34 ++++++++++++++++++++++++++-------- test/test-registry-client.ts | 12 +++++++++--- 4 files changed, 57 insertions(+), 15 deletions(-) diff --git a/src/cmd-add.ts b/src/cmd-add.ts index 10b557eb..858d7a97 100644 --- a/src/cmd-add.ts +++ b/src/cmd-add.ts @@ -80,9 +80,18 @@ const _add = async function ( const pkgsInScope: DomainName[] = []; if (version === undefined || !isPackageUrl(version)) { // verify name - let pkgInfo = await fetchPackageInfo(name); + let pkgInfo = await fetchPackageInfo( + { url: env.registry, auth: env.auth[env.registry] ?? null }, + name + ); if (!pkgInfo && env.upstream) { - pkgInfo = await fetchPackageInfo(name, env.upstreamRegistry); + pkgInfo = await fetchPackageInfo( + { + url: env.upstreamRegistry, + auth: env.auth[env.upstreamRegistry] ?? null, + }, + name + ); if (pkgInfo) isUpstreamPackage = true; } if (!pkgInfo) { diff --git a/src/cmd-view.ts b/src/cmd-view.ts index 6b371975..a20aaddc 100644 --- a/src/cmd-view.ts +++ b/src/cmd-view.ts @@ -31,9 +31,18 @@ export const view = async function ( return 1; } // verify name - let pkgInfo = await fetchPackageInfo(name); + let pkgInfo = await fetchPackageInfo( + { url: env.registry, auth: env.auth[env.registry] ?? null }, + name + ); if (!pkgInfo && env.upstream) - pkgInfo = await fetchPackageInfo(name, env.upstreamRegistry); + pkgInfo = await fetchPackageInfo( + { + url: env.upstreamRegistry, + auth: env.auth[env.upstreamRegistry] ?? null, + }, + name + ); if (!pkgInfo) { log.error("404", `package not found: ${name}`); return 1; diff --git a/src/registry-client.ts b/src/registry-client.ts index 37f9e318..7efd1d4f 100644 --- a/src/registry-client.ts +++ b/src/registry-client.ts @@ -4,17 +4,18 @@ import RegClient, { AddUserResponse, ClientCallback, GetParams, + NpmAuth, } from "another-npm-registry-client"; import log from "./logger"; import request from "request"; import assert, { AssertionError } from "assert"; -import { env } from "./utils/env"; import _ from "lodash"; import { PkgInfo, tryGetLatestVersion } from "./types/pkg-info"; import { DomainName, isInternalPackage } from "./types/domain-name"; import { SemanticVersion } from "./types/semantic-version"; import { packageReference } from "./types/package-reference"; import { RegistryUrl } from "./types/registry-url"; +import { env } from "./utils/env"; export type NpmClient = { rawClient: RegClient; @@ -53,6 +54,11 @@ export type Dependency = { resolved?: boolean; }; +export type Registry = { + url: RegistryUrl; + auth: NpmAuth | null; +}; + type NameVersionPair = { name: DomainName; version: SemanticVersion | "latest" | undefined; @@ -106,14 +112,13 @@ export const getNpmClient = (): NpmClient => { }; // Fetch package info json from registry export const fetchPackageInfo = async function ( - name: DomainName, - registry?: RegistryUrl + registry: Registry, + name: DomainName ): Promise { - if (!registry) registry = env.registry; - const pkgPath = `${registry}/${name}`; + const pkgPath = `${registry.url}/${name}`; const client = getNpmClient(); try { - return await client.get(pkgPath, { auth: env.auth[registry] || undefined }); + return await client.get(pkgPath, { auth: registry.auth || undefined }); // eslint-disable-next-line no-empty } catch (err) {} }; @@ -185,7 +190,14 @@ export const fetchPackageDependencies = async function ( } // try fetching package info from the default registry if (pkgInfo === null) { - pkgInfo = (await fetchPackageInfo(entry.name)) ?? null; + pkgInfo = + (await fetchPackageInfo( + { + url: env.registry, + auth: env.auth[env.registry] ?? null, + }, + entry.name + )) ?? null; if (pkgInfo) { depObj.upstream = false; cachedPacakgeInfoDict[entry.name] = { pkgInfo, upstream: false }; @@ -194,7 +206,13 @@ export const fetchPackageDependencies = async function ( // try fetching package info from the upstream registry if (!pkgInfo) { pkgInfo = - (await fetchPackageInfo(entry.name, env.upstreamRegistry)) ?? null; + (await fetchPackageInfo( + { + url: env.upstreamRegistry, + auth: env.auth[env.upstreamRegistry] ?? null, + }, + entry.name + )) ?? null; if (pkgInfo) { depObj.upstream = true; cachedPacakgeInfoDict[entry.name] = { pkgInfo, upstream: true }; diff --git a/test/test-registry-client.ts b/test/test-registry-client.ts index 01c561f4..60353f9a 100644 --- a/test/test-registry-client.ts +++ b/test/test-registry-client.ts @@ -1,6 +1,6 @@ import "assert"; import "should"; -import { parseEnv } from "../src/utils/env"; +import { env, parseEnv } from "../src/utils/env"; import { fetchPackageInfo } from "../src/registry-client"; import { exampleRegistryUrl, @@ -29,7 +29,10 @@ describe("registry-client", function () { ).should.be.ok(); const pkgInfoRemote = buildPackageInfo(packageA); registerRemotePkg(pkgInfoRemote); - const info = await fetchPackageInfo(packageA); + const info = await fetchPackageInfo( + { url: env.registry, auth: env.auth[env.registry] ?? null }, + packageA + ); should(info).deepEqual(pkgInfoRemote); }); it("404", async function () { @@ -38,7 +41,10 @@ describe("registry-client", function () { ).should.be.ok(); registerMissingPackage(packageA); - const info = await fetchPackageInfo(packageA); + const info = await fetchPackageInfo( + { url: env.registry, auth: env.auth[env.registry] ?? null }, + packageA + ); (info === undefined).should.be.ok(); }); }); From 95928e2d0ad8b9450b8e3d24d16415dae3757e87 Mon Sep 17 00:00:00 2001 From: Ramon Brullo Date: Thu, 14 Dec 2023 18:12:53 +0100 Subject: [PATCH 05/18] refactor: move up env usage Remove usage from registry-client --- src/cmd-add.ts | 37 ++++++++++++++++++++++++------------- src/cmd-deps.ts | 20 +++++++++++++++++--- src/registry-client.ts | 20 ++++---------------- 3 files changed, 45 insertions(+), 32 deletions(-) diff --git a/src/cmd-add.ts b/src/cmd-add.ts index 858d7a97..ee1879e6 100644 --- a/src/cmd-add.ts +++ b/src/cmd-add.ts @@ -8,7 +8,11 @@ import { compareEditorVersion, tryParseEditorVersion, } from "./types/editor-version"; -import { fetchPackageDependencies, fetchPackageInfo } from "./registry-client"; +import { + fetchPackageDependencies, + fetchPackageInfo, + Registry, +} from "./registry-client"; import { DomainName, isDomainName } from "./types/domain-name"; import { SemanticVersion } from "./types/semantic-version"; import { @@ -45,10 +49,22 @@ export const add = async function ( // parse env const envOk = await parseEnv(options, true); if (!envOk) return 1; + + const registry: Registry = { + url: env.registry, + auth: env.auth[env.registry] ?? null, + }; + const upstreamRegistry: Registry = { + url: env.upstreamRegistry, + auth: env.auth[env.upstreamRegistry] ?? null, + }; + // add const results = []; for (const pkg of pkgs) - results.push(await _add(pkg, options.test, options.force)); + results.push( + await _add(registry, upstreamRegistry, pkg, options.test, options.force) + ); const result: AddResult = { code: results.filter((x) => x.code != 0).length > 0 ? 1 : 0, dirty: results.filter((x) => x.dirty).length > 0, @@ -60,6 +76,8 @@ export const add = async function ( }; const _add = async function ( + registry: Registry, + upstreamRegistry: Registry, pkg: PackageReference, testables?: boolean, force?: boolean @@ -80,18 +98,9 @@ const _add = async function ( const pkgsInScope: DomainName[] = []; if (version === undefined || !isPackageUrl(version)) { // verify name - let pkgInfo = await fetchPackageInfo( - { url: env.registry, auth: env.auth[env.registry] ?? null }, - name - ); + let pkgInfo = await fetchPackageInfo(registry, name); if (!pkgInfo && env.upstream) { - pkgInfo = await fetchPackageInfo( - { - url: env.upstreamRegistry, - auth: env.auth[env.upstreamRegistry] ?? null, - }, - name - ); + pkgInfo = await fetchPackageInfo(upstreamRegistry, name); if (pkgInfo) isUpstreamPackage = true; } if (!pkgInfo) { @@ -164,6 +173,8 @@ const _add = async function ( // pkgsInScope if (!isUpstreamPackage) { const [depsValid, depsInvalid] = await fetchPackageDependencies( + registry, + upstreamRegistry, name, version, true diff --git a/src/cmd-deps.ts b/src/cmd-deps.ts index a7f9b1db..55f39153 100644 --- a/src/cmd-deps.ts +++ b/src/cmd-deps.ts @@ -1,6 +1,6 @@ import log from "./logger"; -import { parseEnv } from "./utils/env"; -import { fetchPackageDependencies } from "./registry-client"; +import { env, parseEnv } from "./utils/env"; +import { fetchPackageDependencies, Registry } from "./registry-client"; import { DomainName } from "./types/domain-name"; import { isPackageUrl } from "./types/package-url"; import { @@ -22,14 +22,26 @@ export const deps = async function ( // parse env const envOk = await parseEnv(options, false); if (!envOk) return 1; + + const registry: Registry = { + url: env.registry, + auth: env.auth[env.registry] ?? null, + }; + const upstreamRegistry: Registry = { + url: env.upstreamRegistry, + auth: env.auth[env.upstreamRegistry] ?? null, + }; + // parse name const [name, version] = splitPackageReference(pkg); // deps - await _deps(name, version, options.deep); + await _deps(registry, upstreamRegistry, name, version, options.deep); return 0; }; const _deps = async function ( + registry: Registry, + upstreamRegistry: Registry, name: DomainName, version: VersionReference | undefined, deep?: boolean @@ -38,6 +50,8 @@ const _deps = async function ( throw new Error("Cannot get dependencies for url-version"); const [depsValid, depsInvalid] = await fetchPackageDependencies( + registry, + upstreamRegistry, name, version, deep diff --git a/src/registry-client.ts b/src/registry-client.ts index 7efd1d4f..be860ca9 100644 --- a/src/registry-client.ts +++ b/src/registry-client.ts @@ -15,7 +15,6 @@ import { DomainName, isInternalPackage } from "./types/domain-name"; import { SemanticVersion } from "./types/semantic-version"; import { packageReference } from "./types/package-reference"; import { RegistryUrl } from "./types/registry-url"; -import { env } from "./utils/env"; export type NpmClient = { rawClient: RegClient; @@ -136,6 +135,8 @@ export const fetchPackageInfo = async function ( ] */ export const fetchPackageDependencies = async function ( + registry: Registry, + upstreamRegistry: Registry, name: DomainName, version: SemanticVersion | "latest" | undefined, deep?: boolean @@ -190,14 +191,7 @@ export const fetchPackageDependencies = async function ( } // try fetching package info from the default registry if (pkgInfo === null) { - pkgInfo = - (await fetchPackageInfo( - { - url: env.registry, - auth: env.auth[env.registry] ?? null, - }, - entry.name - )) ?? null; + pkgInfo = (await fetchPackageInfo(registry, entry.name)) ?? null; if (pkgInfo) { depObj.upstream = false; cachedPacakgeInfoDict[entry.name] = { pkgInfo, upstream: false }; @@ -206,13 +200,7 @@ export const fetchPackageDependencies = async function ( // try fetching package info from the upstream registry if (!pkgInfo) { pkgInfo = - (await fetchPackageInfo( - { - url: env.upstreamRegistry, - auth: env.auth[env.upstreamRegistry] ?? null, - }, - entry.name - )) ?? null; + (await fetchPackageInfo(upstreamRegistry, entry.name)) ?? null; if (pkgInfo) { depObj.upstream = true; cachedPacakgeInfoDict[entry.name] = { pkgInfo, upstream: true }; From 668e846488ad33784762f5a0f196365b6ebd8a4d Mon Sep 17 00:00:00 2001 From: Ramon Brullo Date: Thu, 14 Dec 2023 18:23:57 +0100 Subject: [PATCH 06/18] refactor: remove extraneous functions Removes the _ functions that exist in some cmd files by either making them local functions inside the main cmd function or by just moving their content into the main cmd function. The goals for this are: - Move env usage up into just the env functions - Avoid unnecessary parameter passing --- src/cmd-add.ts | 353 ++++++++++++++++++++++------------------------ src/cmd-deps.ts | 19 +-- src/cmd-remove.ts | 97 ++++++------- 3 files changed, 224 insertions(+), 245 deletions(-) diff --git a/src/cmd-add.ts b/src/cmd-add.ts index ee1879e6..3fe768e9 100644 --- a/src/cmd-add.ts +++ b/src/cmd-add.ts @@ -59,201 +59,192 @@ export const add = async function ( auth: env.auth[env.upstreamRegistry] ?? null, }; - // add - const results = []; - for (const pkg of pkgs) - results.push( - await _add(registry, upstreamRegistry, pkg, options.test, options.force) - ); - const result: AddResult = { - code: results.filter((x) => x.code != 0).length > 0 ? 1 : 0, - dirty: results.filter((x) => x.dirty).length > 0, - }; - // print manifest notice - if (result.dirty) - log.notice("", "please open Unity project to apply changes"); - return result.code; -}; - -const _add = async function ( - registry: Registry, - upstreamRegistry: Registry, - pkg: PackageReference, - testables?: boolean, - force?: boolean -): Promise { - // dirty flag - let dirty = false; - // is upstream package flag - let isUpstreamPackage = false; - // parse name - const split = splitPackageReference(pkg); - const name = split[0]; - let version = split[1]; + const addSingle = async function (pkg: PackageReference): Promise { + // dirty flag + let dirty = false; + // is upstream package flag + let isUpstreamPackage = false; + // parse name + const split = splitPackageReference(pkg); + const name = split[0]; + let version = split[1]; - // load manifest - const manifest = loadManifest(env.manifestPath); - if (manifest === null) return { code: 1, dirty }; - // packages that added to scope registry - const pkgsInScope: DomainName[] = []; - if (version === undefined || !isPackageUrl(version)) { - // verify name - let pkgInfo = await fetchPackageInfo(registry, name); - if (!pkgInfo && env.upstream) { - pkgInfo = await fetchPackageInfo(upstreamRegistry, name); - if (pkgInfo) isUpstreamPackage = true; - } - if (!pkgInfo) { - log.error("404", `package not found: ${name}`); - return { code: 1, dirty }; - } - // verify version - const versions = Object.keys(pkgInfo.versions) as SemanticVersion[]; - // eslint-disable-next-line require-atomic-updates - if (!version || version === "latest") - version = tryGetLatestVersion(pkgInfo); - if (versions.filter((x) => x === version).length <= 0) { - log.warn( - "404", - `version ${version} is not a valid choice of: ${versions - .reverse() - .join(", ")}` - ); - return { code: 1, dirty }; - } - - if (version === undefined) - throw new Error("Could not determine package version to add"); - const versionInfo = pkgInfo.versions[version]; - // verify editor version - if (versionInfo.unity) { - const requiredEditorVersion = versionInfo.unityRelease - ? versionInfo.unity + "." + versionInfo.unityRelease - : versionInfo.unity; - if (env.editorVersion) { - const editorVersionResult = tryParseEditorVersion(env.editorVersion); - const requiredEditorVersionResult = tryParseEditorVersion( - requiredEditorVersion + // load manifest + const manifest = loadManifest(env.manifestPath); + if (manifest === null) return { code: 1, dirty }; + // packages that added to scope registry + const pkgsInScope: DomainName[] = []; + if (version === undefined || !isPackageUrl(version)) { + // verify name + let pkgInfo = await fetchPackageInfo(registry, name); + if (!pkgInfo && env.upstream) { + pkgInfo = await fetchPackageInfo(upstreamRegistry, name); + if (pkgInfo) isUpstreamPackage = true; + } + if (!pkgInfo) { + log.error("404", `package not found: ${name}`); + return { code: 1, dirty }; + } + // verify version + const versions = Object.keys(pkgInfo.versions) as SemanticVersion[]; + // eslint-disable-next-line require-atomic-updates + if (!version || version === "latest") + version = tryGetLatestVersion(pkgInfo); + if (versions.filter((x) => x === version).length <= 0) { + log.warn( + "404", + `version ${version} is not a valid choice of: ${versions + .reverse() + .join(", ")}` ); - if (!editorVersionResult) { - log.warn( - "editor.version", - `${env.editorVersion} is unknown, the editor version check is disabled` + return { code: 1, dirty }; + } + + if (version === undefined) + throw new Error("Could not determine package version to add"); + const versionInfo = pkgInfo.versions[version]; + // verify editor version + if (versionInfo.unity) { + const requiredEditorVersion = versionInfo.unityRelease + ? versionInfo.unity + "." + versionInfo.unityRelease + : versionInfo.unity; + if (env.editorVersion) { + const editorVersionResult = tryParseEditorVersion(env.editorVersion); + const requiredEditorVersionResult = tryParseEditorVersion( + requiredEditorVersion ); - } - if (!requiredEditorVersionResult) { - log.warn("package.unity", `${requiredEditorVersion} is not valid`); - if (!force) { - log.notice( - "suggest", - "contact the package author to fix the issue, or run with option -f to ignore the warning" + if (!editorVersionResult) { + log.warn( + "editor.version", + `${env.editorVersion} is unknown, the editor version check is disabled` ); - return { code: 1, dirty }; } - } - if ( - editorVersionResult && - requiredEditorVersionResult && - compareEditorVersion(env.editorVersion, requiredEditorVersion) < 0 - ) { - log.warn( - "editor.version", - `requires ${requiredEditorVersion} but found ${env.editorVersion}` - ); - if (!force) { - log.notice( - "suggest", - `upgrade the editor to ${requiredEditorVersion}, or run with option -f to ignore the warning` + if (!requiredEditorVersionResult) { + log.warn("package.unity", `${requiredEditorVersion} is not valid`); + if (!options.force) { + log.notice( + "suggest", + "contact the package author to fix the issue, or run with option -f to ignore the warning" + ); + return { code: 1, dirty }; + } + } + if ( + editorVersionResult && + requiredEditorVersionResult && + compareEditorVersion(env.editorVersion, requiredEditorVersion) < 0 + ) { + log.warn( + "editor.version", + `requires ${requiredEditorVersion} but found ${env.editorVersion}` ); - return { code: 1, dirty }; + if (!options.force) { + log.notice( + "suggest", + `upgrade the editor to ${requiredEditorVersion}, or run with option -f to ignore the warning` + ); + return { code: 1, dirty }; + } } } } - } - // pkgsInScope - if (!isUpstreamPackage) { - const [depsValid, depsInvalid] = await fetchPackageDependencies( - registry, - upstreamRegistry, - name, - version, - true - ); - // add depsValid to pkgsInScope. - depsValid - .filter((x) => !x.upstream && !x.internal) - .map((x) => x.name) - .forEach((name) => pkgsInScope.push(name)); - // print suggestion for depsInvalid - depsInvalid.forEach((depObj) => { - if (depObj.reason == "package404" || depObj.reason == "version404") { - const resolvedVersion = manifest.dependencies[depObj.name]; - depObj.resolved = Boolean(resolvedVersion); - if (!depObj.resolved) - log.notice( - "suggest", - `to install ${packageReference( - depObj.name, - depObj.version - )} or a replaceable version manually` + // pkgsInScope + if (!isUpstreamPackage) { + const [depsValid, depsInvalid] = await fetchPackageDependencies( + registry, + upstreamRegistry, + name, + version, + true + ); + // add depsValid to pkgsInScope. + depsValid + .filter((x) => !x.upstream && !x.internal) + .map((x) => x.name) + .forEach((name) => pkgsInScope.push(name)); + // print suggestion for depsInvalid + depsInvalid.forEach((depObj) => { + if (depObj.reason == "package404" || depObj.reason == "version404") { + const resolvedVersion = manifest.dependencies[depObj.name]; + depObj.resolved = Boolean(resolvedVersion); + if (!depObj.resolved) + log.notice( + "suggest", + `to install ${packageReference( + depObj.name, + depObj.version + )} or a replaceable version manually` + ); + } + }); + if (depsInvalid.filter((x) => !x.resolved).length > 0) { + if (!options.force) { + log.error( + "missing dependencies", + "please resolve thie issue or run with option -f to ignore the warning" ); + return { code: 1, dirty }; + } } - }); - if (depsInvalid.filter((x) => !x.resolved).length > 0) { - if (!force) { - log.error( - "missing dependencies", - "please resolve thie issue or run with option -f to ignore the warning" - ); - return { code: 1, dirty }; - } - } - } else pkgsInScope.push(name); - } - // add to dependencies - const oldVersion = manifest.dependencies[name]; - addDependency(manifest, name, version); - if (!oldVersion) { - // Log the added package - log.notice("manifest", `added ${packageReference(name, version)}`); - dirty = true; - } else if (oldVersion != version) { - // Log the modified package version - log.notice("manifest", `modified ${name} ${oldVersion} => ${version}`); - dirty = true; - } else { - // Log the existed package - log.notice("manifest", `existed ${packageReference(name, version)}`); - } - if (!isUpstreamPackage) { - // add to scopedRegistries - if (!manifest.scopedRegistries) { - manifest.scopedRegistries = []; - dirty = true; + } else pkgsInScope.push(name); } - let entry = tryGetScopedRegistryByUrl(manifest, env.registry); - if (entry === null) { - const name = url.parse(env.registry).hostname; - if (name === null) throw new Error("Could not resolve registry name"); - entry = scopedRegistry(name, env.registry); - addScopedRegistry(manifest, entry); + // add to dependencies + const oldVersion = manifest.dependencies[name]; + addDependency(manifest, name, version); + if (!oldVersion) { + // Log the added package + log.notice("manifest", `added ${packageReference(name, version)}`); + dirty = true; + } else if (oldVersion != version) { + // Log the modified package version + log.notice("manifest", `modified ${name} ${oldVersion} => ${version}`); dirty = true; + } else { + // Log the existed package + log.notice("manifest", `existed ${packageReference(name, version)}`); } - // apply pkgsInScope - const scopesSet = new Set(entry.scopes || []); - if (isDomainName(env.namespace)) pkgsInScope.push(env.namespace); - pkgsInScope.forEach((name) => { - if (!scopesSet.has(name)) { - scopesSet.add(name); + if (!isUpstreamPackage) { + // add to scopedRegistries + if (!manifest.scopedRegistries) { + manifest.scopedRegistries = []; dirty = true; } - }); - entry.scopes = Array.from(scopesSet).sort(); - } - if (testables) addTestable(manifest, name); - // save manifest - if (dirty) { - if (!saveManifest(env.manifestPath, manifest)) return { code: 1, dirty }; - } - return { code: 0, dirty }; + let entry = tryGetScopedRegistryByUrl(manifest, env.registry); + if (entry === null) { + const name = url.parse(env.registry).hostname; + if (name === null) throw new Error("Could not resolve registry name"); + entry = scopedRegistry(name, env.registry); + addScopedRegistry(manifest, entry); + dirty = true; + } + // apply pkgsInScope + const scopesSet = new Set(entry.scopes || []); + if (isDomainName(env.namespace)) pkgsInScope.push(env.namespace); + pkgsInScope.forEach((name) => { + if (!scopesSet.has(name)) { + scopesSet.add(name); + dirty = true; + } + }); + entry.scopes = Array.from(scopesSet).sort(); + } + if (options.test) addTestable(manifest, name); + // save manifest + if (dirty) { + if (!saveManifest(env.manifestPath, manifest)) return { code: 1, dirty }; + } + return { code: 0, dirty }; + }; + + // add + const results = []; + for (const pkg of pkgs) results.push(await addSingle(pkg)); + const result: AddResult = { + code: results.filter((x) => x.code != 0).length > 0 ? 1 : 0, + dirty: results.filter((x) => x.dirty).length > 0, + }; + // print manifest notice + if (result.dirty) + log.notice("", "please open Unity project to apply changes"); + return result.code; }; diff --git a/src/cmd-deps.ts b/src/cmd-deps.ts index 55f39153..11aa55eb 100644 --- a/src/cmd-deps.ts +++ b/src/cmd-deps.ts @@ -1,13 +1,11 @@ import log from "./logger"; import { env, parseEnv } from "./utils/env"; import { fetchPackageDependencies, Registry } from "./registry-client"; -import { DomainName } from "./types/domain-name"; import { isPackageUrl } from "./types/package-url"; import { packageReference, PackageReference, splitPackageReference, - VersionReference, } from "./types/package-reference"; import { CmdOptions } from "./types/options"; @@ -31,21 +29,8 @@ export const deps = async function ( url: env.upstreamRegistry, auth: env.auth[env.upstreamRegistry] ?? null, }; - - // parse name const [name, version] = splitPackageReference(pkg); - // deps - await _deps(registry, upstreamRegistry, name, version, options.deep); - return 0; -}; -const _deps = async function ( - registry: Registry, - upstreamRegistry: Registry, - name: DomainName, - version: VersionReference | undefined, - deep?: boolean -) { if (version !== undefined && isPackageUrl(version)) throw new Error("Cannot get dependencies for url-version"); @@ -54,7 +39,7 @@ const _deps = async function ( upstreamRegistry, name, version, - deep + options.deep ); depsValid .filter((x) => !x.self) @@ -69,4 +54,6 @@ const _deps = async function ( else if (x.reason == "version404") reason = "missing dependency version"; log.warn(reason, packageReference(x.name, x.version)); }); + + return 0; }; diff --git a/src/cmd-remove.ts b/src/cmd-remove.ts index 08007f77..93ea38ad 100644 --- a/src/cmd-remove.ts +++ b/src/cmd-remove.ts @@ -24,9 +24,57 @@ export const remove = async function ( // parse env const envOk = await parseEnv(options, true); if (!envOk) return 1; + + const removeSingle = async function (pkg: PackageReference) { + // dirty flag + let dirty = false; + // parse name + const split = splitPackageReference(pkg); + const name = split[0]; + let version = split[1]; + if (version) { + log.warn( + "", + `please replace '${packageReference(name, version)}' with '${name}'` + ); + return { code: 1, dirty }; + } + // load manifest + const manifest = loadManifest(env.manifestPath); + if (manifest === null) return { code: 1, dirty }; + // not found array + const pkgsNotFound = []; + version = manifest.dependencies[name]; + if (version) { + log.notice("manifest", `removed ${packageReference(name, version)}`); + removeDependency(manifest, name); + dirty = true; + } else pkgsNotFound.push(pkg); + + const entry = tryGetScopedRegistryByUrl(manifest, env.registry); + if (entry !== null) { + if (hasScope(entry, name)) { + removeScope(entry, name); + const scopesSet = new Set(entry.scopes); + if (isDomainName(env.namespace)) scopesSet.add(env.namespace); + entry.scopes = Array.from(scopesSet).sort(); + dirty = true; + } + } + // save manifest + if (dirty) { + if (!saveManifest(env.manifestPath, manifest)) return { code: 1, dirty }; + } + if (pkgsNotFound.length) { + log.error("404", `package not found: ${pkgsNotFound.join(", ")}`); + return { code: 1, dirty }; + } + return { code: 0, dirty }; + }; + // remove const results = []; - for (const pkg of pkgs) results.push(await _remove(pkg)); + for (const pkg of pkgs) results.push(await removeSingle(pkg)); const result = { code: results.filter((x) => x.code != 0).length > 0 ? 1 : 0, dirty: results.filter((x) => x.dirty).length > 0, @@ -36,50 +84,3 @@ export const remove = async function ( log.notice("", "please open Unity project to apply changes"); return result.code; }; - -const _remove = async function (pkg: PackageReference) { - // dirty flag - let dirty = false; - // parse name - const split = splitPackageReference(pkg); - const name = split[0]; - let version = split[1]; - if (version) { - log.warn( - "", - `please replace '${packageReference(name, version)}' with '${name}'` - ); - return { code: 1, dirty }; - } - // load manifest - const manifest = loadManifest(env.manifestPath); - if (manifest === null) return { code: 1, dirty }; - // not found array - const pkgsNotFound = []; - version = manifest.dependencies[name]; - if (version) { - log.notice("manifest", `removed ${packageReference(name, version)}`); - removeDependency(manifest, name); - dirty = true; - } else pkgsNotFound.push(pkg); - - const entry = tryGetScopedRegistryByUrl(manifest, env.registry); - if (entry !== null) { - if (hasScope(entry, name)) { - removeScope(entry, name); - const scopesSet = new Set(entry.scopes); - if (isDomainName(env.namespace)) scopesSet.add(env.namespace); - entry.scopes = Array.from(scopesSet).sort(); - dirty = true; - } - } - // save manifest - if (dirty) { - if (!saveManifest(env.manifestPath, manifest)) return { code: 1, dirty }; - } - if (pkgsNotFound.length) { - log.error("404", `package not found: ${pkgsNotFound.join(", ")}`); - return { code: 1, dirty }; - } - return { code: 0, dirty }; -}; From ca49020e0f430da5a5c2daaaea4e0cb496d360a8 Mon Sep 17 00:00:00 2001 From: Ramon Brullo Date: Thu, 14 Dec 2023 18:27:16 +0100 Subject: [PATCH 07/18] refactor: move up env usage Remove env from getNpmFetchOptions function --- src/cmd-search.ts | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/src/cmd-search.ts b/src/cmd-search.ts index 0960510b..705ff21a 100644 --- a/src/cmd-search.ts +++ b/src/cmd-search.ts @@ -9,8 +9,8 @@ import { PkgInfo, tryGetLatestVersion } from "./types/pkg-info"; import { env, parseEnv } from "./utils/env"; import { DomainName } from "./types/domain-name"; import { SemanticVersion } from "./types/semantic-version"; -import { RegistryUrl } from "./types/registry-url"; import { CmdOptions } from "./types/options"; +import { Registry } from "./registry-client"; type DateString = string; @@ -26,26 +26,28 @@ export type OldSearchResult = | SearchedPkgInfo[] | Record; -// Get npm fetch options -const getNpmFetchOptions = function (): Options { +/** + * Get npm fetch options + * @param registry The registry for which to get the options + */ +const getNpmFetchOptions = function (registry: Registry): Options { const opts: Options = { log, - registry: env.registry, + registry: registry.url, }; - const auth = env.auth[env.registry]; - if (auth) Object.assign(opts, auth); + const auth = registry.auth; + if (auth !== null) Object.assign(opts, auth); return opts; }; const searchEndpoint = async function ( - keyword: string, - registry?: RegistryUrl + registry: Registry, + keyword: string ): Promise { - if (!registry) registry = env.registry; try { // NOTE: The results of the search will be PkgInfo objects so we can change the type const results = ( - await npmSearch(keyword, getNpmFetchOptions()) + await npmSearch(keyword, getNpmFetchOptions(registry)) ); log.verbose("npmsearch", results.join(os.EOL)); return results.map(getTableRow); @@ -58,12 +60,13 @@ const searchEndpoint = async function ( }; const searchOld = async function ( + registry: Registry, keyword: string ): Promise { // all endpoint try { const results = ( - await npmFetch.json("/-/all", getNpmFetchOptions()) + await npmFetch.json("/-/all", getNpmFetchOptions(registry)) ); let objects: SearchedPkgInfo[] = []; if (results) { @@ -117,12 +120,18 @@ export async function search(keyword: string, options: SearchOptions) { // parse env const envOk = await parseEnv(options, false); if (!envOk) return 1; + + const registry: Registry = { + url: env.registry, + auth: env.auth[env.registry], + }; + const table = getTable(); // search endpoint - let results = await searchEndpoint(keyword); + let results = await searchEndpoint(registry, keyword); // search old search if (results === undefined) { - results = (await searchOld(keyword)) || []; + results = (await searchOld(registry, keyword)) || []; } // search upstream // if (env.upstream) { From c7172c407c83a88827fae220fb0694c209ba3746 Mon Sep 17 00:00:00 2001 From: Ramon Brullo Date: Thu, 14 Dec 2023 18:29:54 +0100 Subject: [PATCH 08/18] refactor: move up env usage Remove env usage from manifest-assertions --- test/manifest-assertions.ts | 9 ++++----- test/test-cmd-add.ts | 33 +++++++++++++++++---------------- test/test-cmd-remove.ts | 11 ++++++----- test/test-pkg-manifest-io.ts | 14 +++++++------- 4 files changed, 34 insertions(+), 33 deletions(-) diff --git a/test/manifest-assertions.ts b/test/manifest-assertions.ts index 7a2e1bd0..62741322 100644 --- a/test/manifest-assertions.ts +++ b/test/manifest-assertions.ts @@ -5,16 +5,15 @@ import { SemanticVersion } from "../src/types/semantic-version"; import { PackageUrl } from "../src/types/package-url"; import { hasScope } from "../src/types/scoped-registry"; import { PkgManifest } from "../src/types/pkg-manifest"; -import { env } from "../src/utils/env"; -export function shouldHaveManifest(): PkgManifest { - const manifest = loadManifest(env.manifestPath); +export function shouldHaveManifestAt(manifestPath: string): PkgManifest { + const manifest = loadManifest(manifestPath); should(manifest).not.be.null(); return manifest!; } -export function shouldHaveNoManifest() { - const manifest = loadManifest(env.manifestPath); +export function shouldHaveNoManifestAt(manifestPath: string) { + const manifest = loadManifest(manifestPath); should(manifest).be.null(); } diff --git a/test/test-cmd-add.ts b/test/test-cmd-add.ts index b58c690b..0f37f6b4 100644 --- a/test/test-cmd-add.ts +++ b/test/test-cmd-add.ts @@ -12,7 +12,7 @@ import { createWorkDir, getWorkDir, removeWorkDir } from "./mock-work-dir"; import { attachMockConsole, MockConsole } from "./mock-console"; import { shouldHaveDependency, - shouldHaveManifest, + shouldHaveManifestAt, } from "./manifest-assertions"; import { buildPackageInfo } from "./data-pkg-info"; import { buildPackageManifest } from "./data-pkg-manifest"; @@ -20,6 +20,7 @@ import { domainName } from "../src/types/domain-name"; import { PackageUrl } from "../src/types/package-url"; import { semanticVersion } from "../src/types/semantic-version"; import { packageReference } from "../src/types/package-reference"; +import { env } from "../src/utils/env"; describe("cmd-add.ts", function () { const packageMissing = domainName("pkg-not-exist"); @@ -162,7 +163,7 @@ describe("cmd-add.ts", function () { it("add pkg", async function () { const retCode = await add(packageA, options); retCode.should.equal(0); - const manifest = shouldHaveManifest(); + const manifest = shouldHaveManifestAt(env.manifestPath); manifest.should.deepEqual(expectedManifestA); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -173,7 +174,7 @@ describe("cmd-add.ts", function () { options ); retCode.should.equal(0); - const manifest = shouldHaveManifest(); + const manifest = shouldHaveManifestAt(env.manifestPath); manifest.should.deepEqual(expectedManifestA); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -181,7 +182,7 @@ describe("cmd-add.ts", function () { it("add pkg@latest", async function () { const retCode = await add(packageReference(packageA, "latest"), options); retCode.should.equal(0); - const manifest = shouldHaveManifest(); + const manifest = shouldHaveManifestAt(env.manifestPath); manifest.should.deepEqual(expectedManifestA); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -197,7 +198,7 @@ describe("cmd-add.ts", function () { options ); retCode2.should.equal(0); - const manifest = shouldHaveManifest(); + const manifest = shouldHaveManifestAt(env.manifestPath); manifest.should.deepEqual(expectedManifestA); mockConsole.hasLineIncluding("out", "modified ").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -213,7 +214,7 @@ describe("cmd-add.ts", function () { options ); retCode2.should.equal(0); - const manifest = shouldHaveManifest(); + const manifest = shouldHaveManifestAt(env.manifestPath); manifest.should.deepEqual(expectedManifestA); mockConsole.hasLineIncluding("out", "existed ").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -224,7 +225,7 @@ describe("cmd-add.ts", function () { options ); retCode.should.equal(1); - const manifest = shouldHaveManifest(); + const manifest = shouldHaveManifestAt(env.manifestPath); manifest.should.deepEqual(defaultManifest); mockConsole .hasLineIncluding("out", "version 2.0.0 is not a valid choice") @@ -235,7 +236,7 @@ describe("cmd-add.ts", function () { const gitUrl = "https://github.com/yo/com.base.package-a" as PackageUrl; const retCode = await add(packageReference(packageA, gitUrl), options); retCode.should.equal(0); - const manifest = shouldHaveManifest(); + const manifest = shouldHaveManifestAt(env.manifestPath); shouldHaveDependency(manifest, packageA, gitUrl); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -244,7 +245,7 @@ describe("cmd-add.ts", function () { const gitUrl = "git@github.com:yo/com.base.package-a" as PackageUrl; const retCode = await add(packageReference(packageA, gitUrl), options); retCode.should.equal(0); - const manifest = shouldHaveManifest(); + const manifest = shouldHaveManifestAt(env.manifestPath); shouldHaveDependency(manifest, packageA, gitUrl); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -253,7 +254,7 @@ describe("cmd-add.ts", function () { const fileUrl = "file../yo/com.base.package-a" as PackageUrl; const retCode = await add(packageReference(packageA, fileUrl), options); retCode.should.equal(0); - const manifest = shouldHaveManifest(); + const manifest = shouldHaveManifestAt(env.manifestPath); shouldHaveDependency(manifest, packageA, fileUrl); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -261,14 +262,14 @@ describe("cmd-add.ts", function () { it("add pkg-not-exist", async function () { const retCode = await add(packageMissing, options); retCode.should.equal(1); - const manifest = shouldHaveManifest(); + const manifest = shouldHaveManifestAt(env.manifestPath); manifest.should.deepEqual(defaultManifest); mockConsole.hasLineIncluding("out", "package not found").should.be.ok(); }); it("add more than one pkgs", async function () { const retCode = await add([packageA, packageB], options); retCode.should.equal(0); - const manifest = shouldHaveManifest(); + const manifest = shouldHaveManifestAt(env.manifestPath); manifest.should.deepEqual(expectedManifestAB); mockConsole .hasLineIncluding("out", "added com.base.package-a") @@ -281,7 +282,7 @@ describe("cmd-add.ts", function () { it("add pkg from upstream", async function () { const retCode = await add(packageUp, upstreamOptions); retCode.should.equal(0); - const manifest = shouldHaveManifest(); + const manifest = shouldHaveManifestAt(env.manifestPath); manifest.should.deepEqual(expectedManifestUpstream); mockConsole .hasLineIncluding("out", "added com.upstream.package-up") @@ -291,7 +292,7 @@ describe("cmd-add.ts", function () { it("add pkg-not-exist from upstream", async function () { const retCode = await add(packageMissing, upstreamOptions); retCode.should.equal(1); - const manifest = shouldHaveManifest(); + const manifest = shouldHaveManifestAt(env.manifestPath); manifest.should.deepEqual(defaultManifest); mockConsole.hasLineIncluding("out", "package not found").should.be.ok(); }); @@ -301,7 +302,7 @@ describe("cmd-add.ts", function () { upstreamOptions ); retCode.should.equal(0); - const manifest = shouldHaveManifest(); + const manifest = shouldHaveManifestAt(env.manifestPath); manifest.should.deepEqual(expectedManifestC); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -309,7 +310,7 @@ describe("cmd-add.ts", function () { it("add pkg with tests", async function () { const retCode = await add(packageA, testableOptions); retCode.should.equal(0); - const manifest = shouldHaveManifest(); + const manifest = shouldHaveManifestAt(env.manifestPath); manifest.should.deepEqual(expectedManifestTestable); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); diff --git a/test/test-cmd-remove.ts b/test/test-cmd-remove.ts index 6ad76d58..ca331e69 100644 --- a/test/test-cmd-remove.ts +++ b/test/test-cmd-remove.ts @@ -7,7 +7,7 @@ import { import { createWorkDir, getWorkDir, removeWorkDir } from "./mock-work-dir"; import { attachMockConsole, MockConsole } from "./mock-console"; import { - shouldHaveManifest, + shouldHaveManifestAt, shouldHaveRegistryWithScopes, shouldNotHaveDependency, } from "./manifest-assertions"; @@ -15,6 +15,7 @@ import { buildPackageManifest } from "./data-pkg-manifest"; import { domainName } from "../src/types/domain-name"; import { semanticVersion } from "../src/types/semantic-version"; import { packageReference } from "../src/types/package-reference"; +import { env } from "../src/utils/env"; const packageA = domainName("com.example.package-a"); const packageB = domainName("com.example.package-b"); @@ -50,7 +51,7 @@ describe("cmd-remove.ts", function () { }; const retCode = await remove(packageA, options); retCode.should.equal(0); - const manifest = shouldHaveManifest(); + const manifest = shouldHaveManifestAt(env.manifestPath); shouldNotHaveDependency(manifest, packageA); shouldHaveRegistryWithScopes(manifest, [ exampleRegistryReverseDomain, @@ -71,7 +72,7 @@ describe("cmd-remove.ts", function () { options ); retCode.should.equal(1); - const manifest = shouldHaveManifest(); + const manifest = shouldHaveManifestAt(env.manifestPath); manifest.should.deepEqual(defaultManifest); mockConsole.hasLineIncluding("out", "please replace").should.be.ok(); }); @@ -84,7 +85,7 @@ describe("cmd-remove.ts", function () { }; const retCode = await remove(missingPackage, options); retCode.should.equal(1); - const manifest = shouldHaveManifest(); + const manifest = shouldHaveManifestAt(env.manifestPath); manifest.should.deepEqual(defaultManifest); mockConsole.hasLineIncluding("out", "package not found").should.be.ok(); }); @@ -97,7 +98,7 @@ describe("cmd-remove.ts", function () { }; const retCode = await remove([packageA, packageB], options); retCode.should.equal(0); - const manifest = shouldHaveManifest(); + const manifest = shouldHaveManifestAt(env.manifestPath); shouldNotHaveDependency(manifest, packageA); shouldNotHaveDependency(manifest, packageB); shouldHaveRegistryWithScopes(manifest, [exampleRegistryReverseDomain]); diff --git a/test/test-pkg-manifest-io.ts b/test/test-pkg-manifest-io.ts index e5662bb0..784fbbe4 100644 --- a/test/test-pkg-manifest-io.ts +++ b/test/test-pkg-manifest-io.ts @@ -7,8 +7,8 @@ import { describe } from "mocha"; import { env, parseEnv } from "../src/utils/env"; import { createWorkDir, getWorkDir, removeWorkDir } from "./mock-work-dir"; import { - shouldHaveManifest, - shouldHaveNoManifest, + shouldHaveManifestAt, + shouldHaveNoManifestAt, shouldNotHaveAnyDependencies, } from "./manifest-assertions"; import { domainName } from "../src/types/domain-name"; @@ -44,7 +44,7 @@ describe("pkg-manifest io", function () { true ) ).should.be.ok(); - const manifest = shouldHaveManifest(); + const manifest = shouldHaveManifestAt(env.manifestPath); manifest.should.be.deepEqual({ dependencies: {} }); }); it("no manifest file", async function () { @@ -54,7 +54,7 @@ describe("pkg-manifest io", function () { false ) ).should.be.ok(); - shouldHaveNoManifest(); + shouldHaveNoManifestAt(env.manifestPath); mockConsole.hasLineIncluding("out", "does not exist").should.be.ok(); }); it("wrong json content", async function () { @@ -64,7 +64,7 @@ describe("pkg-manifest io", function () { true ) ).should.be.ok(); - shouldHaveNoManifest(); + shouldHaveNoManifestAt(env.manifestPath); mockConsole.hasLineIncluding("out", "failed to parse").should.be.ok(); }); it("saveManifest", async function () { @@ -74,11 +74,11 @@ describe("pkg-manifest io", function () { true ) ).should.be.ok(); - const manifest = shouldHaveManifest(); + const manifest = shouldHaveManifestAt(env.manifestPath); shouldNotHaveAnyDependencies(manifest); addDependency(manifest, domainName("some-pack"), semanticVersion("1.0.0")); saveManifest(env.manifestPath, manifest).should.be.ok(); - const manifest2 = shouldHaveManifest(); + const manifest2 = shouldHaveManifestAt(env.manifestPath); manifest2.should.be.deepEqual(manifest); }); }); From adba03326eee1ce2fd06ca1a3f1329bdbb6b094c Mon Sep 17 00:00:00 2001 From: Ramon Brullo Date: Thu, 14 Dec 2023 18:33:44 +0100 Subject: [PATCH 09/18] refactor: move up env usage Remove env usage from writeUnityToken --- src/cmd-login.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/cmd-login.ts b/src/cmd-login.ts index e3cda625..ca4746b0 100644 --- a/src/cmd-login.ts +++ b/src/cmd-login.ts @@ -62,7 +62,9 @@ export const login = async function (options: LoginOptions) { } // write unity token + const configDir = await getUpmConfigDir(env.wsl, env.systemUser); await writeUnityToken( + configDir, _auth, options.alwaysAuth || false, options.basicAuth || false, @@ -179,6 +181,7 @@ export const generateNpmrcLines = function ( * Write npm token to Unity */ const writeUnityToken = async function ( + configDir: string, _auth: Base64 | null, alwaysAuth: boolean, basicAuth: boolean, @@ -186,8 +189,6 @@ const writeUnityToken = async function ( registry: RegistryUrl, token: string | null ) { - // Create config dir if necessary - const configDir = await getUpmConfigDir(env.wsl, env.systemUser); // Read config file const config = (await loadUpmConfig(configDir)) || {}; if (!config.npmAuth) config.npmAuth = {}; From 265467ea20ceb4fb5a7c185f08b22ec6a58f4f39 Mon Sep 17 00:00:00 2001 From: Ramon Brullo Date: Thu, 14 Dec 2023 18:55:39 +0100 Subject: [PATCH 10/18] refactor: extract helper function --- src/types/pkg-manifest.ts | 10 ++++++++++ src/utils/env.ts | 3 ++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/types/pkg-manifest.ts b/src/types/pkg-manifest.ts index 54d99295..cbf2a7bb 100644 --- a/src/types/pkg-manifest.ts +++ b/src/types/pkg-manifest.ts @@ -3,6 +3,7 @@ import { SemanticVersion } from "./semantic-version"; import { PackageUrl } from "./package-url"; import { ScopedRegistry } from "./scoped-registry"; import { RegistryUrl, removeTrailingSlash } from "./registry-url"; +import path from "path"; /** * The content of the package-manifest (manifest.json) of a Unity project @@ -96,3 +97,12 @@ export function addTestable(manifest: PkgManifest, name: DomainName) { if (manifest.testables.indexOf(name) === -1) manifest.testables.push(name); manifest.testables.sort(); } + +/** + * Determines the path to the package manifest based on the working + * directory (Root of Unity project). + * @param workingDirectory The working directory + */ +export function manifestPathFor(workingDirectory: string): string { + return path.join(workingDirectory, "Packages/manifest.json"); +} diff --git a/src/utils/env.ts b/src/utils/env.ts index a50c2ef3..967d7c70 100644 --- a/src/utils/env.ts +++ b/src/utils/env.ts @@ -26,6 +26,7 @@ import { import { encodeBase64 } from "../types/base64"; import { NpmAuth } from "another-npm-registry-client"; import { CmdOptions } from "../types/options"; +import { manifestPathFor } from "../types/pkg-manifest"; type Region = "us" | "cn"; @@ -141,7 +142,7 @@ export const parseEnv = async function ( env.cwd = cwd; } else env.cwd = process.cwd(); // manifest path - const manifestPath = path.join(env.cwd, "Packages/manifest.json"); + const manifestPath = manifestPathFor(env.cwd); if (!fs.existsSync(manifestPath)) { log.error( "manifest", From 034b9d131c5468a48db1f82dc4a79f34b0677ea3 Mon Sep 17 00:00:00 2001 From: Ramon Brullo Date: Thu, 14 Dec 2023 19:03:20 +0100 Subject: [PATCH 11/18] refactor: manifest-path logic Manifest-path is no longer stored on env, but calculated on demand from cwd. The goal here is to shrink env / deduplicate information in it. parseEnv still checks whether manifest-path exists --- src/cmd-add.ts | 4 ++-- src/cmd-remove.ts | 4 ++-- src/utils/env.ts | 4 +--- src/utils/pkg-manifest-io.ts | 28 ++++++++++++++++------------ test/manifest-assertions.ts | 8 ++++---- test/test-cmd-add.ts | 32 ++++++++++++++++---------------- test/test-cmd-remove.ts | 10 +++++----- test/test-env.ts | 4 ---- test/test-pkg-manifest-io.ts | 23 ++++++++++++++--------- 9 files changed, 60 insertions(+), 57 deletions(-) diff --git a/src/cmd-add.ts b/src/cmd-add.ts index 3fe768e9..54fa5162 100644 --- a/src/cmd-add.ts +++ b/src/cmd-add.ts @@ -70,7 +70,7 @@ export const add = async function ( let version = split[1]; // load manifest - const manifest = loadManifest(env.manifestPath); + const manifest = loadManifest(env.cwd); if (manifest === null) return { code: 1, dirty }; // packages that added to scope registry const pkgsInScope: DomainName[] = []; @@ -231,7 +231,7 @@ export const add = async function ( if (options.test) addTestable(manifest, name); // save manifest if (dirty) { - if (!saveManifest(env.manifestPath, manifest)) return { code: 1, dirty }; + if (!saveManifest(env.cwd, manifest)) return { code: 1, dirty }; } return { code: 0, dirty }; }; diff --git a/src/cmd-remove.ts b/src/cmd-remove.ts index 93ea38ad..5887594c 100644 --- a/src/cmd-remove.ts +++ b/src/cmd-remove.ts @@ -40,7 +40,7 @@ export const remove = async function ( return { code: 1, dirty }; } // load manifest - const manifest = loadManifest(env.manifestPath); + const manifest = loadManifest(env.cwd); if (manifest === null) return { code: 1, dirty }; // not found array const pkgsNotFound = []; @@ -63,7 +63,7 @@ export const remove = async function ( } // save manifest if (dirty) { - if (!saveManifest(env.manifestPath, manifest)) return { code: 1, dirty }; + if (!saveManifest(env.cwd, manifest)) return { code: 1, dirty }; } if (pkgsNotFound.length) { log.error("404", `package not found: ${pkgsNotFound.join(", ")}`); diff --git a/src/utils/env.ts b/src/utils/env.ts index 967d7c70..ae514292 100644 --- a/src/utils/env.ts +++ b/src/utils/env.ts @@ -43,7 +43,6 @@ export type Env = { namespace: DomainName | IpAddress; editorVersion: string | null; region: Region; - manifestPath: string; }; export const env: Env = {}; @@ -56,7 +55,6 @@ export const parseEnv = async function ( // set defaults env.registry = registryUrl("https://package.openupm.com"); env.cwd = ""; - env.manifestPath = ""; env.namespace = openUpmReverseDomainName; env.upstream = true; env.color = true; @@ -149,7 +147,7 @@ export const parseEnv = async function ( `can not locate manifest.json at path ${manifestPath}` ); return false; - } else env.manifestPath = manifestPath; + } // editor version const projectVersionPath = path.join( env.cwd, diff --git a/src/utils/pkg-manifest-io.ts b/src/utils/pkg-manifest-io.ts index 33d73ac4..1817698b 100644 --- a/src/utils/pkg-manifest-io.ts +++ b/src/utils/pkg-manifest-io.ts @@ -1,25 +1,25 @@ import fs from "fs"; import { assertIsError } from "./error-type-guards"; import log from "../logger"; -import { PkgManifest } from "../types/pkg-manifest"; +import { manifestPathFor, PkgManifest } from "../types/pkg-manifest"; /** * Attempts to load the manifest from the path specified in env - * @param path The path where the manifest is located + * @param workingDirectory The working directory */ -export const loadManifest = function (path: string): PkgManifest | null { +export const loadManifest = function ( + workingDirectory: string +): PkgManifest | null { + const manifestPath = manifestPathFor(workingDirectory); try { - const text = fs.readFileSync(path, { encoding: "utf8" }); + const text = fs.readFileSync(manifestPath, { encoding: "utf8" }); return JSON.parse(text); } catch (err) { assertIsError(err); if (err.code == "ENOENT") - log.error("manifest", "file Packages/manifest.json does not exist"); + log.error("manifest", `manifest at ${manifestPath} does not exist`); else { - log.error( - "manifest", - `failed to parse Packages/manifest.json at ${path}` - ); + log.error("manifest", `failed to parse manifest at ${manifestPath}`); log.error("manifest", err.message); } return null; @@ -28,13 +28,17 @@ export const loadManifest = function (path: string): PkgManifest | null { /** * Save manifest json file to the path specified in enva - * @param path The path where the manifest is located + * @param workingDirectory The working directory * @param data The manifest to save */ -export const saveManifest = function (path: string, data: PkgManifest) { +export const saveManifest = function ( + workingDirectory: string, + data: PkgManifest +) { + const manifestPath = manifestPathFor(workingDirectory); const json = JSON.stringify(data, null, 2); try { - fs.writeFileSync(path, json); + fs.writeFileSync(manifestPath, json); return true; } catch (err) { assertIsError(err); diff --git a/test/manifest-assertions.ts b/test/manifest-assertions.ts index 62741322..1fd6318b 100644 --- a/test/manifest-assertions.ts +++ b/test/manifest-assertions.ts @@ -6,14 +6,14 @@ import { PackageUrl } from "../src/types/package-url"; import { hasScope } from "../src/types/scoped-registry"; import { PkgManifest } from "../src/types/pkg-manifest"; -export function shouldHaveManifestAt(manifestPath: string): PkgManifest { - const manifest = loadManifest(manifestPath); +export function shouldHaveManifest(workingDirectory: string): PkgManifest { + const manifest = loadManifest(workingDirectory); should(manifest).not.be.null(); return manifest!; } -export function shouldHaveNoManifestAt(manifestPath: string) { - const manifest = loadManifest(manifestPath); +export function shouldHaveNoManifest(workingDirectory: string) { + const manifest = loadManifest(workingDirectory); should(manifest).be.null(); } diff --git a/test/test-cmd-add.ts b/test/test-cmd-add.ts index 0f37f6b4..78476ec7 100644 --- a/test/test-cmd-add.ts +++ b/test/test-cmd-add.ts @@ -12,7 +12,7 @@ import { createWorkDir, getWorkDir, removeWorkDir } from "./mock-work-dir"; import { attachMockConsole, MockConsole } from "./mock-console"; import { shouldHaveDependency, - shouldHaveManifestAt, + shouldHaveManifest, } from "./manifest-assertions"; import { buildPackageInfo } from "./data-pkg-info"; import { buildPackageManifest } from "./data-pkg-manifest"; @@ -163,7 +163,7 @@ describe("cmd-add.ts", function () { it("add pkg", async function () { const retCode = await add(packageA, options); retCode.should.equal(0); - const manifest = shouldHaveManifestAt(env.manifestPath); + const manifest = shouldHaveManifest(env.cwd); manifest.should.deepEqual(expectedManifestA); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -174,7 +174,7 @@ describe("cmd-add.ts", function () { options ); retCode.should.equal(0); - const manifest = shouldHaveManifestAt(env.manifestPath); + const manifest = shouldHaveManifest(env.cwd); manifest.should.deepEqual(expectedManifestA); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -182,7 +182,7 @@ describe("cmd-add.ts", function () { it("add pkg@latest", async function () { const retCode = await add(packageReference(packageA, "latest"), options); retCode.should.equal(0); - const manifest = shouldHaveManifestAt(env.manifestPath); + const manifest = shouldHaveManifest(env.cwd); manifest.should.deepEqual(expectedManifestA); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -198,7 +198,7 @@ describe("cmd-add.ts", function () { options ); retCode2.should.equal(0); - const manifest = shouldHaveManifestAt(env.manifestPath); + const manifest = shouldHaveManifest(env.cwd); manifest.should.deepEqual(expectedManifestA); mockConsole.hasLineIncluding("out", "modified ").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -214,7 +214,7 @@ describe("cmd-add.ts", function () { options ); retCode2.should.equal(0); - const manifest = shouldHaveManifestAt(env.manifestPath); + const manifest = shouldHaveManifest(env.cwd); manifest.should.deepEqual(expectedManifestA); mockConsole.hasLineIncluding("out", "existed ").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -225,7 +225,7 @@ describe("cmd-add.ts", function () { options ); retCode.should.equal(1); - const manifest = shouldHaveManifestAt(env.manifestPath); + const manifest = shouldHaveManifest(env.cwd); manifest.should.deepEqual(defaultManifest); mockConsole .hasLineIncluding("out", "version 2.0.0 is not a valid choice") @@ -236,7 +236,7 @@ describe("cmd-add.ts", function () { const gitUrl = "https://github.com/yo/com.base.package-a" as PackageUrl; const retCode = await add(packageReference(packageA, gitUrl), options); retCode.should.equal(0); - const manifest = shouldHaveManifestAt(env.manifestPath); + const manifest = shouldHaveManifest(env.cwd); shouldHaveDependency(manifest, packageA, gitUrl); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -245,7 +245,7 @@ describe("cmd-add.ts", function () { const gitUrl = "git@github.com:yo/com.base.package-a" as PackageUrl; const retCode = await add(packageReference(packageA, gitUrl), options); retCode.should.equal(0); - const manifest = shouldHaveManifestAt(env.manifestPath); + const manifest = shouldHaveManifest(env.cwd); shouldHaveDependency(manifest, packageA, gitUrl); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -254,7 +254,7 @@ describe("cmd-add.ts", function () { const fileUrl = "file../yo/com.base.package-a" as PackageUrl; const retCode = await add(packageReference(packageA, fileUrl), options); retCode.should.equal(0); - const manifest = shouldHaveManifestAt(env.manifestPath); + const manifest = shouldHaveManifest(env.cwd); shouldHaveDependency(manifest, packageA, fileUrl); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -262,14 +262,14 @@ describe("cmd-add.ts", function () { it("add pkg-not-exist", async function () { const retCode = await add(packageMissing, options); retCode.should.equal(1); - const manifest = shouldHaveManifestAt(env.manifestPath); + const manifest = shouldHaveManifest(env.cwd); manifest.should.deepEqual(defaultManifest); mockConsole.hasLineIncluding("out", "package not found").should.be.ok(); }); it("add more than one pkgs", async function () { const retCode = await add([packageA, packageB], options); retCode.should.equal(0); - const manifest = shouldHaveManifestAt(env.manifestPath); + const manifest = shouldHaveManifest(env.cwd); manifest.should.deepEqual(expectedManifestAB); mockConsole .hasLineIncluding("out", "added com.base.package-a") @@ -282,7 +282,7 @@ describe("cmd-add.ts", function () { it("add pkg from upstream", async function () { const retCode = await add(packageUp, upstreamOptions); retCode.should.equal(0); - const manifest = shouldHaveManifestAt(env.manifestPath); + const manifest = shouldHaveManifest(env.cwd); manifest.should.deepEqual(expectedManifestUpstream); mockConsole .hasLineIncluding("out", "added com.upstream.package-up") @@ -292,7 +292,7 @@ describe("cmd-add.ts", function () { it("add pkg-not-exist from upstream", async function () { const retCode = await add(packageMissing, upstreamOptions); retCode.should.equal(1); - const manifest = shouldHaveManifestAt(env.manifestPath); + const manifest = shouldHaveManifest(env.cwd); manifest.should.deepEqual(defaultManifest); mockConsole.hasLineIncluding("out", "package not found").should.be.ok(); }); @@ -302,7 +302,7 @@ describe("cmd-add.ts", function () { upstreamOptions ); retCode.should.equal(0); - const manifest = shouldHaveManifestAt(env.manifestPath); + const manifest = shouldHaveManifest(env.cwd); manifest.should.deepEqual(expectedManifestC); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -310,7 +310,7 @@ describe("cmd-add.ts", function () { it("add pkg with tests", async function () { const retCode = await add(packageA, testableOptions); retCode.should.equal(0); - const manifest = shouldHaveManifestAt(env.manifestPath); + const manifest = shouldHaveManifest(env.cwd); manifest.should.deepEqual(expectedManifestTestable); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); diff --git a/test/test-cmd-remove.ts b/test/test-cmd-remove.ts index ca331e69..834d54a2 100644 --- a/test/test-cmd-remove.ts +++ b/test/test-cmd-remove.ts @@ -7,7 +7,7 @@ import { import { createWorkDir, getWorkDir, removeWorkDir } from "./mock-work-dir"; import { attachMockConsole, MockConsole } from "./mock-console"; import { - shouldHaveManifestAt, + shouldHaveManifest, shouldHaveRegistryWithScopes, shouldNotHaveDependency, } from "./manifest-assertions"; @@ -51,7 +51,7 @@ describe("cmd-remove.ts", function () { }; const retCode = await remove(packageA, options); retCode.should.equal(0); - const manifest = shouldHaveManifestAt(env.manifestPath); + const manifest = shouldHaveManifest(env.cwd); shouldNotHaveDependency(manifest, packageA); shouldHaveRegistryWithScopes(manifest, [ exampleRegistryReverseDomain, @@ -72,7 +72,7 @@ describe("cmd-remove.ts", function () { options ); retCode.should.equal(1); - const manifest = shouldHaveManifestAt(env.manifestPath); + const manifest = shouldHaveManifest(env.cwd); manifest.should.deepEqual(defaultManifest); mockConsole.hasLineIncluding("out", "please replace").should.be.ok(); }); @@ -85,7 +85,7 @@ describe("cmd-remove.ts", function () { }; const retCode = await remove(missingPackage, options); retCode.should.equal(1); - const manifest = shouldHaveManifestAt(env.manifestPath); + const manifest = shouldHaveManifest(env.cwd); manifest.should.deepEqual(defaultManifest); mockConsole.hasLineIncluding("out", "package not found").should.be.ok(); }); @@ -98,7 +98,7 @@ describe("cmd-remove.ts", function () { }; const retCode = await remove([packageA, packageB], options); retCode.should.equal(0); - const manifest = shouldHaveManifestAt(env.manifestPath); + const manifest = shouldHaveManifest(env.cwd); shouldNotHaveDependency(manifest, packageA); shouldNotHaveDependency(manifest, packageB); shouldHaveRegistryWithScopes(manifest, [exampleRegistryReverseDomain]); diff --git a/test/test-env.ts b/test/test-env.ts index 9e4280b9..3a085b9b 100644 --- a/test/test-env.ts +++ b/test/test-env.ts @@ -37,7 +37,6 @@ describe("env", function () { env.upstreamRegistry.should.equal("https://packages.unity.com"); env.namespace.should.equal("com.openupm"); env.cwd.should.equal(""); - env.manifestPath.should.equal(""); (env.editorVersion === null).should.be.ok(); }); it("check path", async function () { @@ -48,9 +47,6 @@ describe("env", function () { ) ).should.be.ok(); env.cwd.should.be.equal(getWorkDir("test-openupm-cli")); - env.manifestPath.should.be.equal( - path.join(getWorkDir("test-openupm-cli"), "Packages/manifest.json") - ); }); it("can not resolve path", async function () { ( diff --git a/test/test-pkg-manifest-io.ts b/test/test-pkg-manifest-io.ts index 784fbbe4..e65e2880 100644 --- a/test/test-pkg-manifest-io.ts +++ b/test/test-pkg-manifest-io.ts @@ -7,13 +7,14 @@ import { describe } from "mocha"; import { env, parseEnv } from "../src/utils/env"; import { createWorkDir, getWorkDir, removeWorkDir } from "./mock-work-dir"; import { - shouldHaveManifestAt, - shouldHaveNoManifestAt, + shouldHaveManifest, + shouldHaveNoManifest, shouldNotHaveAnyDependencies, } from "./manifest-assertions"; import { domainName } from "../src/types/domain-name"; import { semanticVersion } from "../src/types/semantic-version"; -import { addDependency } from "../src/types/pkg-manifest"; +import { addDependency, manifestPathFor } from "../src/types/pkg-manifest"; +import should from "should"; describe("pkg-manifest io", function () { let mockConsole: MockConsole = null!; @@ -44,7 +45,7 @@ describe("pkg-manifest io", function () { true ) ).should.be.ok(); - const manifest = shouldHaveManifestAt(env.manifestPath); + const manifest = shouldHaveManifest(env.cwd); manifest.should.be.deepEqual({ dependencies: {} }); }); it("no manifest file", async function () { @@ -54,7 +55,7 @@ describe("pkg-manifest io", function () { false ) ).should.be.ok(); - shouldHaveNoManifestAt(env.manifestPath); + shouldHaveNoManifest(env.cwd); mockConsole.hasLineIncluding("out", "does not exist").should.be.ok(); }); it("wrong json content", async function () { @@ -64,7 +65,7 @@ describe("pkg-manifest io", function () { true ) ).should.be.ok(); - shouldHaveNoManifestAt(env.manifestPath); + shouldHaveNoManifest(env.cwd); mockConsole.hasLineIncluding("out", "failed to parse").should.be.ok(); }); it("saveManifest", async function () { @@ -74,11 +75,15 @@ describe("pkg-manifest io", function () { true ) ).should.be.ok(); - const manifest = shouldHaveManifestAt(env.manifestPath); + const manifest = shouldHaveManifest(env.cwd); shouldNotHaveAnyDependencies(manifest); addDependency(manifest, domainName("some-pack"), semanticVersion("1.0.0")); - saveManifest(env.manifestPath, manifest).should.be.ok(); - const manifest2 = shouldHaveManifestAt(env.manifestPath); + saveManifest(env.cwd, manifest).should.be.ok(); + const manifest2 = shouldHaveManifest(env.cwd); manifest2.should.be.deepEqual(manifest); }); + it("manifest-path is correct", function () { + const manifestPath = manifestPathFor("test-openupm-cli"); + should(manifestPath).be.equal("test-openupm-cli/Packages/manifest.json"); + }); }); From 43524db5921054da2d0886c343a9c2925fd9da9d Mon Sep 17 00:00:00 2001 From: Ramon Brullo Date: Thu, 14 Dec 2023 19:20:14 +0100 Subject: [PATCH 12/18] refactor: de-global env Make env a non-global value --- src/cmd-add.ts | 6 +- src/cmd-deps.ts | 6 +- src/cmd-login.ts | 6 +- src/cmd-remove.ts | 6 +- src/cmd-search.ts | 6 +- src/cmd-view.ts | 6 +- src/utils/env.ts | 14 +-- test/mock-work-dir.ts | 3 +- test/test-cmd-add.ts | 47 ++++---- test/test-cmd-remove.ts | 25 ++-- test/test-env.ts | 223 +++++++++++++++++------------------ test/test-pkg-manifest-io.ts | 61 +++------- test/test-registry-client.ts | 23 ++-- 13 files changed, 204 insertions(+), 228 deletions(-) diff --git a/src/cmd-add.ts b/src/cmd-add.ts index 54fa5162..cc654cc8 100644 --- a/src/cmd-add.ts +++ b/src/cmd-add.ts @@ -3,7 +3,7 @@ import url from "url"; import { isPackageUrl } from "./types/package-url"; import { tryGetLatestVersion } from "./types/pkg-info"; import { loadManifest, saveManifest } from "./utils/pkg-manifest-io"; -import { env, parseEnv } from "./utils/env"; +import { parseEnv } from "./utils/env"; import { compareEditorVersion, tryParseEditorVersion, @@ -47,8 +47,8 @@ export const add = async function ( ): Promise { if (!Array.isArray(pkgs)) pkgs = [pkgs]; // parse env - const envOk = await parseEnv(options, true); - if (!envOk) return 1; + const env = await parseEnv(options, true); + if (env === null) return 1; const registry: Registry = { url: env.registry, diff --git a/src/cmd-deps.ts b/src/cmd-deps.ts index 11aa55eb..b9776b71 100644 --- a/src/cmd-deps.ts +++ b/src/cmd-deps.ts @@ -1,5 +1,5 @@ import log from "./logger"; -import { env, parseEnv } from "./utils/env"; +import { parseEnv } from "./utils/env"; import { fetchPackageDependencies, Registry } from "./registry-client"; import { isPackageUrl } from "./types/package-url"; import { @@ -18,8 +18,8 @@ export const deps = async function ( options: DepsOptions ) { // parse env - const envOk = await parseEnv(options, false); - if (!envOk) return 1; + const env = await parseEnv(options, false); + if (env === null) return 1; const registry: Registry = { url: env.registry, diff --git a/src/cmd-login.ts b/src/cmd-login.ts index ca4746b0..07cba4cd 100644 --- a/src/cmd-login.ts +++ b/src/cmd-login.ts @@ -8,7 +8,7 @@ import { loadUpmConfig, saveUpmConfig, } from "./utils/upm-config-io"; -import { env, parseEnv } from "./utils/env"; +import { parseEnv } from "./utils/env"; import { encodeBasicAuth } from "./types/upm-config"; import { Base64 } from "./types/base64"; import { RegistryUrl, removeTrailingSlash } from "./types/registry-url"; @@ -30,8 +30,8 @@ export type LoginOptions = CmdOptions<{ export const login = async function (options: LoginOptions) { // parse env - const envOk = await parseEnv(options, false); - if (!envOk) return 1; + const env = await parseEnv(options, false); + if (env === null) return 1; // query parameters if (!options.username) options.username = await promptUsername(); if (!options.password) options.password = await promptPassword(); diff --git a/src/cmd-remove.ts b/src/cmd-remove.ts index 5887594c..3c7271e0 100644 --- a/src/cmd-remove.ts +++ b/src/cmd-remove.ts @@ -1,6 +1,6 @@ import log from "./logger"; import { loadManifest, saveManifest } from "./utils/pkg-manifest-io"; -import { env, parseEnv } from "./utils/env"; +import { parseEnv } from "./utils/env"; import { isDomainName } from "./types/domain-name"; import { packageReference, @@ -22,8 +22,8 @@ export const remove = async function ( ) { if (!Array.isArray(pkgs)) pkgs = [pkgs]; // parse env - const envOk = await parseEnv(options, true); - if (!envOk) return 1; + const env = await parseEnv(options, true); + if (env === null) return 1; const removeSingle = async function (pkg: PackageReference) { // dirty flag diff --git a/src/cmd-search.ts b/src/cmd-search.ts index 705ff21a..98288538 100644 --- a/src/cmd-search.ts +++ b/src/cmd-search.ts @@ -6,7 +6,7 @@ import { is404Error, isHttpError } from "./utils/error-type-guards"; import * as os from "os"; import assert from "assert"; import { PkgInfo, tryGetLatestVersion } from "./types/pkg-info"; -import { env, parseEnv } from "./utils/env"; +import { parseEnv } from "./utils/env"; import { DomainName } from "./types/domain-name"; import { SemanticVersion } from "./types/semantic-version"; import { CmdOptions } from "./types/options"; @@ -118,8 +118,8 @@ const getTableRow = function (pkg: SearchedPkgInfo): TableRow { export async function search(keyword: string, options: SearchOptions) { // parse env - const envOk = await parseEnv(options, false); - if (!envOk) return 1; + const env = await parseEnv(options, false); + if (env === null) return 1; const registry: Registry = { url: env.registry, diff --git a/src/cmd-view.ts b/src/cmd-view.ts index a20aaddc..40a9101f 100644 --- a/src/cmd-view.ts +++ b/src/cmd-view.ts @@ -2,7 +2,7 @@ import chalk from "chalk"; import log from "./logger"; import assert from "assert"; import { PkgInfo, tryGetLatestVersion } from "./types/pkg-info"; -import { env, parseEnv } from "./utils/env"; +import { parseEnv } from "./utils/env"; import { fetchPackageInfo } from "./registry-client"; import { DomainName } from "./types/domain-name"; import { @@ -19,8 +19,8 @@ export const view = async function ( options: ViewOptions ) { // parse env - const envOk = await parseEnv(options, false); - if (!envOk) return 1; + const env = await parseEnv(options, false); + if (env === null) return 1; // parse name const [name, version] = splitPackageReference(pkg); if (version) { diff --git a/src/utils/env.ts b/src/utils/env.ts index ae514292..fcdff5c1 100644 --- a/src/utils/env.ts +++ b/src/utils/env.ts @@ -45,14 +45,13 @@ export type Env = { region: Region; }; -export const env: Env = {}; - // Parse env export const parseEnv = async function ( options: CmdOptions, checkPath: boolean -) { +): Promise { // set defaults + const env = {}; env.registry = registryUrl("https://package.openupm.com"); env.cwd = ""; env.namespace = openUpmReverseDomainName; @@ -129,13 +128,13 @@ export const parseEnv = async function ( // log.verbose("env.npmAuth", env.npmAuth); // log.verbose("env.auth", env.auth); // return if no need to check path - if (!checkPath) return true; + if (!checkPath) return env; // cwd if (options._global.chdir) { const cwd = path.resolve(options._global.chdir); if (!fs.existsSync(cwd)) { log.error("env", `can not resolve path ${cwd}`); - return false; + return null; } env.cwd = cwd; } else env.cwd = process.cwd(); @@ -146,8 +145,9 @@ export const parseEnv = async function ( "manifest", `can not locate manifest.json at path ${manifestPath}` ); - return false; + return null; } + // editor version const projectVersionPath = path.join( env.cwd, @@ -164,5 +164,5 @@ export const parseEnv = async function ( env.editorVersion = projectVersionContent.m_EditorVersion; } // return - return true; + return env; }; diff --git a/test/mock-work-dir.ts b/test/mock-work-dir.ts index fd795ea5..222340ad 100644 --- a/test/mock-work-dir.ts +++ b/test/mock-work-dir.ts @@ -14,7 +14,7 @@ export const getWorkDir = function (pathToTmp: string): string { export const createWorkDir = function ( pathToTmp: string, { manifest, editorVersion }: ManifestCreationOptions -) { +): string { const workDir = getWorkDir(pathToTmp); fse.mkdirpSync(workDir); if (manifest) { @@ -33,6 +33,7 @@ export const createWorkDir = function ( data ); } + return workDir; }; export const removeWorkDir = function (pathToTmp: string) { const cwd = getWorkDir(pathToTmp); diff --git a/test/test-cmd-add.ts b/test/test-cmd-add.ts index 78476ec7..19efa4b8 100644 --- a/test/test-cmd-add.ts +++ b/test/test-cmd-add.ts @@ -20,9 +20,9 @@ import { domainName } from "../src/types/domain-name"; import { PackageUrl } from "../src/types/package-url"; import { semanticVersion } from "../src/types/semantic-version"; import { packageReference } from "../src/types/package-reference"; -import { env } from "../src/utils/env"; describe("cmd-add.ts", function () { + const workDirName = "test-openupm-cli"; const packageMissing = domainName("pkg-not-exist"); const packageA = domainName("com.base.package-a"); const packageB = domainName("com.base.package-b"); @@ -43,21 +43,21 @@ describe("cmd-add.ts", function () { _global: { registry: exampleRegistryUrl, upstream: false, - chdir: getWorkDir("test-openupm-cli"), + chdir: getWorkDir(workDirName), }, }; const upstreamOptions: AddOptions = { _global: { registry: exampleRegistryUrl, upstream: true, - chdir: getWorkDir("test-openupm-cli"), + chdir: getWorkDir(workDirName), }, }; const testableOptions: AddOptions = { _global: { registry: exampleRegistryUrl, upstream: false, - chdir: getWorkDir("test-openupm-cli"), + chdir: getWorkDir(workDirName), }, test: true, }; @@ -65,12 +65,13 @@ describe("cmd-add.ts", function () { _global: { registry: exampleRegistryUrl, upstream: false, - chdir: getWorkDir("test-openupm-cli"), + chdir: getWorkDir(workDirName), }, force: true, }; describe("add", function () { let mockConsole: MockConsole = null!; + let workDir = ""; const remotePkgInfoA = buildPackageInfo(packageA, (pkg) => pkg.addVersion("0.1.0").addVersion("1.0.0") @@ -135,8 +136,8 @@ describe("cmd-add.ts", function () { ); beforeEach(function () { - removeWorkDir("test-openupm-cli"); - createWorkDir("test-openupm-cli", { + removeWorkDir(workDirName); + workDir = createWorkDir(workDirName, { manifest: true, editorVersion: "2019.2.13f1", }); @@ -155,7 +156,7 @@ describe("cmd-add.ts", function () { mockConsole = attachMockConsole(); }); afterEach(function () { - removeWorkDir("test-openupm-cli"); + removeWorkDir(workDirName); stopMockRegistry(); mockConsole.detach(); }); @@ -163,7 +164,7 @@ describe("cmd-add.ts", function () { it("add pkg", async function () { const retCode = await add(packageA, options); retCode.should.equal(0); - const manifest = shouldHaveManifest(env.cwd); + const manifest = shouldHaveManifest(workDir); manifest.should.deepEqual(expectedManifestA); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -174,7 +175,7 @@ describe("cmd-add.ts", function () { options ); retCode.should.equal(0); - const manifest = shouldHaveManifest(env.cwd); + const manifest = shouldHaveManifest(workDir); manifest.should.deepEqual(expectedManifestA); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -182,7 +183,7 @@ describe("cmd-add.ts", function () { it("add pkg@latest", async function () { const retCode = await add(packageReference(packageA, "latest"), options); retCode.should.equal(0); - const manifest = shouldHaveManifest(env.cwd); + const manifest = shouldHaveManifest(workDir); manifest.should.deepEqual(expectedManifestA); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -198,7 +199,7 @@ describe("cmd-add.ts", function () { options ); retCode2.should.equal(0); - const manifest = shouldHaveManifest(env.cwd); + const manifest = shouldHaveManifest(workDir); manifest.should.deepEqual(expectedManifestA); mockConsole.hasLineIncluding("out", "modified ").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -214,7 +215,7 @@ describe("cmd-add.ts", function () { options ); retCode2.should.equal(0); - const manifest = shouldHaveManifest(env.cwd); + const manifest = shouldHaveManifest(workDir); manifest.should.deepEqual(expectedManifestA); mockConsole.hasLineIncluding("out", "existed ").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -225,7 +226,7 @@ describe("cmd-add.ts", function () { options ); retCode.should.equal(1); - const manifest = shouldHaveManifest(env.cwd); + const manifest = shouldHaveManifest(workDir); manifest.should.deepEqual(defaultManifest); mockConsole .hasLineIncluding("out", "version 2.0.0 is not a valid choice") @@ -236,7 +237,7 @@ describe("cmd-add.ts", function () { const gitUrl = "https://github.com/yo/com.base.package-a" as PackageUrl; const retCode = await add(packageReference(packageA, gitUrl), options); retCode.should.equal(0); - const manifest = shouldHaveManifest(env.cwd); + const manifest = shouldHaveManifest(workDir); shouldHaveDependency(manifest, packageA, gitUrl); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -245,7 +246,7 @@ describe("cmd-add.ts", function () { const gitUrl = "git@github.com:yo/com.base.package-a" as PackageUrl; const retCode = await add(packageReference(packageA, gitUrl), options); retCode.should.equal(0); - const manifest = shouldHaveManifest(env.cwd); + const manifest = shouldHaveManifest(workDir); shouldHaveDependency(manifest, packageA, gitUrl); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -254,7 +255,7 @@ describe("cmd-add.ts", function () { const fileUrl = "file../yo/com.base.package-a" as PackageUrl; const retCode = await add(packageReference(packageA, fileUrl), options); retCode.should.equal(0); - const manifest = shouldHaveManifest(env.cwd); + const manifest = shouldHaveManifest(workDir); shouldHaveDependency(manifest, packageA, fileUrl); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -262,14 +263,14 @@ describe("cmd-add.ts", function () { it("add pkg-not-exist", async function () { const retCode = await add(packageMissing, options); retCode.should.equal(1); - const manifest = shouldHaveManifest(env.cwd); + const manifest = shouldHaveManifest(workDir); manifest.should.deepEqual(defaultManifest); mockConsole.hasLineIncluding("out", "package not found").should.be.ok(); }); it("add more than one pkgs", async function () { const retCode = await add([packageA, packageB], options); retCode.should.equal(0); - const manifest = shouldHaveManifest(env.cwd); + const manifest = shouldHaveManifest(workDir); manifest.should.deepEqual(expectedManifestAB); mockConsole .hasLineIncluding("out", "added com.base.package-a") @@ -282,7 +283,7 @@ describe("cmd-add.ts", function () { it("add pkg from upstream", async function () { const retCode = await add(packageUp, upstreamOptions); retCode.should.equal(0); - const manifest = shouldHaveManifest(env.cwd); + const manifest = shouldHaveManifest(workDir); manifest.should.deepEqual(expectedManifestUpstream); mockConsole .hasLineIncluding("out", "added com.upstream.package-up") @@ -292,7 +293,7 @@ describe("cmd-add.ts", function () { it("add pkg-not-exist from upstream", async function () { const retCode = await add(packageMissing, upstreamOptions); retCode.should.equal(1); - const manifest = shouldHaveManifest(env.cwd); + const manifest = shouldHaveManifest(workDir); manifest.should.deepEqual(defaultManifest); mockConsole.hasLineIncluding("out", "package not found").should.be.ok(); }); @@ -302,7 +303,7 @@ describe("cmd-add.ts", function () { upstreamOptions ); retCode.should.equal(0); - const manifest = shouldHaveManifest(env.cwd); + const manifest = shouldHaveManifest(workDir); manifest.should.deepEqual(expectedManifestC); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); @@ -310,7 +311,7 @@ describe("cmd-add.ts", function () { it("add pkg with tests", async function () { const retCode = await add(packageA, testableOptions); retCode.should.equal(0); - const manifest = shouldHaveManifest(env.cwd); + const manifest = shouldHaveManifest(workDir); manifest.should.deepEqual(expectedManifestTestable); mockConsole.hasLineIncluding("out", "added").should.be.ok(); mockConsole.hasLineIncluding("out", "open Unity").should.be.ok(); diff --git a/test/test-cmd-remove.ts b/test/test-cmd-remove.ts index 834d54a2..68d55d3b 100644 --- a/test/test-cmd-remove.ts +++ b/test/test-cmd-remove.ts @@ -15,15 +15,16 @@ import { buildPackageManifest } from "./data-pkg-manifest"; import { domainName } from "../src/types/domain-name"; import { semanticVersion } from "../src/types/semantic-version"; import { packageReference } from "../src/types/package-reference"; -import { env } from "../src/utils/env"; const packageA = domainName("com.example.package-a"); const packageB = domainName("com.example.package-b"); const missingPackage = domainName("pkg-not-exist"); +const workDirName = "test-openupm-cli"; describe("cmd-remove.ts", function () { describe("remove", function () { let mockConsole: MockConsole = null!; + let workDir = ""; const defaultManifest = buildPackageManifest((manifest) => manifest @@ -32,26 +33,26 @@ describe("cmd-remove.ts", function () { ); beforeEach(function () { - removeWorkDir("test-openupm-cli"); - createWorkDir("test-openupm-cli", { + removeWorkDir(workDirName); + workDir = createWorkDir(workDirName, { manifest: defaultManifest, }); mockConsole = attachMockConsole(); }); afterEach(function () { - removeWorkDir("test-openupm-cli"); + removeWorkDir(workDirName); mockConsole.detach(); }); it("remove pkg", async function () { const options = { _global: { registry: exampleRegistryUrl, - chdir: getWorkDir("test-openupm-cli"), + chdir: workDir, }, }; const retCode = await remove(packageA, options); retCode.should.equal(0); - const manifest = shouldHaveManifest(env.cwd); + const manifest = shouldHaveManifest(workDir); shouldNotHaveDependency(manifest, packageA); shouldHaveRegistryWithScopes(manifest, [ exampleRegistryReverseDomain, @@ -64,7 +65,7 @@ describe("cmd-remove.ts", function () { const options = { _global: { registry: exampleRegistryUrl, - chdir: getWorkDir("test-openupm-cli"), + chdir: workDir, }, }; const retCode = await remove( @@ -72,7 +73,7 @@ describe("cmd-remove.ts", function () { options ); retCode.should.equal(1); - const manifest = shouldHaveManifest(env.cwd); + const manifest = shouldHaveManifest(workDir); manifest.should.deepEqual(defaultManifest); mockConsole.hasLineIncluding("out", "please replace").should.be.ok(); }); @@ -80,12 +81,12 @@ describe("cmd-remove.ts", function () { const options = { _global: { registry: exampleRegistryUrl, - chdir: getWorkDir("test-openupm-cli"), + chdir: workDir, }, }; const retCode = await remove(missingPackage, options); retCode.should.equal(1); - const manifest = shouldHaveManifest(env.cwd); + const manifest = shouldHaveManifest(workDir); manifest.should.deepEqual(defaultManifest); mockConsole.hasLineIncluding("out", "package not found").should.be.ok(); }); @@ -93,12 +94,12 @@ describe("cmd-remove.ts", function () { const options = { _global: { registry: exampleRegistryUrl, - chdir: getWorkDir("test-openupm-cli"), + chdir: workDir, }, }; const retCode = await remove([packageA, packageB], options); retCode.should.equal(0); - const manifest = shouldHaveManifest(env.cwd); + const manifest = shouldHaveManifest(workDir); shouldNotHaveDependency(manifest, packageA); shouldNotHaveDependency(manifest, packageB); shouldHaveRegistryWithScopes(manifest, [exampleRegistryReverseDomain]); diff --git a/test/test-env.ts b/test/test-env.ts index 3a085b9b..b71231e7 100644 --- a/test/test-env.ts +++ b/test/test-env.ts @@ -1,6 +1,5 @@ import "should"; -import { env, parseEnv } from "../src/utils/env"; -import path from "path"; +import { parseEnv } from "../src/utils/env"; import { createWorkDir, getWorkDir, removeWorkDir } from "./mock-work-dir"; import { attachMockConsole, MockConsole } from "./mock-console"; import should from "should"; @@ -31,156 +30,150 @@ describe("env", function () { mockConsole.detach(); }); it("defaults", async function () { - (await parseEnv({ _global: {} }, false)).should.be.ok(); - env.registry.should.equal("https://package.openupm.com"); - env.upstream.should.be.ok(); - env.upstreamRegistry.should.equal("https://packages.unity.com"); - env.namespace.should.equal("com.openupm"); - env.cwd.should.equal(""); - (env.editorVersion === null).should.be.ok(); + const env = await parseEnv({ _global: {} }, false); + should(env).not.be.null(); + env!.registry.should.equal("https://package.openupm.com"); + env!.upstream.should.be.ok(); + env!.upstreamRegistry.should.equal("https://packages.unity.com"); + env!.namespace.should.equal("com.openupm"); + env!.cwd.should.equal(""); + (env!.editorVersion === null).should.be.ok(); }); it("check path", async function () { - ( - await parseEnv( - { _global: { chdir: getWorkDir("test-openupm-cli") } }, - true - ) - ).should.be.ok(); - env.cwd.should.be.equal(getWorkDir("test-openupm-cli")); + const env = await parseEnv( + { _global: { chdir: getWorkDir("test-openupm-cli") } }, + true + ); + should(env).not.be.null(); + env!.cwd.should.be.equal(getWorkDir("test-openupm-cli")); }); it("can not resolve path", async function () { - ( - await parseEnv( - { _global: { chdir: getWorkDir("path-not-exist") } }, - true - ) - ).should.not.be.ok(); + const env = await parseEnv( + { _global: { chdir: getWorkDir("path-not-exist") } }, + true + ); + should(env).be.null(); mockConsole .hasLineIncluding("out", "can not resolve path") .should.be.ok(); }); it("can not locate manifest.json", async function () { - ( - await parseEnv( - { _global: { chdir: getWorkDir("test-openupm-cli-no-manifest") } }, - true - ) - ).should.not.be.ok(); + const env = await parseEnv( + { _global: { chdir: getWorkDir("test-openupm-cli-no-manifest") } }, + true + ); + should(env).be.null(); mockConsole .hasLineIncluding("out", "can not locate manifest.json") .should.be.ok(); }); it("custom registry", async function () { - ( - await parseEnv( - { _global: { registry: "https://registry.npmjs.org" } }, - false - ) - ).should.be.ok(); - env.registry.should.be.equal("https://registry.npmjs.org"); - env.namespace.should.be.equal("org.npmjs"); + const env = await parseEnv( + { _global: { registry: "https://registry.npmjs.org" } }, + false + ); + should(env).not.be.null(); + env!.registry.should.be.equal("https://registry.npmjs.org"); + env!.namespace.should.be.equal("org.npmjs"); }); it("custom registry with splash", async function () { - ( - await parseEnv( - { _global: { registry: "https://registry.npmjs.org/" } }, - false - ) - ).should.be.ok(); - env.registry.should.be.equal("https://registry.npmjs.org"); - env.namespace.should.be.equal("org.npmjs"); + const env = await parseEnv( + { _global: { registry: "https://registry.npmjs.org/" } }, + false + ); + should(env).not.be.null(); + env!.registry.should.be.equal("https://registry.npmjs.org"); + env!.namespace.should.be.equal("org.npmjs"); }); it("custom registry with extra path", async function () { - ( - await parseEnv( - { - _global: { - registry: "https://registry.npmjs.org/some", - }, + const env = await parseEnv( + { + _global: { + registry: "https://registry.npmjs.org/some", }, - false - ) - ).should.be.ok(); - env.registry.should.be.equal("https://registry.npmjs.org/some"); - env.namespace.should.be.equal("org.npmjs"); + }, + false + ); + should(env).not.be.null(); + env!.registry.should.be.equal("https://registry.npmjs.org/some"); + env!.namespace.should.be.equal("org.npmjs"); }); it("custom registry with extra path and splash", async function () { - ( - await parseEnv( - { - _global: { - registry: "https://registry.npmjs.org/some/", - }, + const env = await parseEnv( + { + _global: { + registry: "https://registry.npmjs.org/some/", }, - false - ) - ).should.be.ok(); - env.registry.should.be.equal("https://registry.npmjs.org/some"); - env.namespace.should.be.equal("org.npmjs"); + }, + false + ); + should(env).not.be.null(); + env!.registry.should.be.equal("https://registry.npmjs.org/some"); + env!.namespace.should.be.equal("org.npmjs"); }); it("custom registry without http", async function () { - ( - await parseEnv({ _global: { registry: "registry.npmjs.org" } }, false) - ).should.be.ok(); - env.registry.should.be.equal("http://registry.npmjs.org"); - env.namespace.should.be.equal("org.npmjs"); + const env = await parseEnv( + { _global: { registry: "registry.npmjs.org" } }, + false + ); + should(env).not.be.null(); + env!.registry.should.be.equal("http://registry.npmjs.org"); + env!.namespace.should.be.equal("org.npmjs"); }); it("custom registry with ipv4+port", async function () { - ( - await parseEnv( - { _global: { registry: "http://127.0.0.1:4873" } }, - false - ) - ).should.be.ok(); - env.registry.should.be.equal("http://127.0.0.1:4873"); - env.namespace.should.be.equal("127.0.0.1"); + const env = await parseEnv( + { _global: { registry: "http://127.0.0.1:4873" } }, + false + ); + should(env).not.be.null(); + env!.registry.should.be.equal("http://127.0.0.1:4873"); + env!.namespace.should.be.equal("127.0.0.1"); }); it("custom registry with ipv6+port", async function () { - ( - await parseEnv( - { - _global: { registry: "http://[1:2:3:4:5:6:7:8]:4873" }, - }, - false - ) - ).should.be.ok(); - env.registry.should.be.equal("http://[1:2:3:4:5:6:7:8]:4873"); - env.namespace.should.be.equal("1:2:3:4:5:6:7:8"); + const env = await parseEnv( + { + _global: { registry: "http://[1:2:3:4:5:6:7:8]:4873" }, + }, + false + ); + should(env).not.be.null(); + env!.registry.should.be.equal("http://[1:2:3:4:5:6:7:8]:4873"); + env!.namespace.should.be.equal("1:2:3:4:5:6:7:8"); }); it("upstream", async function () { - (await parseEnv({ _global: { upstream: false } }, false)).should.be.ok(); - env.upstream.should.not.be.ok(); + const env = await parseEnv({ _global: { upstream: false } }, false); + should(env).not.be.null(); + env!.upstream.should.not.be.ok(); }); it("editorVersion", async function () { - ( - await parseEnv( - { _global: { chdir: getWorkDir("test-openupm-cli") } }, - true - ) - ).should.be.ok(); - should(env.editorVersion).be.equal("2019.2.13f1"); + const env = await parseEnv( + { _global: { chdir: getWorkDir("test-openupm-cli") } }, + true + ); + should(env).not.be.null(); + should(env!.editorVersion).be.equal("2019.2.13f1"); }); it("region cn", async function () { - (await parseEnv({ _global: { cn: true } }, false)).should.be.ok(); - env.registry.should.be.equal("https://package.openupm.cn"); - env.upstreamRegistry.should.be.equal("https://packages.unity.cn"); - env.region.should.be.equal("cn"); + const env = await parseEnv({ _global: { cn: true } }, false); + should(env).not.be.null(); + env!.registry.should.be.equal("https://package.openupm.cn"); + env!.upstreamRegistry.should.be.equal("https://packages.unity.cn"); + env!.region.should.be.equal("cn"); }); it("region cn with a custom registry", async function () { - ( - await parseEnv( - { - _global: { - cn: true, - registry: "https://reg.custom-package.com", - }, + const env = await parseEnv( + { + _global: { + cn: true, + registry: "https://reg.custom-package.com", }, - false - ) - ).should.be.ok(); - env.registry.should.be.equal("https://reg.custom-package.com"); - env.upstreamRegistry.should.be.equal("https://packages.unity.cn"); - env.region.should.be.equal("cn"); + }, + false + ); + should(env).not.be.null(); + env!.registry.should.be.equal("https://reg.custom-package.com"); + env!.upstreamRegistry.should.be.equal("https://packages.unity.cn"); + env!.region.should.be.equal("cn"); }); }); }); diff --git a/test/test-pkg-manifest-io.ts b/test/test-pkg-manifest-io.ts index e65e2880..21ad3e42 100644 --- a/test/test-pkg-manifest-io.ts +++ b/test/test-pkg-manifest-io.ts @@ -1,10 +1,8 @@ import { attachMockConsole, MockConsole } from "./mock-console"; import fs from "fs"; import "should"; -import path from "path"; import { saveManifest } from "../src/utils/pkg-manifest-io"; import { describe } from "mocha"; -import { env, parseEnv } from "../src/utils/env"; import { createWorkDir, getWorkDir, removeWorkDir } from "./mock-work-dir"; import { shouldHaveManifest, @@ -16,74 +14,53 @@ import { semanticVersion } from "../src/types/semantic-version"; import { addDependency, manifestPathFor } from "../src/types/pkg-manifest"; import should from "should"; +const workDirName = "test-openupm-cli"; +const wrongJsonWorkDirName = "test-openupm-cli-wrong-json"; + describe("pkg-manifest io", function () { let mockConsole: MockConsole = null!; + let workDir = ""; + let wrongJsonWorkDir = ""; + beforeEach(function () { - removeWorkDir("test-openupm-cli"); - createWorkDir("test-openupm-cli", { manifest: true }); - createWorkDir("test-openupm-cli-wrong-json", { + removeWorkDir(workDirName); + workDir = createWorkDir(workDirName, { manifest: true }); + wrongJsonWorkDir = createWorkDir(wrongJsonWorkDirName, { manifest: true, }); fs.writeFileSync( - path.join( - getWorkDir("test-openupm-cli-wrong-json"), - "Packages/manifest.json" - ), + manifestPathFor(getWorkDir(wrongJsonWorkDirName)), "wrong-json" ); mockConsole = attachMockConsole(); }); afterEach(function () { - removeWorkDir("test-openupm-cli"); - removeWorkDir("test-openupm-cli-wrong-json"); + removeWorkDir(workDirName); + removeWorkDir(wrongJsonWorkDirName); mockConsole.detach(); }); it("loadManifest", async function () { - ( - await parseEnv( - { _global: { chdir: getWorkDir("test-openupm-cli") } }, - true - ) - ).should.be.ok(); - const manifest = shouldHaveManifest(env.cwd); + const manifest = shouldHaveManifest(workDir); manifest.should.be.deepEqual({ dependencies: {} }); }); it("no manifest file", async function () { - ( - await parseEnv( - { _global: { chdir: getWorkDir("path-not-exist") } }, - false - ) - ).should.be.ok(); - shouldHaveNoManifest(env.cwd); + shouldHaveNoManifest("path-not-exist"); mockConsole.hasLineIncluding("out", "does not exist").should.be.ok(); }); it("wrong json content", async function () { - ( - await parseEnv( - { _global: { chdir: getWorkDir("test-openupm-cli-wrong-json") } }, - true - ) - ).should.be.ok(); - shouldHaveNoManifest(env.cwd); + shouldHaveNoManifest(wrongJsonWorkDir); mockConsole.hasLineIncluding("out", "failed to parse").should.be.ok(); }); it("saveManifest", async function () { - ( - await parseEnv( - { _global: { chdir: getWorkDir("test-openupm-cli") } }, - true - ) - ).should.be.ok(); - const manifest = shouldHaveManifest(env.cwd); + const manifest = shouldHaveManifest(workDir); shouldNotHaveAnyDependencies(manifest); addDependency(manifest, domainName("some-pack"), semanticVersion("1.0.0")); - saveManifest(env.cwd, manifest).should.be.ok(); - const manifest2 = shouldHaveManifest(env.cwd); + saveManifest(workDir, manifest).should.be.ok(); + const manifest2 = shouldHaveManifest(workDir); manifest2.should.be.deepEqual(manifest); }); it("manifest-path is correct", function () { - const manifestPath = manifestPathFor("test-openupm-cli"); + const manifestPath = manifestPathFor(workDirName); should(manifestPath).be.equal("test-openupm-cli/Packages/manifest.json"); }); }); diff --git a/test/test-registry-client.ts b/test/test-registry-client.ts index 60353f9a..72188b7a 100644 --- a/test/test-registry-client.ts +++ b/test/test-registry-client.ts @@ -1,6 +1,6 @@ import "assert"; import "should"; -import { env, parseEnv } from "../src/utils/env"; +import { parseEnv } from "../src/utils/env"; import { fetchPackageInfo } from "../src/registry-client"; import { exampleRegistryUrl, @@ -24,25 +24,28 @@ describe("registry-client", function () { stopMockRegistry(); }); it("simple", async function () { - ( - await parseEnv({ _global: { registry: exampleRegistryUrl } }, false) - ).should.be.ok(); + const env = await parseEnv( + { _global: { registry: exampleRegistryUrl } }, + false + ); + should(env).not.be.null(); const pkgInfoRemote = buildPackageInfo(packageA); registerRemotePkg(pkgInfoRemote); const info = await fetchPackageInfo( - { url: env.registry, auth: env.auth[env.registry] ?? null }, + { url: env!.registry, auth: env!.auth[env!.registry] ?? null }, packageA ); should(info).deepEqual(pkgInfoRemote); }); it("404", async function () { - ( - await parseEnv({ _global: { registry: exampleRegistryUrl } }, false) - ).should.be.ok(); - + const env = await parseEnv( + { _global: { registry: exampleRegistryUrl } }, + false + ); + should(env).not.be.null(); registerMissingPackage(packageA); const info = await fetchPackageInfo( - { url: env.registry, auth: env.auth[env.registry] ?? null }, + { url: env!.registry, auth: env!.auth[env!.registry] ?? null }, packageA ); (info === undefined).should.be.ok(); From 05698bd8b3c295fae3d92ac50fec415b64452b99 Mon Sep 17 00:00:00 2001 From: Ramon Brullo Date: Thu, 14 Dec 2023 19:36:48 +0100 Subject: [PATCH 13/18] refactor: registry in env instead of storing urls for regular and upstream registry, as well as all auths, now we store only the auths for the used registries and put them together with the registry urls --- src/cmd-add.ts | 29 +++-------- src/cmd-deps.ts | 14 ++---- src/cmd-remove.ts | 2 +- src/cmd-search.ts | 9 +--- src/cmd-view.ts | 13 +---- src/utils/env.ts | 98 +++++++++++++++++++++++------------- test/test-cmd-remove.ts | 2 +- test/test-env.ts | 26 +++++----- test/test-registry-client.ts | 10 +--- 9 files changed, 94 insertions(+), 109 deletions(-) diff --git a/src/cmd-add.ts b/src/cmd-add.ts index cc654cc8..3c2a750f 100644 --- a/src/cmd-add.ts +++ b/src/cmd-add.ts @@ -8,11 +8,7 @@ import { compareEditorVersion, tryParseEditorVersion, } from "./types/editor-version"; -import { - fetchPackageDependencies, - fetchPackageInfo, - Registry, -} from "./registry-client"; +import { fetchPackageDependencies, fetchPackageInfo } from "./registry-client"; import { DomainName, isDomainName } from "./types/domain-name"; import { SemanticVersion } from "./types/semantic-version"; import { @@ -50,15 +46,6 @@ export const add = async function ( const env = await parseEnv(options, true); if (env === null) return 1; - const registry: Registry = { - url: env.registry, - auth: env.auth[env.registry] ?? null, - }; - const upstreamRegistry: Registry = { - url: env.upstreamRegistry, - auth: env.auth[env.upstreamRegistry] ?? null, - }; - const addSingle = async function (pkg: PackageReference): Promise { // dirty flag let dirty = false; @@ -76,9 +63,9 @@ export const add = async function ( const pkgsInScope: DomainName[] = []; if (version === undefined || !isPackageUrl(version)) { // verify name - let pkgInfo = await fetchPackageInfo(registry, name); + let pkgInfo = await fetchPackageInfo(env.registry, name); if (!pkgInfo && env.upstream) { - pkgInfo = await fetchPackageInfo(upstreamRegistry, name); + pkgInfo = await fetchPackageInfo(env.upstreamRegistry, name); if (pkgInfo) isUpstreamPackage = true; } if (!pkgInfo) { @@ -151,8 +138,8 @@ export const add = async function ( // pkgsInScope if (!isUpstreamPackage) { const [depsValid, depsInvalid] = await fetchPackageDependencies( - registry, - upstreamRegistry, + env.registry, + env.upstreamRegistry, name, version, true @@ -209,11 +196,11 @@ export const add = async function ( manifest.scopedRegistries = []; dirty = true; } - let entry = tryGetScopedRegistryByUrl(manifest, env.registry); + let entry = tryGetScopedRegistryByUrl(manifest, env.registry.url); if (entry === null) { - const name = url.parse(env.registry).hostname; + const name = url.parse(env.registry.url).hostname; if (name === null) throw new Error("Could not resolve registry name"); - entry = scopedRegistry(name, env.registry); + entry = scopedRegistry(name, env.registry.url); addScopedRegistry(manifest, entry); dirty = true; } diff --git a/src/cmd-deps.ts b/src/cmd-deps.ts index b9776b71..5614116c 100644 --- a/src/cmd-deps.ts +++ b/src/cmd-deps.ts @@ -1,6 +1,6 @@ import log from "./logger"; import { parseEnv } from "./utils/env"; -import { fetchPackageDependencies, Registry } from "./registry-client"; +import { fetchPackageDependencies } from "./registry-client"; import { isPackageUrl } from "./types/package-url"; import { packageReference, @@ -21,22 +21,14 @@ export const deps = async function ( const env = await parseEnv(options, false); if (env === null) return 1; - const registry: Registry = { - url: env.registry, - auth: env.auth[env.registry] ?? null, - }; - const upstreamRegistry: Registry = { - url: env.upstreamRegistry, - auth: env.auth[env.upstreamRegistry] ?? null, - }; const [name, version] = splitPackageReference(pkg); if (version !== undefined && isPackageUrl(version)) throw new Error("Cannot get dependencies for url-version"); const [depsValid, depsInvalid] = await fetchPackageDependencies( - registry, - upstreamRegistry, + env.registry, + env.upstreamRegistry, name, version, options.deep diff --git a/src/cmd-remove.ts b/src/cmd-remove.ts index 3c7271e0..74149cfd 100644 --- a/src/cmd-remove.ts +++ b/src/cmd-remove.ts @@ -51,7 +51,7 @@ export const remove = async function ( dirty = true; } else pkgsNotFound.push(pkg); - const entry = tryGetScopedRegistryByUrl(manifest, env.registry); + const entry = tryGetScopedRegistryByUrl(manifest, env.registry.url); if (entry !== null) { if (hasScope(entry, name)) { removeScope(entry, name); diff --git a/src/cmd-search.ts b/src/cmd-search.ts index 98288538..9e448758 100644 --- a/src/cmd-search.ts +++ b/src/cmd-search.ts @@ -121,17 +121,12 @@ export async function search(keyword: string, options: SearchOptions) { const env = await parseEnv(options, false); if (env === null) return 1; - const registry: Registry = { - url: env.registry, - auth: env.auth[env.registry], - }; - const table = getTable(); // search endpoint - let results = await searchEndpoint(registry, keyword); + let results = await searchEndpoint(env.registry, keyword); // search old search if (results === undefined) { - results = (await searchOld(registry, keyword)) || []; + results = (await searchOld(env.registry, keyword)) || []; } // search upstream // if (env.upstream) { diff --git a/src/cmd-view.ts b/src/cmd-view.ts index 40a9101f..c9da6af4 100644 --- a/src/cmd-view.ts +++ b/src/cmd-view.ts @@ -31,18 +31,9 @@ export const view = async function ( return 1; } // verify name - let pkgInfo = await fetchPackageInfo( - { url: env.registry, auth: env.auth[env.registry] ?? null }, - name - ); + let pkgInfo = await fetchPackageInfo(env.registry, name); if (!pkgInfo && env.upstream) - pkgInfo = await fetchPackageInfo( - { - url: env.upstreamRegistry, - auth: env.auth[env.upstreamRegistry] ?? null, - }, - name - ); + pkgInfo = await fetchPackageInfo(env.upstreamRegistry, name); if (!pkgInfo) { log.error("404", `package not found: ${name}`); return 1; diff --git a/src/utils/env.ts b/src/utils/env.ts index fcdff5c1..0cc5bf07 100644 --- a/src/utils/env.ts +++ b/src/utils/env.ts @@ -24,9 +24,10 @@ import { UpmAuth, } from "../types/upm-config"; import { encodeBase64 } from "../types/base64"; -import { NpmAuth } from "another-npm-registry-client"; import { CmdOptions } from "../types/options"; import { manifestPathFor } from "../types/pkg-manifest"; +import { Registry } from "../registry-client"; +import { NpmAuth } from "another-npm-registry-client"; type Region = "us" | "cn"; @@ -35,11 +36,10 @@ export type Env = { color: boolean; systemUser: boolean; wsl: boolean; - npmAuth?: Record; - auth: Record; + npmAuth?: Record; upstream: boolean; - upstreamRegistry: RegistryUrl; - registry: RegistryUrl; + upstreamRegistry: Registry; + registry: Registry; namespace: DomainName | IpAddress; editorVersion: string | null; region: Region; @@ -52,20 +52,24 @@ export const parseEnv = async function ( ): Promise { // set defaults const env = {}; - env.registry = registryUrl("https://package.openupm.com"); + env.registry = { + url: registryUrl("https://package.openupm.com"), + auth: null, + }; env.cwd = ""; env.namespace = openUpmReverseDomainName; env.upstream = true; env.color = true; - env.upstreamRegistry = registryUrl("https://packages.unity.com"); + env.upstreamRegistry = { + url: registryUrl("https://packages.unity.com"), + auth: null, + }; env.systemUser = false; env.wsl = false; env.editorVersion = null; env.region = "us"; // the npmAuth field of .upmconfig.toml env.npmAuth = {}; - // the dict of auth param for npm registry API - env.auth = {}; // log level log.level = options._global.verbose ? "verbose" : "notice"; // color @@ -79,19 +83,60 @@ export const parseEnv = async function ( if (options._global.upstream === false) env.upstream = false; // region cn if (options._global.cn === true) { - env.registry = registryUrl("https://package.openupm.cn"); - env.upstreamRegistry = registryUrl("https://packages.unity.cn"); + env.registry = { + url: registryUrl("https://package.openupm.cn"), + auth: null, + }; + env.upstreamRegistry = { + url: registryUrl("https://packages.unity.cn"), + auth: null, + }; env.region = "cn"; log.notice("region", "cn"); } // registry if (options._global.registry) { - env.registry = coerceRegistryUrl(options._global.registry); + env.registry = { + url: coerceRegistryUrl(options._global.registry), + auth: null, + }; // TODO: Check hostname for null - const hostname = url.parse(env.registry).hostname as string; + const hostname = url.parse(env.registry.url).hostname as string; if (isIpAddress(hostname)) env.namespace = hostname; else env.namespace = namespaceFor(hostname); } + + function tryToNpmAuth(upmAuth: UpmAuth): NpmAuth | null { + if (isTokenAuth(upmAuth)) { + return { + token: upmAuth.token, + alwaysAuth: shouldAlwaysAuth(upmAuth), + }; + } else if (isBasicAuth(upmAuth)) { + const [username, password] = decodeBasicAuth(upmAuth._auth); + return { + username, + password: encodeBase64(password), + email: upmAuth.email, + alwaysAuth: shouldAlwaysAuth(upmAuth), + }; + } + return null; + } + + function tryGetAuthForRegistry(registry: RegistryUrl): NpmAuth | null { + const upmAuth = env.npmAuth![registry]; + if (upmAuth === undefined) return null; + const npmAuth = tryToNpmAuth(upmAuth); + if (npmAuth === null) { + log.warn( + "env.auth", + `failed to parse auth info for ${registry} in .upmconfig.toml: missing token or _auth fields` + ); + } + return null; + } + // auth if (options._global.systemUser) env.systemUser = true; if (options._global.wsl) env.wsl = true; @@ -100,29 +145,10 @@ export const parseEnv = async function ( if (upmConfig) { env.npmAuth = upmConfig.npmAuth; if (env.npmAuth !== undefined) { - (Object.keys(env.npmAuth) as RegistryUrl[]).forEach((reg) => { - const regAuth = env.npmAuth![reg]; - if (isTokenAuth(regAuth)) { - env.auth[reg] = { - token: regAuth.token, - alwaysAuth: shouldAlwaysAuth(regAuth), - }; - } else if (isBasicAuth(regAuth)) { - const [username, password] = decodeBasicAuth(regAuth._auth); - env.auth[reg] = { - username, - password: encodeBase64(password), - email: regAuth.email, - alwaysAuth: shouldAlwaysAuth(regAuth), - }; - } else { - log.warn( - "env.auth", - `failed to parse auth info for ${reg} in .upmconfig.toml: missing token or _auth fields` - ); - log.warn("env.auth", regAuth); - } - }); + env.registry.auth = tryGetAuthForRegistry(env.registry.url); + env.upstreamRegistry.auth = tryGetAuthForRegistry( + env.upstreamRegistry.url + ); } } // log.verbose("env.npmAuth", env.npmAuth); diff --git a/test/test-cmd-remove.ts b/test/test-cmd-remove.ts index 68d55d3b..8eff7b3a 100644 --- a/test/test-cmd-remove.ts +++ b/test/test-cmd-remove.ts @@ -4,7 +4,7 @@ import { exampleRegistryReverseDomain, exampleRegistryUrl, } from "./mock-registry"; -import { createWorkDir, getWorkDir, removeWorkDir } from "./mock-work-dir"; +import { createWorkDir, removeWorkDir } from "./mock-work-dir"; import { attachMockConsole, MockConsole } from "./mock-console"; import { shouldHaveManifest, diff --git a/test/test-env.ts b/test/test-env.ts index b71231e7..3bcbb30e 100644 --- a/test/test-env.ts +++ b/test/test-env.ts @@ -32,9 +32,9 @@ describe("env", function () { it("defaults", async function () { const env = await parseEnv({ _global: {} }, false); should(env).not.be.null(); - env!.registry.should.equal("https://package.openupm.com"); + env!.registry.url.should.equal("https://package.openupm.com"); env!.upstream.should.be.ok(); - env!.upstreamRegistry.should.equal("https://packages.unity.com"); + env!.upstreamRegistry.url.should.equal("https://packages.unity.com"); env!.namespace.should.equal("com.openupm"); env!.cwd.should.equal(""); (env!.editorVersion === null).should.be.ok(); @@ -73,7 +73,7 @@ describe("env", function () { false ); should(env).not.be.null(); - env!.registry.should.be.equal("https://registry.npmjs.org"); + env!.registry.url.should.be.equal("https://registry.npmjs.org"); env!.namespace.should.be.equal("org.npmjs"); }); it("custom registry with splash", async function () { @@ -82,7 +82,7 @@ describe("env", function () { false ); should(env).not.be.null(); - env!.registry.should.be.equal("https://registry.npmjs.org"); + env!.registry.url.should.be.equal("https://registry.npmjs.org"); env!.namespace.should.be.equal("org.npmjs"); }); it("custom registry with extra path", async function () { @@ -95,7 +95,7 @@ describe("env", function () { false ); should(env).not.be.null(); - env!.registry.should.be.equal("https://registry.npmjs.org/some"); + env!.registry.url.should.be.equal("https://registry.npmjs.org/some"); env!.namespace.should.be.equal("org.npmjs"); }); it("custom registry with extra path and splash", async function () { @@ -108,7 +108,7 @@ describe("env", function () { false ); should(env).not.be.null(); - env!.registry.should.be.equal("https://registry.npmjs.org/some"); + env!.registry.url.should.be.equal("https://registry.npmjs.org/some"); env!.namespace.should.be.equal("org.npmjs"); }); it("custom registry without http", async function () { @@ -117,7 +117,7 @@ describe("env", function () { false ); should(env).not.be.null(); - env!.registry.should.be.equal("http://registry.npmjs.org"); + env!.registry.url.should.be.equal("http://registry.npmjs.org"); env!.namespace.should.be.equal("org.npmjs"); }); it("custom registry with ipv4+port", async function () { @@ -126,7 +126,7 @@ describe("env", function () { false ); should(env).not.be.null(); - env!.registry.should.be.equal("http://127.0.0.1:4873"); + env!.registry.url.should.be.equal("http://127.0.0.1:4873"); env!.namespace.should.be.equal("127.0.0.1"); }); it("custom registry with ipv6+port", async function () { @@ -137,7 +137,7 @@ describe("env", function () { false ); should(env).not.be.null(); - env!.registry.should.be.equal("http://[1:2:3:4:5:6:7:8]:4873"); + env!.registry.url.should.be.equal("http://[1:2:3:4:5:6:7:8]:4873"); env!.namespace.should.be.equal("1:2:3:4:5:6:7:8"); }); it("upstream", async function () { @@ -156,8 +156,8 @@ describe("env", function () { it("region cn", async function () { const env = await parseEnv({ _global: { cn: true } }, false); should(env).not.be.null(); - env!.registry.should.be.equal("https://package.openupm.cn"); - env!.upstreamRegistry.should.be.equal("https://packages.unity.cn"); + env!.registry.url.should.be.equal("https://package.openupm.cn"); + env!.upstreamRegistry.url.should.be.equal("https://packages.unity.cn"); env!.region.should.be.equal("cn"); }); it("region cn with a custom registry", async function () { @@ -171,8 +171,8 @@ describe("env", function () { false ); should(env).not.be.null(); - env!.registry.should.be.equal("https://reg.custom-package.com"); - env!.upstreamRegistry.should.be.equal("https://packages.unity.cn"); + env!.registry.url.should.be.equal("https://reg.custom-package.com"); + env!.upstreamRegistry.url.should.be.equal("https://packages.unity.cn"); env!.region.should.be.equal("cn"); }); }); diff --git a/test/test-registry-client.ts b/test/test-registry-client.ts index 72188b7a..dc3444fa 100644 --- a/test/test-registry-client.ts +++ b/test/test-registry-client.ts @@ -31,10 +31,7 @@ describe("registry-client", function () { should(env).not.be.null(); const pkgInfoRemote = buildPackageInfo(packageA); registerRemotePkg(pkgInfoRemote); - const info = await fetchPackageInfo( - { url: env!.registry, auth: env!.auth[env!.registry] ?? null }, - packageA - ); + const info = await fetchPackageInfo(env!.registry, packageA); should(info).deepEqual(pkgInfoRemote); }); it("404", async function () { @@ -44,10 +41,7 @@ describe("registry-client", function () { ); should(env).not.be.null(); registerMissingPackage(packageA); - const info = await fetchPackageInfo( - { url: env!.registry, auth: env!.auth[env!.registry] ?? null }, - packageA - ); + const info = await fetchPackageInfo(env!.registry, packageA); (info === undefined).should.be.ok(); }); }); From f34f9587d84910d2974827a3812e96594d930b12 Mon Sep 17 00:00:00 2001 From: Ramon Brullo Date: Thu, 14 Dec 2023 19:40:22 +0100 Subject: [PATCH 14/18] refactor: remove property from env nmpAuth is only used inside the parse-env function and can be removed from env --- src/utils/env.ts | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/src/utils/env.ts b/src/utils/env.ts index 0cc5bf07..2b91085d 100644 --- a/src/utils/env.ts +++ b/src/utils/env.ts @@ -36,7 +36,6 @@ export type Env = { color: boolean; systemUser: boolean; wsl: boolean; - npmAuth?: Record; upstream: boolean; upstreamRegistry: Registry; registry: Registry; @@ -68,8 +67,6 @@ export const parseEnv = async function ( env.wsl = false; env.editorVersion = null; env.region = "us"; - // the npmAuth field of .upmconfig.toml - env.npmAuth = {}; // log level log.level = options._global.verbose ? "verbose" : "notice"; // color @@ -124,8 +121,14 @@ export const parseEnv = async function ( return null; } + // auth + if (options._global.systemUser) env.systemUser = true; + if (options._global.wsl) env.wsl = true; + const configDir = await getUpmConfigDir(env.wsl, env.systemUser); + const upmConfig = await loadUpmConfig(configDir); + function tryGetAuthForRegistry(registry: RegistryUrl): NpmAuth | null { - const upmAuth = env.npmAuth![registry]; + const upmAuth = upmConfig!.npmAuth![registry]; if (upmAuth === undefined) return null; const npmAuth = tryToNpmAuth(upmAuth); if (npmAuth === null) { @@ -137,19 +140,9 @@ export const parseEnv = async function ( return null; } - // auth - if (options._global.systemUser) env.systemUser = true; - if (options._global.wsl) env.wsl = true; - const configDir = await getUpmConfigDir(env.wsl, env.systemUser); - const upmConfig = await loadUpmConfig(configDir); - if (upmConfig) { - env.npmAuth = upmConfig.npmAuth; - if (env.npmAuth !== undefined) { - env.registry.auth = tryGetAuthForRegistry(env.registry.url); - env.upstreamRegistry.auth = tryGetAuthForRegistry( - env.upstreamRegistry.url - ); - } + if (upmConfig !== undefined && upmConfig.npmAuth !== undefined) { + env.registry.auth = tryGetAuthForRegistry(env.registry.url); + env.upstreamRegistry.auth = tryGetAuthForRegistry(env.upstreamRegistry.url); } // log.verbose("env.npmAuth", env.npmAuth); // log.verbose("env.auth", env.auth); From 36acaa5f1f80eb7ffea8ba3be146fae7524ee528 Mon Sep 17 00:00:00 2001 From: Ramon Brullo Date: Thu, 14 Dec 2023 20:06:25 +0100 Subject: [PATCH 15/18] refactor: remove property from env color is only used inside the parse-env function and can be made local --- src/utils/env.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/utils/env.ts b/src/utils/env.ts index 2b91085d..b674a72b 100644 --- a/src/utils/env.ts +++ b/src/utils/env.ts @@ -33,7 +33,6 @@ type Region = "us" | "cn"; export type Env = { cwd: string; - color: boolean; systemUser: boolean; wsl: boolean; upstream: boolean; @@ -58,7 +57,6 @@ export const parseEnv = async function ( env.cwd = ""; env.namespace = openUpmReverseDomainName; env.upstream = true; - env.color = true; env.upstreamRegistry = { url: registryUrl("https://packages.unity.com"), auth: null, @@ -70,9 +68,9 @@ export const parseEnv = async function ( // log level log.level = options._global.verbose ? "verbose" : "notice"; // color - if (options._global.color === false) env.color = false; - if (process.env.NODE_ENV == "test") env.color = false; - if (!env.color) { + const useColor = + options._global.color === true && process.env.NOVE_ENV !== "test"; + if (!useColor) { chalk.level = 0; log.disableColor(); } From 0d3390f3b2cc0ff0f510c2443baf96a9aad8d5af Mon Sep 17 00:00:00 2001 From: Ramon Brullo Date: Thu, 14 Dec 2023 20:06:45 +0100 Subject: [PATCH 16/18] refactor: remove property from env region is only used inside the parse-env function and can be made local --- src/utils/env.ts | 5 ----- test/test-env.ts | 2 -- 2 files changed, 7 deletions(-) diff --git a/src/utils/env.ts b/src/utils/env.ts index b674a72b..dfa6b968 100644 --- a/src/utils/env.ts +++ b/src/utils/env.ts @@ -29,8 +29,6 @@ import { manifestPathFor } from "../types/pkg-manifest"; import { Registry } from "../registry-client"; import { NpmAuth } from "another-npm-registry-client"; -type Region = "us" | "cn"; - export type Env = { cwd: string; systemUser: boolean; @@ -40,7 +38,6 @@ export type Env = { registry: Registry; namespace: DomainName | IpAddress; editorVersion: string | null; - region: Region; }; // Parse env @@ -64,7 +61,6 @@ export const parseEnv = async function ( env.systemUser = false; env.wsl = false; env.editorVersion = null; - env.region = "us"; // log level log.level = options._global.verbose ? "verbose" : "notice"; // color @@ -86,7 +82,6 @@ export const parseEnv = async function ( url: registryUrl("https://packages.unity.cn"), auth: null, }; - env.region = "cn"; log.notice("region", "cn"); } // registry diff --git a/test/test-env.ts b/test/test-env.ts index 3bcbb30e..d3be0014 100644 --- a/test/test-env.ts +++ b/test/test-env.ts @@ -158,7 +158,6 @@ describe("env", function () { should(env).not.be.null(); env!.registry.url.should.be.equal("https://package.openupm.cn"); env!.upstreamRegistry.url.should.be.equal("https://packages.unity.cn"); - env!.region.should.be.equal("cn"); }); it("region cn with a custom registry", async function () { const env = await parseEnv( @@ -173,7 +172,6 @@ describe("env", function () { should(env).not.be.null(); env!.registry.url.should.be.equal("https://reg.custom-package.com"); env!.upstreamRegistry.url.should.be.equal("https://packages.unity.cn"); - env!.region.should.be.equal("cn"); }); }); }); From dd9b40320890b9ff4a2bafbcce50c3536684bdcc Mon Sep 17 00:00:00 2001 From: Ramon Brullo Date: Fri, 15 Dec 2023 17:27:25 +0100 Subject: [PATCH 17/18] fix: typo in property name This typo caused color to be used in test scenarios which would mess with the mock console --- src/utils/env.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/env.ts b/src/utils/env.ts index dfa6b968..27906a4b 100644 --- a/src/utils/env.ts +++ b/src/utils/env.ts @@ -65,7 +65,7 @@ export const parseEnv = async function ( log.level = options._global.verbose ? "verbose" : "notice"; // color const useColor = - options._global.color === true && process.env.NOVE_ENV !== "test"; + options._global.color === true && process.env.NODE_ENV !== "test"; if (!useColor) { chalk.level = 0; log.disableColor(); From 479731ed61784f7d0215201ba9a28babfdb50adb Mon Sep 17 00:00:00 2001 From: Ramon Brullo Date: Fri, 15 Dec 2023 17:27:59 +0100 Subject: [PATCH 18/18] fix: color option If the color option is undefined it should also count as true --- src/utils/env.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/env.ts b/src/utils/env.ts index 27906a4b..a331d199 100644 --- a/src/utils/env.ts +++ b/src/utils/env.ts @@ -65,7 +65,7 @@ export const parseEnv = async function ( log.level = options._global.verbose ? "verbose" : "notice"; // color const useColor = - options._global.color === true && process.env.NODE_ENV !== "test"; + options._global.color !== false && process.env.NODE_ENV !== "test"; if (!useColor) { chalk.level = 0; log.disableColor();