Repository: compiler-explorer/compiler-explorer
Pull request #7303 (2025-01-25 03:09:29)

Attempt to fix remote library fetching
Libraries were being fetched from wrong URL (on startup);
`Fetching remote libraries from https://godbolt.org:443/api/libraries/c++`

Ended up refactoring a bunch of things to get some unittesting going on

I did not test actual startup and compilation with CE, I might have broken something



Patch:
diff --git a/lib/compiler-finder.ts b/lib/compiler-finder.ts
index 4049b3e41..67d296250 100644
--- a/lib/compiler-finder.ts
+++ b/lib/compiler-finder.ts
@@ -85,6 +85,24 @@ export class CompilerFinder {
         return this.awsPoller.getInstances();
     }
 
+    static prepareRemoteUrlParts(host: string, port: number, uriBase: string, langId: string | null) {
+        const uriSchema = port === 443 ? 'https' : 'http';
+        return {
+            uriSchema: uriSchema,
+            uri: urljoin(`${uriSchema}://${host}:${port}`, uriBase),
+            apiPath: urljoin('/', uriBase || '', 'api/compilers', langId || '', '?fields=all'),
+        };
+    }
+
+    static getRemoteInfo(uriSchema: string, host: string, port: number, uriBase: string, compilerId: string) {
+        return {
+            target: `${uriSchema}://${host}:${port}`,
+            path: urljoin('/', uriBase, 'api/compiler', compilerId, 'compile'),
+            cmakePath: urljoin('/', uriBase, 'api/compiler', compilerId, 'cmake'),
+            basePath: urljoin('/', uriBase),
+        };
+    }
+
     async fetchRemote(
         host: string,
         port: number,
@@ -93,9 +111,7 @@ export class CompilerFinder {
         langId: string | null,
     ): Promise<CompilerInfo[] | null> {
         const requestLib = port === 443 ? https : http;
-        const uriSchema = port === 443 ? 'https' : 'http';
-        const uri = urljoin(`${uriSchema}://${host}:${port}`, uriBase);
-        const apiPath = urljoin('/', uriBase || '', 'api/compilers', langId || '', '?fields=all');
+        const {uriSchema, uri, apiPath} = CompilerFinder.prepareRemoteUrlParts(host, port, uriBase, langId);
         logger.info(`Fetching compilers from remote source ${uri}`);
         return this.retryPromise(
             () => {
@@ -150,11 +166,13 @@ export class CompilerFinder {
                                             if (typeof compiler.alias == 'string') compiler.alias = [compiler.alias];
                                             // End fixup
                                             compiler.exe = '/dev/null';
-                                            compiler.remote = {
-                                                target: `${uriSchema}://${host}:${port}`,
-                                                path: urljoin('/', uriBase, 'api/compiler', compiler.id, 'compile'),
-                                                cmakePath: urljoin('/', uriBase, 'api/compiler', compiler.id, 'cmake'),
-                                            };
+                                            compiler.remote = CompilerFinder.getRemoteInfo(
+                                                uriSchema,
+                                                host,
+                                                port,
+                                                uriBase,
+                                                compiler.id,
+                                            );
                                             return compiler;
                                         });
                                         resolve(compilers);
@@ -369,6 +387,16 @@ export class CompilerFinder {
             .filter(a => a !== '');
     }
 
+    static getRemotePartsFromCompilerName(remoteCompilerName: string) {
+        const bits = remoteCompilerName.split('@');
+        const pathParts = bits[1].split('/');
+        return {
+            host: bits[0],
+            port: parseInt(unwrap(pathParts.shift())),
+            uriBase: pathParts.join('/'),
+        };
+    }
+
     async recurseGetCompilers(
         langId: string,
         compilerName: string,
@@ -376,12 +404,8 @@ export class CompilerFinder {
     ): Promise<PreliminaryCompilerInfo[]> {
         // Don't treat @ in paths as remote addresses if requested
         if (this.args.fetchCompilersFromRemote && compilerName.includes('@')) {
-            const bits = compilerName.split('@');
-            const host = bits[0];
-            const pathParts = bits[1].split('/');
-            const port = parseInt(unwrap(pathParts.shift()));
-            const path = pathParts.join('/');
-            return (await this.fetchRemote(host, port, path, this.ceProps, langId)) || [];
+            const {host, port, uriBase} = CompilerFinder.getRemotePartsFromCompilerName(compilerName);
+            return (await this.fetchRemote(host, port, uriBase, this.ceProps, langId)) || [];
         }
         if (compilerName.indexOf('&') === 0) {
             const groupName = compilerName.substring(1);
diff --git a/lib/options-handler.ts b/lib/options-handler.ts
index 8dc5c086a..7bdb211ca 100755
--- a/lib/options-handler.ts
+++ b/lib/options-handler.ts
@@ -28,6 +28,7 @@ import path from 'path';
 import fs from 'fs-extra';
 import semverParser from 'semver';
 import _ from 'underscore';
+import urlJoin from 'url-join';
 
 import {AppDefaultArguments} from '../app.js';
 import {splitArguments} from '../shared/common-utils.js';
@@ -399,7 +400,7 @@ export class ClientOptionsHandler {
         const remoteId = this.getRemoteId(remoteUrl, language);
         if (!this.remoteLibs[remoteId]) {
             return new Promise(resolve => {
-                const url = remoteUrl + '/api/libraries/' + language;
+                const url = ClientOptionsHandler.getRemoteUrlForLibraries(remoteUrl, language);
                 logger.info(`Fetching remote libraries from ${url}`);
                 let fullData = '';
                 https.get(url, res => {
@@ -428,6 +429,14 @@ export class ClientOptionsHandler {
         await this.getRemoteLibraries(language, target);
     }
 
+    static getFullRemoteUrl(remote): string {
+        return remote.target + remote.basePath;
+    }
+
+    static getRemoteUrlForLibraries(url: string, language: LanguageKey) {
+        return urlJoin(url, '/api/libraries', language);
+    }
+
     async setCompilers(compilers: CompilerInfo[]) {
         const forbiddenKeys = new Set([
             'exe',
@@ -459,7 +468,10 @@ export class ClientOptionsHandler {
             }
 
             if (compiler.remote) {
-                await this.fetchRemoteLibrariesIfNeeded(compiler.lang, compiler.remote.target);
+                await this.fetchRemoteLibrariesIfNeeded(
+                    compiler.lang,
+                    ClientOptionsHandler.getFullRemoteUrl(compiler.remote),
+                );
             }
 
             for (const propKey of Object.keys(compiler)) {
diff --git a/types/compiler.interfaces.ts b/types/compiler.interfaces.ts
index 23b548aab..99c90189f 100644
--- a/types/compiler.interfaces.ts
+++ b/types/compiler.interfaces.ts
@@ -33,6 +33,7 @@ export type Remote = {
     target: string;
     path: string;
     cmakePath: string;
+    basePath: string;
 };
 
 export type CompilerInfo = {


Test patch:
diff --git a/test/analysis-tests.ts b/test/analysis-tests.ts
index 7cc83b3cf..9dc22bd8b 100644
--- a/test/analysis-tests.ts
+++ b/test/analysis-tests.ts
@@ -49,6 +49,7 @@ describe('LLVM-mca tool definition', () => {
                 target: 'foo',
                 path: 'bar',
                 cmakePath: 'cmake',
+                basePath: '/',
             },
             lang: languages.analysis.id,
         });
@@ -92,6 +93,7 @@ describe('LLVM-mca tool definition', () => {
                 target: 'foo',
                 path: 'bar',
                 cmakePath: 'cmake',
+                basePath: '/',
             },
             lang: 'analysis',
             disabledFilters: 'labels,directives,debugCalls' as any,
diff --git a/test/base-compiler-tests.ts b/test/base-compiler-tests.ts
index a54c865e8..91690adcf 100644
--- a/test/base-compiler-tests.ts
+++ b/test/base-compiler-tests.ts
@@ -60,6 +60,7 @@ describe('Basic compiler invariants', () => {
             target: 'foo',
             path: 'bar',
             cmakePath: 'cmake',
+            basePath: '/',
         },
         lang: 'c++',
         ldPath: [],
@@ -119,6 +120,7 @@ describe('Compiler execution', () => {
             target: 'foo',
             path: 'bar',
             cmakePath: 'cmake',
+            basePath: '/',
         },
         lang: 'c++',
         ldPath: [],
@@ -132,6 +134,7 @@ describe('Compiler execution', () => {
             target: 'foo',
             path: 'bar',
             cmakePath: 'cmake',
+            basePath: '/',
         },
         lang: 'c++',
         ldPath: [],
@@ -144,6 +147,7 @@ describe('Compiler execution', () => {
             target: 'foo',
             path: 'bar',
             cmakePath: 'cmake',
+            basePath: '/',
         },
         lang: 'c++',
         ldPath: [],
@@ -154,6 +158,7 @@ describe('Compiler execution', () => {
             target: 'foo',
             path: 'bar',
             cmakePath: 'cmake',
+            basePath: '/',
         },
         lang: 'c++',
         ldPath: [],
@@ -748,6 +753,7 @@ describe('getDefaultExecOptions', () => {
             target: 'foo',
             path: 'bar',
             cmakePath: 'cmake',
+            basePath: '/',
         },
         lang: 'c++',
         ldPath: [],
@@ -818,6 +824,7 @@ describe('Rust overrides', () => {
             target: '',
             path: '',
             cmakePath: '',
+            basePath: '/',
         },
         semver: 'nightly',
         lang: 'rust',
diff --git a/test/d-tests.ts b/test/d-tests.ts
index eba65cf65..f689bdcc0 100644
--- a/test/d-tests.ts
+++ b/test/d-tests.ts
@@ -42,6 +42,7 @@ describe('D', () => {
             target: 'foo',
             path: 'bar',
             cmakePath: 'cmake',
+            basePath: '/',
         },
         lang: languages.d.id,
     };
diff --git a/test/golang-tests.ts b/test/golang-tests.ts
index dbae66a05..8ea277a49 100644
--- a/test/golang-tests.ts
+++ b/test/golang-tests.ts
@@ -41,6 +41,7 @@ const info = {
         target: 'foo',
         path: 'bar',
         cmakePath: 'cmake',
+        basePath: '/',
     },
     lang: languages.go.id,
 };
diff --git a/test/library-tests.ts b/test/library-tests.ts
index 401ce4bd8..62239b024 100644
--- a/test/library-tests.ts
+++ b/test/library-tests.ts
@@ -50,6 +50,7 @@ describe('Library directories (c++)', () => {
             target: 'foo',
             path: 'bar',
             cmakePath: 'cmake',
+            basePath: '/',
         },
         lang: 'c++',
         ldPath: [],
@@ -223,6 +224,7 @@ describe('Library directories (fortran)', () => {
             target: 'foo',
             path: 'bar',
             cmakePath: 'cmake',
+            basePath: '/',
         },
         lang: 'fortran',
         ldPath: [],
diff --git a/test/nim-tests.ts b/test/nim-tests.ts
index 823b2f39a..98ccffa0d 100644
--- a/test/nim-tests.ts
+++ b/test/nim-tests.ts
@@ -44,6 +44,7 @@ describe('Nim', () => {
             target: 'foo',
             path: 'bar',
             cmakePath: 'cmake',
+            basePath: '/',
         },
         lang: languages.nim.id,
     };
diff --git a/test/odin-tests.ts b/test/odin-tests.ts
index 8e396eb99..8daeb3385 100644
--- a/test/odin-tests.ts
+++ b/test/odin-tests.ts
@@ -41,6 +41,7 @@ const info = {
         target: 'example',
         path: 'dummy',
         cmakePath: 'cmake',
+        basePath: '/',
     },
     lang: languages.odin.id,
 };
diff --git a/test/options-handler.ts b/test/options-handler.ts
index 07c572784..a7dee7ce0 100644
--- a/test/options-handler.ts
+++ b/test/options-handler.ts
@@ -30,6 +30,7 @@ import {beforeAll, describe, expect, it} from 'vitest';
 import {AppDefaultArguments} from '../app.js';
 import {BaseCompiler} from '../lib/base-compiler.js';
 import {CompilationEnvironment} from '../lib/compilation-env.js';
+import {CompilerFinder} from '../lib/compiler-finder.js';
 import {ClientOptionsHandler, ClientOptionsType} from '../lib/options-handler.js';
 import * as properties from '../lib/properties.js';
 import {BaseTool} from '../lib/tooling/base-tool.js';
@@ -566,4 +567,30 @@ describe('Options handler', () => {
             },
         });
     });
+
+    it('should correctly resolve remote urls', () => {
+        const compilerName = 'godbolt.org@443/gpu';
+        const {host, port, uriBase} = CompilerFinder.getRemotePartsFromCompilerName(compilerName);
+        expect(host).toEqual('godbolt.org');
+        expect(port).toEqual(443);
+        expect(uriBase).toEqual('gpu');
+
+        const {uriSchema, uri, apiPath} = CompilerFinder.prepareRemoteUrlParts(host, port, uriBase, 'c++');
+        expect(uriSchema).toEqual('https');
+        expect(uri).toEqual('https://godbolt.org:443/gpu');
+        expect(apiPath).toEqual('/gpu/api/compilers/c++?fields=all');
+
+        const info = CompilerFinder.getRemoteInfo(uriSchema, host, port, uriBase, 'g123remote');
+        expect(info).toEqual({
+            target: 'https://godbolt.org:443',
+            path: '/gpu/api/compiler/g123remote/compile',
+            cmakePath: '/gpu/api/compiler/g123remote/cmake',
+            basePath: '/gpu',
+        });
+
+        const fullUrl = ClientOptionsHandler.getFullRemoteUrl(info);
+        const librariesUrl = ClientOptionsHandler.getRemoteUrlForLibraries(fullUrl, 'c++');
+
+        expect(librariesUrl).toEqual('https://godbolt.org:443/gpu/api/libraries/c++');
+    });
 });
diff --git a/test/ppci-tests.ts b/test/ppci-tests.ts
index 49b3a864f..0b5e3cc70 100644
--- a/test/ppci-tests.ts
+++ b/test/ppci-tests.ts
@@ -41,6 +41,7 @@ describe('PPCI', () => {
             target: 'foo',
             path: 'bar',
             cmakePath: 'cmake',
+            basePath: '/',
         },
         lang: languages.c.id,
     };
diff --git a/test/win-path-tests.ts b/test/win-path-tests.ts
index 62c52adc5..7d243bd73 100644
--- a/test/win-path-tests.ts
+++ b/test/win-path-tests.ts
@@ -43,6 +43,7 @@ const info = {
         target: 'foo',
         path: 'bar',
         cmakePath: 'cmake',
+        basePath: '/',
     },
 };