Update our Package.swift to build MapboxMaps from source #125
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## main #125 +/- ##
==========================================
+ Coverage 48.93% 48.97% +0.03%
==========================================
Files 101 102 +1
Lines 5683 5687 +4
==========================================
+ Hits 2781 2785 +4
Misses 2902 2902
Flags with carried forward coverage won't be shown. Click here to find out more.
|
macdrevx
changed the title
macdrevx
reviewed
Feb 19, 2021
macdrevx
reviewed
Feb 19, 2021
macdrevx
reviewed
Feb 19, 2021
| let bundle = Bundle(for: type(of: self)) | ||
| var sdkVersionOptional = bundle.infoDictionary?["MGLSemanticVersionString"] | ||
| if sdkVersionOptional == nil { | ||
| sdkVersionOptional = bundle.infoDictionary?["CFBundleShortVersionString"] | ||
| } | ||
| guard let sdkVersion = sdkVersionOptional as? String else { return } | ||
| let sdkVersion = "10.0.0" |
There was a problem hiding this comment.
Create a ticket to find a better solution
jmkiley
commented
Feb 19, 2021
| @@ -1,4 +1,4 @@ | |||
| binary "https://api.mapbox.com/downloads/v2/carthage/mapbox-common/MapboxCommon-ios.json" "10.0.0-beta.11" | |||
| binary "https://api.mapbox.com/downloads/v2/carthage/mobile-maps-core/MapboxCoreMaps-dynamic.json" "10.0.0-beta.15" | |||
| github "mapbox/mapbox-events-ios" "v0.10.7" | |||
| github "mapbox/mapbox-events-ios" "v0.10.8" | |||
| github "mapbox/turf-swift" "v2.0.0-alpha.2" | |||
There was a problem hiding this comment.
Do we want to update Turf here and in the podspec as well?
There was a problem hiding this comment.
No, we still need this version for Carthage so that it gets built in library evolution mode. For CocoaPods, we will switch to 1.2.0 as part of #118.
macdrevx
approved these changes
Feb 19, 2021
macdrevx
force-pushed
the
jk/package.swift
branch
2 times, most recently
from
3c9e79b to
f298012
Compare
Configuration Changes: - Updated Package.swift to build the project from source - Reorganized the source files according to SPM conventions - Updated the paths of the groups in the MapboxMaps Xcode project to point to the new file locations - Marked OrnamentsLocalizable.strings as a localized resource - Updated MBXGeometry.swift to conditionally compile the @unknown default: of a switch statement with an expression of type Geometry from Turf. This is required since when we build the SDK as an XCFramework, Turf is compiled in library evolution mode, so a default case is required, but when we build the SDK as a Swift package, Turf is not built in library evolution mode, so the default case is unnecessary (and the compiler warns if it is present). - .gitignore updates: - Stopped ignoring IDEWorkspaceCHecks.plist - Stopped ignoring .swiftpm - Stopped ignoring Package.resolved - Started ignoring MapboxAccessToken Dependency Version Changes: - Switched to MapboxMobileEvents v0.10.8 (Carthage & SPM only) - Switched to Turf v1.2.0 (SPM only) Bundle & Info.plist Usage Changes: - Added a Build Pre-Action to MapboxMaps's SPM scheme to copy the Mapbox access token from either ~/.mapbox or ~/mapbox into the new MapboxAccessToken file. The integration tests need this token. Previously, they looked for it in the test bundle's Info.plist, but this approach does not work for SPM. IntegrationTestCase.setupAccessToken() has been updated accordingly. - Added Bundle.mapboxMaps and Bundle.mapboxMapsTests abstractions to allow accessing the SDK's resources. These abstractions handle differences that occur when building the SDK as a Swift package vs building it as a framework. In the SDK source, Bundle.main is used when the intent is to access the bundle of the containing application, and Bundle.mapboxMaps is used when the intent is to use the SDK's bundle. - Replaced usages of Bundle.object(forInfoDictionaryKey:) with Bundle.infoDictionary?[] - Hard-coded the SDK version in EventsManager.swift because we will not be able to customize the SDK's Info.plist when the SDK is built as a Swift package. Fixes: - Fixed an issue in MapViewIntegrationTests.setUpWithError() in which super.setUpWithError() was invoked with try? instead of try, preventing it from being able to skip the test. - Reviewed Xcode 12.4's upgrade recommendations - Fixed a canImport in MapboxMapsAnnotationsTests.swift Deletions: - Deleted unused installation tests - Deleted some placeholder SPM-style tests in the Tests directory - Deleted some unused source files and dangling file references
macdrevx
force-pushed
the
jk/package.swift
branch
from
f298012 to
195f58b
Compare
macdrevx
added
the
feature 🍏
macdrevx
added a commit
that referenced
this pull request
Feb 21, 2021
The change broke building the Examples scheme for testing.
2 tasks
1ec5
reviewed
Feb 22, 2021
| dependencies: [ | ||
| .package(name: "MapboxCommon", url: "https://github.com/mapbox/mapbox-common-ios.git", .exact("10.0.0-beta.11")), | ||
| .package(name: "MapboxCoreMaps", url: "https://github.com/mapbox/mapbox-core-maps-ios.git", .exact("10.0.0-beta.15")), | ||
| .package(name: "MapboxMobileEvents", url: "https://github.com/mapbox/mapbox-events-ios.git", .exact("0.10.8")), |
There was a problem hiding this comment.
Does this need to be exact, or can it be from: like the Turf dependency and Cartfile?
macdrevx
added a commit
that referenced
this pull request
Feb 22, 2021
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull request checklist:
mapbox-maps-ioschangelog:<changelog>MapboxMaps can now be built and tested using Swift Package Manager</changelog>.Summary of changes
Configuration Changes
Package.swiftto build the project from sourceOrnamentsLocalizable.stringsas a localized resourceMBXGeometry.swiftto conditionally compile the@unknown default:of a switch statement with an expression of typeGeometryfrom Turf. This is required since when we build the SDK as an XCFramework, Turf is compiled in library evolution mode, so adefaultcase is required, but when we build the SDK as a Swift package, Turf is not built in library evolution mode, so thedefaultcase is unnecessary (and the compiler warns if it is present)..gitignoreupdates:IDEWorkspaceCHecks.plist.swiftpmPackage.resolvedMapboxAccessTokenDependency Version Changes
Bundle & Info.plist Usage Changes
~/.mapboxor~/mapboxinto the newMapboxAccessTokenfile. The integration tests need this token. Previously, they looked for it in the test bundle'sInfo.plist, but this approach does not work for SPM.IntegrationTestCase.setupAccessToken()has been updated accordingly.Bundle.mapboxMapsandBundle.mapboxMapsTestsabstractions to allow accessing the SDK's resources. These abstractions handle differences that occur when building the SDK as a Swift package vs building it as a framework. In the SDK source,Bundle.mainis used when the intent is to access the bundle of the containing application, andBundle.mapboxMapsis used when the intent is to use the SDK's bundle.Bundle.object(forInfoDictionaryKey:)withBundle.infoDictionary?[]EventsManager.swiftbecause we will not be able to customize the SDK'sInfo.plistwhen the SDK is built as a Swift package.Fixes
MapViewIntegrationTests.setUpWithError()in whichsuper.setUpWithError()was invoked withtry?instead oftry, preventing it from being able to skip the test.canImportinMapboxMapsAnnotationsTests.swiftDeletions
Testsdirectory