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.

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

Update ZIPFoundation to 0.9.16 #1978

Merged
merged 5 commits into from
Mar 4, 2023
Merged

Update ZIPFoundation to 0.9.16 #1978

merged 5 commits into from
Mar 4, 2023

Conversation

calda
Copy link
Member

@calda calda commented Mar 4, 2023

This PR updates our ZIPFoundation dependency to 0.9.16. This fixes #1945, where a dotLottie file would fail to load with a corruptedData error due to an issue in the version of ZIPFoundation that we were previously using.

It's still not really ideal that we integrate ZIPFoundation via source, but it's still the best option. I went ahead and added a README.md to our ZIPFoundation folder that includes a bit of discussion plus instructions for how to update this in the future if necessary.

var generator: String
var version: String?
var author: String?
var generator: String?
Copy link
Member Author

Choose a reason for hiding this comment

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

The animation in #1945 was also failing to load because the manifest didn't provide these properties. Seems fine to make them optional, especially since we don't use these anywhere.

@@ -0,0 +1,24 @@
## ZipFoundation
Copy link
Member Author

Choose a reason for hiding this comment

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

Added some more discussion about our ZIPFoundation dependency, plus instructions for if we have to update this again in the future.


Due to limitations of these package managers, we can't depend on / import
a separate ZIPFoundation module / library. Instead, we include the source
directly within the Lottie library and compile everything as a single unit.
Copy link
Member Author

Choose a reason for hiding this comment

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

🫠

Copy link
Member Author

Choose a reason for hiding this comment

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

At least updating to the most recent version was painless

@calda calda mentioned this pull request Mar 4, 2023
Copy link
Collaborator

@erichoracek erichoracek left a comment

Choose a reason for hiding this comment

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

LGTM! Just a small suggestion


2. Update the URL at the top of this file to indicate what release is being used.

3. Change all of the `public` symbols defined in this module to instead be `internal`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have a script that automates this? might be useful to write as a rake task

Copy link
Member Author

Choose a reason for hiding this comment

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

We'll have to do this pretty infrequently (ideally never but we'll see), so I don't think we need a script for now. If we end up having to do this more regularly then a script would make sense

@calda calda merged commit 70e22dd into master Mar 4, 2023
@calda calda deleted the cal--update-ZIPFoundation branch March 4, 2023 00:48
@RayFor24
Copy link

Does version 4.1.3 not yet include this update?

@reggian
Copy link
Contributor

reggian commented May 3, 2023

I am receiving a build warning when installing via cocoapods:

no rule to process file '.../Pods/lottie-ios/Sources/Private/Model/DotLottie/ZipFoundation/README.md' of type 'net.daringfireball.markdown' for architecture 'arm64'

Any thoughts?

@calda
Copy link
Member Author

calda commented May 3, 2023

Could you file an issue? Seems worth fixing

iago849 pushed a commit to atteamapps/lottie-ios that referenced this pull request Feb 8, 2024
MoroziOS pushed a commit to MoroziOS/tmg-lottie-ios that referenced this pull request May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dotLottie load fail
4 participants