Skip to content

cli, lib: inline grammar definitions into .rs files#4336

Closed
thoughtpolice wants to merge 2 commits into
mainfrom
aseipp/push-zsxzytzomvor
Closed

cli, lib: inline grammar definitions into .rs files#4336
thoughtpolice wants to merge 2 commits into
mainfrom
aseipp/push-zsxzytzomvor

Conversation

@thoughtpolice
Copy link
Copy Markdown
Member

When building Jujutsu crates as a dependency in a Buck project, the usage of #[grammar] requires the .pest files in the code to be in the $PWD relative to where the Rust compiler is invoked on the crate; which isn't identical to how it is done by Cargo, causing a build failure ("file not found").

Rather than special case a Buck build path into the codebase for this (only used by downstream consumers in other projects), inlining the grammar definition into a new Rust file makes this go away.

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

When building Jujutsu crates as a dependency in a Buck project, the usage of
`#[grammar]` requires the `.pest` files in the code to be in the `$PWD` relative
to where the Rust compiler is invoked on the crate; which isn't identical to how
it is done by Cargo, causing a build failure ("file not found").

Rather than special case a Buck build path into the codebase for this (only used
by downstream consumers in other projects), inlining the grammar definition into
a new Rust file makes this go away.

Signed-off-by: Austin Seipp <aseipp@pobox.com>
When building Jujutsu crates as a dependency in a Buck project, the usage of
`#[grammar]` requires the `.pest` files in the code to be in the `$PWD` relative
to where the Rust compiler is invoked on the crate; which isn't identical to how
it is done by Cargo, causing a build failure ("file not found").

Rather than special case a Buck build path into the codebase for this (only used
by downstream consumers in other projects), inlining the grammar definition into
a new Rust file makes this go away.

Signed-off-by: Austin Seipp <aseipp@pobox.com>
@thoughtpolice
Copy link
Copy Markdown
Member Author

thoughtpolice commented Aug 25, 2024

When building Jujutsu crates as a dependency in a Buck project,

To be clear: I mean when I try to use jj-cli and jj-lib in a Buck project separate from this repository; they are being downloaded from crates.io actually. This is not from the Buck branch in #1997 (though coincidentally it does help that branch too.)

@thoughtpolice
Copy link
Copy Markdown
Member Author

thoughtpolice commented Aug 25, 2024

@yuja I know you work a lot on the parsers, more than anyone else, so I'm particularly keen on your feedback. I can maybe keep trying to find workarounds on my side so we don't need this patch, but if this doesn't make your life too much worse or you're ok with it, it would be nice.

@yuja
Copy link
Copy Markdown
Contributor

yuja commented Aug 25, 2024

For ease of maintenance, I prefer a separate .pest file. Can't we fix pest-derive somehow?

When building Jujutsu crates as a dependency in a Buck project, the usage of
#[grammar] requires the .pest files in the code to be in the $PWD relative
to where the Rust compiler is invoked on the crate;

Apparently the search paths are:

  1. $CARGO_MANIFEST_DIR
  2. $CARGO_MANIFEST_DIR/src

https://github.com/pest-parser/pest/blob/master/generator/src/lib.rs#L68

I don't know buck or bazel, but maybe cargo environment variables could be faked?

@thoughtpolice
Copy link
Copy Markdown
Member Author

Aha, I was in fact holding it wrong and was able to diagnose it; there is a flag for setting CARGO_MANIFEST_DIR but I hadn't set it. Nevermind!

@thoughtpolice thoughtpolice deleted the aseipp/push-zsxzytzomvor branch August 25, 2024 02:29
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

Successfully merging this pull request may close these issues.

2 participants