Skip to content

Fix cargo build -p geo_filters --features serde#102

Open
jorendorff wants to merge 2 commits intomainfrom
jorendorff/geo-serde-build
Open

Fix cargo build -p geo_filters --features serde#102
jorendorff wants to merge 2 commits intomainfrom
jorendorff/geo-serde-build

Conversation

@jorendorff
Copy link
Contributor

This previously failed with

❯ cargo build -p geo_filters --features serde
   Compiling geo_filters v0.1.0 (/Users/jorendorff/src/rust-gems/crates/geo_filters)
error[E0433]: failed to resolve: could not find `Deserialize` in `serde`
   --> crates/geo_filters/src/diff_count/sim_hash.rs:28:45
    |
 28 | #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
    |                                             ^^^^^^^^^^^ could not find `Deserialize` in `serde`
    |
note: found an item that was configured out
   --> /Users/jorendorff/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/serde-1.0.214/src/lib.rs:343:24
    |
341 | #[cfg(feature = "serde_derive")]
    |       ------------------------ the item is gated behind the `serde_derive` feature
342 | #[cfg_attr(docsrs, doc(cfg(feature = "derive")))]
343 | pub use serde_derive::{Deserialize, Serialize};
    |                        ^^^^^^^^^^^

...and two other similar errors.

It'd be nice to make a release once this merges, so I bumped the version number.

This previously failed with

```
❯ cargo build -p geo_filters --features serde
   Compiling geo_filters v0.1.0 (/Users/jorendorff/src/rust-gems/crates/geo_filters)
error[E0433]: failed to resolve: could not find `Deserialize` in `serde`
   --> crates/geo_filters/src/diff_count/sim_hash.rs:28:45
    |
 28 | #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
    |                                             ^^^^^^^^^^^ could not find `Deserialize` in `serde`
    |
note: found an item that was configured out
   --> /Users/jorendorff/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/serde-1.0.214/src/lib.rs:343:24
    |
341 | #[cfg(feature = "serde_derive")]
    |       ------------------------ the item is gated behind the `serde_derive` feature
342 | #[cfg_attr(docsrs, doc(cfg(feature = "derive")))]
343 | pub use serde_derive::{Deserialize, Serialize};
    |                        ^^^^^^^^^^^
```

...and two other similar errors.
@jorendorff jorendorff requested a review from a team as a code owner March 10, 2026 22:39
Copilot AI review requested due to automatic review settings March 10, 2026 22:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes the geo_filters crate failing to compile when enabling the serde feature by ensuring Serde’s derive macros are available, and adds a Makefile check to prevent similar feature-gating regressions.

Changes:

  • Enable Serde’s derive feature for the optional serde dependency to make serde::Serialize/serde::Deserialize available.
  • Bump geo_filters crate version to 0.1.1.
  • Add cargo check invocations in make test to validate building geo_filters with no features and with individual features enabled.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
crates/geo_filters/Cargo.toml Enables serde derives under the serde feature and bumps the crate version.
Makefile Adds per-feature cargo check to catch feature-specific build failures early.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +37 to +39
cargo check -p geo_filters --features test-support
cargo check -p geo_filters --features serde
cargo check -p geo_filters --features evaluation
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

The comment says each feature is compiled “in isolation”, but the per-feature cargo check commands don’t pass --no-default-features. That’s equivalent today because default = [], but if default features are added later these checks will no longer validate isolation. Consider adding --no-default-features to the test-support/serde/evaluation checks as well (or updating the comment).

Suggested change
cargo check -p geo_filters --features test-support
cargo check -p geo_filters --features serde
cargo check -p geo_filters --features evaluation
cargo check -p geo_filters --no-default-features --features test-support
cargo check -p geo_filters --no-default-features --features serde
cargo check -p geo_filters --no-default-features --features evaluation

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@itsibitzi itsibitzi left a comment

Choose a reason for hiding this comment

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

Thanks for this!

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.

3 participants