fix(node)!: use full node version for install path (#2187)

BREAKING CHANGE: `node`, `python` and `ruby` based tools are now stored at `/opt/containerbase/<tool-name>/<tool-version>/<node-python-ruby-version>`
containerbase · Feb 13, 2024 · 437c919 · 437c919
1 parent 8950019
commit 437c919
Show file tree

Hide file tree

Showing 5 changed files with 112 additions and 84 deletions.
diff --git a/src/cli/install-tool/index.ts b/src/cli/install-tool/index.ts
@@ -18,7 +18,7 @@ import {
   NodeVersionResolver,
   NpmVersionResolver,
 } from '../tools/node/resolver';
-import { InstallNodeBaseService } from '../tools/node/utils';
+import { InstallNpmBaseService } from '../tools/node/utils';
 import { InstallCocoapodsService } from '../tools/ruby/gem';
 import { InstallRubyBaseService } from '../tools/ruby/utils';
 import { logger } from '../utils';
@@ -104,7 +104,7 @@ export function installTool(
       }
       case 'npm': {
         @injectable()
-        class InstallGenericNpmService extends InstallNodeBaseService {
+        class InstallGenericNpmService extends InstallNpmBaseService {
           override readonly name: string = tool;
 
           override needsPrepare(): boolean {

diff --git a/src/cli/tools/node/index.ts b/src/cli/tools/node/index.ts
@@ -10,7 +10,7 @@ import {
   PathService,
   VersionService,
 } from '../../services';
-import { getDistro, isValid, parse } from '../../utils';
+import { getDistro, parse } from '../../utils';
 import {
   InstallNodeBaseService,
   prepareGlobalConfig,
@@ -198,8 +198,4 @@ fi
       });
     }
   }
-
-  override validate(version: string): Promise<boolean> {
-    return Promise.resolve(isValid(version));
-  }
 }
diff --git a/src/cli/tools/node/npm.ts b/src/cli/tools/node/npm.ts
@@ -1,9 +1,9 @@
 import { execa } from 'execa';
 import { injectable } from 'inversify';
-import { InstallNodeBaseService } from './utils';
+import { InstallNpmBaseService } from './utils';
 
 @injectable()
-export class InstallRenovateService extends InstallNodeBaseService {
+export class InstallRenovateService extends InstallNpmBaseService {
   override readonly name: string = 'renovate';
 
   protected override getAdditionalArgs(): string[] {
@@ -21,7 +21,7 @@ export class InstallRenovateService extends InstallNodeBaseService {
 }
 
 @injectable()
-export class InstallYarnSlimService extends InstallNodeBaseService {
+export class InstallYarnSlimService extends InstallNpmBaseService {
   override readonly name: string = 'yarn-slim';
 
   protected override get tool(): string {
@@ -30,14 +30,15 @@ export class InstallYarnSlimService extends InstallNodeBaseService {
 
   override async install(version: string): Promise<void> {
     await super.install(version);
+    const node = await this.getNodeVersion();
     // TODO: replace with javascript
-    const prefix = await this.pathSvc.findVersionedToolPath(this.name, version);
+    const prefix = this.pathSvc.versionedToolPath(this.name, version);
     await execa(
       'sed',
       [
         '-i',
         's/ steps,/ steps.slice(0,1),/',
-        `${prefix}/node_modules/yarn/lib/cli.js`,
+        `${prefix}/${node}/node_modules/yarn/lib/cli.js`,
       ],
       { stdio: ['inherit', 'inherit', 1] },
     );

diff --git a/src/cli/tools/node/utils.ts b/src/cli/tools/node/utils.ts
@@ -1,4 +1,4 @@
-import fs, { appendFile, mkdir, readFile } from 'node:fs/promises';
+import fs, { appendFile, chmod, mkdir, readFile } from 'node:fs/promises';
 import { join } from 'node:path';
 import { env as penv } from 'node:process';
 import is from '@sindresorhus/is';
@@ -13,10 +13,6 @@ const defaultRegistry = 'https://registry.npmjs.org/';
 
 @injectable()
 export abstract class InstallNodeBaseService extends InstallToolBaseService {
-  protected get tool(): string {
-    return this.name;
-  }
-
   constructor(
     @inject(EnvService) envSvc: EnvService,
     @inject(PathService) pathSvc: PathService,
@@ -25,8 +21,68 @@ export abstract class InstallNodeBaseService extends InstallToolBaseService {
     super(pathSvc, envSvc);
   }
 
+  protected prepareEnv(tmp: string): NodeJS.ProcessEnv {
+    const env: NodeJS.ProcessEnv = {
+      NO_UPDATE_NOTIFIER: '1',
+      npm_config_update_notifier: 'false',
+      npm_config_fund: 'false',
+    };
+
+    if (!penv.npm_config_cache && !penv.NPM_CONFIG_CACHE) {
+      env.npm_config_cache = tmp;
+    }
+
+    const registry = this.envSvc.replaceUrl(defaultRegistry);
+    if (registry !== defaultRegistry) {
+      env.npm_config_registry = registry;
+    }
+
+    return env;
+  }
+
+  protected async updateNodeGyp(
+    prefix: string,
+    tmp: string,
+    env: NodeJS.ProcessEnv,
+    global = false,
+  ): Promise<void> {
+    const res = await execa(
+      join(prefix, 'bin/npm'),
+      [
+        'explore',
+        'npm',
+        ...(global ? ['-g'] : []),
+        '--prefix',
+        prefix,
+        // '--silent',
+        '--',
+        'npm',
+        'install',
+        'node-gyp@latest',
+        '--no-audit',
+        '--cache',
+        tmp,
+        '--silent',
+      ],
+      { reject: false, env, cwd: this.pathSvc.installDir, all: true },
+    );
+
+    if (res.failed) {
+      logger.warn(`Npm error:\n${res.all}`);
+      throw new Error('node-gyp update command failed');
+    }
+  }
+}
+
+@injectable()
+export abstract class InstallNpmBaseService extends InstallNodeBaseService {
+  protected get tool(): string {
+    return this.name;
+  }
+
   override async install(version: string): Promise<void> {
-    const npm = await this.getNodeNpm();
+    const node = await this.getNodeVersion();
+    const npm = this.getNodeNpm(node);
     const tmp = await fs.mkdtemp(
       join(this.pathSvc.tmpDir, 'containerbase-npm-'),
     );
@@ -37,10 +93,15 @@ export abstract class InstallNodeBaseService extends InstallToolBaseService {
       await this.pathSvc.createToolPath(this.name);
     }
 
-    const prefix = await this.pathSvc.createVersionedToolPath(
-      this.name,
-      version,
-    );
+    let prefix = await this.pathSvc.findVersionedToolPath(this.name, version);
+    if (!prefix) {
+      prefix = await this.pathSvc.createVersionedToolPath(this.name, version);
+      // fix perms for later user installs
+      await chmod(prefix, 0o775);
+    }
+
+    prefix = join(prefix, node);
+    await mkdir(prefix);
 
     const res = await execa(
       npm,
@@ -66,9 +127,7 @@ export abstract class InstallNodeBaseService extends InstallToolBaseService {
 
     await fs.symlink(`${prefix}/node_modules/.bin`, `${prefix}/bin`);
     if (this.name === 'npm') {
-      const pkg = await readPackageJson(
-        join(prefix, 'node_modules', this.tool),
-      );
+      const pkg = await readPackageJson(this.packageJsonPath(version, node));
       const ver = parse(pkg.version);
       if (ver.major < 7) {
         // update to latest node-gyp to fully support python3
@@ -83,14 +142,23 @@ export abstract class InstallNodeBaseService extends InstallToolBaseService {
     });
   }
 
+  override async isInstalled(version: string): Promise<boolean> {
+    const node = await this.getNodeVersion();
+    return await this.pathSvc.fileExists(this.packageJsonPath(version, node));
+  }
+
   override async link(version: string): Promise<void> {
     await this.postInstall(version);
   }
 
   override async postInstall(version: string): Promise<void> {
-    const vtPath = this.pathSvc.versionedToolPath(this.name, version);
-    const src = join(vtPath, 'bin');
-    const pkg = await readPackageJson(join(vtPath, 'node_modules', this.tool));
+    const node = await this.getNodeVersion();
+    const src = join(
+      this.pathSvc.versionedToolPath(this.name, version),
+      node,
+      'bin',
+    );
+    const pkg = await readPackageJson(this.packageJsonPath(version, node));
 
     if (!pkg.bin) {
       logger.warn(
@@ -122,70 +190,31 @@ export abstract class InstallNodeBaseService extends InstallToolBaseService {
     return (await this.versionSvc.find('node')) !== null;
   }
 
-  private async getNodeNpm(): Promise<string> {
+  private getNodeNpm(nodeVersion: string): string {
+    return join(this.pathSvc.versionedToolPath('node', nodeVersion), 'bin/npm');
+  }
+
+  protected async getNodeVersion(): Promise<string> {
     const nodeVersion = await this.versionSvc.find('node');
 
     if (!nodeVersion) {
       throw new Error('Node not installed');
     }
-
-    return join(this.pathSvc.versionedToolPath('node', nodeVersion), 'bin/npm');
+    return nodeVersion;
   }
 
   protected getAdditionalArgs(): string[] {
     return [];
   }
 
-  protected prepareEnv(tmp: string): NodeJS.ProcessEnv {
-    const env: NodeJS.ProcessEnv = {
-      NO_UPDATE_NOTIFIER: '1',
-      npm_config_update_notifier: 'false',
-      npm_config_fund: 'false',
-    };
-
-    if (!penv.npm_config_cache && !penv.NPM_CONFIG_CACHE) {
-      env.npm_config_cache = tmp;
-    }
-
-    const registry = this.envSvc.replaceUrl(defaultRegistry);
-    if (registry !== defaultRegistry) {
-      env.npm_config_registry = registry;
-    }
-
-    return env;
-  }
-
-  protected async updateNodeGyp(
-    prefix: string,
-    tmp: string,
-    env: NodeJS.ProcessEnv,
-    global = false,
-  ): Promise<void> {
-    const res = await execa(
-      join(prefix, 'bin/npm'),
-      [
-        'explore',
-        'npm',
-        ...(global ? ['-g'] : []),
-        '--prefix',
-        prefix,
-        // '--silent',
-        '--',
-        'npm',
-        'install',
-        'node-gyp@latest',
-        '--no-audit',
-        '--cache',
-        tmp,
-        '--silent',
-      ],
-      { reject: false, env, cwd: this.pathSvc.installDir, all: true },
+  private packageJsonPath(version: string, node: string): string {
+    return join(
+      this.pathSvc.versionedToolPath(this.name, version),
+      node,
+      'node_modules',
+      this.tool,
+      'package.json',
     );
-
-    if (res.failed) {
-      logger.warn(`Npm error:\n${res.all}`);
-      throw new Error('node-gyp update command failed');
-    }
   }
 }
 
@@ -238,6 +267,6 @@ export async function prepareUserConfig({
 }
 
 async function readPackageJson(path: string): Promise<PackageJson> {
-  const data = await readFile(join(path, 'package.json'), { encoding: 'utf8' });
+  const data = await readFile(path, { encoding: 'utf8' });
   return JSON.parse(data);
 }
diff --git a/test/node/Dockerfile b/test/node/Dockerfile
@@ -324,8 +324,9 @@ RUN npm --version | grep "${NPM_VERSION}"
 
 # TODO: use bats test
 RUN set -ex; \
-  [ -r /opt/containerbase/tools/npm/${NPM_VERSION}/node_modules/npm/node_modules/node-gyp/package.json ] || { echo "missing file"; exit 1; }; \
-  [ "$(cat /opt/containerbase/tools/npm/${NPM_VERSION}/node_modules/npm/node_modules/node-gyp/package.json | jq -r .version)" != "5.1.0" ] \
+  export NODE_VERSION=$(cat /opt/containerbase/versions/node); \
+  [ -r /opt/containerbase/tools/npm/${NPM_VERSION}/${NODE_VERSION}/node_modules/npm/node_modules/node-gyp/package.json ] || { echo "missing file"; exit 1; }; \
+  [ "$(cat /opt/containerbase/tools/npm/${NPM_VERSION}/${NODE_VERSION}/node_modules/npm/node_modules/node-gyp/package.json | jq -r .version)" != "5.1.0" ] \
   && echo node-gyp works || { echo "node-gyp failure"; exit 1; };
 
 #--------------------------------------
@@ -483,7 +484,8 @@ RUN install-tool renovate
 RUN set -ex; \
   renovate --version; \
   renovate-config-validator; \
-  ln -sf /opt/containerbase/tools/renovate/${RENOVATE_VERSION}/node_modules ./node_modules; \
+  export NODE_VERSION=$(cat /opt/containerbase/versions/node); \
+  ln -sf /opt/containerbase/tools/renovate/${RENOVATE_VERSION}/${NODE_VERSION}/node_modules ./node_modules; \
   node -e "new require('re2')('.*').exec('test'); console.log('re2 usable')"; \
   true