From 7e4ff77a43d7a810dc541c1d1f0aa2385c9c98bf Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Tue, 7 Apr 2020 08:33:17 -0700 Subject: [PATCH 1/7] Use `bencher` instead of unstable libtest --- Cargo.toml | 8 +++++++- benches/cache.rs | 18 ++++++++++++++++++ benches/lib.rs | 24 ------------------------ 3 files changed, 25 insertions(+), 25 deletions(-) create mode 100644 benches/cache.rs delete mode 100644 benches/lib.rs diff --git a/Cargo.toml b/Cargo.toml index adcd8e5..923e3a9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -56,7 +56,6 @@ 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"] @@ -82,6 +81,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 +99,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 diff --git a/benches/cache.rs b/benches/cache.rs new file mode 100644 index 0000000..dd3683e --- /dev/null +++ b/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); diff --git a/benches/lib.rs b/benches/lib.rs deleted file mode 100644 index 2d799f4..0000000 --- a/benches/lib.rs +++ /dev/null @@ -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()); - } -} From f5c20abaa198ff8f3dca5aba8e37bef6bc295ce4 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Tue, 7 Apr 2020 08:36:18 -0700 Subject: [PATCH 2/7] Remove unused vtab_v3 feature while I'm here --- Cargo.toml | 2 -- src/vtab/mod.rs | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 923e3a9..2dd3ea0 100644 --- a/Cargo.toml +++ b/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"] diff --git a/src/vtab/mod.rs b/src/vtab/mod.rs index aa07c3f..97124b5 100644 --- a/src/vtab/mod.rs +++ b/src/vtab/mod.rs @@ -70,8 +70,7 @@ unsafe impl Send for Module {} unsafe impl Sync for Module {} // 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() } From f95c3775b2495c65cca83876bb63dc2d701e66bd Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Tue, 7 Apr 2020 09:01:23 -0700 Subject: [PATCH 3/7] Replace the big CI feature list with bundled-full feature --- Cargo.toml | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 2dd3ea0..e724b27 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -57,6 +57,35 @@ modern_sqlite = ["libsqlite3-sys/bundled_bindings"] 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", + "session", + "trace", + "unlock_notify", + "url", + "uuid", + "vtab", + "window", +] + [dependencies] time = "0.1.0" bitflags = "1.0" @@ -108,5 +137,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 From 2f9bd7ed63a05ffca1706f4027978979b9b5044a Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Tue, 7 Apr 2020 09:02:44 -0700 Subject: [PATCH 4/7] Use bundled-full in github CI. Use actions-rs in more places for better error reporting --- .github/workflows/main.yml | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index d3e7735..50aa4e8 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -32,11 +32,22 @@ 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 + + - 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 +58,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 +74,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 --all --features bundled + - uses: actions-rs/cargo@v1 + env: + RUSTFLAGS: -D warnings + with: + command: clippy + # Clippy with bundled-full + args: --all-targets --all --features bundled-full # Ensure patch is formatted. fmt: @@ -97,4 +118,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 From 57749b8dbc91af74e95eee3d65998574bcd7190a Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Tue, 7 Apr 2020 09:12:12 -0700 Subject: [PATCH 5/7] Document bundled-full (add contributing section) --- README.md | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 49646cc..43c1ec1 100644 --- a/README.md +++ b/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 From 74e815fb6d730ca5e60661acbf4a57fa7cacfac2 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Tue, 7 Apr 2020 09:46:35 -0700 Subject: [PATCH 6/7] Use --workspace instead of --all for clippy --- .github/workflows/main.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 50aa4e8..540b547 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -75,14 +75,14 @@ jobs: with: command: clippy # clippy with just bundled - args: --all-targets --all --features bundled + args: --all-targets --workspace --features bundled - uses: actions-rs/cargo@v1 env: RUSTFLAGS: -D warnings with: command: clippy # Clippy with bundled-full - args: --all-targets --all --features bundled-full + args: --all-targets --workspace --features bundled-full # Ensure patch is formatted. fmt: From 6f4c67e1eadc16288b6d85e24f6173deeab0e3ab Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Tue, 7 Apr 2020 14:42:29 -0700 Subject: [PATCH 7/7] Run buildtime_bindgen and session in github CI instead where possible --- .github/workflows/main.yml | 17 +++++++++++++---- Cargo.toml | 1 - 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 540b547..0d40f57 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -43,7 +43,16 @@ jobs: command: test args: --features bundled --workspace --all-targets - - uses: actions-rs/cargo@v1 + - 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 @@ -81,8 +90,8 @@ jobs: RUSTFLAGS: -D warnings with: command: clippy - # Clippy with bundled-full - args: --all-targets --workspace --features bundled-full + # Clippy with all non-conflicting features + args: --all-targets --workspace --features 'bundled-full session buildtime_bindgen' # Ensure patch is formatted. fmt: @@ -118,4 +127,4 @@ jobs: RUSTFLAGS: -D warnings with: command: doc - args: --no-deps --features bundled-full + args: --no-deps --features 'bundled-full session buildtime_bindgen' diff --git a/Cargo.toml b/Cargo.toml index e724b27..1eb9166 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -77,7 +77,6 @@ bundled-full = [ "load_extension", "serde_json", "series", - "session", "trace", "unlock_notify", "url",