From a9bed8232ccaa192fb004953e34c49279d3c779f Mon Sep 17 00:00:00 2001 From: Alex K Date: Mon, 10 Apr 2023 22:23:54 +0200 Subject: [PATCH] feat: allow excluding file paths to filter out commits (#1875) * feat: add exclude for manifest packages * chore: adjust ReleaserConfigJson interface * docs: update documentation * feat: read exclude-paths from configuration * chore: add license to new files * fix: include only based on relevant files * use array index, Array.prototype.at is not available in node 14 which we still support --------- Co-authored-by: Jeff Ching --- docs/manifest-releaser.md | 2 + schemas/config.json | 10 +- src/manifest.ts | 10 +- src/util/commit-exclude.ts | 63 ++++++++ src/util/commit-split.ts | 32 ++-- src/util/commit-utils.ts | 30 ++++ .../manifest/config/exclude-paths.json | 15 ++ test/manifest.ts | 31 ++++ test/util/commit-exclude.ts | 137 ++++++++++++++++++ 9 files changed, 305 insertions(+), 25 deletions(-) create mode 100644 src/util/commit-exclude.ts create mode 100644 src/util/commit-utils.ts create mode 100644 test/fixtures/manifest/config/exclude-paths.json create mode 100644 test/util/commit-exclude.ts diff --git a/docs/manifest-releaser.md b/docs/manifest-releaser.md index 07700523b..faab86777 100644 --- a/docs/manifest-releaser.md +++ b/docs/manifest-releaser.md @@ -262,6 +262,8 @@ defaults (those are documented in comments) ".": { // overrides release-type for node "release-type": "node", + // exclude commits from that path from processing + "exclude-paths": ["path/to/myPyPkgA"] }, // path segment should be relative to repository root diff --git a/schemas/config.json b/schemas/config.json index 66a95e746..b92c20e34 100644 --- a/schemas/config.json +++ b/schemas/config.json @@ -184,6 +184,13 @@ ] } }, + "exclude-paths": { + "description": "Path of commits to be excluded from parsing. If all files from commit belong to one of the paths it will be skipped", + "type": "array", + "items": { + "type": "string" + } + }, "version-file": { "description": "Path to the specialize version file. Used by `ruby` and `simple` strategies.", "type": "string" @@ -394,6 +401,7 @@ "extra-files": true, "version-file": true, "snapshot-label": true, - "initial-version": true + "initial-version": true, + "exclude-paths": true } } diff --git a/src/manifest.ts b/src/manifest.ts index 46a2b7eaa..eaee3672e 100644 --- a/src/manifest.ts +++ b/src/manifest.ts @@ -46,6 +46,7 @@ import { FilePullRequestOverflowHandler, } from './util/pull-request-overflow-handler'; import {signoffCommitMessage} from './util/signoff-commit-message'; +import {CommitExclude} from './util/commit-exclude'; type ExtraJsonFile = { type: 'json'; @@ -125,6 +126,8 @@ export interface ReleaserConfig { extraFiles?: ExtraFile[]; snapshotLabels?: string[]; skipSnapshot?: boolean; + // Manifest only + excludePaths?: string[]; } export interface CandidateReleasePullRequest { @@ -167,6 +170,7 @@ interface ReleaserConfigJson { 'snapshot-label'?: string; // Java-only 'skip-snapshot'?: boolean; // Java-only 'initial-version'?: string; + 'exclude-paths'?: string[]; // manifest-only } export interface ManifestOptions { @@ -637,13 +641,15 @@ export class Manifest { const splitCommits = cs.split(commits); // limit paths to ones since the last release - const commitsPerPath: Record = {}; + let commitsPerPath: Record = {}; for (const path in this.repositoryConfig) { commitsPerPath[path] = commitsAfterSha( path === ROOT_PROJECT_PATH ? commits : splitCommits[path], releaseShasByPath[path] ); } + const commitExclude = new CommitExclude(this.repositoryConfig); + commitsPerPath = commitExclude.excludeCommits(commitsPerPath); // backfill latest release tags from manifest for (const path in this.repositoryConfig) { @@ -1282,6 +1288,7 @@ function extractReleaserConfig( extraLabels: config['extra-label']?.split(','), skipSnapshot: config['skip-snapshot'], initialVersion: config['initial-version'], + excludePaths: config['exclude-paths'], }; } @@ -1616,6 +1623,7 @@ function mergeReleaserConfig( skipSnapshot: pathConfig.skipSnapshot ?? defaultConfig.skipSnapshot, initialVersion: pathConfig.initialVersion ?? defaultConfig.initialVersion, extraLabels: pathConfig.extraLabels ?? defaultConfig.extraLabels, + excludePaths: pathConfig.excludePaths ?? defaultConfig.excludePaths, }; } diff --git a/src/util/commit-exclude.ts b/src/util/commit-exclude.ts new file mode 100644 index 000000000..60127950e --- /dev/null +++ b/src/util/commit-exclude.ts @@ -0,0 +1,63 @@ +// Copyright 2023 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import {Commit} from '../commit'; +import {ReleaserConfig, ROOT_PROJECT_PATH} from '../manifest'; +import {normalizePaths} from './commit-utils'; + +export type CommitExcludeConfig = Pick; + +export class CommitExclude { + private excludePaths: Record = {}; + + constructor(config: Record) { + Object.entries(config).forEach(([path, releaseConfig]) => { + if (releaseConfig.excludePaths) { + this.excludePaths[path] = normalizePaths(releaseConfig.excludePaths); + } + }); + } + + excludeCommits( + commitsPerPath: Record + ): Record { + const filteredCommitsPerPath: Record = {}; + Object.entries(commitsPerPath).forEach(([path, commits]) => { + if (this.excludePaths[path]) { + commits = commits.filter(commit => + this.shouldInclude(commit, this.excludePaths[path], path) + ); + } + filteredCommitsPerPath[path] = commits; + }); + return filteredCommitsPerPath; + } + + private shouldInclude( + commit: Commit, + excludePaths: string[], + packagePath: string + ): boolean { + return ( + !commit.files || + !commit.files + .filter(file => this.isRelevant(file, packagePath)) + .every(file => excludePaths.some(path => this.isRelevant(file, path))) + ); + } + + private isRelevant(file: string, path: string) { + return path === ROOT_PROJECT_PATH || file.indexOf(`${path}/`) === 0; + } +} diff --git a/src/util/commit-split.ts b/src/util/commit-split.ts index 8565ce0c5..c8a2d5648 100644 --- a/src/util/commit-split.ts +++ b/src/util/commit-split.ts @@ -14,6 +14,7 @@ import {Commit} from '../commit'; import {ROOT_PROJECT_PATH} from '../manifest'; +import {normalizePaths} from './commit-utils'; export interface CommitSplitOptions { // Include empty git commits: each empty commit is included @@ -54,29 +55,14 @@ export class CommitSplit { opts = opts || {}; this.includeEmpty = !!opts.includeEmpty; if (opts.packagePaths) { - const paths: string[] = []; - for (let newPath of opts.packagePaths) { - // The special "." path, representing the root of the module, should be - // ignored by commit-split as it is assigned all commits in manifest.ts - if (newPath === ROOT_PROJECT_PATH) { - continue; - } - // normalize so that all paths have leading and trailing slashes for - // non-overlap validation. - // NOTE: GitHub API always returns paths using the `/` separator, - // regardless of what platform the client code is running on - newPath = newPath.replace(/\/$/, ''); - newPath = newPath.replace(/^\//, ''); - newPath = newPath.replace(/$/, '/'); - newPath = newPath.replace(/^/, '/'); - // store them with leading and trailing slashes removed. - newPath = newPath.replace(/\/$/, ''); - newPath = newPath.replace(/^\//, ''); - paths.push(newPath); - } - - // sort by longest paths first - this.packagePaths = paths.sort((a, b) => b.length - a.length); + const paths: string[] = normalizePaths(opts.packagePaths); + this.packagePaths = paths + .filter(path => { + // The special "." path, representing the root of the module, should be + // ignored by commit-split as it is assigned all commits in manifest.ts + return path !== ROOT_PROJECT_PATH; + }) + .sort((a, b) => b.length - a.length); // sort by longest paths first } } diff --git a/src/util/commit-utils.ts b/src/util/commit-utils.ts new file mode 100644 index 000000000..9e2842763 --- /dev/null +++ b/src/util/commit-utils.ts @@ -0,0 +1,30 @@ +// Copyright 2023 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +export const normalizePaths = (paths: string[]) => { + return paths.map(path => { + // normalize so that all paths have leading and trailing slashes for + // non-overlap validation. + // NOTE: GitHub API always returns paths using the `/` separator, + // regardless of what platform the client code is running on + let newPath = path.replace(/\/$/, ''); + newPath = newPath.replace(/^\//, ''); + newPath = newPath.replace(/$/, '/'); + newPath = newPath.replace(/^/, '/'); + // store them with leading and trailing slashes removed. + newPath = newPath.replace(/\/$/, ''); + newPath = newPath.replace(/^\//, ''); + return newPath; + }); +}; diff --git a/test/fixtures/manifest/config/exclude-paths.json b/test/fixtures/manifest/config/exclude-paths.json new file mode 100644 index 000000000..0fbdfbc9c --- /dev/null +++ b/test/fixtures/manifest/config/exclude-paths.json @@ -0,0 +1,15 @@ +{ + "release-type": "simple", + "label": "custom: pending", + "release-label": "custom: tagged", + "exclude-paths": ["path-ignore"], + "packages": { + ".": { + "component": "root", + "exclude-paths": ["path-root-ignore"] + }, + "node-lib": { + "component": "node-lib" + } + } +} diff --git a/test/manifest.ts b/test/manifest.ts index e4dfdacf9..d1ecc6ebb 100644 --- a/test/manifest.ts +++ b/test/manifest.ts @@ -504,6 +504,37 @@ describe('Manifest', () => { 'lang: nodejs', ]); }); + it('should read exclude paths from manifest', async () => { + const getFileContentsStub = sandbox.stub( + github, + 'getFileContentsOnBranch' + ); + getFileContentsStub + .withArgs('release-please-config.json', 'main') + .resolves( + buildGitHubFileContent( + fixturesPath, + 'manifest/config/exclude-paths.json' + ) + ) + .withArgs('.release-please-manifest.json', 'main') + .resolves( + buildGitHubFileContent( + fixturesPath, + 'manifest/versions/versions.json' + ) + ); + const manifest = await Manifest.fromManifest( + github, + github.repository.defaultBranch + ); + expect(manifest.repositoryConfig['.'].excludePaths).to.deep.equal([ + 'path-root-ignore', + ]); + expect(manifest.repositoryConfig['node-lib'].excludePaths).to.deep.equal([ + 'path-ignore', + ]); + }); it('should build simple plugins from manifest', async () => { const getFileContentsStub = sandbox.stub( github, diff --git a/test/util/commit-exclude.ts b/test/util/commit-exclude.ts new file mode 100644 index 000000000..5f3f647ba --- /dev/null +++ b/test/util/commit-exclude.ts @@ -0,0 +1,137 @@ +// Copyright 2023 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import {Commit} from '../../src'; +import { + CommitExclude, + CommitExcludeConfig, +} from '../../src/util/commit-exclude'; +import {expect} from 'chai'; + +describe('commit-exclude', () => { + const commitsPerPath: Record = { + '.': [ + { + sha: 'pack1Pack2', + message: 'commit pack1Pack2', + files: ['pkg1/foo.txt', 'pkg2/bar.txt'], + }, + { + sha: 'rootCommit', + message: 'commit root', + files: ['foo.txt'], + }, + { + sha: 'pack3', + message: 'commit pack3', + files: ['pkg3/bar/foo.txt'], + }, + ], + pkg1: [ + { + sha: 'pack1Pack2', + message: 'commit pack1Pack2', + files: ['pkg1/foo.txt', 'pkg2/bar.txt'], + }, + ], + pkg2: [ + { + sha: 'pack1Pack2', + message: 'commit pack1Pack2', + files: ['pkg1/foo.txt', 'pkg2/bar.txt'], + }, + ], + pkg3: [ + { + sha: 'pack3', + message: 'commit pack3', + files: ['pkg3/foo.txt'], + }, + { + sha: 'pack3sub', + message: 'commit pack3sub', + files: ['pkg3/bar/foo.txt'], + }, + ], + pkg4: [ + { + sha: 'pack3', + message: 'commit pack3', + files: ['pkg3/foo.txt'], + }, + { + sha: 'pack3sub', + message: 'commit pack3sub', + files: ['pkg3/bar/foo.txt'], + }, + ], + }; + + it('should not exclude anything if paths are empty', () => { + const config: Record = {}; + const commitExclude = new CommitExclude(config); + const newCommitsPerPath = commitExclude.excludeCommits(commitsPerPath); + expect(newCommitsPerPath['.'].length).to.equal(3); + expect(newCommitsPerPath['pkg1'].length).to.equal(1); + expect(newCommitsPerPath['pkg2'].length).to.equal(1); + expect(newCommitsPerPath['pkg3'].length).to.equal(2); + }); + + it('should not exclude only if all files are from excluded path', () => { + const config: Record = { + '.': {excludePaths: ['pkg3', 'pkg1']}, + pkg3: {excludePaths: ['pkg3/bar']}, + }; + const commitExclude = new CommitExclude(config); + const newCommitsPerPath = commitExclude.excludeCommits(commitsPerPath); + expect(newCommitsPerPath['.'].length).to.equal(2); + expect(newCommitsPerPath['pkg1'].length).to.equal(1); + expect(newCommitsPerPath['pkg2'].length).to.equal(1); + expect(newCommitsPerPath['pkg3'].length).to.equal(1); + }); + + it('should exclude if all files are from excluded path', () => { + const config: Record = { + '.': {excludePaths: ['pkg3', 'pkg1', 'pkg2']}, + }; + const commitExclude = new CommitExclude(config); + const newCommitsPerPath = commitExclude.excludeCommits(commitsPerPath); + expect(newCommitsPerPath['.'].length).to.equal(1); + expect(newCommitsPerPath['pkg1'].length).to.equal(1); + expect(newCommitsPerPath['pkg2'].length).to.equal(1); + expect(newCommitsPerPath['pkg3'].length).to.equal(2); + }); + + it('should make decision only on relevant files', () => { + const createCommit = (files: string[]) => { + const first = files[0]; + return { + sha: first.split('/')[0], + message: `commit ${first}`, + files, + }; + }; + const commits: Record = { + a: [createCommit(['a/b/c', 'd/e/f', 'd/e/g'])], + d: [createCommit(['a/b/c', 'd/e/f', 'd/e/g'])], + }; + const config: Record = { + d: {excludePaths: ['d/e']}, + }; + const commitExclude = new CommitExclude(config); + const newCommitsPerPath = commitExclude.excludeCommits(commits); + expect(newCommitsPerPath['a'].length).to.equal(1); + expect(newCommitsPerPath['d'].length).to.equal(0); + }); +});