Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
2cb1b15
Initial target code
iker-barriocanal Jul 5, 2021
208575c
Download artifacts in subdirs of tmp dir
iker-barriocanal Jul 5, 2021
205b795
Add `bundleIdPrefix` to config
iker-barriocanal Jul 5, 2021
d401e2f
Spawn process to call the symbol collector
iker-barriocanal Jul 5, 2021
7eaa941
Add GitHub API release-related helper methods
iker-barriocanal Jul 6, 2021
01c98df
Add system helper method to make files executable
iker-barriocanal Jul 6, 2021
6e780f3
Update target config params
iker-barriocanal Jul 6, 2021
e3f4d6d
Add GitHub client
iker-barriocanal Jul 6, 2021
72488d6
Download and use symbol collector
iker-barriocanal Jul 6, 2021
5f3997b
Parallelize symbol download and place them in a subdir
iker-barriocanal Jul 6, 2021
2414f34
Add debug logs
iker-barriocanal Jul 6, 2021
fbd8fba
Add docs to the README
iker-barriocanal Jul 7, 2021
4bf5add
Update target config requirements
iker-barriocanal Jul 8, 2021
c71ed5a
Set default values for some target config params
iker-barriocanal Jul 8, 2021
5a56941
Rename target from `java-symbols` to `native-symbols`
iker-barriocanal Jul 8, 2021
553309f
Remove old requirement of old env var
iker-barriocanal Jul 8, 2021
f6faf72
Rename target config from `java*` to `native*`
iker-barriocanal Jul 8, 2021
186f00b
Add docstrings
iker-barriocanal Jul 8, 2021
6d44b00
Rename target to `symbol-collector`
iker-barriocanal Jul 9, 2021
280bfcb
Use symbol collector available in the path, instead of downloading it
iker-barriocanal Jul 9, 2021
dd2f49d
Remove unused imports
iker-barriocanal Jul 9, 2021
9125c70
Require the symbol collector to be in the path
iker-barriocanal Jul 9, 2021
70b5a62
Update README
iker-barriocanal Jul 9, 2021
83b3d1b
Install `symbol-collector` in the docker image
iker-barriocanal Jul 9, 2021
78bf8d6
Add tests for target config
iker-barriocanal Jul 9, 2021
4c02ab0
Stop and log when no artifacts were found
iker-barriocanal Jul 9, 2021
02b125f
Log output of calling the symbol collector
iker-barriocanal Jul 9, 2021
831abd0
Add publish test
iker-barriocanal Jul 9, 2021
e7940c1
Merge branch 'master' into iker/feat/symbol-collector
iker-barriocanal Jul 9, 2021
69d379c
Update CHANGELOG
iker-barriocanal Jul 9, 2021
d177cdd
Remove unused methods
iker-barriocanal Jul 9, 2021
751deba
Add `target` entry before maven examples
iker-barriocanal Jul 12, 2021
f6b4789
Address feedback
iker-barriocanal Jul 12, 2021
e0bbb33
Simplify test artifacts to string array
iker-barriocanal Jul 12, 2021
872a7d5
Move artifact filtering option parsing to the artifact provider
iker-barriocanal Jul 12, 2021
06f53f0
Update and add tests for parsing the filter options
iker-barriocanal Jul 12, 2021
58baf81
Address feedback
iker-barriocanal Jul 12, 2021
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Unreleased

- feat(maven): Add maven target to deploy to Maven Central (#258)
- feat(symbol-collector): Add symbol-collector target (#266)

## 0.24.4

Expand Down
14 changes: 11 additions & 3 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,31 @@ RUN apt-get -qq update \
git \
ruby-full \
twine \
jq \
unzip \
&& curl -fsSL https://packages.microsoft.com/config/debian/10/packages-microsoft-prod.deb -o /tmp/packages-microsoft-prod.deb \
&& dpkg -i /tmp/packages-microsoft-prod.deb \
&& rm /tmp/packages-microsoft-prod.deb \
&& echo 'deb [arch=amd64] https://download.docker.com/linux/debian buster stable' >> /etc/apt/sources.list \
&& curl -fsSL https://download.docker.com/linux/debian/gpg | apt-key add - \
&& apt-get update -qq \
&& apt-get install -y --no-install-recommends \
dotnet-sdk-3.1 \
docker-ce-cli \
dotnet-sdk-3.1 \
Comment thread
BYK marked this conversation as resolved.
docker-ce-cli \
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/* \
&& curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- --profile minimal -y \
&& cargo --version \
&& cargo install cargo-hack \
# Stick with 3.1.x as 3.2.x doesn't install on Debian Buster for some reason
&& gem update --no-document --system 3.1.5 \
&& gem install cocoapods
&& gem install cocoapods \
# Install https://github.com/getsentry/symbol-collector
&& symbol_collector_url=$(curl -s https://github.com/ghapi/repos/getsentry/symbol-collector/releases/tags/1.2.1 | \
jq -r '.assets[].browser_download_url | select(endswith("symbolcollector-console-linux-x64.zip"))') \
Comment thread
iker-barriocanal marked this conversation as resolved.
&& curl -sL $symbol_collector_url -o "/tmp/sym-collector.zip" \
&& unzip /tmp/sym-collector.zip -d /usr/local/bin/ \
Comment thread
BYK marked this conversation as resolved.
&& chmod +x /usr/local/bin/SymbolCollector.Console

COPY dist/craft /usr/local/bin/craft
RUN chmod +x /usr/local/bin/craft
Expand Down
24 changes: 24 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ then enforces a specific workflow for managing release branches, changelogs, art
- [AWS Lambda Layer (`aws-lambda-layer`)](#aws-lambda-layer-aws-lambda-layer)
- [Unity Package Manager (`upm`)](#unity-package-manager-upm)
- [Maven central (`maven`)](#maven-central-maven)
- [Symbol Collector (`symbol-collector`)](#symbol-collector-symbol-collector)
- [Integrating Your Project with `craft`](#integrating-your-project-with-craft)
- [Pre-release (Version-bumping) Script: Conventions](#pre-release-version-bumping-script-conventions)
- [Post-release Script: Conventions](#post-release-script-conventions)
Expand Down Expand Up @@ -1033,6 +1034,29 @@ targets:
fileReplacerStr: release.aar
```

### Symbol Collector (`symbol-collector`)

Using the [`symbol-collector`](https://github.com/getsentry/symbol-collector) client, uploads native symbols.
The `symbol-collector` needs to be available in the path.

**Configuration**

| Option | Description |
| ---------------- | -------------------------------------------------------------------------------------------- |
| `serverEndpoint` | **optional** The server endpoint. Defaults to `https://symbol-collector.services.sentry.io`. |
| `batchType` | The batch type of the symbols to be uploaded. I.e: `Android`, `macOS`, `iOS`. |
| `bundleIdPrefix` | The prefix of the bundle ID. The new version will be appended to the end of this prefix. |

**Example**

```yaml
targets:
- name: symbol-collector
includeNames: /libsentry(-android)?\.so/
batchType: Android
bundleIdPrefix: android-ndk-
```

## Integrating Your Project with `craft`

Here is how you can integrate your GitHub project with `craft`:
Expand Down
41 changes: 41 additions & 0 deletions src/artifact_providers/__tests__/base.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { stringToRegexp } from '../../utils/filters';
import { parseFilterOptions, RawFilterOptions } from '../base';

describe('parseFilterOptions', () => {
test('empty object', () => {
const rawFilters: RawFilterOptions = {};
const parsedFilters = parseFilterOptions(rawFilters);
expect(parsedFilters).not.toHaveProperty('includeNames');
expect(parsedFilters).not.toHaveProperty('excludeNames');
});

test('undefined properties', () => {
Comment thread
iker-barriocanal marked this conversation as resolved.
const rawFilters: RawFilterOptions = {
includeNames: undefined,
excludeNames: undefined,
};
const parsedFilters = parseFilterOptions(rawFilters);
expect(parsedFilters).not.toHaveProperty('includeNames');
expect(parsedFilters).not.toHaveProperty('excludeNames');
});

test('string properties', () => {
const stringFilter = '/testFilter/';
const rawFilters: RawFilterOptions = {
includeNames: stringFilter,
};
const parsedFilters = parseFilterOptions(rawFilters);
expect(parsedFilters.includeNames).toStrictEqual(
stringToRegexp(stringFilter)
);
});

test('regex properties', () => {
const regexFilter = stringToRegexp('/testFilter/');
const rawFilters: RawFilterOptions = {
includeNames: regexFilter,
};
const parsedFilters = parseFilterOptions(rawFilters);
expect(parsedFilters.includeNames).toStrictEqual(regexFilter);
});
});
41 changes: 38 additions & 3 deletions src/artifact_providers/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
import { clearObjectProperties } from '../utils/objects';
import { ConfigurationError } from '../utils/errors';
import { logger as loggerRaw } from '../logger';
import { stringToRegexp } from '../utils/filters';

/** Maximum concurrency for downloads */
export const MAX_DOWNLOAD_CONCURRENCY = 5;
Expand Down Expand Up @@ -61,14 +62,48 @@ export interface LocalArtifact extends AbstractArtifact {
}

/**
* Fitlering options for artifacts
* Raw filtering options for artifacts.
* They should be parsed by `parseFilterOptions`.
*/
export interface FilterOptions {
export interface RawFilterOptions {
Comment thread
iker-barriocanal marked this conversation as resolved.
/** Include files that match this pattern */
includeNames?: RegExp | string;
/** Exclude files that match this pattern */
excludeNames?: RegExp | string;
}

/**
* Parsed filtering options for artifacts
*/
export interface ParsedFilterOptions {
Comment thread
BYK marked this conversation as resolved.
/** Include files that match this regexp */
includeNames?: RegExp;
/** Exclude files that match this regexp */
excludeNames?: RegExp;
}

/**
* Returns parsed the given raw filters.
*/
export function parseFilterOptions(
Comment thread
BYK marked this conversation as resolved.
rawFilters: RawFilterOptions
): ParsedFilterOptions {
const parsedFilters: ParsedFilterOptions = {};
if (rawFilters.includeNames) {
parsedFilters.includeNames =
typeof rawFilters.includeNames === 'string'
? stringToRegexp(rawFilters.includeNames)
: rawFilters.includeNames;
}
if (rawFilters.excludeNames) {
parsedFilters.excludeNames =
typeof rawFilters.excludeNames === 'string'
? stringToRegexp(rawFilters.excludeNames)
: rawFilters.excludeNames;
}
return parsedFilters;
}

/**
* Configuration options needed for all artifact providers
*/
Expand Down Expand Up @@ -321,7 +356,7 @@ export abstract class BaseArtifactProvider {
*/
public async filterArtifactsForRevision(
revision: string,
filterOptions?: FilterOptions
filterOptions?: ParsedFilterOptions
): Promise<RemoteArtifact[]> {
let filteredArtifacts = await this.listArtifactsForRevision(revision);
if (!filterOptions || filteredArtifacts.length === 0) {
Expand Down
138 changes: 138 additions & 0 deletions src/targets/__tests__/symbolCollector.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
import { withTempDir } from '../../utils/files';
import { NoneArtifactProvider } from '../../artifact_providers/none';
import { checkExecutableIsPresent, spawnProcess } from '../../utils/system';
import { SymbolCollector, SYM_COLLECTOR_BIN_NAME } from '../symbolCollector';

jest.mock('../../utils/files');
jest.mock('../../utils/system');
jest.mock('fs', () => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may wanna lift the common fs mocks into a global mock module as this pattern seems common enough now.

const original = jest.requireActual('fs');
return {
...original,
promises: {
mkdir: jest.fn(() => {
/** do nothing */
}),
},
};
});

const customConfig = {
batchType: 'batchType',
bundleIdPrefix: 'bundleIdPrefix-',
};

function getSymbolCollectorInstance(
config: Record<string, unknown> = { testKey: 'testVal' }
): SymbolCollector {
return new SymbolCollector(
{
name: 'symbol-collector',
...config,
},
new NoneArtifactProvider()
);
}

describe('target config', () => {
test('symbol collector not present in path', () => {
(checkExecutableIsPresent as jest.MockedFunction<
typeof checkExecutableIsPresent
>).mockImplementationOnce(() => {
throw new Error('Checked for executable');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would have been better to make the executable path configurable and make sure we actually throw on a missing file.

});

expect(getSymbolCollectorInstance).toThrowErrorMatchingInlineSnapshot(
`"Checked for executable"`
);
expect(checkExecutableIsPresent).toHaveBeenCalledTimes(1);
expect(checkExecutableIsPresent).toHaveBeenCalledWith(
SYM_COLLECTOR_BIN_NAME
);
});

test('config missing', () => {
(checkExecutableIsPresent as jest.MockedFunction<
typeof checkExecutableIsPresent
>) = jest.fn();

expect(getSymbolCollectorInstance).toThrowErrorMatchingInlineSnapshot(
'"The required `batchType` parameter is missing in the configuration file. ' +
'See the documentation for more details."'
);
expect(checkExecutableIsPresent).toHaveBeenCalledTimes(1);
expect(checkExecutableIsPresent).toHaveBeenCalledWith(
SYM_COLLECTOR_BIN_NAME
);
Comment thread
iker-barriocanal marked this conversation as resolved.
});

test('symbol collector present and config ok', () => {
(checkExecutableIsPresent as jest.MockedFunction<
typeof checkExecutableIsPresent
>) = jest.fn();

const symCollector = getSymbolCollectorInstance(customConfig);
const actualConfig = symCollector.symbolCollectorConfig;
expect(checkExecutableIsPresent).toHaveBeenCalledTimes(1);
expect(checkExecutableIsPresent).toHaveBeenLastCalledWith(
SYM_COLLECTOR_BIN_NAME
);
expect(actualConfig).toHaveProperty('serverEndpoint');
expect(actualConfig).toHaveProperty('batchType');
expect(actualConfig).toHaveProperty('bundleIdPrefix');
});
});

describe('publish', () => {
test('no artifacts found', () => {
const symCollector = getSymbolCollectorInstance(customConfig);
symCollector.getArtifactsForRevision = jest
.fn()
.mockReturnValueOnce(() => []);
expect(spawnProcess).not.toHaveBeenCalled();
});

test('with artifacts', async () => {
(withTempDir as jest.MockedFunction<typeof withTempDir>).mockImplementation(
async cb => await cb('tmpDir')
);
(spawnProcess as jest.MockedFunction<
typeof spawnProcess
>).mockImplementation(() => Promise.resolve(undefined));

const mockedArtifacts = ['artifact1', 'artifact2', 'artifact3'];

const symCollector = getSymbolCollectorInstance(customConfig);
symCollector.getArtifactsForRevision = jest
.fn()
.mockReturnValueOnce(mockedArtifacts);
symCollector.artifactProvider.downloadArtifact = jest.fn();

await symCollector.publish('version', 'revision');

expect(symCollector.getArtifactsForRevision).toHaveBeenCalledTimes(1);
expect(
symCollector.artifactProvider.downloadArtifact
).toHaveBeenCalledTimes(mockedArtifacts.length);

expect(spawnProcess).toHaveBeenCalledTimes(1);
const [cmd, args] = (spawnProcess as jest.MockedFunction<
typeof spawnProcess
>).mock.calls[0] as string[];
expect(cmd).toBe(SYM_COLLECTOR_BIN_NAME);
expect(args).toMatchInlineSnapshot(`
Comment thread
iker-barriocanal marked this conversation as resolved.
Array [
"--upload",
"directory",
"--path",
"tmpDir",
"--batch-type",
"batchType",
"--bundle-id",
"bundleIdPrefix-version",
"--server-endpoint",
"https://symbol-collector.services.sentry.io/",
]
`);
});
});
12 changes: 8 additions & 4 deletions src/targets/base.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { logger as loggerRaw } from '../logger';
import { GithubGlobalConfig, TargetConfig } from '../schemas/project_config';
import { FilterOptions } from '../artifact_providers/base';
import {
parseFilterOptions,
RawFilterOptions,
ParsedFilterOptions,
} from '../artifact_providers/base';
import { stringToRegexp } from '../utils/filters';
import {
BaseArtifactProvider,
Expand All @@ -18,7 +22,7 @@ export class BaseTarget {
/** Unparsed target configuration */
public readonly config: TargetConfig;
/** Artifact filtering options for the target */
public readonly filterOptions: FilterOptions;
public readonly filterOptions: ParsedFilterOptions;
/** Github repo configuration */
public readonly githubRepo?: GithubGlobalConfig;

Expand Down Expand Up @@ -76,10 +80,10 @@ export class BaseTarget {
*/
public async getArtifactsForRevision(
revision: string,
defaultFilterOptions: FilterOptions = {}
defaultFilterOptions: RawFilterOptions = {}
): Promise<RemoteArtifact[]> {
const filterOptions = {
...defaultFilterOptions,
...parseFilterOptions(defaultFilterOptions),
...this.filterOptions,
};
this.logger.debug(
Expand Down
2 changes: 2 additions & 0 deletions src/targets/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { RegistryTarget } from './registry';
import { AwsLambdaLayerTarget } from './awsLambdaLayer';
import { UpmTarget } from './upm';
import { MavenTarget } from './maven';
import { SymbolCollector } from './symbolCollector';

export const TARGET_MAP: { [key: string]: typeof BaseTarget } = {
brew: BrewTarget,
Expand All @@ -31,6 +32,7 @@ export const TARGET_MAP: { [key: string]: typeof BaseTarget } = {
'aws-lambda-layer': AwsLambdaLayerTarget,
upm: UpmTarget,
maven: MavenTarget,
'symbol-collector': SymbolCollector,
};

/** Targets that are treated specially */
Expand Down
5 changes: 1 addition & 4 deletions src/targets/maven.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,10 +218,7 @@ export class MavenTarget extends BaseTarget {
*/
public async upload(revision: string): Promise<void> {
const artifacts = await this.getArtifactsForRevision(revision, {
includeNames:
this.config.includeNames === undefined
? undefined
: stringToRegexp(this.config.includeNames),
includeNames: this.config.includeNames,
});

// We don't want to do this in parallel but in serial, because the gpg-agent
Expand Down
Loading