diff --git a/schemas/config.json b/schemas/config.json index 9b5cec45b..38694cb90 100644 --- a/schemas/config.json +++ b/schemas/config.json @@ -230,6 +230,13 @@ "type": "string" } }, + "additional-paths": { + "description": "Path of commits, from outside the package, to be included in parsing.", + "type": "array", + "items": { + "type": "string" + } + }, "version-file": { "description": "Path to the specialize version file. Used by `ruby` and `simple` strategies.", "type": "string" @@ -486,6 +493,7 @@ "snapshot-label": true, "initial-version": true, "exclude-paths": true, - "component-no-space": false + "component-no-space": false, + "additional-paths": true } } diff --git a/src/manifest.ts b/src/manifest.ts index 576ad8ce4..1a7b2f59a 100644 --- a/src/manifest.ts +++ b/src/manifest.ts @@ -139,6 +139,7 @@ export interface ReleaserConfig { skipSnapshot?: boolean; // Manifest only excludePaths?: string[]; + additionalPaths?: string[]; } export interface CandidateReleasePullRequest { @@ -186,6 +187,7 @@ interface ReleaserConfigJson { 'skip-snapshot'?: boolean; // Java-only 'initial-version'?: string; 'exclude-paths'?: string[]; // manifest-only + 'additional-paths'?: string[]; // manifest-only 'date-format'?: string; } @@ -672,7 +674,12 @@ export class Manifest { this.logger.info(`Splitting ${commits.length} commits by path`); const cs = new CommitSplit({ includeEmpty: true, - packagePaths: Object.keys(this.repositoryConfig), + packagePaths: Object.fromEntries( + Object.entries(this.repositoryConfig).map(([path, config]) => [ + path, + config.additionalPaths || [], + ]) + ), }); const splitCommits = cs.split(commits); @@ -1406,6 +1413,7 @@ function extractReleaserConfig( skipSnapshot: config['skip-snapshot'], initialVersion: config['initial-version'], excludePaths: config['exclude-paths'], + additionalPaths: config['additional-paths'], dateFormat: config['date-format'], }; } @@ -1766,6 +1774,8 @@ function mergeReleaserConfig( initialVersion: pathConfig.initialVersion ?? defaultConfig.initialVersion, extraLabels: pathConfig.extraLabels ?? defaultConfig.extraLabels, excludePaths: pathConfig.excludePaths ?? defaultConfig.excludePaths, + additionalPaths: + pathConfig.additionalPaths ?? defaultConfig.additionalPaths, dateFormat: pathConfig.dateFormat ?? defaultConfig.dateFormat, }; } diff --git a/src/util/commit-split.ts b/src/util/commit-split.ts index c8a2d5648..8b24f9ea9 100644 --- a/src/util/commit-split.ts +++ b/src/util/commit-split.ts @@ -14,7 +14,7 @@ import {Commit} from '../commit'; import {ROOT_PROJECT_PATH} from '../manifest'; -import {normalizePaths} from './commit-utils'; +import {normalizePath, normalizePaths} from './commit-utils'; export interface CommitSplitOptions { // Include empty git commits: each empty commit is included @@ -39,7 +39,7 @@ export interface CommitSplitOptions { // // NOTE: GitHub API always returns paths using the `/` separator, regardless // of what platform the client code is running on - packagePaths?: string[]; + packagePaths?: Record; } /** @@ -50,19 +50,24 @@ export interface CommitSplitOptions { */ export class CommitSplit { includeEmpty: boolean; - packagePaths?: string[]; + packagePaths?: Record; constructor(opts?: CommitSplitOptions) { opts = opts || {}; this.includeEmpty = !!opts.includeEmpty; if (opts.packagePaths) { - 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 + this.packagePaths = Object.fromEntries( + Object.entries(opts.packagePaths) + .map(([path, additionalPaths]) => [ + normalizePath(path), + normalizePaths(additionalPaths), + ]) + .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 + ); } } @@ -93,22 +98,51 @@ export class CommitSplit { // in this edge-case we should not attempt to update the path. if (splitPath.length === 1) continue; - let pkgName; + // first match the file to a primary package. + // Each file can match at most one primary package. + // Files are sorted by longest path first, so the first + // match will be the most specific. ie if there are two packages: ["core", "core/lib"] + // then the file "core/lib/foo.txt" should be assigned to "core/lib" and not "core". + let primaryPkgName; if (this.packagePaths) { // only track paths under this.packagePaths - pkgName = this.packagePaths.find(p => file.indexOf(`${p}/`) === 0); + primaryPkgName = Object.entries(this.packagePaths).find( + ([p]) => file.indexOf(`${p}/`) === 0 + )?.[0]; } else { // track paths by top level folder - pkgName = splitPath[0]; + primaryPkgName = splitPath[0]; } - if (!pkgName || dedupe.has(pkgName)) continue; - else dedupe.add(pkgName); - if (!splitCommits[pkgName]) splitCommits[pkgName] = []; - splitCommits[pkgName].push(commit); + if (!primaryPkgName || dedupe.has(primaryPkgName)) continue; + else dedupe.add(primaryPkgName); + if (!splitCommits[primaryPkgName]) splitCommits[primaryPkgName] = []; + splitCommits[primaryPkgName].push(commit); } + + // next assign the file to additional packages based on their additional paths. + // This is for cases where someone has specified dependencies outside of the + // package directory. For example, if both packages "foo" and "bar" have additional + // path "shared", then commits to "shared/foo.txt" should be assigned to both packages. + commit.files.forEach(file => { + if (this.packagePaths) { + Object.entries(this.packagePaths).forEach( + ([pkgName, additionalPaths]) => { + if ( + additionalPaths.some(path => file.indexOf(`${path}/`) === 0) + ) { + if (dedupe.has(pkgName)) return; + dedupe.add(pkgName); + if (!splitCommits[pkgName]) splitCommits[pkgName] = []; + splitCommits[pkgName].push(commit); + } + } + ); + } + }); + if (commit.files.length === 0 && this.includeEmpty) { if (this.packagePaths) { - for (const pkgName of this.packagePaths) { + for (const pkgName of Object.keys(this.packagePaths)) { splitCommits[pkgName] = splitCommits[pkgName] || []; splitCommits[pkgName].push(commit); } diff --git a/src/util/commit-utils.ts b/src/util/commit-utils.ts index 9e2842763..15c230ef4 100644 --- a/src/util/commit-utils.ts +++ b/src/util/commit-utils.ts @@ -13,18 +13,20 @@ // 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; - }); + return paths.map(normalizePath); +}; + +export const normalizePath = (path: string) => { + // 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/additional-paths.json b/test/fixtures/manifest/config/additional-paths.json new file mode 100644 index 000000000..7e7d380e8 --- /dev/null +++ b/test/fixtures/manifest/config/additional-paths.json @@ -0,0 +1,8 @@ +{ + "release-type": "simple", + "packages": { + "apps/my-app": { + "additional-paths": ["libs/my-lib"] + } + } +} diff --git a/test/manifest.ts b/test/manifest.ts index 90470b7e6..44bca6834 100644 --- a/test/manifest.ts +++ b/test/manifest.ts @@ -537,6 +537,34 @@ describe('Manifest', () => { 'path-ignore', ]); }); + it('should read additional paths from manifest', async () => { + const getFileContentsStub = sandbox.stub( + github, + 'getFileContentsOnBranch' + ); + getFileContentsStub + .withArgs('release-please-config.json', 'main') + .resolves( + buildGitHubFileContent( + fixturesPath, + 'manifest/config/additional-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['apps/my-app'].additionalPaths + ).to.deep.equal(['libs/my-lib']); + }); it('should build simple plugins from manifest', async () => { const getFileContentsStub = sandbox.stub( github, @@ -3738,6 +3766,59 @@ describe('Manifest', () => { ); }); }); + + it('should update manifest for commits in additionalPaths', async () => { + mockReleases(sandbox, github, []); + mockTags(sandbox, github, [ + { + name: 'apps-myapp-v1.0.0', + sha: 'abc123', + }, + ]); + mockCommits(sandbox, github, [ + { + sha: 'aaaaaa', + message: 'fix: my-lib bugfix', + files: ['libs/my-lib/test.txt'], + }, + { + sha: 'abc123', + message: 'chore: release main', + files: [], + pullRequest: { + headBranchName: 'release-please/branches/main/components/myapp', + baseBranchName: 'main', + number: 123, + title: 'chore: release main', + body: '', + labels: [], + files: [], + sha: 'abc123', + }, + }, + ]); + const manifest = new Manifest( + github, + 'main', + { + 'apps/my-app': { + releaseType: 'simple', + component: 'myapp', + additionalPaths: ['libs/my-lib'], + }, + }, + { + 'apps/my-app': Version.parse('1.0.0'), + } + ); + const pullRequests = await manifest.buildPullRequests(); + expect(pullRequests).lengthOf(1); + const pullRequest = pullRequests[0]; + expect(pullRequest.version?.toString()).to.eql('1.0.1'); + expect(pullRequest.headRefName).to.eql( + 'release-please--branches--main--components--myapp' + ); + }); }); describe('createPullRequests', () => { diff --git a/test/util/commit-split.ts b/test/util/commit-split.ts index a8ea1ccf9..656bcb483 100644 --- a/test/util/commit-split.ts +++ b/test/util/commit-split.ts @@ -46,7 +46,7 @@ describe('CommitSplit', () => { }); it('uses path prefixes', () => { const commitSplit = new CommitSplit({ - packagePaths: ['pkg5', 'pkg6/pkg5'], + packagePaths: {pkg5: [], 'pkg6/pkg5': []}, }); const splitCommits = commitSplit.split(commits); expect(splitCommits['pkg1']).to.be.undefined; @@ -70,7 +70,7 @@ describe('CommitSplit', () => { }, ]; const commitSplit = new CommitSplit({ - packagePaths: ['core', 'core/subpackage'], + packagePaths: {core: [], 'core/subpackage': []}, }); const splitCommits = commitSplit.split(commits); expect(splitCommits['core']).lengthOf(1); @@ -90,7 +90,7 @@ describe('CommitSplit', () => { it('should separate commits with limited list of paths', () => { const commitSplit = new CommitSplit({ includeEmpty: true, - packagePaths: ['pkg1', 'pkg4'], + packagePaths: {pkg1: [], pkg4: []}, }); const splitCommits = commitSplit.split(commits); expect(splitCommits['pkg1']).lengthOf(3); @@ -114,7 +114,7 @@ describe('CommitSplit', () => { it('should separate commits with limited list of paths', () => { const commitSplit = new CommitSplit({ includeEmpty: false, - packagePaths: ['pkg1', 'pkg4'], + packagePaths: {pkg1: [], pkg4: []}, }); const splitCommits = commitSplit.split(commits); expect(splitCommits['pkg1']).lengthOf(2); @@ -123,4 +123,17 @@ describe('CommitSplit', () => { expect(splitCommits['pkg4']).to.be.undefined; }); }); + + describe('handles a commit which belongs to multiple components', () => { + it('should share commits', () => { + const commitSplit = new CommitSplit({ + includeEmpty: false, + // both pkg7 and pkg8 depend on pkg1 + packagePaths: {pkg7: ['pkg1'], pkg8: ['pkg1']}, + }); + const splitCommits = commitSplit.split(commits); + expect(splitCommits['pkg7']).lengthOf(2); + expect(splitCommits['pkg8']).lengthOf(2); + }); + }); });