Skip to content

Commit

Permalink
perf: use the pre-generated dictionary first to look up correct packa…
Browse files Browse the repository at this point in the history
…ge names

See: #310
  • Loading branch information
teatimeguest committed Aug 9, 2024
1 parent d89b7d1 commit b2f6f42
Show file tree
Hide file tree
Showing 8 changed files with 171 additions and 68 deletions.
2 changes: 1 addition & 1 deletion .github/tl_packages
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Used for E2E tests.
l3packages
lipsum
shellesc
xparse
10 changes: 10 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions packages/config/eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ export const sources = defineConfig(
'unicorn/no-static-only-class': 'off',
'unicorn/no-unnecessary-polyfills': 'off',
'unicorn/no-useless-undefined': 'off',
'unicorn/no-useless-spread': 'off',
'unicorn/prefer-export-from': 'off',
'unicorn/prefer-number-properties': [
'error',
Expand Down
1 change: 1 addition & 0 deletions packages/texlive/__tests__/releases.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ beforeAll(async () => {
doMock = () => {
return nock('https://ctan.org')
.get('/json/2.0/pkg/texlive')
.query(true)
.reply(200, json);
};
});
Expand Down
91 changes: 63 additions & 28 deletions packages/texlive/__tests__/tlmgr/install.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { afterAll, beforeAll, expect, it, vi } from 'vitest';
import { afterEach, beforeEach, describe, expect, it, test, vi } from 'vitest';

import shellesc from '@setup-texlive-action/fixtures/ctan-api-pkg-shellesc.json';
import stderr2008 from '@setup-texlive-action/fixtures/tlmgr-install.2008.stderr';
Expand All @@ -12,18 +12,16 @@ import { install } from '#texlive/tlmgr/actions/install';
import { TlmgrInternals, set } from '#texlive/tlmgr/internals';
import type { Version } from '#texlive/version';

const toTL: Record<string, string | undefined> = {};
vi.mock('node:timers/promises');
vi.mock('@setup-texlive-action/data/package-names.json', () => ({

This comment has been minimized.

Copy link
@muzimuzhi

muzimuzhi Aug 12, 2024

Contributor

It seems this doesn't match packages/data/data/package-names.json (note the nested data sub-directories), the relative path of the real package-names.json.

This comment has been minimized.

Copy link
@teatimeguest

teatimeguest Aug 12, 2024

Author Owner

Thank you for bringing that to my attention! Actually, it is working as intended, due to the subpath exports:

"exports": {
".": "./src/index.ts",
"./*": "./data/*"
},

This makes the module to be resolved as follows:

  "@setup-texlive-action/data/package-names.json"
// \________________________/ \________________/
//         ┌────┘              ┌───────┘
//         ▼ Package dir       ▼ Exported subpath
//   /‾‾‾‾‾‾‾‾‾‾‾‾‾\ /‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾\
==> "./packages/data/data/package-names.json"

Nevertheless, it is indeed confusing to have nested directories with the same name... (besides, module resolution in Node.js + TypeScript is horribly complicated)

This comment has been minimized.

Copy link
@muzimuzhi

muzimuzhi Aug 13, 2024

Contributor

Thanks for your patient explanation. :)

get toTL() {
return toTL;
},
}));
vi.unmock('@actions/http-client');
vi.unmock('#texlive/tlmgr/actions/install');

beforeAll(async () => {
nock('https://ctan.org')
.persist()
.get('/json/2.0/pkg/shellesc')
.reply(200, shellesc);
});

afterAll(nock.restore);

const setVersion = (version: Version) => {
set(new TlmgrInternals({ TEXDIR: '', version }), true);
};
Expand All @@ -45,28 +43,65 @@ it('installs packages by invoking `tlmgr install`', async () => {
);
});

it.each(
describe.each(
[
['2008', 0, stderr2008],
['2009', 0, stderr2009],
['2014', 0, stderr2014],
['2023', 1, stderr2023],
] as const,
)('tries to determine the TL name (%s)', async (version, exitCode, stderr) => {
setVersion(version);
vi.mocked(TlmgrInternals.prototype.exec).mockResolvedValueOnce(
new ExecResult({
command: '',
exitCode,
stdout: '',
stderr,
}),
);
await expect(install(['shellesc'])).resolves.not.toThrow();
expect(TlmgrInternals.prototype.exec).toHaveBeenCalledWith(
'install',
new Set(['tools']),
expect.anything(),
);
expect(nock.isDone()).toBe(true);
)('tries to determine the TL name (%s)', (version, exitCode, stderr) => {
const error = new ExecResult({ command: '', exitCode, stdout: '', stderr });

beforeEach(() => {
setVersion(version);
vi.mocked(TlmgrInternals.prototype.exec).mockResolvedValueOnce(error);
toTL['shellesc'] = 'tools';
nock('https://ctan.org')
.persist()
.get('/json/2.0/pkg/shellesc')
.query(true)
.reply(200, shellesc);
});

afterEach(nock.cleanAll);

test('using the pre-generated dictionary', async () => {
await expect(install(['shellesc', 'latex'])).resolves.not.toThrow();
expect(TlmgrInternals.prototype.exec).toHaveBeenCalledWith(
'install',
new Set(['shellesc', 'latex']),
expect.anything(),
);
expect(TlmgrInternals.prototype.exec).toHaveBeenCalledWith(
'install',
new Set(['tools']),
expect.anything(),
);
expect(TlmgrInternals.prototype.exec).toHaveBeenCalledTimes(2);
expect(nock.isDone()).toBe(false);
});

test('using the CTAN API', async () => {
toTL['shellesc'] = 'shellesc';
vi.mocked(TlmgrInternals.prototype.exec).mockResolvedValueOnce(error);
await expect(install(['shellesc', 'latex'])).resolves.not.toThrow();
expect(TlmgrInternals.prototype.exec).toHaveBeenCalledWith(
'install',
new Set(['shellesc', 'latex']),
expect.anything(),
);
expect(TlmgrInternals.prototype.exec).toHaveBeenCalledWith(
'install',
new Set(['shellesc']),
expect.anything(),
);
expect(TlmgrInternals.prototype.exec).toHaveBeenCalledWith(
'install',
new Set(['tools']),
expect.anything(),
);
expect(TlmgrInternals.prototype.exec).toHaveBeenCalledTimes(3);
expect(nock.isDone()).toBe(true);
});
});
1 change: 1 addition & 0 deletions packages/texlive/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"@actions/core": "^1.10.1",
"@actions/http-client": "^2.2.1",
"@actions/tool-cache": "^2.0.1",
"@teppeis/multimaps": "^3.0.0",
"class-transformer": "^0.5.1",
"deline": "^1.0.4",
"ts-mixer": "^6.0.4",
Expand Down
31 changes: 24 additions & 7 deletions packages/texlive/src/ctan/api.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,26 @@
import { posix as posixPath } from 'node:path';

import { getJson } from '@setup-texlive-action/utils/http';
import { parseTemplate } from 'url-template';

const API_VERSION = '2.0';
const API_BASE_URL = `https://ctan.org/json/${API_VERSION}`;
const API_BASE_URL: Readonly<URL> = new URL(
parseTemplate('https://ctan.org/json/{version}/pkg/{?drop}').expand({
version: '2.0',
drop: [
'aliases',
'announce',
'bugs',
'ctan',
'descriptions',
'development',
'documentation',
'home',
'index',
'install',
'repository',
'support',
'topics',
],
}),
);

export interface Pkg {
version?: {
Expand All @@ -13,7 +30,7 @@ export interface Pkg {
}

export async function pkg(name: string): Promise<Pkg> {
const path = `/pkg/${name}`;
const endpoint = posixPath.join(API_BASE_URL, path);
return await getJson<Pkg>(endpoint);
const url = new URL(name, API_BASE_URL);
url.search = API_BASE_URL.search;
return await getJson<Pkg>(url);
}
102 changes: 70 additions & 32 deletions packages/texlive/src/tlmgr/actions/install.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { setTimeout } from 'node:timers/promises';

import { toTL } from '@setup-texlive-action/data/package-names.json';
import * as log from '@setup-texlive-action/logger';
import { SetMultimap } from '@teppeis/multimaps';

import * as ctan from '#texlive/ctan';
import { PackageNotFound } from '#texlive/tlmgr/errors';
Expand All @@ -15,27 +19,76 @@ export async function install(packages: Iterable<string>): Promise<void> {
// Some packages have different names in TeX Live and CTAN, and
// the DEPENDS.txt format requires a CTAN name, while
// `tlmgr install` requires a TeX Live one.
// To install such packages with tlmgr,
// the action uses the CTAN API to look up their names in TeX Live.
log.info('Trying to resolve package names: ', error.packages.join(', '));
const result = await Promise.all(error.packages.map((name) => {
return resolvePackageName(name);
}));
const notFound = [] as string[];
const resolved = new Set<string>();
for (const [ctanName, tlName] of result) {
if (tlName !== undefined) {
resolved.add(tlName);
} else {
notFound.push(ctanName);
// To get the correct names for those packages,
// the action first looks up the pre-generated dictionary and
// then falls back to the CTAN API.
log.info(
'Looking up the correct package name(s):',
error.packages.join(', '),
);
let rest: ReadonlySet<string> | undefined = new Set(error.packages);
rest = await tryToInstallWith(rest, async (name) => {
return (toTL as Record<string, string | string[] | undefined>)[name];
});
if (rest !== undefined && rest.size > 0) {
log.info('Querying CTAN:', [...rest].join(', '));
rest = await tryToInstallWith(rest, async (name) => {
try {
const pkg = await ctan.api.pkg(name);
if (typeof pkg.texlive === 'string') {
return pkg.texlive;
}
} catch (error) { // eslint-disable-line @typescript-eslint/no-shadow
log.info({ error }, 'Failed to request package data');
} finally {
await setTimeout(200); // 200ms
}
return undefined;
});
if (rest !== undefined && rest.size > 0) {
throw new PackageNotFound([...rest], { action: 'install' });
}
}
}
}

async function tryToInstallWith(
packages: ReadonlySet<string>,
lookup: (name: string) => Promise<string | string[] | undefined>,
): Promise<ReadonlySet<string> | undefined> {
const fromTL = new SetMultimap<string, string>();
const notFound: string[] = [];
for (const name of packages) {
// eslint-disable-next-line no-await-in-loop
let tlnames = await lookup(name.toLowerCase().split('.', 1)[0] ?? name);
if (tlnames === undefined) {
notFound.push(name);
} else {
tlnames = Array.isArray(tlnames) ? tlnames : [tlnames];
for (const tlname of tlnames) {
fromTL.put(tlname, name);
}
log.info(' %s (in CTAN) => %s (in TeX Live)', ctanName, tlName ?? '???');
log.info(' %s (in CTAN) => %s (in TeX Live)', name, tlnames.join(', '));
}
}
if (fromTL.size === 0) {
return packages;
}
try {
await tryToInstall(new Set(fromTL.keys()));
} catch (error) {
if (!(error instanceof PackageNotFound)) {
throw error;
}
if (notFound.length > 0) {
throw new PackageNotFound(notFound, { action: 'install' });
// This may be a failure to parse logs.
if (error.packages.some((tlname) => !fromTL.has(tlname))) {
log.debug('Unexpected result: %o', fromTL.asMap());
return packages;
}
await tryToInstall(resolved);
notFound.push(...error.packages.flatMap((name) => [...fromTL.get(name)]));
return new Set(notFound);
}
return undefined;
}

async function tryToInstall(packages: ReadonlySet<string>): Promise<void> {
Expand All @@ -57,18 +110,3 @@ async function tryToInstall(packages: ReadonlySet<string>): Promise<void> {
}
}
}

async function resolvePackageName(
name: string,
): Promise<[ctanName: string, tlName?: string]> {
try {
const pkg = await ctan.api.pkg(name);
if (pkg.texlive !== undefined) {
return [name, pkg.texlive];
}
log.info('Unexpected response: %j', pkg);
} catch (error) {
log.info({ error }, 'Failed to request package data');
}
return [name];
}

0 comments on commit b2f6f42

Please sign in to comment.