Merge pull request #687 from thomcc/easier-testing-ci

Improve CI and testing/dev workflow by cleaning up how features are used.

.github/workflows/main.yml

@@ -32,11 +32,31 @@ jobs:
           profile: minimal
           toolchain: stable
           override: true
-      - run: cargo build --features bundled --workspace --all-targets
-      - run: cargo test --features bundled --workspace --all-targets
-      - run: cargo test --features bundled --workspace --doc
-      # We can't use --all-features.
-      - run: cargo test --features 'array backup blob bundled chrono collation csvtab extra_check functions hooks i128_blob limits load_extension serde_json series trace url vtab_v3 window'
+
+      - uses: actions-rs/cargo@v1
+        with:
+          command: build
+          args: --features bundled --workspace --all-targets
+
+      - uses: actions-rs/cargo@v1
+        with:
+          command: test
+          args: --features bundled --workspace --all-targets
+
+      - name: "cargo test --features 'bundled-full session buildtime_bindgen'"
+        if: matrix.platform.os != 'windows-latest'
+        uses: actions-rs/cargo@v1
+        with:
+          command: test
+          args: --features 'bundled-full session buildtime_bindgen' --all-targets --workspace
+
+      - name: "cargo test --features bundled-full"
+        if: matrix.platform.os == 'windows-latest'
+        uses: actions-rs/cargo@v1
+        with:
+          command: test
+          args: --features bundled-full --all-targets --workspace
+
       - name: Static build
         if: matrix.platform.os == 'windows-latest'
         shell: cmd
@@ -47,6 +67,8 @@ jobs:
   # Ensure clippy doesn't complain.
   clippy:
     name: Clippy
+    strategy:
+      fail-fast: false
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v1
@@ -61,7 +83,15 @@ jobs:
           RUSTFLAGS: -D warnings
         with:
           command: clippy
-          args: --all-targets --all --features 'array backup blob bundled chrono collation csvtab extra_check functions hooks i128_blob limits load_extension serde_json series trace url vtab_v3 window' -- -D warnings
+          # clippy with just bundled
+          args: --all-targets --workspace --features bundled
+      - uses: actions-rs/cargo@v1
+        env:
+          RUSTFLAGS: -D warnings
+        with:
+          command: clippy
+          # Clippy with all non-conflicting features
+          args: --all-targets --workspace --features 'bundled-full session buildtime_bindgen'
 
   # Ensure patch is formatted.
   fmt:
@@ -97,4 +127,4 @@ jobs:
           RUSTFLAGS: -D warnings
         with:
           command: doc
-          args: --no-deps --features 'array backup blob bundled chrono collation csvtab extra_check functions hooks i128_blob limits load_extension serde_json series trace url vtab_v3 window'
+          args: --no-deps --features 'bundled-full session buildtime_bindgen'

Cargo.toml

@@ -42,8 +42,6 @@ sqlcipher = ["libsqlite3-sys/sqlcipher"]
 unlock_notify = ["libsqlite3-sys/unlock_notify"]
 # xSavepoint, xRelease and xRollbackTo: 3.7.7 (2011-06-23)
 vtab = ["libsqlite3-sys/min_sqlite_version_3_7_7", "lazy_static"]
-# xShadowName: 3.26.0
-vtab_v3 = ["vtab"]
 csvtab = ["csv", "vtab"]
 # pointer passing interfaces: 3.20.0
 array = ["vtab"]
@@ -56,10 +54,37 @@ series = ["vtab"]
 # check for invalid query.
 extra_check = []
 modern_sqlite = ["libsqlite3-sys/bundled_bindings"]
-unstable = []
 in_gecko = ["modern_sqlite", "libsqlite3-sys/in_gecko"]
 bundled-windows = ["libsqlite3-sys/bundled-windows"]
 
+# Helper feature for enabling both `bundled` and most non-build-related optional
+# features or dependencies. This is useful for running tests / clippy / etc. New
+# features and optional dependencies that don't conflict with anything else
+# should be added here.
+bundled-full = [
+    "array",
+    "backup",
+    "blob",
+    "bundled",
+    "chrono",
+    "collation",
+    "csvtab",
+    "extra_check",
+    "functions",
+    "hooks",
+    "i128_blob",
+    "limits",
+    "load_extension",
+    "serde_json",
+    "series",
+    "trace",
+    "unlock_notify",
+    "url",
+    "uuid",
+    "vtab",
+    "window",
+]
+
 [dependencies]
 time = "0.1.0"
 bitflags = "1.0"
@@ -82,6 +107,9 @@ lazy_static = "1.0"
 regex = "1.0"
 uuid = { version = "0.8", features = ["v4"] }
 unicase = "2.4.0"
+# Use `bencher` over criterion becasue it builds much faster and we don't have
+# many benchmarks
+bencher = "0.1"
 
 [dependencies.libsqlite3-sys]
 path = "libsqlite3-sys"
@@ -97,6 +125,10 @@ name = "deny_single_threaded_sqlite_config"
 [[test]]
 name = "vtab"
 
+[[bench]]
+name = "cache"
+harness = false
+
 [package.metadata.docs.rs]
 features = [ "backup", "blob", "chrono", "collation", "functions", "limits", "load_extension", "serde_json", "trace", "url", "vtab", "window", "modern_sqlite" ]
 all-features = false
@@ -104,5 +136,5 @@ no-default-features = true
 default-target = "x86_64-unknown-linux-gnu"
 
 [package.metadata.playground]
-features = ["array", "backup", "blob", "bundled", "chrono", "collation", "csvtab", "extra_check", "functions", "hooks", "i128_blob", "limits", "load_extension", "modern_sqlite", "serde_json", "series", "trace", "url", "vtab_v3", "vtab", "window"]
+features = ["bundled-full"]
 all-features = false

README.md

@@ -101,7 +101,7 @@ features](https://doc.rust-lang.org/cargo/reference/manifest.html#the-features-s
 * [`array`](https://sqlite.org/carray.html), The `rarray()` Table-Valued Function.
 * `i128_blob` allows storing values of type `i128` type in SQLite databases. Internally, the data is stored as a 16 byte big-endian blob, with the most significant bit flipped, which allows ordering and comparison between different blobs storing i128s to work as expected.
 * `uuid` allows storing and retrieving `Uuid` values from the [`uuid`](https://docs.rs/uuid/) crate using blobs.
-* [`session`](https://sqlite.org/sessionintro.html), Session module extension.
+* [`session`](https://sqlite.org/sessionintro.html), Session module extension. Requires `buildtime_bindgen` feature.
 
 ## Notes on building rusqlite and libsqlite3-sys
 
@@ -166,6 +166,26 @@ enabled if you turn this on, as otherwise you'll need to keep the version of
 SQLite you link with in sync with what rusqlite would have bundled, (usually the
 most recent release of sqlite). Failing to do this will cause a runtime error.
 
+## Contributing
+
+Rusqlite has many features, and many of them impact the build configuration in
+incompatible ways. This is unfortunate, and makes testing changes hard.
+
+To help here: you generally should ensure that you run tests/lint for
+`--features bundled`, and `--features bundled-full session buildtime_bindgen`.
+
+If running bindgen is problematic for you, `--features bundled-full` enables
+bundled and all features which don't require binding generation, and can be used
+instead.
+
+### Checklist
+
+- Run `cargo fmt` to ensure your Rust code is correctly formatted.
+- Ensure `cargo clippy --all-targets --workspace --features bundled` passes without warnings.
+- Ensure `cargo test --all-targets --workspace --features bundled-full session buildtime_bindgen` reports no failures.
+- Ensure `cargo test --all-targets --workspace --features bundled` reports no failures.
+- Ensure `cargo test --all-targets --workspace --features bundled-full session buildtime_bindgen` reports no failures.
+
 ## Author
 
 John Gallagher, johnkgallagher@gmail.com

benches/cache.rs

@@ -0,0 +1,18 @@
+use bencher::{benchmark_group, benchmark_main, Bencher};
+use rusqlite::Connection;
+
+fn bench_no_cache(b: &mut Bencher) {
+    let db = Connection::open_in_memory().unwrap();
+    db.set_prepared_statement_cache_capacity(0);
+    let sql = "SELECT 1, 'test', 3.14 UNION SELECT 2, 'exp', 2.71";
+    b.iter(|| db.prepare(sql).unwrap());
+}
+
+fn bench_cache(b: &mut Bencher) {
+    let db = Connection::open_in_memory().unwrap();
+    let sql = "SELECT 1, 'test', 3.14 UNION SELECT 2, 'exp', 2.71";
+    b.iter(|| db.prepare_cached(sql).unwrap());
+}
+
+benchmark_group!(cache_benches, bench_no_cache, bench_cache);
+benchmark_main!(cache_benches);

benches/lib.rs

@@ -1,24 +0,0 @@
-#![cfg_attr(feature = "unstable", feature(test))]
-
-#[cfg(feature = "unstable")]
-mod bench {
-    extern crate test;
-
-    use rusqlite::Connection;
-    use test::Bencher;
-
-    #[bench]
-    fn bench_no_cache(b: &mut Bencher) {
-        let db = Connection::open_in_memory().unwrap();
-        db.set_prepared_statement_cache_capacity(0);
-        let sql = "SELECT 1, 'test', 3.14 UNION SELECT 2, 'exp', 2.71";
-        b.iter(|| db.prepare(sql).unwrap());
-    }
-
-    #[bench]
-    fn bench_cache(b: &mut Bencher) {
-        let db = Connection::open_in_memory().unwrap();
-        let sql = "SELECT 1, 'test', 3.14 UNION SELECT 2, 'exp', 2.71";
-        b.iter(|| db.prepare_cached(sql).unwrap());
-    }
-}

src/vtab/mod.rs

@@ -70,8 +70,7 @@ unsafe impl<T: VTab> Send for Module<T> {}
 unsafe impl<T: VTab> Sync for Module<T> {}
 
 // Used as a trailing initializer for sqlite3_module -- this way we avoid having
-// the build fail if buildtime_bindgen is on, our bindings have
-// `sqlite3_module::xShadowName`, but vtab_v3 wasn't specified.
+// the build fail if buildtime_bindgen is on
 fn zeroed_module() -> ffi::sqlite3_module {
     // This is safe, as bindgen-generated structs are allowed to be zeroed.
     unsafe { std::mem::MaybeUninit::zeroed().assume_init() }