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

Conversation

evands
Copy link

@evands evands commented Jun 8, 2016

It turned out to be significantly easier and clearer to just create a new branch and pull request for this fix. The first commit just simplifies the podspec to use dependencies to deduplicate code; the second adds FMDB/SQLCipher/FTS3

@ccgus
Copy link
Owner

ccgus commented Jun 8, 2016

@stephanheilner - can you quickly review this (since you seem to use cocoa pods) and let me know if it looks OK?

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 = ...

@stephanheilner
Copy link
Contributor

I added some suggestions and made some comments inline.

…ns: standard, standard/FTS, standalone, standalone/FTS, SQLCipher, and SQLCipher/FTS.

Thanks to @stephanheilner for the helpful review.
@evands
Copy link
Author

evands commented Jun 9, 2016

@stephanheilner: Please see [300d0fe] and let me know if you have any further suggestions. I think we've got a much cleaner podspec now as a result of these efforts.

@evands
Copy link
Author

evands commented Jun 9, 2016

Finally, we'll need to bump the podfile version number and have a corresponding git tag, once this is approved.

@ccgus
Copy link
Owner

ccgus commented Jun 10, 2016

So everyone is happy with this now? All I need to do is commit it?

@stephanheilner
Copy link
Contributor

We'll need to update the version from 2.6.2 to whatever the next tag version will be, but we can do that right before @ccgus creates a new tag. Just give me a heads up, so I can make sure you bump the version in this file as well first.

@evands
Copy link
Author

evands commented Jun 12, 2016

I'm having difficulty with custom tokenizers working in SQLCipher+FTS. Please don't commit yet -- investigating.

@evands
Copy link
Author

evands commented Jun 12, 2016

OK, I figured out the problem but don't have an immediate solution. @stephanheilner would love your input.

  • SQLCipher needs to be compiled with -DDSQLITE_ENABLE_FTS3_TOKENIZER for custom tokenizers to work. Adding this in our podspec doesn't change how the dependency is compiled so doesn't fix the problem.
  • Maybe should be done in the SQLCipher podspec file I think but I can't find a source podspec in their github repo to make a pull request

@pleasantnicholasb
Copy link

Just wondering if this branch is still being worked on.

@evands
Copy link
Author

evands commented Feb 12, 2017

I concluded it probably needs to be fixed at the level of SQLCipher if it is to be properly done and couldn't find anywhere to do that. I've just added this to my Podfile:

post_install do |installer|
 puts "Post-Install: Enabling SQLITE_ENABLE_FTS3_TOKENIZER for SQLCipher..."	
  sqlcipher_targets = installer.pods_project.targets.select { |target| target.name =~ /^SQLCipher$/ }
  sqlcipher_targets.each do |target|
        target.build_configurations.each do |config|
            old_defines = config.build_settings['GCC_PREPROCESSOR_DEFINITIONS'] || []
            config.build_settings['GCC_PREPROCESSOR_DEFINITIONS'] = old_defines + ['$(inherited) SQLITE_ENABLE_FTS3_TOKENIZER=1']
        end
    end
end

@RishatShamsutdinov
Copy link

-DDSQLITE_ENABLE_FTS3_TOKENIZER is not required because you can enable custom tokenizers by following code:
sqlite3_db_config(sqliteHandle, SQLITE_DBCONFIG_ENABLE_FTS3_TOKENIZER, 1, 0);

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.

None yet

5 participants