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

app building fine with rc.2 fails to build with rc.3, in matched_path.rs #1515

Closed
olivier-fs opened this issue Nov 9, 2022 · 11 comments · Fixed by #1517
Closed

app building fine with rc.2 fails to build with rc.3, in matched_path.rs #1515

olivier-fs opened this issue Nov 9, 2022 · 11 comments · Fixed by #1517

Comments

@olivier-fs
Copy link

olivier-fs commented Nov 9, 2022

Bug Report

Version

axum = { version = "0.6.0-rc.3", features = ["http1", "json", "tokio"] }
tokio = { version = "1.21.2", default-features = false, features = ["macros", "rt-multi-thread"] }
tower-http = { version = "0.3.4", features = ["compression-gzip", "fs", "set-header", "timeout", "trace"] }
$ cargo tree | grep axum
├── axum v0.6.0-rc.3
│   ├── axum-core v0.3.0-rc.3
$ cargo --version
cargo 1.65.0 (4bc8f24d3 2022-10-20)

Platform

Windows 10

Description

An axum app that was building fine with rc.2 doesn't build with rc.3.
I fixed some breaking but trivial fixes (e.g. nest() => nest_service()) but still :

$ cargo run --release
   Compiling proc-macro2 v1.0.43
   ...
   ...
   Compiling axum v0.6.0-rc.3
error[E0308]: `if` and `else` have incompatible types
   --> C:\Users\olivier\.cargo\registry\src\github.com-1ecc6299db9ec823\axum-0.6.0-rc.3\src\extract\matched_path.rs:135:12
    |
133 |       let matched_path = if let Some(matched_path) = route_id_to_path.get(&id) {
    |                          ----------------------------------------------------- `if` and `else` have incompatible types
134 |           matched_path
    |           ------------ expected because of this
135 |       } else {
    |  ____________^
136 | |         #[cfg(debug_assertions)]
137 | |         panic!("should always have a matched path for a route id");
138 | |     };
    | |_____^ expected `&Arc<str>`, found `()`
    |
    = note: expected reference `&Arc<str>`
               found unit type `()`

For more information about this error, try `rustc --explain E0308`.
error: could not compile `axum` due to previous error
warning: build failed, waiting for other jobs to finish...

The odd part is :

  • the same code builds fine in debug mode i.e. without --release
  • another axum with the "same" setup i.e. route() with get() handlers, nest_service(), and fallback(), builds OK

rustc, that is nearly always giving good details about the problem and the way to fix it, doesn't in my case, and doesn't even give some filename/line featuring the "culprit" code...

Building with nightly (cargo 1.67.0-nightly (9286a1beb 2022-11-04)) doesn't change anything (as expected, it is a simple if let statement, not the new let chain).

The only difference I can spot for now is the failing build has the "json" feature enabled when the one building fine doesn't, but I don't know if it's related in anyway (and removing the feature makes the app build fail for tons of reason because there are json handlers... It would be possible to comment all these if needed, but I post this issue as is for now).

I'll try to spot more differences between the "OK" code and the the failing one.

@yanns
Copy link
Contributor

yanns commented Nov 9, 2022

I can confirm this issue on Mac and Linux.

@jplatte
Copy link
Member

jplatte commented Nov 9, 2022

the same code builds fine in debug mode i.e. without --release

That is because of the #[cfg(debug_assertions)]. I think that just has to be removed for this to work again.

@jplatte
Copy link
Member

jplatte commented Nov 9, 2022

I think adding this to your Cargo.toml should make it work until we fix it properly:

[profile.release.package]
axum = { debug = true }

@davidpdrsn
Copy link
Member

Ah yes I see. I'll fix that an ship a new release!

@lolo32
Copy link

lolo32 commented Nov 9, 2022

It's the same for the just released rc.4

Edited: disabled my comment

@davidpdrsn
Copy link
Member

@lolo32 There is no rc.4 so not sure what you mean.

@olivier-fs
Copy link
Author

[profile.release.package]
axum = { debug = true }

doesn't fix it on my box.

FYI - even if it's almost surely not related at all - I have

[profile.release]
strip = true
panic = "abort"
opt-level = "s"
lto = true
codegen-units = 1

Anyway I just wanted to report the issue as soon as possible, because that's the whole point of "rc" releases.
I can revert eaasily revert to rc-2, and wait for the next rc-4 or reference the github version now that the problem is spotted.

I still can't catch what difference in source code makes it build in release mode in one app and not the other...

Still eager to try asap the fantastic work both of you guys are doing on axum ! thanks a lot.

@davidpdrsn
Copy link
Member

davidpdrsn commented Nov 9, 2022

When #1517 is merged I'll do a new release. Hold your horses 😅

@lolo32
Copy link

lolo32 commented Nov 9, 2022

@lolo32 There is no rc.4 so not sure what you mean.

Sorry, my bad, forget my comment

@olivier-fs
Copy link
Author

Just switched my Cargo.toml to

axum = { git = "https://github.com/tokio-rs/axum", features = ["http1", "json", "tokio"] }

and I can confirm you fixed it ♛ !

I don't close this issue myself as you may want to add comments / more fixes.

@davidpdrsn
Copy link
Member

Alright 0.6.0-rc.4 is out with the fix! Thanks for reporting it!

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 a pull request may close this issue.

5 participants