From 4322ae8283abb381a6d52f8c84d3515267645126 Mon Sep 17 00:00:00 2001 From: Christoph Ludwig Date: Fri, 19 Aug 2022 13:49:01 +0200 Subject: [PATCH] fix(pip_requirements): ignore extras in test for hashes (#16910) Co-authored-by: Rhys Arkins Co-authored-by: Michael Kriese --- .../pip_requirements/artifacts.spec.ts | 61 +++++++++++++++++-- .../manager/pip_requirements/artifacts.ts | 49 +++++++++++---- .../manager/pip_requirements/extract.ts | 2 +- 3 files changed, 95 insertions(+), 17 deletions(-) diff --git a/lib/modules/manager/pip_requirements/artifacts.spec.ts b/lib/modules/manager/pip_requirements/artifacts.spec.ts index 1ca4eb14f37555..bfc998dde76ee3 100644 --- a/lib/modules/manager/pip_requirements/artifacts.spec.ts +++ b/lib/modules/manager/pip_requirements/artifacts.spec.ts @@ -20,9 +20,20 @@ const adminConfig: RepoGlobalConfig = { const config: UpdateArtifactsConfig = { constraints: { python: '3.10.2' } }; -const newPackageFileContent = `atomicwrites==1.4.0 \ ---hash=sha256:03472c30eb2c5d1ba9227e4c2ca66ab8287fbfbbda3888aa93dc2e28fc6811b4 \ ---hash=sha256:75a9445bac02d8d058d5e1fe689654ba5a6556a1dfd8ce6ec55a0ed79866cfa6`; +/* + * Sample package file content that exhibits dependencies with and without + * "extras" specifications as well as line continuations and additional + * (valid) whitespace. + */ +const newPackageFileContent = `atomicwrites==1.4.0 \\\n\ + --hash=sha256:03472c30eb2c5d1ba9227e4c2ca66ab8287fbfbbda3888aa93dc2e28fc6811b4 \\\n\ + --hash=sha256:75a9445bac02d8d058d5e1fe689654ba5a6556a1dfd8ce6ec55a0ed79866cfa6\n\ + boto3-stubs[iam] == 1.24.36.post1 \ +--hash=sha256:39acbbc8c87a101bdf46e058fbb012d044b773b43f7ed02cc4c24192a564411e \ +--hash=sha256:ca3b3066773fc727fea0dbec252d098098e45fe0def011b22036ef674344def2\n\ +botocore==1.27.46 \ +--hash=sha256:747b7e94aef41498f063fc0be79c5af102d940beea713965179e1ead89c7e9ec \ +--hash=sha256:f66d8305d1f59d83334df9b11b6512bb1e14698ec4d5d6d42f833f39f3304ca7`; describe('modules/manager/pip_requirements/artifacts', () => { beforeEach(() => { @@ -60,7 +71,7 @@ describe('modules/manager/pip_requirements/artifacts', () => { expect( await updateArtifacts({ packageFileName: 'requirements.txt', - updatedDeps: [{ depName: 'atomicwrites' }], + updatedDeps: [{ depName: 'atomicwrites' }, { depName: 'boto3-stubs' }], newPackageFileContent, config, }) @@ -71,6 +82,10 @@ describe('modules/manager/pip_requirements/artifacts', () => { cmd: 'hashin atomicwrites==1.4.0 -r requirements.txt', options: { cwd: '/tmp/github/some/repo' }, }, + { + cmd: "hashin 'boto3-stubs[iam] == 1.24.36.post1' -r requirements.txt", + options: { cwd: '/tmp/github/some/repo' }, + }, ]); }); @@ -80,7 +95,43 @@ describe('modules/manager/pip_requirements/artifacts', () => { expect( await updateArtifacts({ packageFileName: 'requirements.txt', - updatedDeps: [{ depName: 'atomicwrites' }], + updatedDeps: [{ depName: 'atomicwrites' }, { depName: 'boto3-stubs' }], + newPackageFileContent, + config, + }) + ).toEqual([ + { + file: { + type: 'addition', + path: 'requirements.txt', + contents: 'new content', + }, + }, + ]); + + expect(execSnapshots).toMatchObject([ + { + cmd: 'hashin atomicwrites==1.4.0 -r requirements.txt', + options: { cwd: '/tmp/github/some/repo' }, + }, + { + cmd: "hashin 'boto3-stubs[iam] == 1.24.36.post1' -r requirements.txt", + options: { cwd: '/tmp/github/some/repo' }, + }, + ]); + }); + + it('ignores falsy depNames', async () => { + fs.readLocalFile.mockResolvedValueOnce('new content'); + const execSnapshots = mockExecAll(); + expect( + await updateArtifacts({ + packageFileName: 'requirements.txt', + updatedDeps: [ + { depName: '' }, + { depName: 'atomicwrites' }, + { depName: undefined }, + ], newPackageFileContent, config, }) diff --git a/lib/modules/manager/pip_requirements/artifacts.ts b/lib/modules/manager/pip_requirements/artifacts.ts index 060ca4c2ac1030..f9dcf2734787c8 100644 --- a/lib/modules/manager/pip_requirements/artifacts.ts +++ b/lib/modules/manager/pip_requirements/artifacts.ts @@ -1,11 +1,38 @@ import is from '@sindresorhus/is'; +import { quote } from 'shlex'; import { TEMPORARY_ERROR } from '../../../constants/error-messages'; import { logger } from '../../../logger'; import { exec } from '../../../util/exec'; import type { ExecOptions } from '../../../util/exec/types'; import { ensureCacheDir, readLocalFile } from '../../../util/fs'; -import { newlineRegex, regEx } from '../../../util/regex'; +import { escapeRegExp, regEx } from '../../../util/regex'; import type { UpdateArtifact, UpdateArtifactsResult } from '../types'; +import { extrasPattern } from './extract'; + +/** + * Create a RegExp that matches the first dependency pattern for + * the named dependency that is followed by package hashes. + * + * The regular expression defines a single named group `depConstraint` + * that holds the dependency constraint without the hash specifiers. + * The substring matched by this named group will start with the dependency + * name and end with a non-whitespace character. + * + * @param depName the name of the dependency + */ +function dependencyAndHashPattern(depName: string): RegExp { + const escapedDepName = escapeRegExp(depName); + + // extrasPattern covers any whitespace between the dep name and the optional extras specifier, + // but it does not cover any whitespace in front of the equal signs. + // + // Use a non-greedy wildcard for the range pattern; otherwise, we would + // include all but the last hash specifier into depConstraint. + return regEx( + `^\\s*(?${escapedDepName}${extrasPattern}\\s*==.*?\\S)\\s+--hash=`, + 'm' + ); +} export async function updateArtifacts({ packageFileName, @@ -21,18 +48,18 @@ export async function updateArtifacts({ try { const cmd: string[] = []; const rewrittenContent = newPackageFileContent.replace(regEx(/\\\n/g), ''); - const lines = rewrittenContent - .split(newlineRegex) - .map((line) => line.trim()); for (const dep of updatedDeps) { - const hashLine = lines.find( - (line) => - // TODO: types (#7154) - line.startsWith(`${dep.depName!}==`) && line.includes('--hash=') + if (!dep.depName) { + continue; + } + const depAndHashMatch = dependencyAndHashPattern(dep.depName).exec( + rewrittenContent ); - if (hashLine) { - const depConstraint = hashLine.split(' ')[0]; - cmd.push(`hashin ${depConstraint} -r ${packageFileName}`); + if (depAndHashMatch) { + // If there's a match, then the regular expression guarantees + // that the named subgroup deepConstraint did match as well. + const depConstraint = depAndHashMatch.groups!.depConstraint; + cmd.push(`hashin ${quote(depConstraint)} -r ${quote(packageFileName)}`); } } if (!cmd.length) { diff --git a/lib/modules/manager/pip_requirements/extract.ts b/lib/modules/manager/pip_requirements/extract.ts index 64bb9f01322fa5..f699b375cf4d99 100644 --- a/lib/modules/manager/pip_requirements/extract.ts +++ b/lib/modules/manager/pip_requirements/extract.ts @@ -11,7 +11,7 @@ import type { PackageDependency, PackageFile } from '../types'; export const packagePattern = '[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9._-]*[a-zA-Z0-9]'; -const extrasPattern = '(?:\\s*\\[[^\\]]+\\])?'; +export const extrasPattern = '(?:\\s*\\[[^\\]]+\\])?'; const packageGitRegex = regEx( /(?(?:git\+)(?git|ssh|https):\/\/(?(?:(?[^@]+)@)?(?[\w.-]+)(?\/)(?.*\/(?[\w/-]+))(\.git)?(?:@(?.*))))/ );