Generate pest grammar through an integration test

[djc]

handlebars recently became a transitive dependency of one of my work projects via async-graphql. I noticed that it pulls the pest_derive crates into my dependency graph, but this is actually not necessary. By doing the code generation in an integration test, we can move the dependencies here into dev-dependencies, moving the effort of code generation (including the dependencies required to do so) from downstream users to this repository. By running it as part of your test suite, your CI setup will ensure that the generated code and the grammar remain in sync.

I actually did something similar for async-graphql itself (async-graphql/async-graphql#901) and a few other projects. Let me know what you think!

(Submitting this against the 4.x branch since it's semver-compatible anyway and so would be nice to get as a minor version bump. If you think this should wait for 5.x -- though I cannot think of any reason to do so -- we can change the target branch.)

[djc]

Looks like some of the test failures are unrelated to my changes. Should I target the 5.x branch instead? Any guess as to when you'll release a 5.0.0?

[sunng87]

Thank you @djc. Just keep the target branch to 4.x and let me deal with cherry-picking.

I'm OK with the idea to remove pest_derive as a runtime dependency. The only question is if we can keep generated code out of SCM. I'm not sure if build.rs will work for this case, like how prost-build deals with code generation.

[djc]

Why do you want to keep it out of Git? build.rs doesn't really help, because then downstream users will still need to compile the dependencies needed for the build script. I guess you could do some custom thing if you have some release scripting, but IMO keeping it in Git is not a problem -- rustfmt in CI can easily be fixed up by running rustfmt instead of prettyplease on the code generation output, and @generated in the first line is something that GitHub uses to make sure that diffs from the generated code get collapsed by default (like Cargo.lock).

[sunng87]

Yes, as you said, with generated sources in git,

1. you will need a custom workflow to sync it, format it and commit it. The code change happens after cargo test but it's not the test sematic designed for.
2. when multiple developers are working on the grammar files, it causes conflicts that are not easy to resolve.

I agree with the idea to remove pest_derive from runtime. Just let me do some experiment for a better workflow if you are not in hurry.

[djc]

I'm not in any particular hurry.

I think the conflicts are easy to resolve, by resolving the conflicts in the .pest file, then rerunning the test.

[djc]

Ping? Any updates on this? Is there anything I can do?

[sunng87]

@djc sorry for being inactive for this. After some thinking I'm still not a big fan of checking-in generated sources although I agree on the benefit of lighter runtime dependencies. I was thinking about moving these macro crates to build-dependencies and generating sources in build.rs. However, I realize it make no differences from user's perspective and has very little benefit.

Checking-in generated sources requires CI tasks to check the consistency, and manual task to update. Recently I have been blocked by an issue in arrow-rs for how it handles generated protobuf sources. The dependencies for code generated are pinned to avoid unwanted changes, which introduces more issues.

I want to thank you for proposing this but in pest-derive I see little benefit for the change. If it really matters in your context, it is possible to do it via the fork.