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

Add WATT support to cqrs-codegen (#3) #5

Merged
merged 4 commits into from
Nov 7, 2019
Merged

Conversation

ffuugoo
Copy link

@ffuugoo ffuugoo commented Nov 6, 2019

Resolves #3

  • By default cqrs-codegen would build WATT-version of macro.
  • Regular version could be enabled with cargo <command> --no-default-features --features no-watt (or similar options in Cargo.toml, though command-line arguments won't work for crates inside workspaces, as --no-default-features is ignored for workspaces; --features also works in surprising ways in workspaces).
  • By default WATT-version would use cqrs-codegen/src/codegen.wasm if it's present or build it, if it's absent and cqrs-codegen/impl dir is available.
  • There is also an option to create cqrs-codegen/.watch-cqrs-codegen-impl file, and then build.rs will try to be clever about rebuilding codegen.wasm, e.g., rebuild it, if sources in cqrs-codegen/impl/src changed.

Problems and solutions

synstructure can not be build for wasm32-unknown-unknown target

  • implemented quick-fix for synstructure in a fork and temporarily switched cqrs-codegen to it
  • opened an issue and submitted a pull-request to synstructure (though, it needs some further work)

Single crate can't be used as a proc-macro-crate and some other crate-type at the same time

I.e., it's impossible for a crate to be a proc-macro-crate and also produce WASM binaries required for WATT. At least, not without some radical hacks.

Extracted cqrs-codegen-impl crate (located in cqrs-codegen/impl).

Building WASM binaries for WATT requires using special WATT version of proc_macro2

It also requires to use cargo [patch] section to patch proc_macro2 for syn and quote as well. Which may be problematic in some cases, cause...

In workspaces [patch] sections are only allowed in workspace root crate (and ignored everywhere else)

However, setting required [patch] for proc_macro2 in workspace root is also problematic, cause...

[patch] sections patch dependencies for all dependencies as well and even for proc-macro dependencies

I.e., when patched, all proc-macro dependencies used in a crate/workspace would use this WATT version of proc_macro2, which is very likely to break stuff.

The following workaround was found for the three problems above:

If an empty [workspace] section added to Cargo.toml of some member-of-workspace-crate, then when running cargo from this crate's root cargo won't treat it as part of workspace.

So, to build cqrs-codegen-impl for WATT we can:

  • cd <cqrs-codegen-impl-dir>
  • add [workspace] section to Cargo.toml
  • cargo build --target wasm32-unknown-unknown --features watt
  • revert changes made to Cargo.toml

This solution was codified in build.rs for cqrs-codegen crate, however, during work on build.rs another smaller problem occured.

It's impossible to recursively run cargo from build.rs

As recursive cargo invocation would lock waiting for target directory to be released (which is locked by first cargo invocation, waiting for build.rs to finish, i.e., deadlock).

This can be circumvented by specifying separate target directory for recursive cargo calls.

As, in our case, we are building cqrs-codegen-impl for other target and also with patched version of proc_macro2, it's not a problem at all (and, maybe, even desirable).

@ffuugoo ffuugoo self-assigned this Nov 6, 2019
@ffuugoo
Copy link
Author

ffuugoo commented Nov 7, 2019

FCM

Add WATT version of 'cqrs-codegen' crate

- extract 'cqrs-codegen/imp' sub-crate from 'cqrs-codegen'
- add 'build.rs' file which builds WASM binary of `cqrs-codegen/impl` to 'cqrs-codegen'
- hide original implementation behind "no-watt" feature

@ffuugoo ffuugoo requested a review from tyranron November 7, 2019 05:32
@tyranron tyranron added the enhancement New feature or request label Nov 7, 2019
@tyranron tyranron changed the title WIP: Add WATT support to cqrs-codegen (#3) Add WATT support to cqrs-codegen (#3) Nov 7, 2019
@tyranron tyranron marked this pull request as ready for review November 7, 2019 12:46
Copy link
Member

@tyranron tyranron left a comment

Choose a reason for hiding this comment

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

Well enough! 👍

@ffuugoo ffuugoo merged commit 31bc69b into master Nov 7, 2019
@ffuugoo ffuugoo deleted the cqrs-codegen-watt branch November 7, 2019 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WATT version of proc_macro in cqrs-codegen crate
2 participants