Skip to content

Commit

Permalink
perf: reduce overhead by combining tlmgr update calls into one (#226)
Browse files Browse the repository at this point in the history
  • Loading branch information
teatimeguest committed Nov 4, 2024
1 parent f0689a7 commit 66d427e
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 47 deletions.
17 changes: 2 additions & 15 deletions packages/action/__tests__/runs/main/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { CacheService } from '#action/cache';
import { main } from '#action/runs/main';
import { Config } from '#action/runs/main/config';
import { install } from '#action/runs/main/install';
import { adjustTexmf, updateTlmgr } from '#action/runs/main/update';
import { adjustTexmf, update } from '#action/runs/main/update';

vi.unmock('#action/runs/main');

Expand Down Expand Up @@ -138,20 +138,7 @@ it.each(cacheTypes)(
async (...kind) => {
setCacheType(kind);
await expect(main()).resolves.not.toThrow();
expect(updateTlmgr).toHaveBeenCalledOnce();
},
);

it.each(cacheTypes)(
'updates all packages if `update-all-packages` is true (%s)',
async (...kind) => {
setCacheType(kind);
config.updateAllPackages = true;
await expect(main()).resolves.not.toThrow();
expect(tlmgr.update).toHaveBeenCalledWith({
all: true,
reinstallForciblyRemoved: true,
});
expect(update).toHaveBeenCalledOnce();
},
);

Expand Down
44 changes: 34 additions & 10 deletions packages/action/__tests__/runs/main/update.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import {
} from '@setup-texlive-action/texlive';

import { CacheService } from '#action/cache';
import { updateTlmgr } from '#action/runs/main/update';
import { update } from '#action/runs/main/update';

const { update, repository: { list, remove } } = tlmgr;
const { repository: { list, remove } } = tlmgr;

const updateCache = vi.fn();

Expand All @@ -35,26 +35,26 @@ const failedToInitialize = new TlpdbError('', {
});

it('move to historic', async () => {
vi.mocked(update).mockRejectedValueOnce(versionOutdated);
vi.mocked(tlmgr.update).mockRejectedValueOnce(versionOutdated);
const opts = { version: LATEST_VERSION };
await expect(updateTlmgr(opts)).resolves.not.toThrow();
await expect(update(opts)).resolves.not.toThrow();
expect(updateCache).toHaveBeenCalled();
});

it('move to historic master', async () => {
vi
.mocked(update)
.mocked(tlmgr.update)
.mockRejectedValueOnce(versionOutdated)
.mockRejectedValueOnce(failedToInitialize);
const opts = { version: LATEST_VERSION };
await expect(updateTlmgr(opts)).resolves.not.toThrow();
await expect(update(opts)).resolves.not.toThrow();
expect(updateCache).toHaveBeenCalled();
});

it('does not move to historic if repository set', async () => {
vi.mocked(update).mockRejectedValueOnce(versionOutdated);
vi.mocked(tlmgr.update).mockRejectedValueOnce(versionOutdated);
const opts = { version: LATEST_VERSION, repository: new URL(MOCK_URL) };
await expect(updateTlmgr(opts)).rejects.toThrow(versionOutdated);
await expect(update(opts)).rejects.toThrow(versionOutdated);
});

it('removes tlcontrib', async () => {
Expand All @@ -63,7 +63,7 @@ it('removes tlcontrib', async () => {
yield { tag: 'tlcontrib', path: MOCK_URL };
});
const opts = { version: '2023' } as const;
await expect(updateTlmgr(opts)).resolves.not.toThrow();
await expect(update(opts)).resolves.not.toThrow();
expect(remove).toHaveBeenCalledWith('tlcontrib');
});

Expand All @@ -73,6 +73,30 @@ it('removes tlpretest', async () => {
yield { tag: 'tlcontrib', path: MOCK_URL };
});
const opts = { version: LATEST_VERSION };
await expect(updateTlmgr(opts)).resolves.not.toThrow();
await expect(update(opts)).resolves.not.toThrow();
expect(remove).toHaveBeenCalledWith('main');
});

it('defaults to update only tlmgr', async () => {
await expect(update({ version: LATEST_VERSION }))
.resolves
.not
.toThrow();
expect(tlmgr.update).toHaveBeenCalledWith({
self: true,
all: false,
reinstallForciblyRemoved: false,
});
});

it('calls `tlmgr update --all` if `updateAllPackages: true`', async () => {
await expect(update({ version: LATEST_VERSION, updateAllPackages: true }))
.resolves
.not
.toThrow();
expect(tlmgr.update).toHaveBeenCalledWith({
self: true,
all: true,
reinstallForciblyRemoved: true,
});
});
23 changes: 9 additions & 14 deletions packages/action/src/runs/main/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
import { CacheService } from '#action/cache';
import { Config } from '#action/runs/main/config';
import { install } from '#action/runs/main/install';
import { adjustTexmf, updateTlmgr } from '#action/runs/main/update';
import { adjustTexmf, update } from '#action/runs/main/update';

export async function main(): Promise<void> {
const config = await Config.load();
Expand Down Expand Up @@ -45,19 +45,14 @@ export async function main(): Promise<void> {

if (cache.restored) {
if (profile.version >= previous.version) {
await log.group(
profile.version >= latest.version
? 'Updating tlmgr'
: 'Checking the package repository status',
async () => {
await updateTlmgr(config);
},
);
if (config.updateAllPackages) {
await log.group(`Updating packages`, async () => {
await tlmgr.update({ all: true, reinstallForciblyRemoved: true });
});
}
const msg = config.updateAllPackages
? 'Updating packages'
: profile.version >= latest.version
? 'Updating tlmgr'
: 'Checking the package repository status';
await log.group(msg, async () => {
await update(config);
});
}
await adjustTexmf(profile);
}
Expand Down
22 changes: 15 additions & 7 deletions packages/action/src/runs/main/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@ import { CacheService } from '#action/cache';
export interface UpdateOptions {
readonly version: Version;
readonly repository?: Readonly<URL> | undefined;
readonly updateAllPackages?: boolean | undefined;
}

export async function updateTlmgr(options: UpdateOptions): Promise<void> {
export async function update(options: UpdateOptions): Promise<void> {
try {
await updateRepositories(options);
} catch (error) {
Expand All @@ -28,17 +29,23 @@ export async function updateTlmgr(options: UpdateOptions): Promise<void> {
&& options.repository === undefined
) {
log.info({ error });
await moveToHistoric(options.version);
await moveToHistoric(options);
} else {
throw error;
}
}
}

async function updateTlmgr(options: UpdateOptions): Promise<void> {
const tlmgr = Tlmgr.use();
const all = options.updateAllPackages ?? false;
await tlmgr.update({ self: true, all, reinstallForciblyRemoved: all });
}

async function updateRepositories(options: UpdateOptions): Promise<void> {
const tlmgr = Tlmgr.use();
const { latest, previous } = ReleaseData.use();
const version = options.version;
const { version } = options;
let repository = options.repository;
if (version >= previous.version) {
for await (const { path, tag } of tlmgr.repository.list()) {
Expand All @@ -60,23 +67,25 @@ async function updateRepositories(options: UpdateOptions): Promise<void> {
}
if (repository !== undefined) {
await changeRepository('main', repository);
} else {
await tlmgr.update({ self: true });
}
await updateTlmgr(options);
}

async function moveToHistoric(version: Version): Promise<void> {
async function moveToHistoric(options: UpdateOptions): Promise<void> {
const cache = CacheService.use();
const { version } = options;
const tag = 'main';
try {
await changeRepository(tag, tlnet.historic(version));
await updateTlmgr(options);
} catch (error) {
if (
error instanceof TlpdbError
&& error.code === TlpdbError.Code.FAILED_TO_INITIALIZE
) {
log.info({ error });
await changeRepository(tag, tlnet.historic(version, { master: true }));
await updateTlmgr(options);
} else {
throw error;
}
Expand All @@ -97,7 +106,6 @@ async function changeRepository(
}
await tlmgr.repository.remove(tag);
await tlmgr.repository.add(url, tag);
await tlmgr.update({ self: true });
}

export async function adjustTexmf(profile: Profile): Promise<void> {
Expand Down
3 changes: 2 additions & 1 deletion packages/config/eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -307,10 +307,11 @@ export const sources = defineConfig(
'unicorn/no-array-for-each': 'off',
'unicorn/no-array-reduce': 'off',
'unicorn/no-negated-condition': 'off',
'unicorn/no-nested-ternary': 'off',
'unicorn/no-static-only-class': 'off',
'unicorn/no-unnecessary-polyfills': 'off',
'unicorn/no-useless-undefined': 'off',
'unicorn/no-useless-spread': 'off',
'unicorn/no-useless-undefined': 'off',
'unicorn/prefer-export-from': 'off',
'unicorn/prefer-number-properties': [
'error',
Expand Down

0 comments on commit 66d427e

Please sign in to comment.