Skip to content

Commit 68f1121

Browse files
committed
Fix repo links in updated changelogs
When this tool populates the Unreleased section of the changelog, it does not just rewrite that section, but rather, it rewrites the entire changelog. That behavior comes from the `updateChangelog` function in `@metamask/auto-changelog`. This function takes the changes to apply, but it also takes a repository URL, which it will use to construct links for all of the releases in the changelog. Unfortunately, the repository URL that this tool passes to `updateChangelog` is not always the same URL that `auto-changelog` uses. Specifically, if the developer used a different URL to clone the repo than is listed in `package.json` under `repository.url`, then when creating a new release branch, all of the links in the changelog will be modified to reflect this new URL. This is incorrect, the resulting changelog will not pass validation when run through `auto-changelog validate`, forcing the developer to revert the changes to the links in order to merge the release PR. This commit addresses this pain point by updating the logic used to obtain the repository URL to better match what is in `@metamask/auto-changelog`. This means that the aforementioned links in the changelog should stay the same when creating a new release branch.
1 parent ee05a76 commit 68f1121

12 files changed

+276
-168
lines changed

src/functional.test.ts

+3-14
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,9 @@ describe('create-release-branch (functional)', () => {
7777
},
7878
});
7979

80-
expect(await environment.readJsonFile('package.json')).toStrictEqual({
80+
expect(await environment.readJsonFile('package.json')).toMatchObject({
8181
name: '@scope/monorepo',
8282
version: '2.0.0',
83-
private: true,
84-
workspaces: ['packages/*'],
85-
scripts: { foo: 'bar' },
86-
packageManager: '[email protected]',
8783
});
8884
expect(
8985
await environment.readJsonFileWithinPackage('a', 'package.json'),
@@ -190,13 +186,9 @@ describe('create-release-branch (functional)', () => {
190186
},
191187
});
192188

193-
expect(await environment.readJsonFile('package.json')).toStrictEqual({
189+
expect(await environment.readJsonFile('package.json')).toMatchObject({
194190
name: '@scope/monorepo',
195191
version: '1.1.0',
196-
private: true,
197-
workspaces: ['packages/*'],
198-
scripts: { foo: 'bar' },
199-
packageManager: '[email protected]',
200192
});
201193
expect(
202194
await environment.readJsonFileWithinPackage('a', 'package.json'),
@@ -945,12 +937,9 @@ __metadata:
945937
},
946938
});
947939

948-
expect(await environment.readJsonFile('package.json')).toStrictEqual({
940+
expect(await environment.readJsonFile('package.json')).toMatchObject({
949941
name: '@scope/monorepo',
950942
version: '2.0.0',
951-
private: true,
952-
workspaces: ['packages/*'],
953-
packageManager: '[email protected]',
954943
});
955944
expect(
956945
await environment.readJsonFileWithinPackage('a', 'package.json'),

src/misc-utils.test.ts

+46
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
runCommand,
1010
getStdoutFromCommand,
1111
getLinesFromCommand,
12+
convertToHttpsGitHubRepositoryUrl,
1213
} from './misc-utils.js';
1314

1415
jest.mock('which');
@@ -211,4 +212,49 @@ describe('misc-utils', () => {
211212
expect(lines).toStrictEqual([' line 1', 'line 2', ' line 3 ']);
212213
});
213214
});
215+
216+
describe('convertToHttpsRepositoryUrl', () => {
217+
it('returns the URL of the "origin" remote of the given repo if it looks like a HTTPS public GitHub repo URL', () => {
218+
expect(
219+
convertToHttpsGitHubRepositoryUrl(
220+
'https://github.com/example-org/example-repo',
221+
),
222+
).toBe('https://github.com/example-org/example-repo');
223+
});
224+
225+
it('lops ".git" off from the HTTPS public GitHub repo URL', () => {
226+
expect(
227+
convertToHttpsGitHubRepositoryUrl(
228+
'https://github.com/example-org/example-repo.git',
229+
),
230+
).toBe('https://github.com/example-org/example-repo');
231+
});
232+
233+
it('converts an SSH GitHub repo URL into an HTTPS URL (without the trailing ".git")', () => {
234+
expect(
235+
convertToHttpsGitHubRepositoryUrl(
236+
'[email protected]:example-org/example-repo.git',
237+
),
238+
).toBe('https://github.com/example-org/example-repo');
239+
});
240+
241+
it.each([
242+
'foo',
243+
'http://github.com/example-org',
244+
'https://github.com/example-org',
245+
'http://github.com/example-org/example-repo',
246+
'https://github.comzzzz/example-org/example-repo',
247+
'https://gitbar.foo/example-org/example-repo',
248+
'[email protected]:example-org',
249+
'[email protected]:example-org/example-repo.git',
250+
'[email protected]:example-org/example-repo.foo',
251+
])(
252+
'throws if the URL is in an invalid format such as "%s"',
253+
(repositoryUrl) => {
254+
expect(() => convertToHttpsGitHubRepositoryUrl(repositoryUrl)).toThrow(
255+
`Unrecognized repository URL: ${repositoryUrl}`,
256+
);
257+
},
258+
);
259+
});
214260
});

src/misc-utils.ts

+39
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,20 @@ export { isObject };
1515
*/
1616
export const debug = createDebug('create-release-branch:impl');
1717

18+
/**
19+
* Matches URLs in the formats:
20+
*
21+
* - "https://github.com/OrganizationName/RepoName"
22+
* - "https://github.com/OrganizationName/RepoName.git"
23+
*/
24+
const HTTPS_GITHUB_URL_REGEX =
25+
/^https:\/\/github\.com\/(.+?)\/(.+?)(?:\.git)?$/u;
26+
27+
/**
28+
* Matches a URL in the format "[email protected]/OrganizationName/RepoName.git".
29+
*/
30+
const SSH_GITHUB_URL_REGEX = /^git@github\.com:(.+?)\/(.+?)\.git$/u;
31+
1832
/**
1933
* Type guard for determining whether the given value is an instance of Error.
2034
* For errors generated via `fs.promises`, `error instanceof Error` won't work,
@@ -172,3 +186,28 @@ export async function getLinesFromCommand(
172186
const { stdout } = await execa(command, args, options);
173187
return stdout.split('\n').filter((value) => value !== '');
174188
}
189+
190+
/**
191+
* Converts the given GitHub repository URL to its HTTPS version.
192+
*
193+
* A "GitHub repository URL" looks like one of:
194+
*
195+
* - https://github.com/OrganizationName/RepositoryName
196+
* - [email protected]:OrganizationName/RepositoryName.git
197+
*
198+
* If the URL does not match either of these patterns, an error is thrown.
199+
*
200+
* @param url - The URL to convert.
201+
* @returns The HTTPS URL of the repository, e.g.
202+
* `https://github.com/OrganizationName/RepositoryName`.
203+
*/
204+
export function convertToHttpsGitHubRepositoryUrl(url: string): string {
205+
const match =
206+
url.match(HTTPS_GITHUB_URL_REGEX) ?? url.match(SSH_GITHUB_URL_REGEX);
207+
208+
if (match) {
209+
return `https://github.com/${match[1]}/${match[2]}`;
210+
}
211+
212+
throw new Error(`Unrecognized repository URL: ${url}`);
213+
}

src/project.test.ts

+73-4
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@ import path from 'path';
33
import { when } from 'jest-when';
44
import { SemVer } from 'semver';
55
import * as actionUtils from '@metamask/action-utils';
6-
import { withSandbox } from '../tests/helpers.js';
6+
import { withProtectedProcessEnv, withSandbox } from '../tests/helpers.js';
77
import {
88
buildMockPackage,
99
buildMockProject,
1010
createNoopWriteStream,
1111
} from '../tests/unit/helpers.js';
12+
import * as miscUtils from './misc-utils.js';
1213
import {
14+
getValidRepositoryUrl,
1315
readProject,
1416
restoreChangelogsForSkippedPackages,
1517
updateChangelogsForChangedPackages,
@@ -42,6 +44,11 @@ describe('project', () => {
4244
validatedManifest: {
4345
workspaces: ['packages/a', 'packages/subpackages/*'],
4446
},
47+
unvalidatedManifest: {
48+
repository: {
49+
url: projectRepositoryUrl,
50+
},
51+
},
4552
},
4653
);
4754
const workspacePackages = {
@@ -59,9 +66,6 @@ describe('project', () => {
5966
};
6067
const projectTagNames = ['tag1', 'tag2', 'tag3'];
6168
const stderr = createNoopWriteStream();
62-
when(jest.spyOn(repoModule, 'getRepositoryHttpsUrl'))
63-
.calledWith(projectDirectoryPath)
64-
.mockResolvedValue(projectRepositoryUrl);
6569
when(jest.spyOn(repoModule, 'getTagNames'))
6670
.calledWith(projectDirectoryPath)
6771
.mockResolvedValue(projectTagNames);
@@ -126,6 +130,71 @@ describe('project', () => {
126130
});
127131
});
128132
});
133+
134+
describe('getValidRepositoryUrl', () => {
135+
describe('if the `npm_package_repository_url` environment variable is set', () => {
136+
it('returns the HTTPS version of this URL', async () => {
137+
await withProtectedProcessEnv(async () => {
138+
process.env.npm_package_repository_url =
139+
'[email protected]:example-org/example-repo.git';
140+
const packageManifest = {};
141+
const repositoryDirectoryPath = '/path/to/project';
142+
143+
expect(
144+
await getValidRepositoryUrl(
145+
packageManifest,
146+
repositoryDirectoryPath,
147+
),
148+
).toBe('https://github.com/example-org/example-repo');
149+
});
150+
});
151+
});
152+
153+
describe('if package.json has a string "repository" field', () => {
154+
it('returns the HTTPS version of this URL', async () => {
155+
const packageManifest = {
156+
repository: '[email protected]:example-org/example-repo.git',
157+
};
158+
const repositoryDirectoryPath = '/path/to/project';
159+
160+
expect(
161+
await getValidRepositoryUrl(packageManifest, repositoryDirectoryPath),
162+
).toBe('https://github.com/example-org/example-repo');
163+
});
164+
});
165+
166+
describe('if package.json has an object "repository" field with a string "url" field', () => {
167+
it('returns the HTTPS version of this URL', async () => {
168+
const packageManifest = {
169+
repository: {
170+
url: '[email protected]:example-org/example-repo.git',
171+
},
172+
};
173+
const repositoryDirectoryPath = '/path/to/project';
174+
175+
expect(
176+
await getValidRepositoryUrl(packageManifest, repositoryDirectoryPath),
177+
).toBe('https://github.com/example-org/example-repo');
178+
});
179+
});
180+
181+
describe('if package.json does not have a "repository" field', () => {
182+
it('returns the HTTPS version of this URL', async () => {
183+
const packageManifest = {};
184+
const repositoryDirectoryPath = '/path/to/project';
185+
when(jest.spyOn(miscUtils, 'getStdoutFromCommand'))
186+
.calledWith('git', ['config', '--get', 'remote.origin.url'], {
187+
cwd: repositoryDirectoryPath,
188+
})
189+
.mockResolvedValue('[email protected]:example-org/example-repo.git');
190+
191+
expect(
192+
await getValidRepositoryUrl(packageManifest, repositoryDirectoryPath),
193+
).toBe('https://github.com/example-org/example-repo');
194+
});
195+
});
196+
});
197+
129198
describe('restoreChangelogsForSkippedPackages', () => {
130199
it('should reset changelog for packages with changes not included in release', async () => {
131200
const project = buildMockProject({

src/project.ts

+70-3
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,25 @@
11
import { WriteStream } from 'fs';
22
import { resolve } from 'path';
33
import { getWorkspaceLocations } from '@metamask/action-utils';
4+
import { isPlainObject } from '@metamask/utils';
45
import { WriteStreamLike, fileExists } from './fs.js';
56
import {
67
Package,
78
readMonorepoRootPackage,
89
readMonorepoWorkspacePackage,
910
updatePackageChangelog,
1011
} from './package.js';
11-
import { getRepositoryHttpsUrl, getTagNames, restoreFiles } from './repo.js';
12+
import { getTagNames, restoreFiles } from './repo.js';
1213
import { SemVer } from './semver.js';
13-
import { PackageManifestFieldNames } from './package-manifest.js';
14+
import {
15+
PackageManifestFieldNames,
16+
UnvalidatedPackageManifest,
17+
} from './package-manifest.js';
1418
import { ReleaseSpecification } from './release-specification.js';
19+
import {
20+
convertToHttpsGitHubRepositoryUrl,
21+
getStdoutFromCommand,
22+
} from './misc-utils.js';
1523

1624
/**
1725
* The release version of the root package of a monorepo extracted from its
@@ -83,13 +91,16 @@ export async function readProject(
8391
projectDirectoryPath: string,
8492
{ stderr }: { stderr: WriteStreamLike },
8593
): Promise<Project> {
86-
const repositoryUrl = await getRepositoryHttpsUrl(projectDirectoryPath);
8794
const tagNames = await getTagNames(projectDirectoryPath);
8895
const rootPackage = await readMonorepoRootPackage({
8996
packageDirectoryPath: projectDirectoryPath,
9097
projectDirectoryPath,
9198
projectTagNames: tagNames,
9299
});
100+
const repositoryUrl = await getValidRepositoryUrl(
101+
rootPackage.unvalidatedManifest,
102+
projectDirectoryPath,
103+
);
93104
const releaseVersion = examineReleaseVersion(
94105
rootPackage.validatedManifest.version,
95106
);
@@ -132,6 +143,62 @@ export async function readProject(
132143
};
133144
}
134145

146+
/**
147+
* Returns the https-prefixed GitHub repository URL for the project, so that we
148+
* know how to construct links to releases inside of the changelog.
149+
*
150+
* This URL is obtained by looking at the following sources, in this order:
151+
*
152+
* - `process.env.npm_package_repository_url` (to support MetaMask projects
153+
* which still use NPM or Yarn 1.x)
154+
* - The `repository` field inside of `package.json` (to support most MetaMask
155+
* projects)
156+
* - The URL of the "origin" remote (to support newer projects which for some
157+
* reason do not have a "repository" field)
158+
*
159+
* Note that there is a function in `@metamask/auto-changelog` that offers a
160+
* similar capability, and in fact, this function should match it as closely as
161+
* possible so that when changelogs are updated they also pass
162+
* `@metamask/auto-changelog`'s validation step.
163+
*
164+
* @param packageManifest - The manifest for the package that represents a
165+
* project.
166+
* @param repositoryDirectoryPath - The path to the repository for the project.
167+
* @returns The HTTPS URL of the repository, e.g.
168+
* `https://github.com/OrganizationName/RepositoryName`.
169+
* @throws If the URL obtained via `package.json` or the "origin" remote is in
170+
* in invalid format.
171+
*/
172+
export async function getValidRepositoryUrl(
173+
packageManifest: UnvalidatedPackageManifest,
174+
repositoryDirectoryPath: string,
175+
): Promise<string> {
176+
// Set automatically by NPM or Yarn 1.x
177+
const npmPackageRepositoryUrl = process.env.npm_package_repository_url;
178+
179+
if (npmPackageRepositoryUrl) {
180+
return convertToHttpsGitHubRepositoryUrl(npmPackageRepositoryUrl);
181+
}
182+
183+
if (typeof packageManifest.repository === 'string') {
184+
return convertToHttpsGitHubRepositoryUrl(packageManifest.repository);
185+
}
186+
187+
if (
188+
isPlainObject(packageManifest.repository) &&
189+
typeof packageManifest.repository.url === 'string'
190+
) {
191+
return convertToHttpsGitHubRepositoryUrl(packageManifest.repository.url);
192+
}
193+
194+
const gitConfigUrl = await getStdoutFromCommand(
195+
'git',
196+
['config', '--get', 'remote.origin.url'],
197+
{ cwd: repositoryDirectoryPath },
198+
);
199+
return convertToHttpsGitHubRepositoryUrl(gitConfigUrl);
200+
}
201+
135202
/**
136203
* Updates the changelog files of all packages that have changes since latest release to include those changes.
137204
*

0 commit comments

Comments
 (0)