getsentry / sentry-cocoa

The official Sentry SDK for iOS, tvOS, macOS, watchOS.
https://sentry.io/for/cocoa/
MIT License
807 stars 319 forks source link

Provide pre-built binaries for SPM (compiling current package takes >20 seconds) #1369

Closed arielelkin closed 8 months ago

arielelkin commented 3 years ago

The current Swift Package involves building the entire Sentry SDK from source, which takes 10-30 seconds:

image

Could we have a swift package that just provides the the Sentry binary .frameworks. Implementation is really straightforward: https://developer.apple.com/documentation/swift_packages/distributing_binary_frameworks_as_swift_packages

A broader discussion is whether Sentry's default swift package should contain source code or the pre-built framework. I presume most people using Sentry are just integrating it as a third-party framework (rather than developing it) and faster compilation times are more important to them than having access to the source code. Have you considered providing two separate Swift packages; one for the source and one for the pre-built binary?

philipphofmann commented 3 years ago

Thanks, @arielelkin, for pointing that out. I agree that this should be a relatively easy task. We are happy to accept PRs if you are motivated :)

arielelkin commented 3 years ago

I could take a quick look...

For this to work, the zip file you make available in Releases must have the xcframework in its root directory (currently it's in Carthage/Build/Sentry.xcframework).

Could you please fix that?

philipphofmann commented 3 years ago

Thanks, for the info. I guess it's better if I do it then because getting all the paths right without breaking something might be a bit hard. Thanks for your help.

arielelkin commented 1 year ago

@philipphofmann hello! I'd like to follow up on this.

I believe this is important. Building Sentry from source as is currently the case makes sense if most developers were interested in inspecting or debugging the Firebase code, but I suspect that's a tiny fraction of us. Feels like the rest of us waste a lot of computing time by constantly building from source instead of using precompiled binaries.

What's the value in building Sentry from source instead of using precompiled binaries?

I'd be happy to send a PR addressing this. As I mentioned, the zip file you make available in Releases must have the xcframework in its root directory (currently it's in Carthage/Build/Sentry.xcframework).

Could you please fix that?

philipphofmann commented 1 year ago

@arielelkin, sorry we didn't get to this yet. If you could open a PR, it would be awesome 😃 . I think it should be an easy fix.

arielelkin commented 1 year ago

@philipphofmann The binary asset's url specified in Package.swift needs to point to a release asset and a tag, but that creates a chicken and egg problem.

One solution is the following: https://forums.swift.org/t/spm-support-basic-auth-for-non-git-binary-dependency-hosts/37878/103

Point the Package.swift to the draft release assets Push the updated Package.swift + tag Update the draft release to use the new tag Move the release from draft to published

Alternatively, we could have a separate repo hosting the release assets.. Easier to implement, but has some redundancy.

What are your thoughts?

philipphofmann commented 1 year ago

Sorry, @arielelkin will get back to you tomorrow.

philipphofmann commented 1 year ago

Alternatively, we could have a separate repo hosting the release assets. Easier to implement, but has some redundancy.

I would like to avoid a repo for hosting, as we need to set it up, maintain, etc.

@arielelkin, we know the binary URL before creating the release. See https://github.com/getsentry/sentry-cocoa/releases/tag/8.3.1 has https://github.com/getsentry/sentry-cocoa/releases/download/8.3.1/Sentry.xcframework.zip.

Another question: can't we use the binarytarget(name:path:)?

arielelkin commented 1 year ago

we know the binary URL before creating the release. See https://github.com/getsentry/sentry-cocoa/releases/tag/8.3.1 has https://github.com/getsentry/sentry-cocoa/releases/download/8.3.1/Sentry.xcframework.zip.

Unfortunately that doesn't help; the issue is knowing the binary URL before creating the tag, not the release:

Another question: can't we use the binarytarget(name:path:)?

For that to work, the .xcframework must be checked into the repo.

philipphofmann commented 1 year ago

We know the URL beforehand, as it follows the pattern https://github.com/getsentry/sentry-cocoa/releases/download/sdk-version/Sentry.xcframework.zip , but the problem is the checksum. It doesn't matter if we host our own repo for the framework.zip or if we use the artifacts of GH releases. We need to make a commit with the checksum of the framework.zip after GH actions created it.

Currently, our release process pulls the artifact from the commit created by crafts preReleaseCommand, such as 8.3.2. After a Sentry manager approves the release, such as 8.3.2, the publish GH action workflow would need to pull the artifact and create another commit to update the checksum of the SPM .binaryTarget. As far as I know, this is not possible now with our process of managing release. Craft would need a similar hook as the pre-release-command, which runs during craft publish. The hook would need to run before publishing the targets.

We understand that there is a valid use case for this improvement, but it's not a low-hanging fruit, and therefore, we can't give you an ETA for it.

tcwalther commented 1 year ago

Another option is to create a second repository for this, like a mirror. I've found https://github.com/malcommac/sentry-cocoa-sdk-xcframeworks/ that does exactly that, but trusting a binary framework from an unofficial source is always a bit "meh" (plus, rn their workflow script is a bit broken and doesn't actually store the framework in the root folder - I created an issue for this, but it shows it'd be better to have an official channel).

philipphofmann commented 1 year ago

Thanks for the update, @tcwalther, but I prefer using the same repository. For everyone, please upvote in the first comment in this issue https://github.com/getsentry/sentry-cocoa/issues/1369#issue-1022475796 to let us know how many users would love to see this being implemented.

scogeo commented 1 year ago

I'm chiming in for a +1 to have a separate repository to host the Package.swift containing binaryTargets and keep the current repository as a build from source(s) package. This gives the most flexibility for building the library from source if needed and gives an option to access the officially sanctioned pre-built library at a different URL. For example, if there was a new repo named sentry-cocoa-xcframework, then one could just change the URL in the package reference.

// Use for source version
.package(url: "https://github.com/getsentry/sentry-cocoa", from: "8.3.2"),
// Or, use the xcframework version
.package(url: "https://github.com/getsentry/sentry-cocoa-xcframework", from: "8.3.2"),

The repo that hosts the xcframework doesn't have to actually host or build the xcframework, it could just point at URLs back in the main sentry-coca repository releases. Then just add a workflow in GitHub that updates the xcframework repo whenever a release is published in the main repo.

It would just need a Package.swift file similar to the following:

let package = Package(
    name: "Sentry",
    platforms: [.iOS(.v11), .macOS(.v10_13), .tvOS(.v11), .watchOS(.v4)],
    products: [
        .library(name: "Sentry", targets: ["Sentry"]),
    ],
    dependencies: [],
    targets: [
        .binaryTarget(name: "Sentry",
                      url: "https://github.com/getsentry/sentry-cocoa/releases/download/8.3.2/Sentry.xcframework.zip",
                      checksum: "e25814a875efe595b24e0a5fddd519b899aab129ab8e257673be90ae85fb4bc8")
    ]
)

Each release just needs to update the URL and the checksum. Then just update the Carthage build so that the .zip file has the xcframework at the root and it will be compatible with both SPM and Carthage.

arielelkin commented 1 year ago

In the meantime, here's how to easily host your sentry xcframework binaries:

  1. create a new folder
  2. download the latest sentry xcframework from https://github.com/getsentry/sentry-cocoa/releases, unzip and place it at the root of the directory
  3. create the following Package.swift file
  4. commit and push
  5. in your xcode project's swift package section, replace the sentry package with this
  6. reduce your build time substantially
// swift-tools-version:5.5
import PackageDescription

let package = Package(
    name: "Sentry-binaries",
    platforms: [.iOS(.v11), .macOS(.v10_13), .tvOS(.v11), .watchOS(.v4)],
    products: [
        .library(
            name: "Sentry-binaries",
            targets: ["Sentry"]
        )
    ],
    targets: [
        .binaryTarget(
            name: "Sentry",
            path: "Sentry.xcframework"
        )
    ]
)
lionel-alves commented 9 months ago

Hi, any update on this? Thanks!

philipphofmann commented 9 months ago

@lionel-alves, no update, sorry.

brustolin commented 8 months ago

We added pre-built binaries in this PR and will be available with the next release.