Attempt to fix remote library fetching (#7303)

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. (but @mattgodbolt tested and it looked great)
This commit is contained in:
Patrick Quist
2025-01-25 16:27:03 +01:00
committed by GitHub
parent 10bc41a592
commit 7234169c86
13 changed files with 97 additions and 16 deletions

View File

@@ -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);

View File

@@ -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)) {

View File

@@ -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,

View File

@@ -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',

View File

@@ -42,6 +42,7 @@ describe('D', () => {
target: 'foo',
path: 'bar',
cmakePath: 'cmake',
basePath: '/',
},
lang: languages.d.id,
};

View File

@@ -41,6 +41,7 @@ const info = {
target: 'foo',
path: 'bar',
cmakePath: 'cmake',
basePath: '/',
},
lang: languages.go.id,
};

View File

@@ -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: [],

View File

@@ -44,6 +44,7 @@ describe('Nim', () => {
target: 'foo',
path: 'bar',
cmakePath: 'cmake',
basePath: '/',
},
lang: languages.nim.id,
};

View File

@@ -41,6 +41,7 @@ const info = {
target: 'example',
path: 'dummy',
cmakePath: 'cmake',
basePath: '/',
},
lang: languages.odin.id,
};

View File

@@ -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++');
});
});

View File

@@ -41,6 +41,7 @@ describe('PPCI', () => {
target: 'foo',
path: 'bar',
cmakePath: 'cmake',
basePath: '/',
},
lang: languages.c.id,
};

View File

@@ -43,6 +43,7 @@ const info = {
target: 'foo',
path: 'bar',
cmakePath: 'cmake',
basePath: '/',
},
};

View File

@@ -33,6 +33,7 @@ export type Remote = {
target: string;
path: string;
cmakePath: string;
basePath: string;
};
export type CompilerInfo = {