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

fix(npm): don't delete lockfiles early #28472

Merged
merged 2 commits into from
Apr 18, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions docs/usage/self-hosted-experimental.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@ Skipping the check will speed things up, but may result in versions being return

If set to any value, Renovate will always paginate requests to GitHub fully, instead of stopping after 10 pages.

## `RENOVATE_REUSE_PACKAGE_LOCK`

If set to "false" (string), Renovate will remove any existing `package-lock.json` before trying to update it.

## `RENOVATE_USER_AGENT`

If set to any string, Renovate will use this as the `user-agent` it sends with HTTP requests.
Expand Down
8 changes: 4 additions & 4 deletions lib/modules/manager/npm/extract/index.spec.ts
viceice marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { codeBlock } from 'common-tags';
import { extractAllPackageFiles } from '..';
import { Fixtures } from '../../../../../test/fixtures';
import { fs } from '../../../../../test/util';
import { logger } from '../../../../logger';
Expand Down Expand Up @@ -980,10 +981,9 @@ describe('modules/manager/npm/extract/index', () => {
describe('.extractAllPackageFiles()', () => {
it('runs', async () => {
fs.readLocalFile.mockResolvedValueOnce(input02Content);
const res = await npmExtract.extractAllPackageFiles(
defaultExtractConfig,
['package.json'],
);
const res = await extractAllPackageFiles(defaultExtractConfig, [
'package.json',
]);
expect(res).toEqual([
{
deps: [
Expand Down
16 changes: 0 additions & 16 deletions lib/modules/manager/npm/post-update/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,22 +204,6 @@ describe('modules/manager/npm/post-update/index', () => {
expect(git.getFile).toHaveBeenCalledOnce();
});

it('works no reuse lockfiles', async () => {
await expect(
writeExistingFiles(
{ ...updateConfig, reuseLockFiles: false },
additionalFiles,
),
).resolves.toBeUndefined();

expect(fs.writeLocalFile).toHaveBeenCalledOnce();
expect(fs.deleteLocalFile.mock.calls).toEqual([
['package-lock.json'],
['yarn.lock'],
['yarn.lock'],
]);
});

it('writes .npmrc files', async () => {
await writeExistingFiles(updateConfig, {
npm: [
Expand Down
137 changes: 61 additions & 76 deletions lib/modules/manager/npm/post-update/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,91 +141,76 @@ export async function writeExistingFiles(
const npmLock = packageFile.managerData.npmLock;
if (npmLock) {
const npmLockPath = npmLock;
if (
process.env.RENOVATE_REUSE_PACKAGE_LOCK === 'false' ||
config.reuseLockFiles === false
) {
logger.debug(`Ensuring ${npmLock} is removed`);
await deleteLocalFile(npmLockPath);
} else {
logger.debug(`Writing ${npmLock}`);
let existingNpmLock: string;
try {
existingNpmLock = (await getFile(npmLock)) ?? '';
} catch (err) /* istanbul ignore next */ {
logger.warn({ err }, 'Error reading npm lock file');
existingNpmLock = '';
}
const { detectedIndent, lockFileParsed: npmLockParsed } =
parseLockFile(existingNpmLock);
if (npmLockParsed) {
const packageNames =
'packages' in npmLockParsed
? Object.keys(npmLockParsed.packages)
: [];
const widens: string[] = [];
let lockFileChanged = false;
for (const upgrade of config.upgrades) {
if (upgrade.lockFiles && !upgrade.lockFiles.includes(npmLock)) {
continue;
}
if (!upgrade.managerData) {
continue;
}
logger.debug(`Writing ${npmLock}`);
let existingNpmLock: string;
try {
existingNpmLock = (await getFile(npmLock)) ?? '';
} catch (err) /* istanbul ignore next */ {
logger.warn({ err }, 'Error reading npm lock file');
existingNpmLock = '';
}
const { detectedIndent, lockFileParsed: npmLockParsed } =
parseLockFile(existingNpmLock);
if (npmLockParsed) {
const packageNames =
'packages' in npmLockParsed
? Object.keys(npmLockParsed.packages)
: [];
const widens: string[] = [];
let lockFileChanged = false;
for (const upgrade of config.upgrades) {
if (upgrade.lockFiles && !upgrade.lockFiles.includes(npmLock)) {
continue;
}
if (!upgrade.managerData) {
continue;
}
if (
upgrade.rangeStrategy === 'widen' &&
upgrade.managerData.npmLock === npmLock
) {
// TODO #22198
widens.push(upgrade.depName!);
}
const { depName } = upgrade;
for (const packageName of packageNames) {
if (
upgrade.rangeStrategy === 'widen' &&
upgrade.managerData.npmLock === npmLock
'packages' in npmLockParsed &&
(packageName === `node_modules/${depName}` ||
packageName.startsWith(`node_modules/${depName}/`))
) {
// TODO #22198
widens.push(upgrade.depName!);
}
const { depName } = upgrade;
for (const packageName of packageNames) {
if (
'packages' in npmLockParsed &&
(packageName === `node_modules/${depName}` ||
packageName.startsWith(`node_modules/${depName}/`))
) {
logger.trace({ packageName }, 'Massaging out package name');
lockFileChanged = true;
delete npmLockParsed.packages[packageName];
}
logger.trace({ packageName }, 'Massaging out package name');
lockFileChanged = true;
delete npmLockParsed.packages[packageName];
}
}
if (widens.length) {
logger.debug(
`Removing ${String(widens)} from ${npmLock} to force an update`,
);
lockFileChanged = true;
try {
if (
'dependencies' in npmLockParsed &&
npmLockParsed.dependencies
) {
widens.forEach((depName) => {
// TODO #22198
delete npmLockParsed.dependencies![depName];
});
}
} catch (err) /* istanbul ignore next */ {
logger.warn(
{ npmLock },
'Error massaging package-lock.json for widen',
);
}
if (widens.length) {
logger.debug(
`Removing ${String(widens)} from ${npmLock} to force an update`,
);
lockFileChanged = true;
try {
if ('dependencies' in npmLockParsed && npmLockParsed.dependencies) {
widens.forEach((depName) => {
// TODO #22198
delete npmLockParsed.dependencies![depName];
});
}
} catch (err) /* istanbul ignore next */ {
logger.warn(
{ npmLock },
'Error massaging package-lock.json for widen',
);
}
if (lockFileChanged) {
logger.debug('Massaging npm lock file before writing to disk');
existingNpmLock = composeLockFile(npmLockParsed, detectedIndent);
}
await writeLocalFile(npmLockPath, existingNpmLock);
}
if (lockFileChanged) {
logger.debug('Massaging npm lock file before writing to disk');
existingNpmLock = composeLockFile(npmLockParsed, detectedIndent);
}
await writeLocalFile(npmLockPath, existingNpmLock);
}
}
const { yarnLock } = packageFile.managerData;
if (yarnLock && config.reuseLockFiles === false) {
await deleteLocalFile(yarnLock);
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion lib/modules/manager/npm/post-update/npm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ export async function generateLockFile(
} catch (err) /* istanbul ignore next */ {
logger.debug(
{ err, lockFileName },
'Error removing package-lock.json for lock file maintenance',
'Error removing `package-lock.json` for lock file maintenance',
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/modules/manager/npm/post-update/pnpm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export async function generateLockFile(
} catch (err) /* istanbul ignore next */ {
logger.debug(
{ err, lockFileName },
'Error removing yarn.lock for lock file maintenance',
'Error removing `pnpm-lock.yaml` for lock file maintenance',
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ exports[`workers/repository/updates/generate generateBranchConfig() handles @typ
"prettyDepType": "dependency",
"recreateClosed": false,
"releaseTimestamp": undefined,
"reuseLockFiles": true,
"upgrades": [
{
"branchName": "some-branch",
Expand Down Expand Up @@ -108,7 +107,6 @@ exports[`workers/repository/updates/generate generateBranchConfig() handles @typ
"prettyDepType": "dependency",
"recreateClosed": false,
"releaseTimestamp": undefined,
"reuseLockFiles": true,
"upgrades": [
{
"branchName": "some-branch",
Expand Down Expand Up @@ -186,7 +184,6 @@ exports[`workers/repository/updates/generate generateBranchConfig() handles lock
"prettyDepType": "dependency",
"recreateClosed": true,
"releaseTimestamp": undefined,
"reuseLockFiles": true,
"upgrades": [
{
"branchName": "some-branch",
Expand Down Expand Up @@ -231,7 +228,6 @@ exports[`workers/repository/updates/generate generateBranchConfig() handles lock
"prettyNewVersion": "v1.0.1",
"recreateClosed": false,
"releaseTimestamp": undefined,
"reuseLockFiles": true,
"upgrades": [
{
"branchName": "some-branch",
Expand Down
1 change: 0 additions & 1 deletion lib/workers/repository/updates/generate.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ describe('workers/repository/updates/generate', () => {
lockedVersion: '1.0.0',
newValue: '^1.0.0',
newVersion: '1.0.1',
reuseLockFiles: true,
prettyNewVersion: 'v1.0.1',
upgrades: [
{
Expand Down
3 changes: 0 additions & 3 deletions lib/workers/repository/updates/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -338,9 +338,6 @@ export function generateBranchConfig(
...config.upgrades[0],
releaseTimestamp: releaseTimestamp!,
}; // TODO: fixme (#9666)
config.reuseLockFiles = config.upgrades.every(
(upgrade) => upgrade.updateType !== 'lockFileMaintenance',
);
config.dependencyDashboardApproval = config.upgrades.some(
(upgrade) => upgrade.dependencyDashboardApproval,
);
Expand Down