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

Add Additional Features / Options #563

Merged
merged 24 commits into from
May 25, 2018
Merged

Conversation

gjrtimmer
Copy link
Collaborator

New README Preview

ADD

  • SQLITE_ALLOW_URI_AUTHORITY
  • SQLITE_ENABLE_API_ARMOR
  • SQLITE_DEFAULT_FOREIGN_KEYS
  • SQLITE_INTROSPECTION_PRAGMAS
  • SQLITE_SECURE_DELETE
  • SQLITE_ENABLE_STAT4
  • SQLITE_DEFAULT_AUTOVACUUM

Update

  • README
    • Feature Table
    • TOC
    • Renamed files of existing features to aggregate them based on filename.

@coveralls
Copy link

coveralls commented May 23, 2018

Coverage Status

Coverage remained the same at 68.061% when pulling 4d6bb50 on GJRTimmer:update/options into 29ac65f on mattn:master.

@mattn
Copy link
Owner

mattn commented May 24, 2018

Do you want to add some of them into .travis.yaml?

@mattn
Copy link
Owner

mattn commented May 24, 2018

I'd like to say for everyone that see this PR, I will not promise that the options will stable in feature. I possibly may include some of them into default feature. :)

@mattn
Copy link
Owner

mattn commented May 24, 2018

And I regret that these some tags should have sqlite3- prefix. If possible, I want to the tags which does not have the prefix of sqlite3- be deprecated.

@gjrtimmer
Copy link
Collaborator Author

gjrtimmer commented May 24, 2018

I understand and agree. I will add them to Travis CI.

I understand that some options will be enabled by default in the future, but some should be left to the user, or we should follow SQLite itself. For example; foreign keys check is still not enabled by default by SQLite itself. So should we not follow the same convention ? Let me know

Deprecation

To make sure I understand you completly, you want to deprecate the tag; icu json1 fts5 and you want me to change them to sqlite_icu and keep the original isu for some time. At the moment you want both tags to work. And in the future you will remove the old one.

I will make the changes you want.

@mattn
Copy link
Owner

mattn commented May 24, 2018

Thanks. So I want to rename all of opt tags include . Could you please add sqlite_ prefix for them?

fts5 => sqlite_fts5 fts5
icu => sqlite_icu icu
json1 => sqlite_json1 json1
see => sqlite_see see
trace => sqlite_trace trace
userauth => sqlite_userauth userauth
vtable => sqlite_vtable vtable

I'll keep older tag name for the time being to be possible to build with both tags. And I'll make the feature-tags which doesn't include sqlite_ prefix deprecated.

@mattn
Copy link
Owner

mattn commented May 24, 2018

One more please, update .travis.yml with using new sqlite_ prefix.

gjrtimmer added 8 commits May 24, 2018 11:43
* Keep current build tags backwards compliant
* Added alias for `sqlite_json1` => `sqlite_json`
Fix: New build tag(s) names
Add documentation to feature table for tag `trace`
Usage of new build tag names
@gjrtimmer
Copy link
Collaborator Author

gjrtimmer commented May 24, 2018

@mattn I've renamed everything to what you wanted.

Updates

  • Travis CI now uses new names
  • Addition: Travis CI > for sqlite_userauth (Not present in this branch, will you make the build tag change in the branch where this tag is used ?)
  • Addition: Travis CI < for sqlite_see (Not present in this branch, will you make the build tag change in the branch where this tag is used ?)
  • json1 => sqlite_json sqlite_json1 json1 (sqlite_json :: This is just for user conveniance)

Added Cross-Compile jobs for additional feature(s)
@gjrtimmer
Copy link
Collaborator Author

gjrtimmer commented May 24, 2018

Travis CI now fully updated, forgot to add the additional jobs for the Windows Cross-Compile.
These have now been added.

@mattn
Copy link
Owner

mattn commented May 24, 2018

Thanks. I'll merge this after CI succeeded.

@gjrtimmer
Copy link
Collaborator Author

gjrtimmer commented May 24, 2018

You will update the see branch with the sqlite_see build tag and the branch where the sqlite_userauth tag is used right ?

@mattn
Copy link
Owner

mattn commented May 24, 2018

No, that was garbage. Thanks.

@gjrtimmer
Copy link
Collaborator Author

Then I suggest we leave the sqlite_see tag and sqlite_userauth tag in travis-ci.yml for the future.

@gjrtimmer
Copy link
Collaborator Author

gjrtimmer commented May 24, 2018

@mattn on OSX module ICU fails: unicode/utypes.h is missing I do not own a Mac do you know which package we need to install ?

Required for building SQLite module `icu`
@mattn
Copy link
Owner

mattn commented May 24, 2018

now travis stop?

@gjrtimmer
Copy link
Collaborator Author

Yeah stop Travis. I will make it shorter

@gjrtimmer
Copy link
Collaborator Author

@mattn Travis-CI Updated, currently running hope that you can merge this PR when its done.

@gjrtimmer
Copy link
Collaborator Author

@mattn Still trouble with compiling the icu module on OSX. Do you know how to do this, adding brew install icu4c had no effect.

@mattn
Copy link
Owner

mattn commented May 25, 2018

@gjrtimmer Thanks your working on this. Sorry, I don't have OSX. So can not help you...

@gjrtimmer
Copy link
Collaborator Author

@mattn I'm now trying with Stackoverflow to fix this.

@gjrtimmer
Copy link
Collaborator Author

@mattn Think I succesfully fixed it, will merge my fix/darwin-icu branch into this one, then rebuild and we should have a complete succesfull build for ALL feature modules on several platforms 👍

For the icu module I've added additional CFLAGS and LDFLAGS only for darwin, apparently you have to manually include it on OSX.

@gjrtimmer
Copy link
Collaborator Author

@mattn Please cancel #709 #710 Travis Builds for me

gjrtimmer added 2 commits May 25, 2018 12:24
Removed: 32Bit Windows Cross-Compile
Aggregated: sqlite_trace into main module build
@gjrtimmer
Copy link
Collaborator Author

@mattn Build successful

@gjrtimmer
Copy link
Collaborator Author

@mattn Ready to merge 👍

@mattn
Copy link
Owner

mattn commented May 25, 2018

Thanks. I'll check this on Windows later.

@gjrtimmer gjrtimmer mentioned this pull request May 25, 2018
@mattn
Copy link
Owner

mattn commented May 25, 2018

LGTM on Windows. Thank you.

@mattn mattn merged commit c5dd6c3 into mattn:master May 25, 2018
@gjrtimmer gjrtimmer deleted the update/options branch May 26, 2018 19:04
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