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

Why is virtually all the macro code inside askama_shared #676

Closed
jplatte opened this issue Apr 25, 2022 · 6 comments
Closed

Why is virtually all the macro code inside askama_shared #676

jplatte opened this issue Apr 25, 2022 · 6 comments

Comments

@jplatte
Copy link
Contributor

jplatte commented Apr 25, 2022

I was surprised to see that syn, quote and proc-macro2 are not just build dependencies of my project, they're regular dependencies too through askama_shared. When I looked into it, I found all of the macro code being in askama_shared. What is the reason for this?

@djc
Copy link
Owner

djc commented Apr 26, 2022

The reason is to minimize coupling between some of the other crates, which created some semver hazards before.

I'm struggling to come up with a reason for desiring a split here. What's the concern here? If you use serde for example, it will depend on serde_derive which depends on syn/quote/proc-macro2. Is there an issue with binary sizes?

@jplatte
Copy link
Contributor Author

jplatte commented Apr 26, 2022

If compiling for a different target triple than the host's, these libraries that are only ever used as part of the proc-macro code (as a compiler plugin on the host) will be compiled for both host and target. I haven't been cross-compiling so far, I just noticed this when analyzing dependencies of turbo.fish with cargo-depgraph. It seems wrong to me to have these be part of the runtime dependency graph.

An alternative to moving the code into the proc-macro crate would be moving it behind a feature flag that's activated by the proc-macro crate, though then you end up compiling the shared crate twice (at least with resolver = "2") even for target=host because the build-dependencies graph has a different feature set on askama_shared than the regular dependency graph.

@djc
Copy link
Owner

djc commented Apr 26, 2022

I think the problem is that proc-macro crates can only export procedural macros, and no other kinds of symbols. That may also complicate the testing situation. That said, I'm not opposed to someone experimenting with moving a bunch of code (back) into the askama_derive crate, but it probably won't make my priority list (though I'd be happy to review).

@thburghout
Copy link

Thanks for fixing this issue! We ran into this in our cross-compiling toolchain with an error like meta-rust/meta-rust#266 . It was fixed by using the git dependency of askama. When are you planning on releasing these changes?

Great project btw!

@djc
Copy link
Owner

djc commented Jun 15, 2022

I think we can do that soon.

@djc
Copy link
Owner

djc commented Jun 17, 2022

Going to close this for now. Will probably release after #700 has been merged.

@djc djc closed this as completed Jun 17, 2022
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

3 participants