Bring back xcframework based Carthage support for MapboxCoreNavigation #3031
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
S2Ler
added
build
S2Ler
added a commit
that referenced
this pull request
May 28, 2021
The motivation for migrating to new snapshot testing library is described here: #2791 (comment). Main consequence is that we test only on one device. The env for current snapshots are: iPhone 12 Pro Max, iOS 14.5. Xcode based project isn't updated yet because the work depends on #3031.
1ec5
approved these changes
Jun 2, 2021
S2Ler
force-pushed
the
s2ler/fix-carthage
branch
3 times, most recently
from
748cead to
a15918e
Compare
S2Ler
commented
Jun 3, 2021
| @@ -4,12 +4,11 @@ step-library: | |||
| - &restore-cache | |||
| restore_cache: | |||
| keys: | |||
| - nav-cache-v5-{{ .Environment.CIRCLE_JOB }}-{{ checksum "Cartfile.resolved" }} | |||
| - nav-cache-v5-{{ .Environment.CIRCLE_JOB }} # used if checksum fails | |||
There was a problem hiding this comment.
This line broke the cache for me. See https://app.circleci.com/pipelines/github/mapbox/mapbox-navigation-ios/3758/workflows/d308c9bd-af8a-4a23-8298-cbd9c24345d8/jobs/34125 run.
@1ec5 Can you please clarify what it does and if I can remove it?
There was a problem hiding this comment.
This step restores the Carthage folder (with all the checkouts and build products) from a cache from the previous build. It’s based on a checksum of the resolved file, so that any time a dependency version changes, there’s a cache miss. It’s also possible to force a cache miss by incrementing the v5 to a different version.
There was a problem hiding this comment.
As far as I see the cache with a key - nav-cache-v5-{{ .Environment.CIRCLE_JOB }} # used if checksum fails isn't saved anywhere.
|
Wiki PR: git clone --branch s2ler/carthage-migration https://github.com/mapbox/mapbox-navigation-ios.wiki.git
git diff 611aa2596fbb62b5b107bbe37b6b72ef7e403059^! |
S2Ler
commented
Jun 3, 2021
S2Ler
commented
Jun 3, 2021
1ec5
approved these changes
Jun 3, 2021
It is partially replaced with #3030.
This scheme depends on mapbox/mapbox-maps-ios, which doesn't support Carthage yet. Tracking issue: mapbox/mapbox-maps-ios#66. So, to keep Carthage happy for MapboxCoreNavigation, which works with Carthage, we must remove other unbuildable framework schemes.
- We don't need a workaround anymore. - Update Carthage to 0.38.0 with xcframework support.
This will allow you to reset caches by changing CARTHAGE_CACHE_VERSION environment variable.
- Remove the mention of the v1 from documentations. Motivation: the requirements and installation steps for v1 and v2 becomes so different that it hards to maintain. We should have separate docs for v1 anyway. - Update minimum Carthage version to 0.38 - Remove duplicates information from `custom-navigation.md` - Update CHANGELOG.md
The motivation for keeping this script is have backward compatibility. The developers who uses this script for workaround won't need to change anything to use new distribution.
Example doesn't depend on Carthage
S2Ler
added a commit
that referenced
this pull request
Jun 4, 2021
The motivation for migrating to new snapshot testing library is described here: #2791 (comment). Main consequence is that we test only on one device. The env for current snapshots are: iPhone 12 Pro Max, iOS 14.5. Xcode based project isn't updated yet because the work depends on #3031.
S2Ler
added a commit
that referenced
this pull request
Jun 11, 2021
The motivation for migrating to new snapshot testing library is described here: #2791 (comment). The env for current snapshots are: - iPhone 12 Pro Max, iOS 14.4 - iPhone 11 Pro Max, iOS 13.7 Xcode based project isn't updated yet because the work depends on #3031. Fix regression in test due to global dependency on applied Theme in tests. Some tests change the global theme. To return it to the expected state, we apply the needed theme in our snapshot tests. Temporary fix to InstructionLabel clear cache functionality Check #3043 (comment) for more info.
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.
Description
Our Carthage support is broken currently. This PR fixes it for MapboxCoreNavigation framework.
We can't add support for MapboxNavigation at the moment, because it depends on https://github.com/mapbox/mapbox-maps-ios, which doesn't support Carthage distribution at the moment. Tracking issue: mapbox/mapbox-maps-ios#66.
Implementation