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

Fix podspec versioning and tests #599

Merged
merged 2 commits into from
Mar 5, 2020
Merged

Conversation

ilammy
Copy link
Collaborator

@ilammy ilammy commented Mar 2, 2020

Remove the hack that we have hastily added to work around a packaging issue with GRKOpenSSL 1.0.2.19 (see #538, #539). We still pin the version to 1.0.2.18 though, because the library has never been updated and 1.0.2.19 is still broken as it was. However, we can't release the next version of Themis pinned to 0.12.1. Use the tagged version. This also lets the podspec to validate without warnings (from our part, at least).

Also remove the Podfile.lock from our unit test project. It is meant to be used with the latest version from master as evident from the Podfile. However, presence of Podfile.lock makes "pod install" to install an older version of Themis 0.11 which is lacks some new features that are being tested. (Bitrise is fine because it removes the lockfile manually.)

Remove the lock file to ensure that we are testing the trunk version of Themis, and add Podfile.lock to .gitignore to prevent it from being committed again on accident.

Checklist

  • Change is covered by automated tests (somewhat, Bitrise does not validate podspec)
  • The coding guidelines are followed
  • Public API has proper documentation
  • Example projects and code samples are up-to-date

Remove the hack that we have hastily added to work around a packaging
issue with GRKOpenSSL 1.0.2.19. We still pin the version to 1.0.2.18
though, because the library has never been updated and 1.0.2.19 is still
broken as it was. However, we can't release the next version of Themis
pinned to 0.12.1. Use the tagged version. This also lets the podspec
to validate without warnings (from our part, at least).

Also remove the Podfile.lock from our unit test project. It is meant to
be used with the latest version from master as evident from the Podfile.
However, presence of Podfile.lock makes "pod install" to install an
older version of Themis 0.11 which is lacks some new features that are
being tested. (Bitrise is fine because it removes the lockfile manually.)

Remove the lock file to ensure that we are testing the trunk version
of Themis, and add Podfile.lock to .gitignore to prevent it from being
committed again on accident.
@ilammy ilammy added O-iOS 📱 Operating system: iOS O-macOS 💻 Operating system: macOS tests Themis test suite M-CocoaPods Package manager: CocoaPods, Objective-C and Swift, iOS and macOS labels Mar 2, 2020
@ilammy ilammy mentioned this pull request Mar 2, 2020
9 tasks
Copy link
Contributor

@vixentael vixentael left a comment

Choose a reason for hiding this comment

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

Whenever I agree that not-commiting podfile.lock will allow to see/test problems with versions as early as possible, I'd like to emphasize that it also will lead to unstable unit testing project. Imagine developers download Themis and try to run tests/see code first, but "It's suddenly broken we don't know why it was broken and what prev versions were working fine".

tests/objcthemis/.gitignore Outdated Show resolved Hide resolved
@ilammy
Copy link
Collaborator Author

ilammy commented Mar 5, 2020

I admit that this change is not ideal, but at least it improves the use case we care more about: when the user checks out default branch (master) they'd expect tests to work.

What user checks out What Themis version tests are actually using
Before PRAfter PRIdeal
master0.11mastermaster
stable0.11masterstable
0.110.11master0.11

Current approach ‘breaks’ historical branches, in a sense that we use newer Themis with older tests. However, it should still work if we’re careful enough with backwards compatibility.

The ideal approach would be to use something like development pods:

-pod 'themis', :git => "https://github.com/cossacklabs/themis.git"
+pod 'themis', :path => "../.."

so that tests use exactly the same version that's in the checked out tree. In my shallow experience, this often ends up broken and not working, but we might give it a try.

I've tried doing this, followed by pod update... and CocoaPods ate all remaining RAM, freezing up my Mac.

I take that as a no.

Another option is to manually update the Podfile.lock in a separate commit after we update unit-tests, but I don't really want to play this silly game to please the machine. It's easy to forget to do this, unless we put that into a checklist. And I don't want to remove local version hacks from CI because that means we'd be merging red pull requests... So I'd rather ‘break’ historical versions (which is easily fixed by manually editing Podfile after checkout, if you really need to test an old version against old tests).

@ilammy ilammy merged commit e33f4e4 into cossacklabs:master Mar 5, 2020
@ilammy ilammy deleted the podspec-version branch March 5, 2020 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
M-CocoaPods Package manager: CocoaPods, Objective-C and Swift, iOS and macOS O-iOS 📱 Operating system: iOS O-macOS 💻 Operating system: macOS tests Themis test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants