Breaking changes in latest release

[alice-i-cecile]

The most recent minor version release has breaking changes in it, which appears to have broken tera.

This in turn broke our CI; see this failed run.

[alice-i-cecile]

Upstream issue: Keats/tera#739

[NoahTheDuke]

Thanks for the report!

[regar42]

I believe the culprit is here: 723d006
There's a mismatch between how the path to the grammar should be referred to:

Grammar definitions reside in custom .pest files located in the src directory. Their path is relative to src and is specified between the derive attribute and empty struct that Parser will be derived on.

(from pest/derive/src/lib.rs)

and the new include_str behavior (from the linked commit message):

when emitting include_str! the original path (relative to the Rust source file) should be used

[mockersf]

I tried fixing the path in Tera, a lot of other errors appeared after

error: could not compile `tera` due to 379 previous errors; 1 warning emitted

[MarijnS95]

@regar42 I've also "bisected" this to be caused by #522; that change doesn't account for modules/folders deeper than $CARGO_MANIFEST_DIR/src/<the file>.pest it seems.

@mockersf Indeed, when fixing the relative path for include_str, the generator then fails which has not been updated to deal with relative paths to the source file. See also #669 (comment).

[regar42]

Could we maybe yank the release 2.2.0 until this is fixed? 😊

[tomtau]

Could we maybe yank the release 2.2.0 until this is fixed? 😊

It's yanked now.

[MarijnS95]

@tomtau Shall #522 be reverted so that 2.3.0 can be published? It's both a breaking change and something that I don't see how to fix in a trivial way.

[tomtau]

@MarijnS95 yes: #673
Could you try to test to use the version with the reverted commit if everything else works as before?

pest = { git = "https://github.com/tomtau/pest.git", rev = "39595d56729c6613e5792e0bf3e6c11d9b89d03d" }

I'm not sure if there are any other sneaky breaking changes.

[MarijnS95]

@tomtau Tested, fixes the errors again!

Note that I couldn't use your branch as a patch as it started complain about bootstrap, had to check it out locally to make it bootstrap first 😄 (to generate meta/src/grammar.rs).

[alice-i-cecile]

Thank you very much for the prompt fix!

[CAD97]

Note that I couldn't use your branch as a patch as it started complain about bootstrap, had to check it out locally to make it bootstrap first 😄

Yeah, that's (unfortunately) a known issue. I've tried multiple times to come up with a scheme that allows bootstrap to run automatically for git dependencies and local development but be precompiled for crates-io usage, and it's unfortunately just not a use case that cargo lends itself to.

The most resilient choice seems to be committing the generated file(s) and updating it in a test (this is what r-a does), but our bootstrapped parser changes much more frequently than r-a's generated AST types, so doing so would cause us to run into significantly more conflicts from committing the generated file.

....But now I've come up with a super sketchy idea for a cfg(bootstrap) solution....

[SafariMonkey]

The most resilient choice seems to be committing the generated file(s) and updating it in a test (this is what r-a does), but our bootstrapped parser changes much more frequently than r-a's generated AST types, so doing so would cause us to run into significantly more conflicts from committing the generated file.

This may not help for automated merges on GitHub etc., but looking into solutions I found an approach that can ease the pain of merging generated files during local dev:

https://www.charpeni.com/blog/use-custom-merge-driver-to-simplify-git-conflicts

Pest is used by a lot of popular crates (good job!), and having a transitive dependency sometimes fail to compile and require manual intervention (or trying again and again until after 15 minutes it randomly worked, according to a friend) isn't ideal. I hope you can find a relatively painless solution!