sentry-cocoa icon indicating copy to clipboard operation
sentry-cocoa copied to clipboard

chore: Move XCFrameworks to root after building

Open philipphofmann opened this issue 1 year ago • 3 comments

Running make build-xcframework-sample relies on having the Sentry.xcframework.zip file in the repository root folder see https://github.com/getsentry/sentry-cocoa/blob/6bc31df2a007b41dcdac3c456dc9a97d6a2c49bb/scripts/create-carthage-json.sh#L4

In CI, we archive the XCFramework zips here https://github.com/getsentry/sentry-cocoa/blob/6bc31df2a007b41dcdac3c456dc9a97d6a2c49bb/.github/workflows/build.yml#L132

and then downloading the artifacts puts them in the repository root folder before running make build-xcframework-sample

https://github.com/getsentry/sentry-cocoa/blob/6bc31df2a007b41dcdac3c456dc9a97d6a2c49bb/.github/workflows/build.yml#L145-L153

Running make build-xcframework locally doesn't put the XCFrameworks zip files into the repository root folder, which breaks running make build-xcframework-sample locally. This PR fixes this problem.

#skip-changelog

philipphofmann avatar Mar 19 '24 11:03 philipphofmann

IMO, it would be better to update the sample to point to the right place.

The problem is that GH actions download the artifact into the root folder of the repository. I could also then move the downloaded artifacts to Carthage/ on GH actions, but that also feels kind of weird.

philipphofmann avatar Mar 19 '24 13:03 philipphofmann

@brustolin, please have a look at https://github.com/getsentry/sentry-cocoa/pull/3766#issuecomment-2007125832. I either want to merge or close this PR.

philipphofmann avatar Apr 02 '24 11:04 philipphofmann

@brustolin, please have a look at #3766 (comment). I either want to merge or close this PR.

I dont mind, we can change it, but you still need to address the 3 points in this comment

brustolin avatar Apr 03 '24 08:04 brustolin

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • [x] Sources/Sentry/SentrySwizzle.m

github-actions[bot] avatar May 06 '24 09:05 github-actions[bot]

Performance metrics :rocket:

  Plain With Sentry Diff
Startup time 1210.87 ms 1234.88 ms 24.01 ms
Size 21.58 KiB 616.72 KiB 595.14 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
408f43e509276bf959d3780c88aa4a4e95368675 1229.31 ms 1247.52 ms 18.21 ms
33b2f5140a89fa9ba5de0dd0d3a23a1c5a2645ce 1251.24 ms 1259.18 ms 7.94 ms
c0f08e718b7ca8828429d95d382d548b9f300de7 1230.67 ms 1246.31 ms 15.63 ms
6604dbbf68e19f201a6368d709536ceacfabb6d4 1248.35 ms 1256.14 ms 7.79 ms
6943de0faec429fe6e29ae8c9cdb31770ad05e94 1235.98 ms 1246.88 ms 10.90 ms
8aec30ebd6b60262f8b935b5475741c92da38834 1249.29 ms 1252.63 ms 3.35 ms
d3abae0e1782d1d9e495befcfa1326f25ca64604 1200.36 ms 1224.22 ms 23.87 ms
e5dcbd5e2d4fff30521db1ad3c569ee287f5b7a2 1223.47 ms 1243.90 ms 20.43 ms
f1b1deca8ba27285f15b61b6c88865a0c84058d7 1234.27 ms 1249.10 ms 14.84 ms
18d491a20edc61eae9b34201fea7c88c77dd558d 1226.86 ms 1251.35 ms 24.49 ms

App size

Revision Plain With Sentry Diff
408f43e509276bf959d3780c88aa4a4e95368675 21.58 KiB 573.17 KiB 551.59 KiB
33b2f5140a89fa9ba5de0dd0d3a23a1c5a2645ce 20.76 KiB 432.17 KiB 411.41 KiB
c0f08e718b7ca8828429d95d382d548b9f300de7 21.58 KiB 573.84 KiB 552.26 KiB
6604dbbf68e19f201a6368d709536ceacfabb6d4 22.84 KiB 402.56 KiB 379.72 KiB
6943de0faec429fe6e29ae8c9cdb31770ad05e94 20.76 KiB 393.33 KiB 372.57 KiB
8aec30ebd6b60262f8b935b5475741c92da38834 21.58 KiB 616.76 KiB 595.18 KiB
d3abae0e1782d1d9e495befcfa1326f25ca64604 20.76 KiB 434.92 KiB 414.16 KiB
e5dcbd5e2d4fff30521db1ad3c569ee287f5b7a2 22.85 KiB 414.09 KiB 391.24 KiB
f1b1deca8ba27285f15b61b6c88865a0c84058d7 21.58 KiB 616.14 KiB 594.56 KiB
18d491a20edc61eae9b34201fea7c88c77dd558d 21.58 KiB 544.86 KiB 523.28 KiB

github-actions[bot] avatar May 06 '24 10:05 github-actions[bot]