Skip to content

Commit

Permalink
Strip json comments (#486)
Browse files Browse the repository at this point in the history
  • Loading branch information
sorenlouv authored Dec 13, 2023
1 parent 923f34e commit a04eb0a
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 36 deletions.
25 changes: 24 additions & 1 deletion src/lib/git.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,30 @@ describe('createBackportBranch', () => {
backportBranch,
}),
).rejects.toThrowErrorMatchingInlineSnapshot(
`"The branch "4.x" is invalid or doesn't exist"`,
`"The remote "'https://github.com/elastic/kibana.git'" is invalid or doesn't exist"`,
);
});

it('should throw "is not a commit" error', async () => {
expect.assertions(1);
const err = new childProcess.SpawnError({
code: 128,
cmdArgs: [],
stdout: '',
stderr:
"fatal: 'origin/remote-branch-name' is not a commit and a branch 'local-branch-name' cannot be created from it",
});

jest.spyOn(childProcess, 'spawnPromise').mockRejectedValueOnce(err);
await expect(
createBackportBranch({
options,
sourceBranch,
targetBranch,
backportBranch,
}),
).rejects.toThrowErrorMatchingInlineSnapshot(
`"The branch "origin/remote-branch-name'" is invalid or doesn't exist"`,
);
});

Expand Down
45 changes: 29 additions & 16 deletions src/lib/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -504,9 +504,9 @@ export async function createBackportBranch({
}) {
const spinner = ora(options.interactive, 'Pulling latest changes').start();

try {
const cwd = getRepoPath(options);
const cwd = getRepoPath(options);

try {
await spawnPromise('git', ['reset', '--hard'], cwd);
await spawnPromise('git', ['clean', '-d', '--force'], cwd);

Expand Down Expand Up @@ -534,35 +534,48 @@ export async function createBackportBranch({
try {
await spawnPromise('git', ['branch', '-D', tmpBranchName], cwd);
} catch (e) {
//
// swallow error
}

// fetch commits for source branch
await fetchBranch(options, sourceBranch);

spinner.succeed();
} catch (e) {
spinner.fail();

if (e instanceof SpawnError) {
const isBranchInvalid =
e.context.stderr.toLowerCase().includes(`couldn't find remote ref`) ||
e.context.stderr.toLowerCase().includes(`invalid refspec`) ||
e.context.stderr
.toLowerCase()
.includes(
`is not a commit and a branch '${backportBranch}' cannot be created from it`,
);

if (isBranchInvalid) {
const invalidRemoteRef = e.context.stderr
.toLowerCase()
.match(/couldn't find remote ref (.*)/)?.[1];

const invalidCommit = e.context.stderr
.toLowerCase()
.match(
/'(.+) is not a commit and a branch .+ cannot be created from it/,
)?.[1];

const invalidBranch = invalidRemoteRef || invalidCommit;

if (invalidBranch) {
throw new BackportError(
`The branch "${targetBranch}" is invalid or doesn't exist`,
`The branch "${invalidBranch}" is invalid or doesn't exist`,
);
}

const invalidRefSpec = e.context.stderr
.toLowerCase()
.match(/invalid refspec (.*)/)?.[1];

if (invalidRefSpec) {
throw new BackportError(
`The remote "${invalidRefSpec}" is invalid or doesn't exist`,
);
}
}

throw e;
}

spinner.succeed();
}

export async function deleteBackportBranch({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
} from '../../../git';
import { logger } from '../../../logger';
import {
parseRemoteConfig,
parseRemoteConfigFile,
swallowMissingConfigFileException,
} from '../../../remoteConfig';
import {
Expand Down Expand Up @@ -145,7 +145,7 @@ async function getRemoteConfigFileOptions(
}

logger.info('Remote config: Parsing.');
return parseRemoteConfig(remoteConfig);
return parseRemoteConfigFile(remoteConfig);
}

function throwIfInsufficientPermissions(
Expand Down
9 changes: 3 additions & 6 deletions src/lib/remoteConfig.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import gql from 'graphql-tag';
import { ConfigFileOptions } from '../entrypoint.api';
import { withConfigMigrations } from '../options/config/readConfigFile';
import { parseConfigFile } from '../options/config/readConfigFile';
import { GithubV4Exception } from './github/v4/apiRequestV4';
import { logger } from './logger';

Expand Down Expand Up @@ -40,11 +39,9 @@ export interface RemoteConfigHistory {
};
}

export function parseRemoteConfig(remoteConfig: RemoteConfig) {
export function parseRemoteConfigFile(remoteConfig: RemoteConfig) {
try {
return withConfigMigrations(
JSON.parse(remoteConfig.file.object.text),
) as ConfigFileOptions;
return parseConfigFile(remoteConfig.file.object.text);
} catch (e) {
logger.info('Parsing remote config failed', e);
return;
Expand Down
4 changes: 2 additions & 2 deletions src/lib/sourceCommit/parseSourceCommit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import gql from 'graphql-tag';
import { differenceBy } from 'lodash';
import { ValidConfigOptions } from '../../options/options';
import {
parseRemoteConfig,
RemoteConfigHistory,
RemoteConfigHistoryFragment,
parseRemoteConfigFile,
} from '../remoteConfig';
import {
TargetPullRequest,
Expand Down Expand Up @@ -281,6 +281,6 @@ function getSourceCommitBranchLabelMapping(
sourcePullRequest?.mergeCommit.remoteConfigHistory.edges?.[0]?.remoteConfig;

if (remoteConfig) {
return parseRemoteConfig(remoteConfig)?.branchLabelMapping;
return parseRemoteConfigFile(remoteConfig)?.branchLabelMapping;
}
}
15 changes: 6 additions & 9 deletions src/options/config/readConfigFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,21 @@ export async function readConfigFile(
filepath: string,
): Promise<ConfigFileOptions> {
const fileContents = await readFile(filepath, 'utf8');
const configWithoutComments = stripJsonComments(fileContents);

try {
return withConfigMigrations(JSON.parse(configWithoutComments));
return parseConfigFile(fileContents);
} catch (e) {
throw new BackportError(
`"${filepath}" contains invalid JSON:\n\n${fileContents}`,
);
}
}
// ensure backwards compatability when config options are renamed
export function withConfigMigrations({
upstream,
labels,
branches,
addOriginalReviewers,
...config
}: ConfigFileOptions) {
export function parseConfigFile(fileContents: string): ConfigFileOptions {
const configWithoutComments = stripJsonComments(fileContents);
const { upstream, labels, branches, addOriginalReviewers, ...config } =
JSON.parse(configWithoutComments);

const { repoName, repoOwner } = parseUpstream(upstream, config);

return excludeUndefined({
Expand Down

0 comments on commit a04eb0a

Please sign in to comment.