Updated Fixes for the Linux version of GRDB
Fixes for the Linux version of GRDB
UPDATE this PR merges some upstream changes into my fork.
This PR contains some fixes for the Linux version of GRDB, mainly these fixes include:
- Fixing linker errors on Linux
- Some fixes for differences between
Foundationon Linux and Apple platforms - Fixing the unittests and making them succeed on Linux
- Ignoring Apple platform specific functionality like
CombineorNSFileCoordinator, etc.
Some of these fixes are inspired by #1708 by @marcprux. This PR attemps to make these fixes in
a platform-agnostic way so that GRDB will build with SPM on more platforms than Linux alone,
for instance, on Android and Windows. These patches fix the build of GRDB on Swift 6.1 and later.
There are still some issues building on Swift 6.0, which I will attempt to fix later.
Fixing the GRDB Build
Most of GRDB actually builds on Linux just fine. The only change is that I needed to do to make it build was adding a
@preconcurrency import Dispatch
to handle non-Sendable protocol conformance in GRDB/Core/DispatchQueueActor.swift. Not sure if that's the right approach but it made everything build!
Fixing GRDB linking on Linux
The main issue with GRDB on Linux is actually a linker error. The standard libsqlite3-dev installed with apt or dnf doesn't have WAL enabled and when trying to use GRDB in another swift package or app in Linux, you get a linker error. Functions like sqlite3_snapshot_get, sqlite3_snapshot_open are not found by the linker,
and therefore the package isn't usable.
There would be two ways around it I think, either:
- include a custom
sqlitebuild forGRDBon Linux, or - fix linking by adding extra compiler directives/conditions for Linux
I chose the latter for now as it is a more easy fix. I updated the following conditional compilation directive:
#if SQLITE_ENABLE_SNAPSHOT || (!GRDBCUSTOMSQLITE && !GRDBCIPHER)
to:
#if (SQLITE_ENABLE_SNAPSHOT || (!GRDBCUSTOMSQLITE && !GRDBCIPHER)) && !os(Linux)
This is a quick fix, but it would be better to change the SQLITE_ENABLE_SNAPSHOT to return false on Linux.
Note this should be better handled as a fix to SQLITE_ENABLE_SNAPSHOT as later on Linux may also provide
a custom sqlite build to enable WAL. In those cases, the code inside these blocks should be compiled.
This compiler directive is present (and updated) in the following files:
GRDB/Core/DatabasePool.swiftGRDB/Core/DatabaseSnapshotPool.swiftGRDB/Core/WALSnapshot.swiftGRDB/Core/WALSnapshotTransaction.swiftGRDB/ValueObservation/Observers/ValueConcurrentObserver.swift
Updating the compiler directive as above will fix the build with the standard libsqlite3-dev on Linux. If that's fine we
can keep it at that. I'm not completely sure yet what WAL does but if I understand correctly, using it is mostly a
performance consideration. If it would be a nice to have on Linux too, I can research adding a custom sqlite build for
Linux with WAL enabled. I wonder why it is not included in the standard libsqlite3-dev though? Adding the changes above
will fix linking of the GRDB code.
Fixes for open source Foundation versions
Open source versions of Foundation are slightly different than the Foundation Apple bundles with XCode. Also, some
packages, like for instance CoreGraphics, are not available on non-Apple platforms. Some primitive types like CGFloat
are available on non-Apple platforms and are defined in Foundation. In some cases, there are also differences where a
initializer is defined as convenience init in open source Foundation and as a normal init in Apple Foundation. This
causes some problems with Apple specific code in some cases; it will fail to build on open source Foundation. In GRDB in
many cases this code is surrounded by a #if !os(Linux) compiler directive. The fixes below try to solve the build issues on
Linux and try to get rid of these compiler directives. Also fixing the unit tests at the same time.
CGFloat.swift
CoreGraphics is not available on Linux. But CGFloat is available via Foundation. Changed the compiler directive in CGFloat.swift and its test from:
#if canImport(CoreGraphics)
to:
#if canImport(CoreGraphics)
import CoreGraphics
#elseif !os(Darwin)
import Foundation
#endif
The same fix should be added to GRDBTests/CGFloatTests.swift.
Decimal.swift
Removed the #if !os(Linux) compiler directive in the Decimal extension, as it compiles fine under swift 6. As discussed
with @groue the minimum version for GRDB is swift 6.0. I tested these changes with swift 6.1 and swift 6.2 and they work fine.
Note: swift 6.0 doesn't build, we should look into fixing the build for this version.
NSString, NSDate, Date
Some foundation NS* types (NSString, NSData, Date, etc.) were excluding the DatabaseValueConvertible extensions on Linux but build fine on swift 6.1 and swift 6.2.
Removed #if !os(Linux) in:
NSString.swiftNSData.swiftDate.swift
NSNumber
There's a problem with the following code in NSNumber.swift on Linux:
public static func fromDatabaseValue(_ dbValue: DatabaseValue) -> Self? {
switch dbValue.storage {
case .int64(let int64):
return self.init(value: int64)
case .double(let double):
return self.init(value: double)
case let .string(string):
// Must match Decimal.fromDatabaseValue(_:)
guard let decimal = Decimal(string: string, locale: posixLocale) else { return nil }
return NSDecimalNumber(decimal: decimal) as? Self
default:
return nil
}
}
both the .int64 and .double case above do not compile. The error is the following:
error: constructing an object of class type 'Self' with a metatype value must use a 'required' initializer
The NSNumber(value: Int64) and NSNumber(value: Double) are marked as convenience init on Linux. This may cause the build to fail. The issue with this approach is that it doesn't work for NSDecimalNumber, which is a subclass of NSNumber.
It looks like the main issue to begin with is that NSNumber defines NSNumber(value: Int64) and NSNumber(value: Double as convenience init, whereas their NSDecimalNumber equivalents are non-convenience. I could solve it by changing the code
above like this:
public static func fromDatabaseValue(_ dbValue: DatabaseValue) -> Self? {
switch dbValue.storage {
case .int64(let int64) where self is NSDecimalNumber.Type:
let number = NSDecimalNumber(value: int64)
return number as? Self
case .int64(let int64):
let number = NSNumber(value: int64)
return number as? Self
case .double(let double) where self is NSDecimalNumber.Type:
let number = NSDecimalNumber(value: double)
return number as? Self
case .double(let double):
let number = NSNumber(value: double)
return number as? Self
case .string(let string):
// Must match Decimal.fromDatabaseValue(_:)
guard let decimal = Decimal(string: string, locale: posixLocale) else { return nil }
return NSDecimalNumber(decimal: decimal) as? Self
default:
return nil
}
}
}
This is a lot more verbose than the existing code. Also, this code may fail if there are other subclasses of NSNumber
that need to be supported. For each of these classes we should add a case for .int64 and double. I think an alternative
approach would be better, but I don't have one at the moment.
All tests in:
Tests/GRDBTests/FoundationNSNumberTests.swiftTests/GRDBTests/FoundationNSDecimalNumberTests.swift
succeeded with these changes.
URL.swift
The NSURL is a specific case. Apparently, the NSURL absoluteString property is non-optional on Linux and optional on Apple platforms. The fix is the following:
/// Returns a TEXT database value containing the absolute URL.
public var databaseValue: DatabaseValue {
#if !os(Darwin)
absoluteString.databaseValue
#else
absoluteString?.databaseValue ?? .null
#endif
}
UUID.swift
UUID.swift contains a compiler directive excluding the DatabaseValueConvertible from building
on Linux and Windows. On Linux the problem is the following:
public static func fromDatabaseValue(_ dbValue: DatabaseValue) -> Self? {
switch dbValue.storage {
case .blob(let data) where data.count == 16:
return data.withUnsafeBytes {
self.init(uuidBytes: $0.bindMemory(to: UInt8.self).baseAddress)
}
case .string(let string):
return self.init(uuidString: string)
default:
return nil
}
}
The NSUUID(uuidBytes:) constructor takes an optional UnsafePointer<UInt8>, see here, whereas this constructor on Linux takes a non-optional.
This could be fixed with a guard statement. However, similarly to the case for NSNumber in addition, the
self.init(uuidString: string) doesn't compile on Linux. The error is:
Constructing an object of class type 'Self' with a metatype value must use a 'required' initializer
Added the following fix:
public static func fromDatabaseValue(_ dbValue: DatabaseValue) -> Self? {
switch dbValue.storage {
case .blob(let data) where data.count == 16:
return data.withUnsafeBytes {
#if canImport(Darwin)
self.init(uuidBytes: $0.bindMemory(to: UInt8.self).baseAddress)
#else
guard let uuidBytes = $0.bindMemory(to: UInt8.self).baseAddress else {
return nil as Self?
}
return NSUUID(uuidBytes: uuidBytes) as? Self
#endif
}
case .string(let string):
return NSUUID(uuidString: string) as? Self
default:
return nil
}
}
This also makes all unittests succeed. It should be checked whethere we would want to support any subclass of NSUUID.
If so, a similar solution as for NSNumber above can be used.
Use of DispatchQueue private APIs
In some of the tests for the DispatchQueue APIs, some private DispatchQueue apis are used to get the label of the
current DispatchQueue. These APIs are unavailable on non-Darwin platforms. It's mainly about some tests in for example DatabasePoolConcurrencyTests.swift. For instance, the following test:
func testDefaultLabel() throws {
let dbPool = try makeDatabasePool()
dbPool.writeWithoutTransaction { db in
XCTAssertEqual(db.configuration.label, nil)
XCTAssertEqual(db.description, "GRDB.DatabasePool.writer")
// This test CAN break in future releases: the dispatch queue labels
// are documented to be a debug-only tool.
let label = String(utf8String: __dispatch_queue_get_label(nil))
XCTAssertEqual(label, "GRDB.DatabasePool.writer")
I understood from @groue that the goal of those tests is to make sure that the Xcode, LLDB, and crash reports mention the label of the current database in the name of the current DispatchQueue. This is a nicety that aims at helping debugging, and worth a test. The label of the current dispatch queue can only be tested with the private __dispatch_queue_get_label api, which is only available on Darwin systems.
Those tests can be avoided on non-Darwin platforms, as @marcprux did in #1708:
#if canImport(Darwin) // __dispatch_queue_get_label unavailable on non-Darwin platforms
// This test CAN break in future releases: the dispatch queue labels
// are documented to be a debug-only tool.
let label = String(utf8String: __dispatch_queue_get_label(nil))
XCTAssertEqual(label, "GRDB.DatabasePool.writer")
#endif
I implemented this approach for:
Utils.swiftDatabasePoolConcurrencyTests.swiftDatabaseQueueTests.swiftDatabaseSnapshotTests.swift
NSError
Bridging of DatabaseError to NSError does not seem to work on Linux. The testNSErrorBridging therefore
fails, we should probably ignore the test on non-Darwin platforms as NSError is mostly a left-over from ObjectiveC times
and APIs.
func testNSErrorBridging() throws {
#if !canImport(Darwin)
throw XCTSkip("NSError bridging not available on non-Darwin platforms")
#else
let dbQueue = try makeDatabaseQueue()
try dbQueue.inDatabase { db in
try db.create(table: "parents") { $0.column("id", .integer).primaryKey() }
try db.create(table: "children") { $0.belongsTo("parent") }
do {
try db.execute(sql: "INSERT INTO children (parentId) VALUES (1)")
} catch let error as NSError {
XCTAssertEqual(DatabaseError.errorDomain, "GRDB.DatabaseError")
XCTAssertEqual(error.domain, DatabaseError.errorDomain)
XCTAssertEqual(error.code, 787)
XCTAssertNotNil(error.localizedFailureReason)
}
}
#endif
}
Fixing Unit tests
Tests using NSFileCoordinator
There are some tests using NSFileCoordinator which is not available on non-Darwin platforms. Most notably
testConcurrentOpening in DatabasePoolConcurrencyTests.swift contributes 400+ test failures when run on
Linux. testConcurrentOpening, that runs about 500 tests (a for loop of 50 iterations runs 10 times some things in parallel
with DispatchQueue.concurrentPerform. The test uses the NSFileCoordinator which isn't available on Linux.
These tests are ignored because they test whether NSFileCoordinator works with GRDB. This is Darwin specific functionality:
We're talking about
testConcurrentOpening, right? The goal ofDispatchQueue.concurrentPerformis just to spawn parallel jobs. In this case each job opens aDatabasePool, protected by aNSFileCoordinator, and we're asserting that not error is thrown. This is a test for the correct integration ofGRDBandNSFileCoordinator, so that we can confidently recommend usingNSFileCoordinatoras a way to protect a database that is shared between multiple processes.
We could say that those tests should be removed, arguing that they are just asserting the behavior of
NSFileCoordinatorand that "testing the framework" is a bad practice - but we're also asserting thatDatabasePoolevolution must be compatible with user code that usesNSFileCoordinatorand that any deadlock must be prevented.)
Arguably DispatchQueue.concurrentPerform could be replaced with any other modern technique that guarantees a minimum level of parallelism, such as withTaskGroup.
We should check whether we need to test similar behaviour on Linux. For now we just ignore the test. @marcprux solved it like this:
func testConcurrentOpening() throws {
#if !canImport(ObjectiveC)
throw XCTSkip("NSFileCoordinator unavailable")
#else
We solved it similarly.
Other Unit tests
XCTIssueis not available on non-Darwin platforms, it's used in:GRDBTests/FailureTestCase.swiftignored this test completely with#if os(Darwin)on non-Darwin platformsGRDBTests/ValueObservationRecorderTests.swiftignored this test also.
Support.swiftactually needsCombine, so all tests that use it needCombineas well:DatabaseMigratorTests.swifthas several tests using theTestclass defined inSupport.swift; used an#if !canImport(Combine)clause to skip several tests in this test suite.ValueObservationTests.swiftalso uses thisTestclass
DatabaseRegionObservationTests.swifttests someCombinefunctionality intestAnyDatabaseWriter; also wrapped them in a#if canImport(Combine)- When integrating the upstream changes I had some conflicts in
DatabaseMigratorTests.swiftI tried to merge them but not sure everything went ok. I was struggling a bit integrating them. This file should be checked if all upstream changes have been merged in.
TODOs
- [ ] Fix the build under
swift 6.0 - [ ] Check if we can provide an alternative for
XCTIssue - [ ] See if we can find an alternative for
__dispatch_queue_get_labelon Linux - [ ] Setup a
CI.ymlpipeline to build the Linux version of the package automatically and run the unit tests
With this we:
Executed 2768 tests, with 46 tests skipped and 0 failures (0 unexpected) in 234.863 (234.863) seconds
I did an initial scan, and my initial takeaways:
- Great work!
- The reformatting and whitespace changes (probably resulting from a linter run) make this challenging to review. There's probably not much that can be done about it at this point, though.
- I think that Linux should added to the CI matrix in
.github/workflows/CI.yml.
I did an initial scan, and my initial takeaways:
1. Great work!
Thanks!
2. The reformatting and whitespace changes (probably resulting from a linter run) make this challenging to review. There's probably not much that can be done about it at this point, though.
Actually, I tried to check this with SwiftLint but I saw a lot of linting warnings in code I hadn't touched too. So was not sure how I should proceed. How can I improve this?
3. I think that Linux should added to the CI matrix in `.github/workflows/CI.yml`.
That would be nice, I discussed this with @groue earlier, but he wanted to wait a bit with this. It would be good in the long run though. If the build fails I could check it and fix it faster. Well let's see :wink:
Hello @thinkpractice,
That's huge, and I'm so thankful π₯³
It's a pity that the diff messed up with white space π¬. The git blame will be unusable, and code archeology will become very difficult. I'm very sorry but I have to ask you to redo all your commits and remove all white space changes.
~Fortunately I can review the changes by asking GitHub UI to hide the white space changes in the diff view π~
[EDIT] It looks like you actually applied a code formatter. Again, I'm very sorry, but this is not mergeable. And I can not review in correct conditions with so many unrelated changes. I hope VSCode can be configured so that it does not mess up with existing files.
Let's not have code formatting pollute the review. So I looked at SQLITE_ENABLE_SNAPSHOT. A pull request is on the way.
@groue No worries! Ughh this is due to my inexperience contributing to open source projects like this :disappointed: (and part of my learning curve). It would help me a ton if you give me some pointers as to what your setup according to code formatting is. I think my main mistake is not disabling swift-format for this project, I will investigate how.
The other thing is configuring SwiftLint. When I ran it yesterday on my PR (and also on the original repo) SwiftLint threw a ton of errors, so I must be doing something wrong. I'll try to fix the formatting, but I'm not sure how much time I will have this weekend. Maybe in the train to Leeds :wink:
Looking forward to your SQLITE_ENABLE_SNAPSHOT changes, when the PR arrives I can try them on Linux :smile:
The pull request for snapshots: https://github.com/groue/GRDB.swift/pull/1826
@groue No worries! Ughh this is due to my inexperience contributing to open source projects like this π (and part of my learning curve).
π I hope I did not sound too bad at you! As I said above, the real problem is the git blame.
It would help me a ton if you give me some pointers as to what your setup according to code formatting is. I think my main mistake is not disabling
swift-formatfor this project, I will investigate how.
There is no formatting spec such as an .editorconfig in the project, because .editorconfig can not be made compatible with the default Xcode formatting. As for Swift formatters, I also think none of them can be made compatible with the default Xcode formatting. Yeah people can be pretty much opinionated.
My best suggestion is to disable all code formatting tools, and to control what you commit.
The other thing is configuring
SwiftLint. When I ran it yesterday on my PR (and also on the original repo)SwiftLintthrew a ton of errors, so I must be doing something wrong. I'll try to fix the formatting, but I'm not sure how much time I will have this weekend. Maybe in the train to Leeds π
I don't think SwiftLint is invoked from SPM. I suppose you can ignore it for now?
Looking forward to your
SQLITE_ENABLE_SNAPSHOTchanges, when the PR arrives I can try them on Linux π
I hope #1826 helps! π€
@groue No worries! Ughh this is due to my inexperience contributing to open source projects like this π (and part of my learning curve).
π I hope I did not sound too bad at you! As I said above, the real problem is the git blame.
Nah it was really fine. To be honest I didn't even think about the code formatting being an issue. Then when @marcprux told me yesterday that code reviewing would be a challenge I realised that the git diffs would be messed up because of that. My mistake! If I cannot fix it on this PR then I will just redo the changes and submit it again. Hoping the third time's a charm!
It would help me a ton if you give me some pointers as to what your setup according to code formatting is. I think my main mistake is not disabling
swift-formatfor this project, I will investigate how.There is no formatting spec such as an
.editorconfigin the project, because.editorconfigcan not be made compatible with the default Xcode formatting. As for Swift formatters, I also think none of them can be made compatible with the default Xcode formatting. Yeah people can be pretty much opinionated.
Oh, I just downloaded a vscode EditorConfig extension in the hope that it would take the .editorconfig in the root of the repo into account. Do I then understand correctly that I should ignore it completely?
My best suggestion is to disable all code formatting tools, and to control what you commit.
The other thing is configuring
SwiftLint. When I ran it yesterday on my PR (and also on the original repo)SwiftLintthrew a ton of errors, so I must be doing something wrong. I'll try to fix the formatting, but I'm not sure how much time I will have this weekend. Maybe in the train to Leeds πI don't think SwiftLint is invoked from SPM. I suppose you can ignore it for now?
Looking forward to your
SQLITE_ENABLE_SNAPSHOTchanges, when the PR arrives I can try them on Linux π
I found out that you have yml for SwiftLint in the Scripts directory. This works and shows only 2 errors on the branch that I'm using. But for now I will just disable all code formatting and check it before submitting.
Oh, @thinkpractice, I certainly do not want to sound like I am "ableist" regarding the tooling. If you have difficulties with formatting, this is quite OK: I will perform the necessary cleanup. Please do not feel embarrassed! This PR is such a boon π
I'm mostly trying to get my head around some of the git commands related to PRs. I'm used to using branches but not really use to PR. Trying to figure out how to commit changes to this PR for instance?
UPDATE just found out that I can just change my forked branch and the changes will be reflected here DUH. Well that's really awesome! Will go and check how to get the formatting in a good shape again :)
@groue how do you use git blame to see my changes? And I guess you only want to see the lines I changed right? I'm not sure what would be faster... changing back each file or applying each change I made on a fresh fork. I finally got my head around working with PRs so this time it should be fine :)
@groue how do you use
git blameto see my changes?
To see your changes, I just use the GitHub interface, that's OK. I do not use git blame at this stage.
git blame is useful when one wonders why the code is like it is, and wants to better understand. The blame helps finding which commits have changed some code, and this greatly help performing "code archaeology" - but only when the commits are meaningful. GRDB is old, more than 10 years old now. Code archaeology is necessary to help my aging memory.
EDIT: So a good practice is to strictly distinguish commits that format from commit that change code meaningfully. This way, archeology is still possible: formatting commits can be ignored, and commits that change code meaningfully have no noise.
This rule is not an absolute, of course. But here all thresholds have been crossed ;-)
@groue as I trial I tried to reformat Core/Support/Foundation/Decimal.swift but if I now look at commit diff here in GitHub I'm seeing the changes in Decimal.swift as related to my earlier changes. I guess this is not what you want to see, but you would like to see the changes as related to your original code... Is there a way to get back to that besides starting of a fresh fork+branch and redoing the changes?
EDIT: I have just discovered the Files Changes tab as opposed to the commits view (sorry for being such a GitHub noob) and seeing that that integrates my changes. Will experiment a bit if that makes cleaning up easier. I'm seeing now what you and @marcprux mean by the code-formatter making it a challenge to review!
@groue I found out there's some weird behaviour with my vscode. One time tabs are converted to spaces, and then another indentation which was 4 spaces gets converted back to a tab :angry: Will need to check how to solve this. Already changed my editor settings, so either I missed some setting or it's a bug in vscode....
Found it apparently there is something like "editor.trimAutoWhitespace" in vscode which removes "trailing auto inserted whitespace"
@groue I have been updating some more files to reflect the original formatting and make it easier for me to see my changes. Before these changes would be synced to this PR, but now I'm not seeing them anymore. Is this something that sound familiar to you? I have already tried the solution mentioned here, i.e. updating the base to the same branch and saving it. So far, this hasn't helped, and if I try it again I'm getting an error saying the base is already being updated. I will check later if I can see the changes, but if they don't show up, do you have any tips?
UPDATE some of my changes are starting to show up... so I guess I it's working again. Let's see if the other changes I made later will show up as well!
@thinkpractice I have pushed on the groue:grdb_linux_changes branch all changes of your own branch, fully rebased on the latest development branch, with all code formatting removed. I took care that all commits are attributed to you.
I had to heavily amend your initial changes, so I hope I haven't broken or forgotten anything.
If this branch builds as well as yours on Linux, and if you agree, maybe you could force-push this branch to yours? That will make the review, and future changes, much easier.
@groue did you see I spend some time cleaning up the code as well? Hope you could use that. Thanks for taking care of the rest! Will take care to format my code in the right way on any future changes. I am fine with force pushing the changes to my branch if that makes things easier. I am still traveling so I cannot really test your changes now (only brought my MacBook). I will be home next weekend so then I can test everything. Hope thatβs ok!
did you see I spend some time cleaning up the code as well? Hope you could use that.
Yes, thanks for your efforts! Now starting over from the development branch was the best way to reach the ideal minimal diff.
Will take care to format my code in the right way on any future changes.
I hope .vscode/settings.json solves this for good, yes.
I am fine with force pushing the changes to my branch if that makes things easier. I am still traveling so I cannot really test your changes now (only brought my MacBook). I will be home next weekend so then I can test everything. Hope thatβs ok!
If you look at the new commits, you can see that they perform minimal changes. They do indeed make things much easier. I just don't know if I faithfully reported all your changes. So please take your time, check this branch on your Linux system, and make sure I did not forget anything! π
@groue checked out your branch and it compiled successfully! Also the test project compiled and ran and we now have:
Executed 2771 tests, with 46 tests skipped and 0 failures (0 unexpected) in 236.873 (236.873) seconds
So it looks like we have the build working on Linux again. I will merge the changes you made into my branch and then hopefully we can merge. Do you think whether there would be any additional things I can do to improve the support on Linux? I'm thinking along the lines of adding some equivalent tests for the Darwin specific tests we commented out, adding a custom sqlite build for WAL, adding a CI for the linux build (can we have separate tags to indicate build failures on different platforms like the package index does?), or maybe investigating the new swift container framework so we can also test the Linux build on macOS?
Welcome back, @thinkpractice -:
@groue checked out your branch and it compiled successfully! Also the test project compiled and ran and we now have:
Executed 2771 tests, with 46 tests skipped and 0 failures (0 unexpected) in 236.873 (236.873) seconds
You can be proud :-)
So it looks like we have the build working on Linux again. I will merge the changes you made into my branch and then hopefully we can merge.
If you do not mind, please force-push my branch to yours instead, so that only "my" commits are in your branch, and only them. This will remove from the git history all the formatting and de-formatting commits.
[EDIT]: I'll help you if this is not quite clear - please just tell.
Do you think whether there would be any additional things I can do to improve the support on Linux? I'm thinking along the lines of adding some equivalent tests for the Darwin specific tests we commented out, adding a custom
sqlitebuild for WAL, adding a CI for the linux build (can we have separate tags to indicate build failures on different platforms like the package index does?), or maybe investigating the new swift container framework so we can also test the Linux build on macOS?
I'm looking forward for all the goos ideas you will suggest :-) For now, let's merge this. In parallel, there was some work in progress regarding SQLCipher, in #1827, in which a serious issue was discovered - it slipped in the development branch while working on SQLITE_ENABLE_SNAPSHOT and package traits. I'll have to deal with this before everything else. Fortunately, I do not think this has any impact on your pull request. It's just that there are several topics to deal with, and I'll address them one after the other.
I'm really curious to see what will be shipped in the next release :-)
Welcome back, @thinkpractice -:
Thanks! Good to be back :) Although I had a blast at SwiftLeeds and in the UK in general :smile:
@groue checked out your branch and it compiled successfully! Also the test project compiled and ran and we now have:
Executed 2771 tests, with 46 tests skipped and 0 failures (0 unexpected) in 236.873 (236.873) secondsYou can be proud :-)
Thanks! Happy it was relatively easy to get it to run, thanks for all the prior work done :smile:
So it looks like we have the build working on Linux again. I will merge the changes you made into my branch and then hopefully we can merge.
If you do not mind, please force-push my branch to yours instead, so that only "my" commits are in your branch, and only them. This will remove from the git history all the formatting and de-formatting commits.
[EDIT]: I'll help you if this is not quite clear - please just tell.
Yeah would prefer some help. So I have your repo as upstream so I guess it would be something like this?
git fetch upstream
git reset --hard upstream/grdb_linux_changes
Would this do the job?
I'm looking forward for all the goos ideas you will suggest :-) For now, let's merge this. In parallel, there was some work in progress regarding SQLCipher, in #1827, in which a serious issue was discovered - it slipped in the
developmentbranch while working onSQLITE_ENABLE_SNAPSHOTand package traits. I'll have to deal with this before everything else. Fortunately, I do not think this has any impact on your pull request. It's just that there are several topics to deal with, and I'll address them one after the other.
I think I will check first if I can get a CI for Linux to work, parallel to the macOS one. This would allow us to identify whether any of the changes made would break on either platform. This would also not interfere with the work you are currently doing on SQLCipher. If I could help there, just let me know :smile: After that, I'm thinking I will start evaluating GRDB on Linux and see if any issues pop up... If the SQLCipher part works, I think we will be able to use that work to make custom sqlite builds on linux too. That would be than my next focus.
I'm really curious to see what will be shipped in the next release :-)
That makes two of us! Looks like there are a couple of exciting changes coming up!
@groue just in case you missed it. Is this the right way to apply your changes to my branch?
git fetch upstream
git reset --hard upstream/grdb_linux_changes
If so then I can merge and fix the remaining issues this week :smile:
Hi @thinkpractice, sorry I had a busy weekend. OK so let's go:
My current remotes are origin for groue/GRDB.swift, and R4N because I follow #1827:
9:35 groue@Renart ~/Documents/git/groue/GRDB.swift % git remote -v
R4N [email protected]:R4N/GRDB.swift.git (fetch)
R4N [email protected]:R4N/GRDB.swift.git (push)
origin [email protected]:groue/GRDB.swift.git (fetch)
origin [email protected]:groue/GRDB.swift.git (push)
Let's add a remote for your own fork, where I can find your branch grdb_linux_changes:
9:35 groue@Renart ~/Documents/git/groue/GRDB.swift % git remote add thinkpractice [email protected]:thinkpractice/GRDB.swift.git
Let's checkout your branch grdb_linux_changes:
9:41 groue@Renart ~/Documents/git/groue/GRDB.swift % git fetch thinkpractice
9:41 groue@Renart ~/Documents/git/groue/GRDB.swift % git checkout -b grdb_linux_changes --track thinkpractice/grdb_linux_changes
branch 'grdb_linux_changes' set up to track 'thinkpractice/grdb_linux_changes'.
Switched to a new branch 'grdb_linux_changes'
Is it what we expect? Yes it is. These are your latest commits:
9:42 groue@Renart ~/Documents/git/groue/GRDB.swift % git log -2
commit 2ddf51026ef0b739b7ea911e6a1417e3266969c2 (HEAD -> grdb_linux_changes, thinkpractice/grdb_linux_changes)
Author: Tim De Jong <[email protected]>
Date: Fri Oct 10 17:34:12 2025 +0100
Restored some of the original indentation
commit 1785f63cce380972bcd8c702048571506ce9e838
Author: Tim De Jong <[email protected]>
Date: Fri Oct 10 17:29:28 2025 +0100
Restored code layout messed up by swift-format.
Let's reset its state to my branch grdb_linux_changes:
9:43 groue@Renart ~/Documents/git/groue/GRDB.swift % git reset --hard origin/grdb_linux_changes
HEAD is now at 3dd3bbf0c vscode config
Is it what we expect? Yes it is. These are my latest commits:
9:43 groue@Renart ~/Documents/git/groue/GRDB.swift % git log -2
commit 3dd3bbf0c5ab23fb8576f380d9a5564bc83ca846 (HEAD -> grdb_linux_changes, origin/grdb_linux_changes)
Author: Tim De Jong <[email protected]>
Date: Sun Oct 12 16:43:31 2025 +0200
vscode config
commit d7cf0f9e59c46054d1dcbbe1b145c19c8ed8de30
Author: Tim De Jong <[email protected]>
Date: Sun Oct 12 17:17:47 2025 +0200
NSError bridging is not available, or doesn't work the same on non-Darwin platforms. Skip testNSErrorBridging on these platforms.
We can now force-push the content of my branch to yours:
9:43 groue@Renart ~/Documents/git/groue/GRDB.swift % git push --force
Total 0 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0)
To github.com:thinkpractice/GRDB.swift.git
+ 2ddf51026...3dd3bbf0c grdb_linux_changes -> grdb_linux_changes (forced update)
And we're done:
- This pull request now only contains the rewritten commits.
- There's no trace of indentation churn anywhere, no trace at all.
- We're going to merge soon.
On your side, since I did force-push to you branch, you will not be able to pull. You will have to reset your local branch to its new remote state. The latest commit is 3dd3bbf0c5ab23fb8576f380d9a5564bc83ca846, so you'll write in your terminal:
# Grab latest commits
git fetch
# Ignore all local commits in your local grdb_linux_changes branch,
# and just apply the latest commit from the remote.
git reset --hard 3dd3bbf0c5ab23fb8576f380d9a5564bc83ca846
@groue Thanks so much for the detailed explanation. As you see I'm not that familiar with these specialized git commands, but the above helps me a lot to learn :smile: So I guess the command I showed above would've worked too but was missing the git push --force above. Will reset my branch soon and so nice that we can merge soon! I will have a look this week if I can:
- Add a CI for Linux that builds the code and runs the test
- Use the GRDB from this branch in some toy project I've been developing and see how it works in production
- If the toy project works out fine, I will start testing some more complex examples
@groue Thanks so much for the detailed explanation. As you see I'm not that familiar with these specialized git commands, but the above helps me a lot to learn π
That's fine! I frequently use an LLM to help me, or an app that puts a GUI in front of git. The hardest part is frequently to know what is even possible to do π
I will have a look this week if I can:
- Add a CI for Linux that builds the code and runs the test
- Use the GRDB from this branch in some toy project I've been developing and see how it works in production
- If the toy project works out fine, I will start testing some more complex examples
Yes, please play with it as much as you can - that's not something I can easily do!
For the CI part, I'd prefer, if it is possible, that we do not add Linux tests to the .github/workflows/CI.yml file. Instead, we should create a dedicated Linux workflow with its dedicated file. My goal here is that we can detect Linux build failures (that's what we are currently lacking), but without putting a red dot on GRDB itself β not until Linux is an "officially" supported platform.
@groue Thanks so much for the detailed explanation. As you see I'm not that familiar with these specialized git commands, but the above helps me a lot to learn π
That's fine! I frequently use an LLM to help me, or an app that puts a GUI in front of git. The hardest part is frequently to know what is even possible to do π
I agree git has a lot of possibilities and sometimes I wonder which alternative to pick. I often use a LLM too, but in this case I wanted to be better safe than sorry; to be sure I wouldn't mess up the changes we made.
I will have a look this week if I can:
- Add a CI for Linux that builds the code and runs the test
- Use the GRDB from this branch in some toy project I've been developing and see how it works in production
- If the toy project works out fine, I will start testing some more complex examples
Yes, please play with it as much as you can - that's not something I can easily do!
For the CI part, I'd prefer, if it is possible, that we do not add Linux tests to the .github/workflows/CI.yml file. Instead, we should create a dedicated Linux workflow with its dedicated file. My goal here is that we can detect Linux build failures (that's what we are currently lacking), but without putting a red dot on GRDB itself β not until Linux is an "officially" supported platform.
I will see if we can add a separate pipeline next to the Apple one. It would be especially nice if we could see when the Apple build is successful, and the Linux one is not. Just curious: what would be need to make Linux an officially supported platform?
Just curious: what would be need to make Linux an officially supported platform?
When someone commits to fix bugs and answer support requests in a reasonable delay.
This is, in practice, synonymous with:
When anyone can assume that the versions that are released over time work fine and have no known issue.
(This is basically what anyone expects from a trusted library).
Today, I do not even see when I unintentionally break the Linux build. With the CI build, I'll know it π
When I see a Linux failure, I'll try to fix it if I can. But a CI build is very slow, and it is not a real build on your machine, that you can debug. I'll thus be pretty much hindered. And pushing random commits until the CI is happy is... well no I won't do that.
I may ask for help, but what is important is that this Linux failure will not be a blocker, because it is not officially supported. I'll be able to ship a version of the library anyway.
If Linux were officially supported, I just could not ship such a broken version.
When someone commits to fix bugs and answer support requests in a reasonable delay.
This person could be me, if I knew how to put my fingers into Swift Linux builds, in the long run. But I do not run Linux, and it looks like one quickly becomes obsolete when one does not frequently swims in these waters.
And pushing random commits until the CI is happy is... well no I won't do that.
I personally wouldn't be so quick to dismiss that as an option. That's how I do things with cross-platform frameworks supporting Windows and Linux and Android. Once the underlying Linux support is in place, I wouldn't envision that breakage would be all that frequent, and it'll run in parallel with the macOS/iOS builds and likely be the fastest component of the PR's CI.
I'd recommend just plopping Linux into the CI.yml as another job, like I did at https://github.com/marcprux/GRDB.swift/actions/runs/18848586041/job/53779109250. If it becomes a burden or headache, you can always remove it.
And pushing random commits until the CI is happy is... well no I won't do that.
I personally wouldn't be so quick to dismiss that as an option. That's how I do things with cross-platform frameworks supporting Windows and Linux and Android. Once the underlying Linux support is in place, I wouldn't envision that breakage would be all that frequent, and it'll run in parallel with the macOS/iOS builds and likely be the fastest component of the PR's CI.
I think indeed it would not be breaking that often any more, but of course we don't know what future changes entail. Things have become a lot better on Linux the last couple of swift versions, code that didn't use to build now builds. Also the GRDB package was building already, the changes made to it minor, and overall I added some minor improvements for Linux. The major problem was that the package was not linking, that's why we would need to run the tests as well. That being said, I wouldn't want to disrupt the current development flow yet. I would like to see if we can do something in parallel first and "battle test" GRDB on Linux first. Question is when we have had enough testing!
I'd recommend just plopping Linux into the CI.yml as another job, like I did at https://github.com/marcprux/GRDB.swift/actions/runs/18848586041/job/53779109250. If it becomes a burden or headache, you can always remove it.
Nice! @marcprux could this be used as a start for the work I want to do adding a Linux specific pipeline?
And pushing random commits until the CI is happy is... well no I won't do that.
I personally wouldn't be so quick to dismiss that as an option.
You're right, I did not write with enough nuance. What I wanted to say is that hammering the CI is not always a solution, because the CI hides information that only a local run can show.
I'd recommend just plopping Linux into the CI.yml as another job. If it becomes a burden or headache, you can always remove it.
I understand, but no. The README shows the CI status, and I do not want it to be stuck on the red color just because I'm not hammering the CI well enough. That's why I want the Linux CI to run in a distinct workflow.
As I said above, someone has to commit to fix bugs and answer support requests in a reasonable delay, and it is unlikely I am the best choice for this role. Until then, Linux will remain community-supported, and Linux failures will not prevent new versions from shipping.