Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(util/exec): Fix strict null errors #12909

Merged
merged 9 commits into from
Dec 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 11 additions & 11 deletions lib/util/exec/buildpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,21 +74,21 @@ export async function resolveConstraint(
}

const pkgReleases = await getPkgReleases(toolConfig);
if (!pkgReleases?.releases?.length) {
throw new Error('No tool releases found.');
}

const allVersions = pkgReleases.releases.map((r) => r.version);
const matchingVersions = allVersions.filter(
(v) => !constraint || versioning.matches(v, constraint)
);
const releases = pkgReleases?.releases ?? [];
const versions = releases.map((r) => r.version);
const resolvedVersion = versions
.filter((v) => !constraint || versioning.matches(v, constraint))
.pop();

if (matchingVersions.length) {
const resolvedVersion = matchingVersions.pop();
if (resolvedVersion) {
logger.debug({ toolName, constraint, resolvedVersion }, 'Resolved version');
return resolvedVersion;
}
const latestVersion = allVersions.filter((v) => versioning.isStable(v)).pop();

const latestVersion = versions.filter((v) => versioning.isStable(v)).pop();
if (!latestVersion) {
throw new Error('No tool releases found.');
}
logger.warn(
{ toolName, constraint, latestVersion },
'No matching tool versions found for constraint - using latest version'
Expand Down
29 changes: 16 additions & 13 deletions lib/util/exec/docker/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,17 @@ function uniq<T = unknown>(

function prepareVolumes(volumes: VolumeOption[] = []): string[] {
const expanded: (VolumesPair | null)[] = volumes.map(expandVolumeOption);
const filtered: VolumesPair[] = expanded.filter((vol) => vol !== null);
const filtered: VolumesPair[] = expanded.filter(
(vol): vol is VolumesPair => vol !== null
);
const unique: VolumesPair[] = uniq<VolumesPair>(filtered, volumesEql);
return unique.map(([from, to]) => `-v "${from}":"${to}"`);
}

function prepareCommands(commands: Opt<string>[]): string[] {
return commands.filter((command) => command && typeof command === 'string');
return commands.filter<string>((command): command is string =>
is.string(command)
);
}

export async function getDockerTag(
Expand Down Expand Up @@ -97,9 +101,8 @@ export async function getDockerTag(
logger.debug('Filtering out unstable versions');
versions = versions.filter((version) => ver.isStable(version));
}
versions = versions.sort(ver.sortVersions.bind(ver));
if (versions.length) {
const version = versions.pop();
const version = versions.sort(ver.sortVersions.bind(ver)).pop();
if (version) {
logger.debug(
{ depName, scheme, constraint, version },
`Found compatible image version`
Expand All @@ -117,12 +120,12 @@ export async function getDockerTag(
return 'latest';
}

function getContainerName(image: string, prefix?: string): string {
return `${prefix || 'renovate_'}${image}`.replace(regEx(/\//g), '_');
function getContainerName(image: string, prefix?: string | undefined): string {
return `${prefix ?? 'renovate_'}${image}`.replace(regEx(/\//g), '_');
}

function getContainerLabel(prefix: string): string {
return `${prefix || 'renovate_'}child`;
function getContainerLabel(prefix: string | undefined): string {
return `${prefix ?? 'renovate_'}child`;
}

export async function removeDockerContainer(
Expand Down Expand Up @@ -199,7 +202,7 @@ export async function generateDockerCommand(
): Promise<string> {
const { envVars, cwd, tagScheme, tagConstraint } = options;
let image = options.image;
const volumes = options.volumes || [];
const volumes = options.volumes ?? [];
const {
localDir,
cacheDir,
Expand All @@ -221,7 +224,7 @@ export async function generateDockerCommand(
if (envVars) {
result.push(
...uniq(envVars)
.filter((x) => typeof x === 'string')
.filter(is.string)
.map((e) => `-e ${e}`)
);
}
Expand All @@ -232,11 +235,11 @@ export async function generateDockerCommand(

image = `${ensureTrailingSlash(dockerImagePrefix ?? 'renovate')}${image}`;

let tag: string;
let tag: string | null = null;
if (options.tag) {
tag = options.tag;
} else if (tagConstraint) {
const tagVersioning = tagScheme || 'semver';
const tagVersioning = tagScheme ?? 'semver';
tag = await getDockerTag(image, tagConstraint, tagVersioning);
logger.debug(
{ image, tagConstraint, tagVersioning, tag },
Expand Down
4 changes: 2 additions & 2 deletions lib/util/exec/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -713,7 +713,7 @@ describe('util/exec/index', () => {
actualCmd.push(execCmd);
actualOpts.push(execOpts);
callback(null, { stdout: '', stderr: '' });
return undefined;
return undefined as never;
});
GlobalConfig.set({ cacheDir, localDir: cwd, ...adminConfig });
await exec(cmd as string, inOpts);
Expand All @@ -729,7 +729,7 @@ describe('util/exec/index', () => {
cpExec.mockImplementation((execCmd, execOpts, callback) => {
actualCmd.push(execCmd);
callback(null, { stdout: '', stderr: '' });
return undefined;
return undefined as never;
});

GlobalConfig.set({ binarySource: 'global' });
Expand Down
83 changes: 41 additions & 42 deletions lib/util/exec/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import is from '@sindresorhus/is';
import { dirname, join } from 'upath';
import { GlobalConfig } from '../../config/global';
import { TEMPORARY_ERROR } from '../../constants/error-messages';
Expand All @@ -11,50 +12,50 @@ import type {
ExecOptions,
ExecResult,
ExtraEnv,
Opt,
RawExecOptions,
} from './types';

function getChildEnv({
extraEnv = {},
extraEnv,
env: forcedEnv = {},
}: ExecOptions): ExtraEnv<string> {
}: ExecOptions): Record<string, string> {
const globalConfigEnv = GlobalConfig.get('customEnvVariables');

const inheritedKeys = Object.entries(extraEnv).reduce(
(acc, [key, val]) =>
val === null || val === undefined ? acc : [...acc, key],
[]
);
const inheritedKeys: string[] = [];
for (const [key, val] of Object.entries(extraEnv ?? {})) {
if (is.string(val)) {
inheritedKeys.push(key);
}
}

const parentEnv = getChildProcessEnv(inheritedKeys);
const childEnv = Object.entries({
const combinedEnv = {
...extraEnv,
...parentEnv,
...globalConfigEnv,
...forcedEnv,
}).reduce(
(acc, [key, val]) =>
val === null || val === undefined
? acc
: { ...acc, [key]: val.toString() },
{}
);
return childEnv;
};

const result: Record<string, string> = {};
for (const [key, val] of Object.entries(combinedEnv)) {
if (is.string(val)) {
result[key] = `${val}`;
}
}

return result;
}

function dockerEnvVars(
extraEnv: ExtraEnv,
childEnv: ExtraEnv<string>
): string[] {
function dockerEnvVars(extraEnv: ExtraEnv, childEnv: ExtraEnv): string[] {
const extraEnvKeys = Object.keys(extraEnv || {});
return extraEnvKeys.filter(
(key) => typeof childEnv[key] === 'string' && childEnv[key].length > 0
);
return extraEnvKeys.filter((key) => is.nonEmptyString(childEnv[key]));
}

function getCwd({ cwd, cwdFile }: ExecOptions): string {
const defaultCwd = GlobalConfig.get('localDir');
const paramCwd = cwdFile ? join(defaultCwd, dirname(cwdFile)) : cwd;
return paramCwd || defaultCwd;
return paramCwd ?? defaultCwd;
}

function getRawExecOptions(opts: ExecOptions): RawExecOptions {
Expand All @@ -78,11 +79,11 @@ function getRawExecOptions(opts: ExecOptions): RawExecOptions {
}

// Set default max buffer size to 10MB
rawExecOptions.maxBuffer = rawExecOptions.maxBuffer || 10 * 1024 * 1024;
rawExecOptions.maxBuffer = rawExecOptions.maxBuffer ?? 10 * 1024 * 1024;
return rawExecOptions;
}

function isDocker({ docker }: ExecOptions): boolean {
function isDocker(docker: Opt<DockerOptions>): docker is DockerOptions {
const { binarySource } = GlobalConfig.get();
return binarySource === 'docker' && !!docker;
}
Expand All @@ -103,7 +104,7 @@ async function prepareRawExec(

let rawCommands = typeof cmd === 'string' ? [cmd] : cmd;

if (isDocker(opts)) {
if (isDocker(docker)) {
logger.debug('Using docker to execute');
const extraEnv = { ...opts.extraEnv, ...customEnvVariables };
const childEnv = getChildEnv(opts);
Expand All @@ -112,7 +113,7 @@ async function prepareRawExec(
const dockerOptions: DockerOptions = { ...docker, cwd, envVars };
const preCommands = [
...(await generateInstallCommands(opts.toolConstraints)),
...(opts.preCommands || []),
...(opts.preCommands ?? []),
];
const dockerCommand = await generateDockerCommand(
rawCommands,
Expand All @@ -136,12 +137,12 @@ export async function exec(
opts: ExecOptions = {}
): Promise<ExecResult> {
const { docker } = opts;
const { dockerChildPrefix } = GlobalConfig.get();
const dockerChildPrefix = GlobalConfig.get('dockerChildPrefix');

const { rawCommands, rawOptions } = await prepareRawExec(cmd, opts);
const useDocker = isDocker(opts);
const useDocker = isDocker(docker);

let res: ExecResult | null = null;
let res: ExecResult = { stdout: '', stderr: '' };
viceice marked this conversation as resolved.
Show resolved Hide resolved
for (const rawCmd of rawCommands) {
const startTime = Date.now();
if (useDocker) {
Expand Down Expand Up @@ -173,17 +174,15 @@ export async function exec(
throw err;
}
const durationMs = Math.round(Date.now() - startTime);
if (res) {
logger.debug(
{
cmd: rawCmd,
durationMs,
stdout: res.stdout,
stderr: res.stderr,
},
'exec completed'
);
}
logger.debug(
{
cmd: rawCmd,
durationMs,
stdout: res.stdout,
stderr: res.stderr,
},
'exec completed'
);
}

return res;
Expand Down
4 changes: 2 additions & 2 deletions test/exec-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export function mockExecAll(
throw execResult;
}
callback(null, execResult);
return undefined;
return undefined as never;
});
return snapshots;
}
Expand All @@ -67,7 +67,7 @@ export function mockExecSequence(
throw execResult;
}
callback(null, execResult);
return undefined;
return undefined as never;
});
});
return snapshots;
Expand Down
10 changes: 8 additions & 2 deletions tsconfig.strict.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@
"lib/util/cache/*.ts",
"lib/util/clone.ts",
"lib/util/date.ts",
"lib/util/exec/common.ts",
"lib/util/exec/types.ts",
"lib/util/exec",
"lib/util/fs",
"lib/util/git/config.ts",
"lib/util/git/types.ts",
Expand Down Expand Up @@ -68,6 +67,13 @@
"lib/logger/err-serializer.spec.ts",
"lib/manager/gradle/shallow/types.ts",
"lib/util/cache/*.spec.ts",
"lib/util/exec/buildpack.spec.ts",
"lib/util/exec/buildpack.ts",
"lib/util/exec/docker/index.spec.ts",
"lib/util/exec/docker/index.ts",
"lib/util/exec/env.spec.ts",
"lib/util/exec/index.spec.ts",
"lib/util/exec/index.ts",
"lib/util/fs/index.spec.ts"
]
}