Skip to content

Commit

Permalink
Fix the bug cfg!(feature = "production") in c_at_e_main crate alway…
Browse files Browse the repository at this point in the history
…s will be `true`

This fixes the bug that is `cfg!(feature = "production")` always will be evaluated as `true`.
For example, the following code in `c_at_e_main` will always returns `"this is production"`
even if we run `make build_debug -j RELEASE_CHANNEL=canary`.

```rust
fn main() {
    let message = if cfg!(feature = "production") {
        "production"
    } else if cfg!(feature = "canary") {
        "canary"
    } else {
        "others"
    };
    println!("this is {}", message);
}
```

This bug is caused accidentally by our build system.

We set `default` feature for `c_at_e_main` to run
many rust toolchain that are not integrated for our build system
easily.

But our _release channel_ model is not mutually exclusive.
[cargo does not support it](rust-lang/cargo#2980).
So when we invoke `cargo build --feature canary`, then cargo will
enables both of `production` and `canary` feature because cargo enables
`default` feature set implicitly too.

Therefore, he above code will be expanded to:

```rust
fn main() {
    let message = if (true) {
        "production"
    } else if (true) {
        "canary"
    } else {
        "others"
    };
    println!("This is {}", message);
}
```

This causes the bug.

This change set `--no-default-features` CLI flags to cargo.
By this change, cargo will not enable `default` feature implicitly
and the bug will be fixed.

This change does not remove `default` feature from
`c_at_e_main`'s Cargo.toml
to still allow to invoke rust-analyzer without zero-configuration.

Note
-----

Can we set cargo's default CLI arguments without our build system?
=============

No.
[`.cargo/config.toml`](https://doc.rust-lang.org/cargo/reference/config.html)
has no way to disable default features without passing CLI flags.

Can we configure rust-analyzer to disable all default features?
==========

Yes, we can config it by vscode's setting.
But rust-analyzer does not have a way to editor agnostic settings.

- rust-lang/rust-analyzer#11010

There are rust-project.json but it's for non-cargo based project.
We would not like to use it.
https://rust-analyzer.github.io/manual.html#non-cargo-based-projects
  • Loading branch information
tetsuharuohzeki committed Sep 26, 2022
1 parent 65bd0d9 commit 03764f2
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 0 deletions.
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ RELEASE_CHANNEL_FEATURES := $(APPLICATION_NAME)/canary
endif

CARGO_FEATURES_CLI_FLAGS := \
--no-default-features \
--features $(RELEASE_CHANNEL_FEATURES),$(ADDITIONAL_FEATURES)

CLIPPY_RULES := \
Expand Down
2 changes: 2 additions & 0 deletions crates/main/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ rust-version.workspace = true
fastly = "^0.8.7"

[features]
# Our build system requires to set _release channel_ explicitly by using `--no-default-features`.
# This default set is only used for toolchains that are not integrated our build systems (e.g. rust-analyzer).
default = [
# We want to be able to rollout this application to production at all time.
# Thus we use _production_ as our default build feature set.
Expand Down

0 comments on commit 03764f2

Please sign in to comment.