Skip to content

Prep for public release to CocoaPods #314

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

Merged
merged 5 commits into from
Jan 10, 2016
Merged

Prep for public release to CocoaPods #314

merged 5 commits into from
Jan 10, 2016

Conversation

mikemee
Copy link
Collaborator

@mikemee mikemee commented Jan 8, 2016

@stephencelis, as discussed, here is a branch with a proposed update to become the first "public" cocoapod version.

Summary of changes:

Known CocoaPod hassles:

  • I couldn't convince it to generate its own module.modulemap file, so one is included
  • has spurious warnings about a missing include: Umbrella header for module 'SQLite' does not include header 'fts3_tokenizer.h (all tests, including tokenizer, run fine)
  • pod lib lint does not work to validate the pod, but pod spec lint is fine (per pod lib lint doesn't respect preserve_paths? CocoaPods/CocoaPods#4607) (Note: does not currently pass either as it needs a branch, but it did pass in testing on my repo with the url modified).

Next Steps

If this PR is accepted, the following should be done to make CocoaPods and Carthage happy:

Trying this PR now

I encourage others to try this PR. Because it's on a branch, it is a trivial change to your existing files (or manually as before):

Carthage Cartfile:

github "stephencelis/SQLite.swift" "publicpod"

CocoaPod Podfile:

use_frameworks!

pod 'SQLite.swift',
  git: 'https://github.com/stephencelis/SQLite.swift.git', :branch => 'publicpod'

After this

Other TODOs

  • Remove SQLCipher references and installation instructions from README, Documentation

 * Remove `SQLiteCipher` (for now)
 * rebuild clean from "New Project" with Xcode 7.2
 * fix build glitches with Carthage
 * fix build glitches with CocoaPods
 * separate build targets from test targets with @testable import
 * make sqlite3 a Swift system module so `import SQLite3` works in `.swift` files (for this framework)
 * use an Xcode project file instead of Workspace
 * folder structure rearranged
 * CocoaPod hassles: doesn't generate its own module.modulemap file, has spurious warning about missing include
_No good deed goes unpunished._
@@ -1,10 +1,23 @@
# OS X
.DS_Store
Copy link
Owner

Choose a reason for hiding this comment

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

Good change. Prepares us for swiftpm compatibility 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thx :) I can't take full credit. This just seemed to be the norm in similar projects I viewed for comparison when I did "new project", so I copy pasted. Plus I've removed more than a few .DS_Store files over time.

@stephencelis
Copy link
Owner

This is great! Thanks for all the work you put into it! I left a few comments/questions, but I think this is really close to being ready.

@stephencelis
Copy link
Owner

This'll need to get rebased (given #317). Once you think things are ready, give me a nod, I'll take a final look and merge!

@stephencelis
Copy link
Owner

(Oh, and as far as a separate SQLiteCipher.swift project goes, should I create a repo and add you?)

@mikemee
Copy link
Collaborator Author

mikemee commented Jan 10, 2016

Rebase is done. @testable done. deployment target added to podspec.

Working on the other items. (Trying private for fts3_tokenizer.h just to see... so far Xcode happy, but still need to test Carthage (likely fine since Xcode is) and of course CocoaPods to be sure).

Re SQLiteCipher - sure. Can't promise anything, but I do like to see things through... :)

@stephencelis
Copy link
Owner

Yeah, I think the other repo at least needs to work very basically since people depend on SQLCipher in their projects. Let me see if I can get something basic working against this branch.

I also just noticed that SQLCipher installation instructions are still in the README/Documentation on this branch; we should remove/update them before merging.

@mikemee
Copy link
Collaborator Author

mikemee commented Jan 10, 2016

Private for fts3_tokenizer.h still generates the same warning, so I'll leave it as is (project), unless you have strong feelings about it.

Will fix the readme/documentation re SQLCipher

Re the module map. I just realized that even if I fix it via a magic script for Cocoa, it's still "wrong" for Swift Package Manager / current Xcode style file in SQLite3/module.modulemap, i.e.:

module SQLite3 [system] {
    header "/usr/include/sqlite3.h"
    link "sqlite3"
    export *
}

And I don't know any simple way of changing that, hmm, except, I suppose by setting up multiple files, then changing the Swift Include file path in Xcode... all of which seems more messy for a problem that will go away with Swift Package Manager soon enough anyway (I hope!)

I.e. these are all identical on my machine:

/usr/include/sqlite3.h 
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk/usr/include/sqlite3.h
/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS.sdk/usr/include/sqlite3.h 
/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator.sdk/usr/include/sqlite3.h

Given that all these files are identical anyway, I propose sticking with the original checkin and not complicating the CocoaPod build further.

But I'll accept whatever you prefer ... it's your project (though I'm not excited to hack the script I will admit! ;)

mikemee and others added 2 commits January 10, 2016 15:00
 - remove @testable
 - remove SQLCipher references in docs
 - add deployment target to pod spec
 - merge recent master changes
stephencelis added a commit that referenced this pull request Jan 10, 2016
Prep for public release to CocoaPods
@stephencelis stephencelis merged commit c2c3d52 into master Jan 10, 2016
@stephencelis stephencelis deleted the publicpod branch January 10, 2016 23:33
@stephencelis
Copy link
Owner

💯

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.

3 participants