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

Broken feature unification when multiple packages depend on askama #810

Open
dpc opened this issue Apr 3, 2023 · 9 comments
Open

Broken feature unification when multiple packages depend on askama #810

dpc opened this issue Apr 3, 2023 · 9 comments

Comments

@dpc
Copy link

dpc commented Apr 3, 2023

I'm trying to make a workspace that in two different places of dependency tree uses askama

One imports only askama, the other one askama_axum. The one that only imports askama fails with:

error[E0433]: failed to resolve: could not find `askama_axum` in the list of imported crates                                                                    
   --> /home/dpc/.cargo/git/checkouts/uniffi-rs-5e0d6f877b5a61c0/b24850f/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs:120:10
    |                                                                                                                                                           
120 | #[derive(Template)]                                                                                                                                       
    |          ^^^^^^^^ could not find `askama_axum` in the list of imported crates
    |                                                                                                                                                           
    = note: this error originates in the derive macro `Template` (in Nightly builds, run with -Z macro-backtrace for more info)                                          

Seems to me that the other crate enabled with-axum feature, and askama_derive is being picked up by the uniffi-rs so it now generates code that wants to access askama_axum, which is not a dependency of it.

I've read through #588 (comment) which seems somewhat related.

Unless I'm missing something somewhere, this might be a hard circle to square. Downstream crates with different extenson-sets, can't share the askama_derive, it seems.

@djc
Copy link
Owner

djc commented Apr 3, 2023

Yeah, not sure if there's a solution for this. The easiest solution is probably to have both of your crates enable with with-axum feature.

@Kijewski
Copy link
Collaborator

Kijewski commented Apr 3, 2023

Hm, I think in this case you have to copy https://github.com/djc/askama/blob/ea8be44/askama_axum/src/lib.rs and call into_response(&Template) manually.

Features are additive, and I we add an "anti-feature" like "dont-implement-into", then this would be the same as if you omit "with-axum" in your main crate.

@dpc
Copy link
Author

dpc commented Apr 3, 2023

The problem is that the crate where this is happening is a dependency of a dependency of a dependency... . I can [patch] it on my fork, but in principle this is not a viable solution.

Idea: Possibly, instead of relaying on cargo features to build the right code, somehow the user of the macro should inject the list of backends they're interested in.

Possibly askama_axum could even somehow export Template to be askama::Template([with-axum]). In essence: it has to come from the end caller, not from cargo features.

This way askama_derive can build support for all crates that are being used (feature unification is OK here), but then each user of askama templates gets the proc macro to generate only code that they locally use and have imported.

If you can somehow make an askama_axum::Template=askama_derive::Template([with_axum]) alias then the usability won't suffer. If not, then derive(Template(axum)) is still not terrible.

@djc
Copy link
Owner

djc commented Apr 4, 2023

Having to decorate each type with all the integrations needed seems like a pain in the ass. But, we could set this up in the configuration? So you declare in the askama.toml for your crate which integrations you want.

@dpc
Copy link
Author

dpc commented Apr 4, 2023

If that's possible, then it does seem like an improvement.

@5iddy
Copy link

5iddy commented Apr 5, 2023

I am experiencing this issue as well but with with-actix-web feature.
Here is the errror:

error[E0433]: failed to resolve: could not find `askama_actix` in the list of imported crates
 --> src/templates/index.rs:3:17
  |
3 | #[derive(Debug, Template)]
  |                 ^^^^^^^^ could not find `askama_actix` in the list of imported crates
  |
  = note: this error originates in the derive macro `Template` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0433`.
error: could not compile `xprs` due to previous error

@djc
Copy link
Owner

djc commented Apr 5, 2023

@5iddy you should be pulling the askama_actix crate into your dependency graph.

@qsantos
Copy link

qsantos commented Jan 2, 2024

I have encountered the same issue as @dpc. To give some explanation as to why it is non-trivial to solve this:

When the with-axum feature is set, the Template derive macro will generate some code that depends on askama_axum:

// Implement Axum's `IntoResponse`.
#[cfg(feature = "with-axum")]
fn impl_axum_into_response(&mut self, buf: &mut Buffer) -> Result<(), CompileError> {
self.write_header(buf, "::askama_axum::IntoResponse", None)?;
buf.writeln("#[inline]")?;
buf.writeln(
"fn into_response(self)\
-> ::askama_axum::Response {",
)?;
buf.writeln("::askama_axum::into_response(&self)")?;
buf.writeln("}")?;
buf.writeln("}")
}

This code might not be called by the crate which depends on askama and not askama_axum. However, when resolving the dependencies at the workspace level, having another crate depending on askama_axum will enable the with-axum feature (features are meant to be additive only precisely to make this possible). So, we still need to generate the code, so we still need to refer to askama_axum.

In other situations, it would just mean adding askama_axum as an optional dependency to askama that is enabled when the with-axum feature is set and re-exporting it (so that the code generated by the macro can find it at ::askama::askama_axum). However, here, askama_axum actually depends on askama. So, this would be a cyclic dependency, which is not allowed.

In short, making this work as-expected would require a deep rework of the Askama workspace.

@djc
Copy link
Owner

djc commented Jan 2, 2024

I'm open to reviewing a rework of the Askama workspace if we think there is a way to solve this, but the right way to solve this is not obvious to me. In particular, having all the integrations be Cargo features in the askama crate is not a workable situation -- that is what we had before, but it leads to issues because the Askama crate ends up conditionally depending on a whole lot of other crates some of which might conflict (like, Rocket might depend on a different version of hyper than Axum, which is a pain in the ass to maintain).

The problem here is that we have Askama usage in different crates, and whether or not to generate code for an integration is propagated via Cargo features. Askama crates get compiled with a unified set of features, while the support code necessary for the integration might not available in each crate that has templates in it. However, it's not obvious how procedural macro invocations across crates might communicate with each other (alternatively, how we could block procedural macro invocations across crates from communicating with each other, I suppose?). askama.toml as a configuration method functions on a per-crate basis.

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

No branches or pull requests

5 participants