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

Replacement for #401: Update podspec for SQLCipher+fts3 #503

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 23 additions & 14 deletions FMDB.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@ Pod::Spec.new do |s|
s.author = { 'August Mueller' => 'gus@flyingmeat.com' }
s.source = { :git => 'https://github.com/ccgus/fmdb.git', :tag => "#{s.version}" }
s.requires_arc = true
s.default_subspec = 'standard'
s.default_subspec = 'standard'

s.subspec 'common' do |ss|
ss.source_files = 'src/fmdb/FM*.{h,m}'
ss.exclude_files = 'src/fmdb.m'
end

# use the built-in library version of sqlite3
s.subspec 'standard' do |ss|
Copy link
Contributor

Choose a reason for hiding this comment

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

s.subspec 'standard' do |ss| should just have the ss.dependency 'FMDB/common' dependency instead of duplicating the source_files and exclude_files.

Copy link
Author

Choose a reason for hiding this comment

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

Agreed - that was an oversight. Thanks for catching it.

Expand All @@ -26,24 +31,28 @@ Pod::Spec.new do |s|
s.subspec 'standalone' do |ss|
ss.xcconfig = { 'OTHER_CFLAGS' => '$(inherited) -DFMDB_SQLITE_STANDALONE' }
ss.dependency 'sqlite3'
ss.source_files = 'src/fmdb/FM*.{h,m}'
ss.exclude_files = 'src/fmdb.m'
end

# build with FTS support and custom FTS tokenizer source files
ss.dependency 'FMDB/common'
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation on end is wrong here

Copy link
Contributor

Choose a reason for hiding this comment

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

I would add the subspec for FTS here as well:

s.subspec 'standard' do |ss|
    ss.library = 'sqlite3'
    ss.source_files = 'src/fmdb/FM*.{h,m}'
    ss.exclude_files = 'src/fmdb.m'

    # use the built-in library version of sqlite3 with custom FTS tokenizer source files
    ss.subspec 'FTS' do |sss|
        sss.source_files = 'src/extra/fts3/*.{h,m}'
        sss.pod_target_xcconfig = { 'OTHER_CFLAGS' => '$(inherited) -DSQLITE_ENABLE_FTS4=1 -DSQLITE_ENABLE_FTS3_PARENTHESIS=1' }
    end
end

And then get rid of the separate subspec for 'FTS'

Copy link
Author

Choose a reason for hiding this comment

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

Agreed that this is better. I'd just kept the separate subspec to minimize changes from the existing podfile.

# build the latest stable version of sqlite3
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd tweak this comment to something like:
# Use the standalone version of sqlite3/fts and include custom FTS Tokenizer code

# with FTS support and custom FTS tokenizer source files
s.subspec 'standalone-fts' do |ss|
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a sub-subspec to s.subspec 'standalone' do |ss| for FTS instead of this weird standalone-fts one. Something like this:

s.subspec 'standalone' do |ss|
    ss.xcconfig = { 'OTHER_CFLAGS' => '$(inherited) -DFMDB_SQLITE_STANDALONE' }
    ss.dependency 'sqlite3'
    ss.dependency 'FMDB/common'

    ss.subspec 'FTS' do |sss|
        sss.dependency 'sqlite3/fts'
        sss.source_files = 'src/extra/fts3/*.{h,m}'
    end
end

Copy link
Author

Choose a reason for hiding this comment

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

I like this idea and it's similar to how it was being done a long time ago in this podfile. As above, I left the weirdness to minimize the changes I was making. Better to just fix it properly.

ss.xcconfig = { 'OTHER_CFLAGS' => '$(inherited) -DFMDB_SQLITE_STANDALONE' }
ss.source_files = 'src/fmdb/FM*.{h,m}', 'src/extra/fts3/*.{h,m}'
ss.exclude_files = 'src/fmdb.m'
ss.dependency 'FMDB/standalone'
ss.dependency 'sqlite3/fts'
ss.source_files = 'src/extra/fts3/*.{h,m}'
end

# use SQLCipher and enable -DSQLITE_HAS_CODEC flag
s.subspec 'SQLCipher' do |ss|
ss.dependency 'SQLCipher'
ss.source_files = 'src/fmdb/FM*.{h,m}'
ss.exclude_files = 'src/fmdb.m'
ss.xcconfig = { 'OTHER_CFLAGS' => '$(inherited) -DSQLITE_HAS_CODEC -DHAVE_USLEEP=1' }
end

ss.dependency 'FMDB/common'
Copy link
Contributor

Choose a reason for hiding this comment

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

Though it wouldn't affect how this functions, the indentation on this is wrong:

ss.dependency 'SQLCipher'
ss.dependency 'FMDB/common'
ss.xcconfig = ...

ss.xcconfig = { 'OTHER_CFLAGS' => '$(inherited) -DSQLITE_HAS_CODEC -DHAVE_USLEEP=1' }

# FMDB/SQLCipher/FTS subspec, which uses SQLCipher
Copy link
Contributor

Choose a reason for hiding this comment

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

These comments need to be indented correctly

# and adds FTS support + custom FTS tokenizer source files
ss.subspec 'FTS' do |sss|
sss.dependency 'SQLCipher/fts'
sss.dependency 'FMDB/FTS'
end
end
end