Skip to content

Conversation

marcprux
Copy link

@marcprux marcprux commented Jan 26, 2025

This is a less ambitious alternative to #1701 that enables adding a SQLCipher source dependency to the Package.swift based on the GRDBCIPHER environment variable. Tests can be run with the dependency with a command like:

GRDBCIPHER="https://github.com/skiptools/swift-sqlcipher.git#1.2.1" swift test

Running it without the environment variable will leave things exactly as they are. The majority of changes in this PR are simply re-ordering the import header checks for GRDBCIPHER and SWIFT_PACKAGE, since we would need the former to take precedence of the latter.

An advantage of doing it this way is that it facilitates creating a GRDB-SQLCipher fork that can follow the releases of the upstream. The only change in such a fork would be to hardwire the GRDBCIPHER setting in Package.swift to some SQLCipher source repository (either mine, or one that you maintain). When a client package wants to switch between GRDB and GRDB-SQLCipher, they would just need to swap their dependency URL. (this is no longer needed when using package traits; see comment below)

This PR also adds in support building and testing against Android, mostly by stubbing out the unit tests that can't compile due to missing NSFileCoordinator, Combine, and the like. Android tests can be run with skip:

GRDBCIPHER="https://github.com/skiptools/swift-sqlcipher.git#1.2.1" skip android test

Most of the tests pass, but there are a few that need to be investigated. I'll look into them next.

Closes #1701

@marcprux
Copy link
Author

marcprux commented Jan 27, 2025

Addendum: out of curiosity, I ran some comparisons between the built-in SQLite and the SQLCipher build with the following script (and adding a GRDB_PERFORMANCE_TESTS check to enable GRDBPerformanceTests in Package.swift).

The results over a dozen runs are a fairly consistent slowdown of around 10% when using SQLCipher. It'd be interesting to tweak the SQLite build flags in swift-sqlcipher to see what might increase/decrease the performance.

echo "With GRDBCIPHER"; rm -r .build; GRDB_PERFORMANCE_TESTS=1 GRDBCIPHER="https://github.com/skiptools/swift-sqlcipher.git#1.2.1" swift test --filter GRDBPerformanceTests 2>&1 | grep --after-context=2 "Test Suite 'GRDBPackageTests.xctest' passed"
echo "Without GRDBCIPHER"; rm -r .build; GRDB_PERFORMANCE_TESTS=1 swift test --filter GRDBPerformanceTests 2>&1 | grep --after-context=2 "Test Suite 'GRDBPackageTests.xctest' passed"

With GRDBCIPHER
Test Suite 'GRDBPackageTests.xctest' passed at 2025-01-26 22:33:13.477.
	 Executed 23 tests, with 0 failures (0 unexpected) in 353.144 (353.146) seconds
Test Suite 'Selected tests' passed at 2025-01-26 22:33:13.477.
Without GRDBCIPHER
Test Suite 'GRDBPackageTests.xctest' passed at 2025-01-26 22:38:59.398.
	 Executed 23 tests, with 0 failures (0 unexpected) in 314.684 (314.686) seconds

Update: this was simply because I wasn't building in release mode. See comment below, where the SQLCipher build is actually 10% faster.

@Joannis
Copy link

Joannis commented Jan 30, 2025

@marcprux the Android NDK doesn't seem to bundle sqlite3.h, whereas GRDB and your PR do rely on that being available. How are you providing that right now when building and testing GRDB?

@marcprux
Copy link
Author

@marcprux the Android NDK doesn't seem to bundle sqlite3.h, whereas GRDB and your PR do rely on that being available. How are you providing that right now when building and testing GRDB?

It is included with the SQLCipher source module: https://github.com/skiptools/swift-sqlcipher/tree/main/Sources/SQLCipher/sqlite

Copy link
Owner

@groue groue left a comment

Choose a reason for hiding this comment

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

Hello @marcprux,

I like that very much. This pull request is a great gift 🎁! I'll shortly play with the new targets.

I made a first review and preliminary comments. I hope we agree on the broad outlines!

@groue
Copy link
Owner

groue commented Feb 1, 2025

The results over a dozen runs are a fairly consistent slowdown of around 10% when using SQLCipher. It'd be interesting to tweak the SQLite build flags in swift-sqlcipher to see what might increase/decrease the performance.

Isn't it just the cipher? (EDIT: I'm wrong because performance tests don't run on encrypted databases, see comment below.)

SQLCipher can also be extremely slow when it opens a connection. Some people have reported up to 0.5 seconds (😱). To the point I had to ship #1350.

@groue
Copy link
Owner

groue commented Feb 1, 2025

I ran the same performance tests, but this time in release configuration:

echo "With GRDBCIPHER"; rm -rf .build; GRDB_PERFORMANCE_TESTS=1 GRDBCIPHER="https://github.com/skiptools/swift-sqlcipher.git#1.2.1" swift test --configuration release --filter GRDBPerformanceTests 2>&1 | grep --after-context=2 "Test Suite 'GRDBPackageTests.xctest' passed"
echo "Without GRDBCIPHER"; rm -rf .build; GRDB_PERFORMANCE_TESTS=1 swift test --configuration release --filter GRDBPerformanceTests 2>&1 | grep --after-context=2 "Test Suite 'GRDBPackageTests.xctest' passed"

With GRDBCIPHER
Test Suite 'GRDBPackageTests.xctest' passed at 2025-02-01 13:53:37.700.
	 Executed 23 tests, with 0 failures (0 unexpected) in 72.258 (72.260) seconds
Test Suite 'Selected tests' passed at 2025-02-01 13:53:37.700.
Without GRDBCIPHER
Test Suite 'GRDBPackageTests.xctest' passed at 2025-02-01 13:57:26.264.
	 Executed 23 tests, with 0 failures (0 unexpected) in 80.139 (80.141) seconds
Test Suite 'Selected tests' passed at 2025-02-01 13:57:26.264.

This time, SQLCipher tests are faster.

@marcprux
Copy link
Author

marcprux commented Feb 1, 2025

SQLCipher can also be extremely slow when it opens a connection

Agreed, but the performance hit should only occur when encryption is actually enabled on the database, which shouldn't affect any of the perf tests.

This time, SQLCipher tests are faster.

Of course! Foolish of me to not do that. Otherwise, it is comparing the debug build of the sqlcipher/sqlite source build against the release build of the vendored sqlite.

It is heartening to see that superior performance can be squeezed out of a source build. I wonder how much more might be managed with strategic performance-related flags (e.g., sbooth/CSQLite#59 (comment)).

@marcprux
Copy link
Author

marcprux commented Feb 6, 2025

Apologies for the long delay in addressing the feedback (I was traveling).

I believe I've handled each of the requested changes. Let me know if I can do anything else to help get this PR over the finish line – it has grown quite large, so I'm nervous that rebasing is going to be difficult if it starts to become stale.

@Joannis
Copy link

Joannis commented Feb 7, 2025

We've been using this branch successfully.

@marcprux
Copy link
Author

marcprux commented Feb 12, 2025

I'll follow up on the list of needs at https://mastodon.social/@[email protected]/113990450491505387 here, just to keep it in the PR record.

  • Provides built-in SQLCipher support that works for 80+% of people.

This should work out of the box for anyone that just changes:

    dependencies: [
        .package(url: "https://github.com/groue/GRDB.swift.git", from: "7.0.0"),
    ]

to the (theoretical) locally-maintained fork:

    dependencies: [
        .package(url: "https://github.com/groue/GRDB-SQLCipher.swift.git", from: "7.0.0"),
    ]
  • No extra dependency (we must control the SQLCipher compilation options).

You are welcome to relocate https://github.com/skiptools/swift-sqlcipher.git into the groue/ org and use that – then you would have control over releases and flags.

  • No binaries (they rot).

swift-sqlcipher.git is a pure source build. There are no binaries.

  • A clear documentation that tells users what to do to get GRDB+SQLCipher with SPM, written for people who are not build experts.

This isn't part of this PR, but it would only be a couple of lines in the README.

  • Testable in GitHub CI.

This is part of the PR. The new SPMSQLCipher job just does:

GRDBCIPHER="https://github.com/skiptools/swift-sqlcipher.git#1.2.1" swift test
  • Runs the full GRDB test suite with unencrypted databases, AND with encrypted databases (we already do that for SQLCipher with CocoaPods).

The tests do run against SQLCipher, but against an unencrypted database. I didn't see how this is being done with SQLCipher for CocoaPods, but I suppose we could just do the same setup here (perhaps just based on checking the GRDBCIPHER environment variable)

  • Requires minimum efforts for debugging (one can debug tests from Xcode).

This shouldn't be an issue. Just activate (i.e., un-comment) the GRDBCIPHER line in the Package.swift and run the test cases in Xcode.

  • Requires minimum efforts for maintenance (i.e. bumping SQLCipher version and adjusting compile options when needed)

For the SQLCipher dependency, you just need to make a new amalgamated sqlite.c from sqlcipher, commit it, and then tag a release. This is done with the script: https://github.com/skiptools/swift-sqlcipher/blob/main/scripts/build_sqlcipher.sh

An example of this sort of update commit is the bump to sqlite 3.46.1 and sqlcipher 4.6.1: skiptools/swift-sqlcipher@5ea2f8b

For GRDB itself, my proposal is that you maintain a fork of GRDB.swift.git at https://github.com/groue/GRDB-SQLCipher.swift.git. The fork is pristine, with the one exception that the GRDBCIPHER line in the Package.swift is un-commented. Whenever you make a release of GRDB.swift.git, you just update the GRDB-SQLCipher.swift.git fork and sync the tags.

It is a little bit of overhead, but it could be added as another step in any scripts you might already be using to maintain releases.

  • A technique thats enables SQLCipher support in companion packages as well (GRDBQuery, GRDBSnapshotTesting, RxGRDB).

This is certainly a shortcoming of this method. I unfortunately don't see any solution other than to also have a GRDBQuery-SQLCipher that depends on the GRDB-SQLCipher.swift.git fork.

This last point is indeed painful. However, I think this PR gets us a lot, with minimal intrusion on the existing GRDB structure. A more "correct" solution would be along the lines of #1701, which requires a large refactoring and had performance implications, and so was rejected.

We can have a working GRDB-SQLCipher implementation (plus Android support!) today with this PR, while still continuing to design a more ideal future implementation.

Update: the fork is no longer necessary when using package traits (see comment below)

@groue
Copy link
Owner

groue commented Feb 24, 2025

I'm sorry I did not answer yet. I had to cope with the problems created with Xcode 16.3 beta. I'll get back to you soon.

@Alex293
Copy link

Alex293 commented Mar 12, 2025

This PR saved me today while integrating a closed source framework which links SQLCipher while we were already extensively using grdb.

@marcprux
Copy link
Author

marcprux commented Apr 23, 2025

I've updated this PR to use the new package traits in Swift 6.1, which seem designed for a scenario exactly like this. Now, rather than having to create and maintain a custom GRDB-SQLCipher fork that enables GRDBCIPHER, a consumer of this package can simply enable the GRDBCIPHER trait when adding a dependency on GRDB:

    dependencies: [
        .package(url: "https://github.com/groue/GRDB.swift", from: "7.5.0", traits: ["GRDBCIPHER"])
    ]

Adding the trait to the dependency will activate the GRDBCIPHER conditionals as well as adding a conditional dependency on the swift-sqlcipher repository, whereas leaving the trait empty (the default) will result in GRDB continuing to use the vendored SQLite3.

For the purpose of testing and CI, the "GRDBCIPHER" environment variable is still checked, but only to see whether to enable the trait by default in the Package.swift, so you can run:

# test with SQLCipher
GRDBCIPHER=1 swift test 
# test with SQLite3
GRDBCIPHER=0 swift test 

The performance test environment can also still be used, which demonstrates that the custom SQLCipher build continues to be ~10% faster than the vendored SQLite3, at least on my machine:

# SQLCipher performance tests
GRDBCIPHER=1 GRDB_PERFORMANCE_TESTS=1 swift test -c release --filter GRDBPerformanceTests 
# Executed 23 tests, with 0 failures (0 unexpected) in 73.005 (73.009) seconds

# SQLite3 performance tests
GRDBCIPHER=0 GRDB_PERFORMANCE_TESTS=1 swift test -c release --filter GRDBPerformanceTests 
# Executed 23 tests, with 0 failures (0 unexpected) in 81.398 (81.401) seconds

The only drawback is that since traits are simple booleans, you can no longer specify a custom repository with the "GRDBCIPHER" environment variable, so you need to depend on a specific repository. But an advantage is that the swift-sqlcipher.git repository could now publish its own traits that will control various SQLite build flags, which means that consumers of the package could potentially have fine-grained control over the SQLite build itself.

@marcprux marcprux changed the title Rework import order for SQLCipher; add test conditionals for Android Support SQLCipher using package traits Apr 23, 2025
@marcprux
Copy link
Author

@groue Hoping you can take another look at this PR with the new Package traits dependency. I think it really improves on the original idea.

@marcprux
Copy link
Author

We need this for our work, so I'll maintain GRDB with SQLCipher support in our fork at https://github.com/swift-everywhere/grdb-sqlcipher.git and track upstream releases, starting with v7.5.0.

@aehlke
Copy link

aehlke commented Aug 29, 2025

@groue We'd love to see this get merged. It would be very useful for using with Skip and other native Android Swift usage.

It would also let us use SharingGRDB on Android which would be an incredible cross-platform story. GRDB is the only part needed for that.

Is it waiting on you finding time for review or are there further changes needed? Thank you

@groue
Copy link
Owner

groue commented Sep 10, 2025

Hello, sorry for being late to the party. I'm slow on those topics because I'm really not excited by build topics.

Let's discuss traits!

The Trait unification chapter of the proposal says:

At this point, it is important to talk about the trait unification across the entire dependency graph. After dependency resolution the union of enabled traits per package is calculated. This is then used to determine both the enabled optional dependencies and the enabled traits for the compile time checks. Since the enabled traits of a dependency are specified on a per package level and not from the root of the tree, any combination of enabled traits must be supported. A consequence of this is that all traits should be additive. Enabling a trait should not disable functionality i.e. remove API or lead to any other SemVer-incompatible change.

A SQLCipher trait can not be 100% compatible with the SQLite version that ships on Apple operating systems (the default trait on Apple OS). We just have to wait for the two SQLite version to ship with a different set of compile options, or for the operating system to ship a newer SQLite version with more features.

In case the problem is not clear, we just need:

  • An application that depends on Dependency1 and Dependency2
  • Dependency1 depends on GRDB(standard) and uses an API X (does not need to be a GRDB API, it can be a SQLite C function, or an SQL syntax).
  • Dependency2 depends, or starts depending on GRDB(+SQLCipher)

With bad luck, API X does not work with SQLCipher, and has no replacement (linker error, runtime error, whatever, SOME ERROR). The application owner complains to the Dependency1 maintainers, who say they're not responsible (they are correct). Owner complains to Dependency2, who say they require SQLCipher (perfectly legit). What are the options of the application owner? All painful. They are entitled to turn back to this repository, complaining that this is a misuse of package traits (and they would be correct: GRDB should not have exposed SQLCipher as a trait, to reuse the emphasized terminology of the proposal).

And I'm not even talking about the import SQLite3 that the app and Dependency1 are entitled to use, with bad consequences whenever SQLCipher enters the picture.

Thoughts?

@marcprux
Copy link
Author

A diamond dependency pattern does indeed have the potential to introduce merged trait conflicts. But I envision that the preference for which SQLite runtime to use (vendored SQLite3 vs. custom SQLCipher) would be something generally decided at the application level, and so the trait declarations would be pushed up as high as possible. Dependency1 and Dependency2 ought not mandate any particular compile options, but instead adapt to the SQLite runtime in use, such as by introspecting on pragma COMPILE_OPTIONS (and potentially throwing a runtime error if there is an unsupported operation). E.g.:

func encryptDatabase() throws {
    if try exec("pragma COMPILE_OPTIONS").contains("SQLCIPHER") == false { 
        throw RuntimeError("Unsupported database configuration…") 
    } 
}

I'm not sure how many middleware packages like this are out there currently, other than sharing-grdb. If you know of any, it would be interesting to examine them to see if they presume any particular compile options.

Also, I'll note that the potential issue of compile option mismatch already exists on Linux: the .apt(["libsqlite3-dev"]) dependency in use will likely have different compile options on different distributions, and these won't generally match the exact compile options on Apple platforms (which aren't even themselves consistent: macOS and iOS have different compile options, and I wouldn't be surprised if tvOS, visionOS, etc. also have mismatches). And lastly, these options can and do change in between releases. So, in short, I think it makes sense to advocate for defensive programming and validating any compile option assumptions with a check against the runtime COMPILE_OPTIONS.

And I'm not even talking about the import SQLite3 that the app and Dependency1 are entitled to use, with bad consequences whenever SQLCipher enters the picture.

Yes, well, that would have to be discouraged. If GRDB were to re-export the SQLite3 or SQLCipher module, then a dependent package ought to just be able to pick up the symbols from there without needing to ever import SQLite3. This is somewhat orthogonal to the traits technique I'm suggesting; any world in which GRDB might depend on SQLCipher vs. SQLite3 — regardless of how it is implemented — will have this issue.

@groue
Copy link
Owner

groue commented Sep 11, 2025

Thanks @marcprux for your careful consideration. This is well appreciated.

What I envision is that users wildly exploit any possible technique. That's my rule of thumb, and it's been quite efficient so far.

I do not assume that the preference will be decided at the application level. It is reasonable to imagine a third-party library that encrypts its private data and sets the SQLCipher trait.

We do not have to wonder if conflicts can happen: they will. And runtime checks can't fix linker errors.

The correct attitude is vigilant optimism - and probably a fair amount of advice and empathy in the documentation.


On the topic of exposing the C SQLite APIs, which is an important GRDB feature - several people depend on it:

If GRDB were to re-export the SQLite3 or SQLCipher module, then a dependent package ought to just be able to pick up the symbols from there without needing to ever import SQLite3.

That's what GRDB was doing until GRDB 7. There were too many reports of build errors, so I shipped #1600, forcing users to write explicit imports of their desired SQLite flavor. Since then, the flow of build error reports has nearly stopped. As far as I can tell, the root cause is probably an Xcode bug.

What I mean here is that I can't just re-export and call it a day. In an ideal world, this would work. But it does not, for reasons I can't address, and I'm unable to report (the error does happen, I saw it, but it is very difficult to reproduce, and it's not only a matter of cleaning caches).


Finally, there's the "official" SQLCipher package, on which you have made several comments. If I understand well, the big problem is that a binary distribution would not work on all platforms, and that's why you suggest a source distribution. I agree with you.

Why did you pick https://github.com/skiptools/swift-sqlcipher? What are the other options? If there's a bias due to your focus on Android, would you please make it explicit, and describe the consequences of this choice for other platforms?


So… Using package traits for SQLCipher will create a new flow of issues.

And this is not a subject I know enough about. I do not want open issues to pile up, especially when the issue lies elsewhere, and I'm unable to prove it. I could move all those issues to discussions (some repos do that in order to keep the number of issues small), but that does not address anything. Finally I'm slow to address those topics, as everybody can see.

In the interest of all, I wish someone would volunteer for the position of “build support” in GRDB.

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.

5 participants