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

feat: add additional-paths property to manifest releaser #2336

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion __snapshots__/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ Options:
the first major release
[boolean] [default: false]
--prerelease-type type of the prerelease, e.g., alpha [string]
--extra-files extra files for the strategy to consider
--extra-files extra files for the strategy to update
[string]
--version-file path to version file to update, e.g.,
version.rb [string]
Expand Down
4 changes: 2 additions & 2 deletions docs/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ Extra options:
| `--pull-request-title-pattern` | `string` | Override the pull request title pattern. Defaults to `chore${scope}: release${component} ${version}` |
| `--pull-request-header` | `string` | Override the pull request header. Defaults to `:robot: I have created a release *beep* *boop*` |
| `--pull-request-footer` | `string` | Override the pull request footer. Defaults to `This PR was generated with Release Please. See documentation.` |
| `--extra-files` | `string[]` | Extra file paths for the release strategy to consider |
| `--extra-files` | `string[]` | Extra file paths for the release strategy to update |
| `--version-file` | `string` | Ruby only. Path to the `version.rb` file |

## Creating/updating release PRs
Expand Down Expand Up @@ -115,7 +115,7 @@ need to specify your release options:
| `--pull-request-header` | `string` | Override the pull request header. Defaults to `:robot: I have created a release *beep* *boop*` |
| `--pull-request-footer` | `string` | Override the pull request footer. Defaults to `This PR was generated with Release Please. See documentation.` |
| `--signoff` | string | Add [`Signed-off-by`](https://git-scm.com/docs/git-commit#Documentation/git-commit.txt---signoff) line at the end of the commit log message using the user and email provided. (format "Name \<email@example.com\>") |
| `--extra-files` | `string[]` | Extra file paths for the release strategy to consider |
| `--extra-files` | `string[]` | Extra file paths for the release strategy to update |
| `--version-file` | `string` | Ruby only. Path to the `version.rb` file |
| `--skip-labeling` | `boolean` | If set, labels will not be applied to pull requests |
| `--include-v-in-tags` | `boolean` | Include "v" in tag versions. Defaults to `true`. |
Expand Down
2 changes: 2 additions & 0 deletions docs/manifest-releaser.md
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,8 @@ defaults (those are documented in comments)
"release-type": "node",
// exclude commits from that path from processing
"exclude-paths": ["path/to/myPyPkgA"]
// include commits from that path in processing
"additional-paths": ["path/to/externalPkgB"]
},

// path segment should be relative to repository root
Expand Down
10 changes: 9 additions & 1 deletion schemas/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,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"
Expand Down Expand Up @@ -443,6 +450,7 @@
"version-file": true,
"snapshot-label": true,
"initial-version": true,
"exclude-paths": true
"exclude-paths": true,
"additional-paths": true
}
}
2 changes: 1 addition & 1 deletion src/bin/release-please.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ function pullRequestStrategyOptions(yargs: yargs.Argv): yargs.Argv {
type: 'string',
})
.option('extra-files', {
describe: 'extra files for the strategy to consider',
describe: 'extra files for the strategy to update',
type: 'string',
coerce(arg?: string) {
if (arg) {
Expand Down
12 changes: 11 additions & 1 deletion src/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ export interface ReleaserConfig {
skipSnapshot?: boolean;
// Manifest only
excludePaths?: string[];
additionalPaths?: string[];
}

export interface CandidateReleasePullRequest {
Expand Down Expand Up @@ -183,6 +184,7 @@ interface ReleaserConfigJson {
'skip-snapshot'?: boolean; // Java-only
'initial-version'?: string;
'exclude-paths'?: string[]; // manifest-only
'additional-paths'?: string[]; // manifest-only
}

export interface ManifestOptions {
Expand Down Expand Up @@ -658,7 +660,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);

Expand Down Expand Up @@ -1372,6 +1379,7 @@ function extractReleaserConfig(
skipSnapshot: config['skip-snapshot'],
initialVersion: config['initial-version'],
excludePaths: config['exclude-paths'],
additionalPaths: config['additional-paths'],
signoff: config['signoff'],
};
}
Expand Down Expand Up @@ -1727,6 +1735,8 @@ function mergeReleaserConfig(
initialVersion: pathConfig.initialVersion ?? defaultConfig.initialVersion,
extraLabels: pathConfig.extraLabels ?? defaultConfig.extraLabels,
excludePaths: pathConfig.excludePaths ?? defaultConfig.excludePaths,
additionalPaths:
pathConfig.additionalPaths ?? defaultConfig.additionalPaths,
};
}

Expand Down
37 changes: 23 additions & 14 deletions src/util/commit-split.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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<string, string[]>;
}

/**
Expand All @@ -50,19 +50,24 @@ export interface CommitSplitOptions {
*/
export class CommitSplit {
includeEmpty: boolean;
packagePaths?: string[];
packagePaths?: Record<string, string[]>;
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
);
}
}

Expand Down Expand Up @@ -93,10 +98,14 @@ export class CommitSplit {
// in this edge-case we should not attempt to update the path.
if (splitPath.length === 1) continue;

let pkgName;
let pkgName: string | undefined;
if (this.packagePaths) {
// only track paths under this.packagePaths
pkgName = this.packagePaths.find(p => file.indexOf(`${p}/`) === 0);
pkgName = Object.entries(this.packagePaths).find(
([path, additionalPaths]) =>
file.indexOf(`${path}/`) === 0 ||
additionalPaths.some(path => file.indexOf(`${path}/`) === 0)
)?.[0];
} else {
// track paths by top level folder
pkgName = splitPath[0];
Expand All @@ -108,7 +117,7 @@ export class CommitSplit {
}
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);
}
Expand Down
30 changes: 16 additions & 14 deletions src/util/commit-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
8 changes: 8 additions & 0 deletions test/fixtures/manifest/config/additional-paths.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"release-type": "simple",
"packages": {
"apps/my-app": {
"additional-paths": ["libs/my-lib"]
}
}
}
81 changes: 81 additions & 0 deletions test/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,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,
Expand Down Expand Up @@ -3519,6 +3547,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', () => {
Expand Down
8 changes: 4 additions & 4 deletions test/util/commit-split.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down