From d3f864a903a48a2cacc1823b32dda310d557977b Mon Sep 17 00:00:00 2001 From: Norbert Szulc Date: Thu, 15 Feb 2024 13:09:58 +0100 Subject: [PATCH 01/11] Workaround --- lib/modules/manager/pip-compile/artifacts.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/modules/manager/pip-compile/artifacts.ts b/lib/modules/manager/pip-compile/artifacts.ts index 48172d6f601624..fb042a783f1326 100644 --- a/lib/modules/manager/pip-compile/artifacts.ts +++ b/lib/modules/manager/pip-compile/artifacts.ts @@ -76,8 +76,11 @@ export async function updateArtifacts({ config.lockFiles, )})`, ); + // workaround for Renovate not passing all relevant lock files to updateArtifacts + const inferedLockfile = upath.changeExt(inputFileName, '.txt'); + const lockFiles = [...config.lockFiles, inferedLockfile]; const result: UpdateArtifactsResult[] = []; - for (const outputFileName of config.lockFiles) { + for (const outputFileName of lockFiles) { const existingOutput = await readLocalFile(outputFileName, 'utf8'); if (!existingOutput) { logger.debug('pip-compile: No output file found'); From 0da176c7d662f986d6150f356c469f6e40ec262e Mon Sep 17 00:00:00 2001 From: Norbert Szulc Date: Thu, 15 Feb 2024 13:17:38 +0100 Subject: [PATCH 02/11] add PR number --- lib/modules/manager/pip-compile/artifacts.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/modules/manager/pip-compile/artifacts.ts b/lib/modules/manager/pip-compile/artifacts.ts index fb042a783f1326..822f612c30c4c4 100644 --- a/lib/modules/manager/pip-compile/artifacts.ts +++ b/lib/modules/manager/pip-compile/artifacts.ts @@ -76,7 +76,7 @@ export async function updateArtifacts({ config.lockFiles, )})`, ); - // workaround for Renovate not passing all relevant lock files to updateArtifacts + // workaround for Renovate not passing all relevant lock files to updateArtifacts, see: #27319 const inferedLockfile = upath.changeExt(inputFileName, '.txt'); const lockFiles = [...config.lockFiles, inferedLockfile]; const result: UpdateArtifactsResult[] = []; From e3e0ea80199f053e2892d3ffdc6763b9ef18de88 Mon Sep 17 00:00:00 2001 From: Norbert Szulc Date: Thu, 15 Feb 2024 13:52:37 +0100 Subject: [PATCH 03/11] attempt 2 --- lib/modules/manager/pip-compile/artifacts.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/modules/manager/pip-compile/artifacts.ts b/lib/modules/manager/pip-compile/artifacts.ts index 822f612c30c4c4..cbb7e0151dc048 100644 --- a/lib/modules/manager/pip-compile/artifacts.ts +++ b/lib/modules/manager/pip-compile/artifacts.ts @@ -16,6 +16,7 @@ import { getExecOptions, getRegistryUrlVarsFromPackageFile, } from './common'; +import { extractAllPackageFiles } from '.'; export function constructPipCompileCmd( content: string, @@ -77,8 +78,19 @@ export async function updateArtifacts({ )})`, ); // workaround for Renovate not passing all relevant lock files to updateArtifacts, see: #27319 - const inferedLockfile = upath.changeExt(inputFileName, '.txt'); - const lockFiles = [...config.lockFiles, inferedLockfile]; + let lockFiles: string[]; + if (config.isLockFileMaintenance) { + lockFiles = config.lockFiles; + } else { + const packageFiles = await extractAllPackageFiles(config, config.lockFiles); + if (!packageFiles) { + return null; + } + lockFiles = + packageFiles.find((file) => file.packageFile === inputFileName)! + .lockFiles ?? []; + } + // end workaround const result: UpdateArtifactsResult[] = []; for (const outputFileName of lockFiles) { const existingOutput = await readLocalFile(outputFileName, 'utf8'); From 79ebdff5ed122f4b8cd7c59642edfc34f5df3b18 Mon Sep 17 00:00:00 2001 From: Norbert Szulc Date: Thu, 15 Feb 2024 14:22:26 +0100 Subject: [PATCH 04/11] Revert "Workaround" This reverts commit d3f864a903a48a2cacc1823b32dda310d557977b. --- lib/modules/manager/pip-compile/artifacts.ts | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/lib/modules/manager/pip-compile/artifacts.ts b/lib/modules/manager/pip-compile/artifacts.ts index cbb7e0151dc048..48172d6f601624 100644 --- a/lib/modules/manager/pip-compile/artifacts.ts +++ b/lib/modules/manager/pip-compile/artifacts.ts @@ -16,7 +16,6 @@ import { getExecOptions, getRegistryUrlVarsFromPackageFile, } from './common'; -import { extractAllPackageFiles } from '.'; export function constructPipCompileCmd( content: string, @@ -77,22 +76,8 @@ export async function updateArtifacts({ config.lockFiles, )})`, ); - // workaround for Renovate not passing all relevant lock files to updateArtifacts, see: #27319 - let lockFiles: string[]; - if (config.isLockFileMaintenance) { - lockFiles = config.lockFiles; - } else { - const packageFiles = await extractAllPackageFiles(config, config.lockFiles); - if (!packageFiles) { - return null; - } - lockFiles = - packageFiles.find((file) => file.packageFile === inputFileName)! - .lockFiles ?? []; - } - // end workaround const result: UpdateArtifactsResult[] = []; - for (const outputFileName of lockFiles) { + for (const outputFileName of config.lockFiles) { const existingOutput = await readLocalFile(outputFileName, 'utf8'); if (!existingOutput) { logger.debug('pip-compile: No output file found'); From 9feba35fbfe5748d4b5466e840b84fe062376915 Mon Sep 17 00:00:00 2001 From: Norbert Szulc Date: Thu, 15 Feb 2024 14:28:53 +0100 Subject: [PATCH 05/11] modify config before updateArtifacts --- lib/workers/repository/update/branch/get-updated.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/workers/repository/update/branch/get-updated.ts b/lib/workers/repository/update/branch/get-updated.ts index ab15de8e7b74c1..af7ba42ada998f 100644 --- a/lib/workers/repository/update/branch/get-updated.ts +++ b/lib/workers/repository/update/branch/get-updated.ts @@ -361,6 +361,10 @@ export async function getUpdatedPackageFiles( const packageFileContents = updatedFileContents[packageFile] || (await getFile(packageFile, config.baseBranch)); + // workaround, see #27319 + if (config.packageFiles?.[packageFile]?.lockFiles) { + config.lockFiles = config.packageFiles![packageFile].lockFiles; + } const results = await updateArtifacts({ packageFileName: packageFile, updatedDeps: [], From bf0e2fbb43e9e7b715550782694df04544107992 Mon Sep 17 00:00:00 2001 From: Norbert Szulc Date: Thu, 15 Feb 2024 15:29:04 +0100 Subject: [PATCH 06/11] attempt 3 --- .../repository/update/branch/get-updated.ts | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/lib/workers/repository/update/branch/get-updated.ts b/lib/workers/repository/update/branch/get-updated.ts index af7ba42ada998f..6794f38c8c5fe9 100644 --- a/lib/workers/repository/update/branch/get-updated.ts +++ b/lib/workers/repository/update/branch/get-updated.ts @@ -6,6 +6,7 @@ import { get } from '../../../../modules/manager'; import type { ArtifactError, PackageDependency, + PackageFile, } from '../../../../modules/manager/types'; import { getFile } from '../../../../util/git'; import type { FileAddition, FileChange } from '../../../../util/git/types'; @@ -361,15 +362,16 @@ export async function getUpdatedPackageFiles( const packageFileContents = updatedFileContents[packageFile] || (await getFile(packageFile, config.baseBranch)); - // workaround, see #27319 - if (config.packageFiles?.[packageFile]?.lockFiles) { - config.lockFiles = config.packageFiles![packageFile].lockFiles; - } + const results = await updateArtifacts({ packageFileName: packageFile, updatedDeps: [], newPackageFileContent: packageFileContents!, - config, + config: patchConfigForArtifactsUpdate( + config, + manager, + packageFile, + ), }); if (is.nonEmptyArray(results)) { for (const res of results) { @@ -393,3 +395,22 @@ export async function getUpdatedPackageFiles( artifactErrors, }; } + +// workaround, see #27319 +function patchConfigForArtifactsUpdate( + config: BranchConfig, + manager: string, + packageFile: string, +): BranchConfig { + const updatedConfig = { ...config }; + if (updatedConfig.packageFiles?.[manager]) { + const packageFiles: PackageFile[] = updatedConfig.packageFiles?.[manager]; + const _packageFile: PackageFile | undefined = packageFiles.find( + (p) => p.packageFile === packageFile, + ); + if (_packageFile) { + updatedConfig.lockFiles = _packageFile.lockFiles; + } + } + return updatedConfig; +} From 7f421f58497781d48027ddea88d61e9575452776 Mon Sep 17 00:00:00 2001 From: Norbert Szulc Date: Thu, 15 Feb 2024 17:22:20 +0100 Subject: [PATCH 07/11] Try writing unit test --- .../update/branch/get-updated.spec.ts | 36 +++++++++++++++++++ .../repository/update/branch/get-updated.ts | 11 +++--- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/lib/workers/repository/update/branch/get-updated.spec.ts b/lib/workers/repository/update/branch/get-updated.spec.ts index ab63ca482a5246..9847d29a413af8 100644 --- a/lib/workers/repository/update/branch/get-updated.spec.ts +++ b/lib/workers/repository/update/branch/get-updated.spec.ts @@ -8,6 +8,7 @@ import * as _helmv3 from '../../../../modules/manager/helmv3'; import * as _npm from '../../../../modules/manager/npm'; import * as _pep621 from '../../../../modules/manager/pep621'; import * as _poetry from '../../../../modules/manager/poetry'; +import type { PackageFile } from '../../../../modules/manager/types'; import type { BranchConfig, BranchUpgradeConfig } from '../../../types'; import * as _autoReplace from './auto-replace'; import { getUpdatedPackageFiles } from './get-updated'; @@ -204,6 +205,41 @@ describe('workers/repository/update/branch/get-updated', () => { }); }); + it('for lockFileMaintenance passes proper lockFiles', async () => { + config.upgrades.push({ + manager: 'composer', + updateType: 'lockFileMaintenance', + branchName: 'some-branch', + } satisfies BranchUpgradeConfig); + config.lockFiles = ['different.lock']; + config.packageFiles = { + composer: [ + { + packageFile: 'composer.json', + lockFiles: ['composer.lock'], + deps: [], + }, + ] satisfies PackageFile[], + }; + composer.updateArtifacts.mockResolvedValueOnce([ + { + file: { + type: 'addition', + path: 'composer.json', + contents: 'some contents', + }, + }, + ]); + const res = await getUpdatedPackageFiles(config); + expect(composer.updateArtifacts).toBeCalledWith( + expect.objectContaining({ + config: expect.objectContaining({ + lockFiles: ['different.lock'], + }), + }), + ); + }); + it('handles isRemediation success', async () => { config.upgrades.push({ manager: 'npm', diff --git a/lib/workers/repository/update/branch/get-updated.ts b/lib/workers/repository/update/branch/get-updated.ts index 6794f38c8c5fe9..72a2c8cb66bd03 100644 --- a/lib/workers/repository/update/branch/get-updated.ts +++ b/lib/workers/repository/update/branch/get-updated.ts @@ -400,13 +400,14 @@ export async function getUpdatedPackageFiles( function patchConfigForArtifactsUpdate( config: BranchConfig, manager: string, - packageFile: string, + packageFileName: string, ): BranchConfig { const updatedConfig = { ...config }; - if (updatedConfig.packageFiles?.[manager]) { - const packageFiles: PackageFile[] = updatedConfig.packageFiles?.[manager]; - const _packageFile: PackageFile | undefined = packageFiles.find( - (p) => p.packageFile === packageFile, + if (is.nonEmptyArray(updatedConfig.packageFiles?.[manager])) { + const managerPackageFiles: PackageFile[] = + updatedConfig.packageFiles?.[manager]; + const _packageFile: PackageFile | undefined = managerPackageFiles.find( + (p) => p.packageFile === packageFileName, ); if (_packageFile) { updatedConfig.lockFiles = _packageFile.lockFiles; From fa985956301dd3a61ac16e71f7a70e00a6b681a1 Mon Sep 17 00:00:00 2001 From: Norbert Szulc Date: Thu, 15 Feb 2024 17:54:10 +0100 Subject: [PATCH 08/11] Apply suggestions from code review Co-authored-by: Michael Kriese --- lib/workers/repository/update/branch/get-updated.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/workers/repository/update/branch/get-updated.ts b/lib/workers/repository/update/branch/get-updated.ts index 72a2c8cb66bd03..eecbc869c8ef97 100644 --- a/lib/workers/repository/update/branch/get-updated.ts +++ b/lib/workers/repository/update/branch/get-updated.ts @@ -406,11 +406,11 @@ function patchConfigForArtifactsUpdate( if (is.nonEmptyArray(updatedConfig.packageFiles?.[manager])) { const managerPackageFiles: PackageFile[] = updatedConfig.packageFiles?.[manager]; - const _packageFile: PackageFile | undefined = managerPackageFiles.find( + const packageFile = managerPackageFiles.find( (p) => p.packageFile === packageFileName, ); - if (_packageFile) { - updatedConfig.lockFiles = _packageFile.lockFiles; + if (packageFile) { + updatedConfig.lockFiles = packageFile.lockFiles; } } return updatedConfig; From adbea9e237244d172d71aaf57be378e8a167d747 Mon Sep 17 00:00:00 2001 From: Norbert Szulc Date: Thu, 15 Feb 2024 17:56:30 +0100 Subject: [PATCH 09/11] remove assignment --- lib/workers/repository/update/branch/get-updated.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/workers/repository/update/branch/get-updated.spec.ts b/lib/workers/repository/update/branch/get-updated.spec.ts index 9847d29a413af8..1366e83db50ac0 100644 --- a/lib/workers/repository/update/branch/get-updated.spec.ts +++ b/lib/workers/repository/update/branch/get-updated.spec.ts @@ -230,7 +230,7 @@ describe('workers/repository/update/branch/get-updated', () => { }, }, ]); - const res = await getUpdatedPackageFiles(config); + await getUpdatedPackageFiles(config); expect(composer.updateArtifacts).toBeCalledWith( expect.objectContaining({ config: expect.objectContaining({ From 2edbb79d51cb511e3b8b5201129fc221fd03948a Mon Sep 17 00:00:00 2001 From: Norbert Szulc Date: Mon, 19 Feb 2024 12:53:47 +0100 Subject: [PATCH 10/11] Fix test by add packageFile name to upgrade --- lib/workers/repository/update/branch/get-updated.spec.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/workers/repository/update/branch/get-updated.spec.ts b/lib/workers/repository/update/branch/get-updated.spec.ts index 1366e83db50ac0..123485920d9c87 100644 --- a/lib/workers/repository/update/branch/get-updated.spec.ts +++ b/lib/workers/repository/update/branch/get-updated.spec.ts @@ -209,6 +209,7 @@ describe('workers/repository/update/branch/get-updated', () => { config.upgrades.push({ manager: 'composer', updateType: 'lockFileMaintenance', + packageFile: 'composer.json', branchName: 'some-branch', } satisfies BranchUpgradeConfig); config.lockFiles = ['different.lock']; @@ -231,10 +232,10 @@ describe('workers/repository/update/branch/get-updated', () => { }, ]); await getUpdatedPackageFiles(config); - expect(composer.updateArtifacts).toBeCalledWith( + expect(composer.updateArtifacts).toHaveBeenCalledWith( expect.objectContaining({ config: expect.objectContaining({ - lockFiles: ['different.lock'], + lockFiles: ['composer.lock'], }), }), ); From 776c03a2b6b0d2f2238f01099874f243440b94fb Mon Sep 17 00:00:00 2001 From: Norbert Szulc Date: Tue, 27 Feb 2024 09:15:47 +0100 Subject: [PATCH 11/11] Update lib/workers/repository/update/branch/get-updated.ts Co-authored-by: Rhys Arkins --- lib/workers/repository/update/branch/get-updated.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/workers/repository/update/branch/get-updated.ts b/lib/workers/repository/update/branch/get-updated.ts index cf7ecfd3e74dae..70882094557b1e 100644 --- a/lib/workers/repository/update/branch/get-updated.ts +++ b/lib/workers/repository/update/branch/get-updated.ts @@ -380,7 +380,7 @@ function patchConfigForArtifactsUpdate( (p) => p.packageFile === packageFileName, ); if (packageFile) { - updatedConfig.lockFiles = packageFile.lockFiles; + updatedConfig.lockFiles ??= packageFile.lockFiles; } } return updatedConfig;