groue / GRDB.swift

A toolkit for SQLite databases, with a focus on application development
MIT License
6.7k stars 687 forks source link

SPM support #72

Closed zmeyc closed 7 years ago

zmeyc commented 8 years ago

(copied from Twitter discussion) It would be nice to have SPM support in GRDB so it can be used with server-side apps.

I'm working on Telegram Bots SDK: https://github.com/zmeyc/telegram-bot-swift and wanted to add an example of using a DB to show how to maintain sessions etc.

It’d also help telling which kind of bot you’d like to build on top of your SDK.

A template project to build other bots on. Probably a "shopping list" bot or something similar - for example, if you want to create a list of items to buy and share with other people, just add them to a group chat with the bot and everyone will be able to modify the list.

I've already ported another lib, SQLite.swift to Swift3/SPM, but it lacks some of the functionality I had to add: Records, migrations, upsert etc which GRDB supports out of the box.

SPM requires a lot of work, and last time I tried (early after the release of OpenSource Swift), I could never link to sqlite.

Currently it links without problems to system sqlite, but it's located in different dirs on OS X (if installed using brew) and Linux and requires using two different packages to workaround this. At first, I opted to compile it from source on OS X to install it into /usr/local instead: https://github.com/zmeyc/CSQLite

But, in the end I bundled sqlite with the Swift lib itself which requires no additional build steps from user and doesn't require adding flags to 'swift build': https://github.com/zmeyc/SQLite.swift/blob/master/Package.swift https://github.com/zmeyc/SQLite.swift/tree/master/Sources/CSQLite

Another problem with SPM is that it works only with 'master' branch now. So, if it's going to be supported, probably Swift3 branch has to be renamed to master, and current 'master' renamed to Stable and probably made the default on github. I believe other package managers don't have this limitation. This needs to be carefully tested.

Folder structure also needs to be adjusted a bit, but it's not going to be too hard.

groue commented 8 years ago

Hello @zmeyc, thanks for your interest, and the context you have provided.

I prefer answering no.

First of all, GRDB focuses on applications, and applications do not need Swift Package Manager. At least, not yet. Future will tell.

Next, shipping GRDB as a Swift package would raise quality issues, because packaging implies server support, when the library is not ready for the server. Precisely speaking:

  1. The behavior of GRDB in case of multiple writers on a single database is not documented enough, without any clear roadmap to avoid concurrency errors.

    Database changes observation is only reliable when there is a single writer to the database.

    The last point makes FetchedRecordsController unsuitable for the server.

  2. GRDB caches some database information, and the cache is very naive, implemented with simple dictionaries. The memory management API has not been audited for the server.

As a conclusion, packaging GRDB requires a more complex packaging than the one you talk about, and a dedication that lasts longer than a demo.

Keep on thinking on your goal. Maybe GRDB will remain your best option. But there are other databases, and some of them already have Swift bindings packaged with the SPM.

zmeyc commented 8 years ago

Ok, thank you for the detailed explanation!

zmeyc commented 8 years ago

@groue I tried using thin SQLite wrapper from Perfect, but it was a pain to use and resulted in very verbose code. So in the end I forked GRDB, added SPM's Package.swift, excluded Objective C code (SQLCipher etc) and added SQLiteMacOSX package as an external dependency (probably unnecessary, I'll try to use the one in GRDB later): https://github.com/zmeyc/GRDB.swift It built without problems, I had to add "import Foundation" in a few files. I'll try to use it in my server apps and see what issues I run into. Even considering the issues you mentioned there aren't many alternatives.

groue commented 8 years ago

OK @zmeyc. Thanks for your work. I don't quite know how your bot is architected, but: if there are several DatabasePool/Queue instantiated on a single database file at a given time, you will experience SQLITE_BUSY errors as soon as two connections will want to write concurrently.

That's because GRDB has been tuned for applications, not the server. Please read carefully the following:

To avoid SQLITE_BUSY errors, you will need a "busy handler". For example:

var config = Configuration()
config.busyMode = .timeout(10) // Wait 10 seconds before throwing SQLITE_BUSY error
let dbQueue = DatabaseQueue(path: "", configuration: config)

You will also avoid some errors (not all) by setting the default transaction kind to DEFERRED (instead of the default IMMEDIATE):

config.defaultTransactionKind = .deferred

Relevant GRDB documentation:

Must-read SQLite documentation:

zmeyc commented 8 years ago

@groue Thanks, this is very important info.

I started reading about SQLITE_BUSY after becoming surprised by this busy-loop in Perfect.swift :)) https://github.com/PerfectlySoft/Perfect-SQLite/blob/master/Sources/SQLite/SQLite.swift#L227 I wonder why they didn't use busy_handler / busy_timeout instead.

Bot will use a single database queue. It's an app which is constantly running, not started per request. Telegram throttles requests anyway, so DB is unlikely to be a bottleneck. But I'll add busyMode handler for situations where external apps also try to work with DB file.

cfilipov commented 8 years ago

@zmeyc Looking at your fork I see you added Pckage.swift but haven't re-arranged the source files into a Sources/ directory. Does that work (I haven't had a chance to try your repo yet)?

SPM's docs say that it's possible to configure sources location, but never describe how.

zmeyc commented 8 years ago

@cfilipov Yes, if Sources/ dir is missing, it treats every folder it finds in main folder as a module. I just excluded everything else from compilation.

cfilipov commented 8 years ago

Perfect! I wish they would have documented that! Other than having to import Foundation and keeping it all on master it looks like no other changes were necessary? Should be relatively easy to keep in sync with upstream.

zmeyc commented 8 years ago

@cfilipov @groue Yes, it should be easy to keep in sync.

On Github it's possible to make another branch the default one and I believe other package managers can work with non-master branch, so theoretically it can be kept in one repo. But users who cloned the lib probably won't be able to upgrade easily if branches are renamed.

In this fork I added SQLite as an external dependency, but this is unnecessary. The one in GRDB repo can be added as another module. I forgot how to specify dependencies between local modules in Package.swift, but this is definitely possible.

zmeyc commented 8 years ago

To use the lib in a project, add to Package.swift:

            .Package(url: "https://github.com/zmeyc/GRDB.swift.git", majorVersion: 0)

swift build will work without additional steps, but when generating .xcodeproj using swift package generate-xcodeproj:

Choose TARGETS -> GRDB -> Build Settings -> Import Paths and add: Packages/SQLiteMacOSX-0.0.2/Sources Otherwise it won't be able to import SQLiteMaxOSX module. This seems to be an SPM bug.

groue commented 8 years ago

I'm very pleased to read all that. I'm all ears for your experience in running GRDB with in an SPM environment. Let me reopen the issue, since the subject is alive thanks to both of you @zmeyc and @cfilipov!

zmeyc commented 8 years ago

Found an example:

let package = Package(
    name: "SQLite.swift",
    targets: [
        Target(
            name: "SQLite", // <-- lib source files
            dependencies: [
                .Target(name: "CSQLite") // <-- module with .c files
            ]),
        Target(
            name: "CSQLite")
    ]
)

We also need to find a way of specifying if system SQLite or locally compiled one should be used (checking an env var? or for a presence of config file in project dir or package dir?)

Latest SPM also seems to support pkgConfig and package providers (apt-get, brew), but it's undocumented and I haven't checked how it works yet. Examples:

https://github.com/IBM-Swift/CCurl/commit/99010c8a2e7d543e3fee426256a2dc8390e87c81 https://github.com/IBM-Swift/CCurl/blob/master/Package.swift

zmeyc commented 8 years ago

I tried to built the local SQLite module, but the problem is SPM can now only build modules which contain source files only. It's not possible to execute custom build steps such as running autotools or a Makefile command. So, current options are using system sqlite or the one installed by brew/apt-get or the one from Xcode (using hardcoded path) or building SQLite from amalgamated source file.

A benefit of external SQLite package is that the user can override it by specifying his own version in his project's Package.swift. AFAIK it will be used instead of GRDB's one if module name will match. I'm not sure if local package can be overridden this way.

cfilipov commented 8 years ago

Happy to see this re-opened. the ability to build GRDB using SPM without having to re-organize the project structure should make it much easier to maintain (I was planning on having a fork that did only this). Other than a few added imports in some code (and the question about supporting custom built SQLite as @zmeyc mentioned), it looks like it's mainly just having an appropriately-configured Package.swift file in the master branch.

This will also make things much cleaner for me. I have a Swift package LibA that would depend on the GRDB package. Then another Swift package cmdA that is a command line utility for libA. Now I can easily build that command line tool with swift build and it will statically link GRDB. I can also use swift package generate-xcodeproj and link against the same static libA in my iOS project.

zmeyc commented 8 years ago

@groue @cfilipov I have misunderstood how tags work. If a hash is tagged in any branch, SPM should find it. It doesn't recognize "vX.Y.Z" naming scheme and expects tags in "X.Y.Z" format. So, it should be possible to keep "vX.Y.Z" tags for Swift 2 and use "X.Y.Z" tags for Swift 3 versions in Swift3 branch. @groue what do you think?

zmeyc commented 8 years ago

Btw, adding import path to generated .xcodeproj can be automated. Kitura uses xcodeproj gem for patching the main project's xcodeproj: https://github.com/IBM-Swift/Kitura-Build/blob/master/build/fix_xcode_project.rb

groue commented 8 years ago

I have misunderstood how tags work. If a hash is tagged in any branch, SPM should find it. It doesn't recognize "vX.Y.Z" naming scheme and expects tags in "X.Y.Z" format. So, it should be possible to keep "vX.Y.Z" tags for Swift 2 and use "X.Y.Z" tags for Swift 3 versions in Swift3 branch. @groue what do you think?

Thanks for the investigation @zmeyc. We'll switch to X.Y.Z version tags for Swift 3 (when it leaves the beta phase), since it helps supporting SPM.

zmeyc commented 8 years ago

@groue Sorry to misinform you, I've rechecked and SPM does pick up "vX.Y.Z" tags. :( I also verified that it works with branches.

swiftlyfalling commented 8 years ago

@zmeyc: As a general note on custom SQLite builds, it is only safe to use the SQLite amalgamation source files as-is (with the default settings). If you want to enable/disable SQLite features or change any SQLite compilation options or flags, you must re-generate the amalgamation sources from the raw SQLite source with those flags set (which is what the GRDBCustomSQLite target does).

See: https://www.sqlite.org/howtocompile.html#amal

groue commented 8 years ago

Hello @zmeyc

What would be your recommendation about splitting GRDB in several modules / submodules (in a single package if possible, or in separate packages if necessary), so that we could independently load:

That's the split I'm thinking of, but you may have better ideas. When you have time, I'd be curious hearing your thoughts.

zmeyc commented 8 years ago

@groue Sorry for late response, I'm currently travelling. I'm not sure if splitting the library into submodules will give any benefits as it's unlikely that they will be used separately. Also, it will probably complicate interaction between submodules. I would only extract CSQLite (SQLiteCustom) into it's own module to make it overridable keeping everything else as a single module.

adamnemecek commented 7 years ago

What's the situation with this? I've spent like 3 hours trying to get GRDB to compile but I have some issues with every single one of installation methods. Swift Package Manager has never given me this much of a headache (I'm not blaming GRDB, it's mostly the other package managers or the build system).

First of all, GRDB focuses on applications, and applications do not need Swift Package Manager.

Why don't applications need SPM? It's so much better than carthage and cocoapods.

groue commented 7 years ago

Hello @adamnemecek

I have some issues with every single one of installation methods.

The three documented installation methods (CocoaPods, Carthage, and manual) are supposed to succeed with the latest version of GRDB (0.101.1), and the requirements mentionned at the very top of the README file. If this is not your case, please provide detailed information in a new, dedicated, issue.

First of all, GRDB focuses on applications, and applications do not need Swift Package Manager.

Why don't applications need SPM? It's so much better than carthage and cocoapods.

Things may have changed since my initial comment was written. I'm not familiar with Xcode and SPM integration: if you have useful information, please share it with us.

adamnemecek commented 7 years ago

I've actually figured out the issue (I had an older Xcode version (8.1) which apparently mattered). However, I'm willing to help out with migration to SPM regardless.

I'm not familiar with Xcode and SPM integration: if you have useful information, please share it with us.

That's the best thing, SPM works independently of Xcode project files, you use the package manager to generate a new project file. It makes things a lot easier and cleaner. I can answer some questions about it if you want (note that I too run into issues but compared with pods and carthage it's a walk in the park).

zmeyc commented 7 years ago

@adamnemecek SPM support can be added fairly easily: https://github.com/groue/GRDB.swift/commit/a18dee3b9bc07ae421c305d2510a6f09825592a7 But we haven't decided how to use customized builds of sqlite in this setup.

adamnemecek commented 7 years ago

Why not include this on master?

zmeyc commented 7 years ago

@adamnemecek Ideally we need a way to use a custom built sqlite. I mean integrate SPM with this somehow: https://github.com/groue/GRDB.swift/blob/master/Documentation/CustomSQLiteBuilds.md

adamnemecek commented 7 years ago

Little by little. Having it working 20% (i.e. without custom builds) is better than having it working 0%. But this does seem somewhat tricky.

zmeyc commented 7 years ago

@groue What do you think? We can start by linking to a system sqlite in a separate CSQLite package. Shall I send a PR? upd: or rather to a brew installed one and apt-get one on Linux.

groue commented 7 years ago

SPM works independently of Xcode project files, you use the package manager to generate a new project file.

I had read that but were unable to get the consequences. Thanks!

Custom build/System?

I prefer to target the system Sqlite. Why? Because it closes so many doors. Conversely, custom builds want to be customized. And unless I'm mistaken, SPM is still far from letting package MyApp tell its dependency package GRDB how to compile its own dependcy package CSQLite.

Let me know if you have any question. I'm far from my computer right now, and won't be get back until a week. Can I let you both synchronize so that you don't spend the same energy on the same PR? There are three tasks: supporting SPM with system SQLite, checking that other installation methods and sqlite variants still work, and updating the documentation. I will be able to remotely assist.

Thanks to both of you!

zmeyc commented 7 years ago

@groue Could you create a new repo named "SQLiteSDK"? I'll send a PR with contents.

It'll use a system sqlite by default. SPM doesn't support forks right now, so to customize it users will need either to:

  1. Fork SQLiteSDK and customize it (put sqlite3.c in there etc). Fork GRDB.swift and change URL in Package.swift to point to their fork of SQLiteSDK.

or (probably easier):

  1. Use editable packages feature of SPM: swift package edit SQLiteSDK --branch tmp This will put SQLiteSDK package into "editable" mode and put it into Packages/SQLiteSDK dir, where it can be replaced by anything else, for example by user's own fork of the package. This can be automated by using a Makefile (I'll add an example to the doc).
groue commented 7 years ago

@groue Could you create a new repo named "SQLiteSDK"? I'll send a PR with contents.

Done: https://github.com/groue/SQLiteSDK

It'll use a system sqlite by default.

It'll use the system sqlite only.

Customization is a big topic. GRDB not only handles custom SQLite builds, but also exposes new APIs depending on the customization (such as support for the FTS5 full-text engine).

So I recommend that you focus on a very precise topic: supporting system SQLite through SPM, and forget the topic of customization entirely, for now.

zmeyc commented 7 years ago

@groue Could you create LICENSE / README files, I can't fork empty repo. The button is grayed out.

groue commented 7 years ago

@zmeyc Is the README enough?

zmeyc commented 7 years ago

@groue Yes, thanks!

zmeyc commented 7 years ago

@groue I've opened 2 PRs:

Modulemap: https://github.com/groue/SQLiteSDK/pull/1

SPM support (no tests yet): https://github.com/groue/GRDB.swift/pull/201

groue commented 7 years ago

Update: #202

zmeyc commented 7 years ago

@groue I've replied in PR. Please rename the repo and I'll do the remaining adjustments.

Regarding test_SPM_install: I'm not sure how it should work. To test that it still builds with SPM, it should be enough to call

swift package clean
swift build

Unit tests require a flat layout: Tests/BlablaTests/File.swift Every dir in Tests/ becomes a module. I don't know where the code shared between tests should go, probably we'll have to ask SPM devs.

groue commented 7 years ago

Closing in favor of #202.