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

Linux compilation fixes #1401

Merged
merged 5 commits into from
Jul 9, 2023
Merged

Linux compilation fixes #1401

merged 5 commits into from
Jul 9, 2023

Conversation

kustra
Copy link
Collaborator

@kustra kustra commented Jun 29, 2023

Hello,

I'd like to fix the compilation errors of GRDB 6.15.x on Linux. I've read that it's not officially supported, but that one-off PRs are welcome.

This PR gets the repo to the point where GRDB compiles on Linux with SQLite 3.20.0. Additionally, please find below some of my findings while working on this.

SQLite 3.20.0 conditional compilations

Several places in the code use the following pattern.

#if GRDBCUSTOMSQLITE || GRDBCIPHER
    // new version of the code (1)
#else
    if #available(iOS 12, macOS 10.14, tvOS 12, watchOS 5, *) { // SQLite 3.20+
        // new version of the code (2)
        // same as (1)
    } else {
        // old version of the code
    }
#endif

When compiling on Linux with SQLite 3.19.3 installed in the system, compilation chooses new version of the code (2). I wonder how to fix this pattern to choose according to the actual SQLite version available.
However, this is not a critical issue, as I can just choose to compile using SQLite 3.20.0.

Outdated code?

There's some #if os(Linux) blocks around this line:

// sqlite3_trace_v2 and sqlite3_expanded_sql were introduced in SQLite 3.14.0

I guess this logic assumes that any Linux builds will run SQLite < 3.14.0, but the docs state that the minimum supported SQLite version is 3.19.3, so this logic seems outdated.

Dockerfile for testing

With SQLite 3.19.3. (GRDB doesn't compile with this setup, see above.)

FROM swift:5.7-amazonlinux2

RUN yum -y install make && \
    yum clean all

ARG sqlite_year=2017
ARG sqlite_version=3190300

RUN curl -O --output-dir . https://www.sqlite.org/${sqlite_year}/sqlite-autoconf-${sqlite_version}.tar.gz && \
    tar xvzf sqlite-autoconf-${sqlite_version}.tar.gz && \
    cd sqlite-autoconf-${sqlite_version} && \
    CFLAGS="-Os" ./configure --prefix=/usr && \
    make && \
    make install && \
    cd .. && rm -rf sqlite-autoconf-${sqlite_version}*

To test with SQLite 3.20.0, just set ARG sqlite_version to 3200000. (Which compiles for me.)

Build the image - copy-paste the above Dockerfile somewhere, and run from the same folder:
docker rmi -f grdb-build && docker build . -t grdb-build

Then from the repo root, run:
docker run --rm -t -v ".:/src" -w "/src/" grdb-build bash -c "swift package clean && swift build"

Pull Request Checklist

  • CONTRIBUTING: You have read https://github.com/groue/GRDB.swift/blob/master/CONTRIBUTING.md
  • BRANCH: This pull request is submitted against the development branch.
  • DOCUMENTATION: Inline documentation has been updated.
  • DOCUMENTATION: README.md or another dedicated guide has been updated.
  • TESTS: Changes are tested.
  • TESTS: The make smokeTest terminal command runs without failure.

@groue
Copy link
Owner

groue commented Jun 29, 2023

Hello @kustra! This is a very welcome PR, written with dedication 👍 I'll look into details in the next few days, please hold on :-)

@groue
Copy link
Owner

groue commented Jul 2, 2023

💝

I'd like to fix the compilation errors of GRDB 6.15.x on Linux. I've read that it's not officially supported, but that one-off PRs are welcome.

Totally.

It's hard for me to set Linux as an officially supported platform in the long run, because I lack the expertise for testing and debugging it. You address this need below.

The risk is that future GRDB versions will break Linux support again.

SQLite 3.20.0 conditional compilations

Several places in the code use the following pattern.

#if GRDBCUSTOMSQLITE || GRDBCIPHER
    // new version of the code (1)
#else
    if #available(iOS 12, macOS 10.14, tvOS 12, watchOS 5, *) { // SQLite 3.20+
        // new version of the code (2)
        // same as (1)
    } else {
        // old version of the code
    }
#endif

When compiling on Linux with SQLite 3.19.3 installed in the system, compilation chooses new version of the code (2). I wonder how to fix this pattern to choose according to the actual SQLite version available. However, this is not a critical issue, as I can just choose to compile using SQLite 3.20.0.

The main problem is that Swift does not import the C SQLITE_VERSION_NUMBER macro. So we use various techniques in order to enable or disable features, and link against available C functions, depending on the version of SQLite that GRDB links against:

  • Apple system lib: We can use @available and if #available.
  • Custom SQLite build, locked at a specific version (currently 3.39.3). Detected with #if GRDBCUSTOMSQLITE.
  • SQLCipher 3.4.2+ via CocoaPods, for which we can only assume SQLite >= 3.20.0. Detected with #if GRDBCIPHER.

We also use compiler options for some specific needs. For example, the full-text engine FTS5 is enabled by default with SPM and the Xcode project, but not for custom SQLite builds (where support for FTS5 can not be implicit).

Finally, we use sqlite3_libversion_number() when runtime checks are enough.

I'm not sure the GRDB code base is 100% consistent in the way it targets one SQLite version or another, because this has grown in an "organic way", and I've learned techniques on the go.

In the end, this is not very pretty, and I never took the time to think about a better way to handle this.

This PR gets the repo to the point where GRDB compiles on Linux with SQLite 3.20.0.

This makes Linux similar to SQLCipher, then. Not that this has to stay in that way, but it can help :-)

Outdated code?

There's some #if os(Linux) blocks around this line:

// sqlite3_trace_v2 and sqlite3_expanded_sql were introduced in SQLite 3.14.0

I guess this logic assumes that any Linux builds will run SQLite < 3.14.0, but the docs state that the minimum supported SQLite version is 3.19.3, so this logic seems outdated.

It certainly is. All #if os(Linux) that git blame links to me were just an attempt at not breaking things, but I didn't check anything.

sqlite3_trace_v2 was introduced in SQLite 3.14.0, so we can use it on Linux builds as well, now. Do you want to add a commit for that?

This point aside, the PR is perfect for me. But you may want to grab this opportunity.

Dockerfile for testing

👍 I'll have a more detailed look at this section.

@groue
Copy link
Owner

groue commented Jul 7, 2023

Hello @kustra,

Maybe you missed my question about using sqlite3_trace_v2 in Linux builds. What do you think?

@kustra
Copy link
Collaborator Author

kustra commented Jul 7, 2023

Maybe you missed my question about using sqlite3_trace_v2 in Linux builds. What do you think?

Yep, as soon as the smoke tests succeed, I'll push a commit that removes the legacy logic. I was on holiday with the family, hence my lack of response for the last few days. :)

@groue
Copy link
Owner

groue commented Jul 7, 2023

👍 Take all the time you want! I was just wondering if I wasn't blocking the PR with my imprecise wording. Now that I know we're synced, I'm relieved!

@kustra
Copy link
Collaborator Author

kustra commented Jul 7, 2023

SQLite 3.20.0 conditional compilations

Several places in the code use the following pattern.

#if GRDBCUSTOMSQLITE || GRDBCIPHER
    // new version of the code (1)
#else
    if #available(iOS 12, macOS 10.14, tvOS 12, watchOS 5, *) { // SQLite 3.20+
        // new version of the code (2)
        // same as (1)
    } else {
        // old version of the code
    }
#endif

When compiling on Linux with SQLite 3.19.3 installed in the system, compilation chooses new version of the code (2). I wonder how to fix this pattern to choose according to the actual SQLite version available. However, this is not a critical issue, as I can just choose to compile using SQLite 3.20.0.

The main problem is that Swift does not import the C SQLITE_VERSION_NUMBER macro. So we use various techniques in order to enable or disable features, and link against available C functions, depending on the version of SQLite that GRDB links against:

  • Apple system lib: We can use @available and if #available.
  • Custom SQLite build, locked at a specific version (currently 3.39.3). Detected with #if GRDBCUSTOMSQLITE.
  • SQLCipher 3.4.2+ via CocoaPods, for which we can only assume SQLite >= 3.20.0. Detected with #if GRDBCIPHER.

We also use compiler options for some specific needs. For example, the full-text engine FTS5 is enabled by default with SPM and the Xcode project, but not for custom SQLite builds (where support for FTS5 can not be implicit).

Finally, we use sqlite3_libversion_number() when runtime checks are enough.

I'm not sure the GRDB code base is 100% consistent in the way it targets one SQLite version or another, because this has grown in an "organic way", and I've learned techniques on the go.

In the end, this is not very pretty, and I never took the time to think about a better way to handle this.

I think the cleanest possible solution is to create shims in a C header that's available for all setups, not just CSQLite variant. For all the affected API calls, something along the lines of:

int sqlite3_prepare_shim(
  sqlite3 *db,              /* Database handle. */
  const char *zSql,         /* UTF-8 encoded SQL statement. */
  int nBytes,               /* Length of zSql in bytes. */
  unsigned int prepFlags,   /* Zero or more SQLITE_PREPARE_* flags */
  sqlite3_stmt **ppStmt,    /* OUT: A pointer to the prepared statement */
  const char **pzTail       /* OUT: End of parsed string */
){
#if SQLITE_VERSION_NUMBER >= 3200000
  return sqlite3_prepare_v3(...);
#else
  return sqlite3_prepare_v2(...);
#endif
}

In any case, I think this PR is good to go. :)

@groue
Copy link
Owner

groue commented Jul 9, 2023

I think the cleanest possible solution is to create shims in a C header that's available for all setups, not just CSQLite variant.

This is a very good idea. I could even replace the existing static inline C functions defined in shim.h with a "proper" .c file.

In any case, I think this PR is good to go. :)

Let's merge, then. Thank you very much for your contribution!

Would you accept an invitation that grants you a push access to GRDB? I do intend to keep the lead of the project, but it is good for everyone that people of various levels of expertise can improve the repository. Of course, your free time would remain just as free as it was before!

@groue groue merged commit 3206d04 into groue:development Jul 9, 2023
@kustra kustra deleted the dev/linux-compile-fixes branch July 10, 2023 09:27
@kustra
Copy link
Collaborator Author

kustra commented Jul 10, 2023

Would you accept an invitation that grants you a push access to GRDB? I do intend to keep the lead of the project, but it is good for everyone that people of various levels of expertise can improve the repository. Of course, your free time would remain just as free as it was before!

I contribute to GRDB as part of my work. I may be inactive for long stretches of time. If that's okay, I'll accept. Thank you. :)

@groue
Copy link
Owner

groue commented Jul 10, 2023

Well, the invitation is sent, and it comes without any obligation of any kind :-)

Contributing to this repo is a matter of pleasure, free will, and the desire to build a good companion for app developers.

If you want to contribute more, whatever it means, or if your job has specific needs, please tell, and we'll figure out what's best for everybody :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants