Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: macos compilation due to cocoa SDK changes #1602

Merged
merged 1 commit into from
Aug 18, 2023
Merged

fix: macos compilation due to cocoa SDK changes #1602

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion .github/workflows/flutter.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ jobs:
steps:
- uses: actions/checkout@v3
# https://github.com/CocoaPods/CocoaPods/issues/5275#issuecomment-315461879
- run: pod lib lint ios/sentry_flutter.podspec --configuration=Debug --skip-import-validation --allow-warnings
- run: pod lib lint ios/sentry_flutter.podspec --configuration=Debug --skip-import-validation --allow-warnings --verbose

swift-lint:
runs-on: ubuntu-latest
Expand Down
13 changes: 1 addition & 12 deletions flutter/example/ios/Podfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
wanted_project_target = '11.0'

# Uncomment this line to define a global platform for your project
platform :ios, wanted_project_target
platform :ios, '11.0'

# CocoaPods analytics sends network stats synchronously affecting flutter build latency.
ENV['COCOAPODS_DISABLE_STATS'] = 'true'
Expand Down Expand Up @@ -37,15 +35,6 @@ target 'Runner' do
end

post_install do |installer|
# remove after https://github.com/flutter/flutter/issues/124340 getting into all channels
installer.generated_projects.each do |project|
project.targets.each do |target|
target.build_configurations.each do |config|
config.build_settings['IPHONEOS_DEPLOYMENT_TARGET'] = wanted_project_target
end
end
end

installer.pods_project.targets.each do |target|
flutter_additional_ios_build_settings(target)
end
Expand Down
5 changes: 5 additions & 0 deletions flutter/ios/Classes/SentryFlutterPluginApple.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,7 @@ public class SentryFlutterPluginApple: NSObject, FlutterPlugin {
}

private func fetchNativeAppStart(result: @escaping FlutterResult) {
#if os(iOS) || os(tvOS)
Copy link
Contributor

Choose a reason for hiding this comment

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

@vaind @brustolin This was available on macOS as well, this change will remove the feature on Flutter-macOS, I'd rather expose appStartMeasurement again for macOS on the Cocoa SDK, it's ok to be a quick fix, but I'd also open issues on the Cocoa SDK to fix this and later remove these checks on the Flutter SDK as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@armcknight do you want to pitch in here since you've made the changes to remove these for macOS in getsentry/sentry-cocoa#3157 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or @brustolin , both work on Cocoa.

Copy link
Member

Choose a reason for hiding this comment

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

Oof, just saw this, sorry for the delay. Really there was no feature functionality removed from Flutter-macOS, since the app start tracker did nothing on macOS: https://github.com/getsentry/sentry-cocoa/pull/3157/files#diff-d13c2b7f6a0357a11c275412ab996c4ce37cd3923403c9d287e36e8e27274470L49

I agree it's a pain that this kind of stuff is breaking stuff downstream. We need a way to test sentry-cocoa integration so we can avoid or be aware of future breakages.

guard let appStartMeasurement = PrivateSentrySDKOnly.appStartMeasurement else {
print("warning: appStartMeasurement is null")
result(nil)
Expand All @@ -481,6 +482,10 @@ public class SentryFlutterPluginApple: NSObject, FlutterPlugin {
]

result(item)
#else
print("note: appStartMeasurement not available on this platform")
result(nil)
#endif
}

private var totalFrames: UInt = 0
Expand Down
2 changes: 1 addition & 1 deletion flutter/ios/sentry_flutter.podspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Pod::Spec.new do |s|
Sentry SDK for Flutter with support to native through sentry-cocoa.
DESC
s.homepage = 'https://sentry.io'
s.license = { :file => '../LICENSE' }
s.license = { :type => 'MIT', :file => '../LICENSE' }
s.authors = "Sentry"
s.source = { :git => "https://github.com/getsentry/sentry-dart.git",
:tag => s.version.to_s }
Expand Down
Loading