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

Grammar file not found on workspace #325

Closed
librelois opened this issue Oct 28, 2018 · 16 comments · Fixed by #702
Closed

Grammar file not found on workspace #325

librelois opened this issue Oct 28, 2018 · 16 comments · Fixed by #702

Comments

@librelois
Copy link

I use pest in a crate of a workspace, so this crate has no src folder. I was forced to create an empty src subfolder in my crate so that pest could find my file, it's not clean.

It's easy to reproduce this bug, create a new workspace project and try to import pest grammar file from a crate of workspace.

@dragostis
Copy link
Contributor

The question is, how does one solve this? It makes sense for the grammar file to be in src since that will make sure it's included in the crate.

@CAD97
Copy link
Contributor

CAD97 commented Oct 29, 2018

I've been using pest in a workspace, and this hasn't been a problem for me.

What I might see being a problem is a binary crate using bin instead of src. But for my own workspace, I've had no issues; the src path is the src of the crate I'm working in.

Could you provide an exact reproduction steps? (Also, OS may play a role here, unfortunately.)

@dragostis
Copy link
Contributor

I think he means that it will not work where no src is present within a crate?

@librelois
Copy link
Author

I have created an example repository that reproduces the bug.
If you clone the repository it does not compile. If you create an empty folder named src in my-usb-crate then it compiles.

https://github.com/librelois/workspace_use_pest

@librelois
Copy link
Author

The question is, how does one solve this?

My opinion is that the path should start from the root folder of the crate, and not from the sub-folder src :)

@CAD97
Copy link
Contributor

CAD97 commented Oct 29, 2018

Oh, that project is set up in a very unconventional manner.

Most workspaces are set up where each subcrate has a src directory containing the sources, rather than redefining where the entry point is in Cargo.toml. (They also can have other "magic" directories, such as tests and examples.)

Your problem isn't that you're in a workspace, but that you've defined a different lib.path.

A common workspace layout
/Cargo.toml
/derive/Cargo.toml
/derive/src/lib.rs
/derive/tests/test001.rs
/traits/Cargo.toml
/traits/src/lib.rs

Any change to where the #[grammar = ""] path starts from is technically a breaking change. The path is currently defined to be relative to the src directory, so for full strict backwards compatibility, that needs to be the default for a relative path.

One solution is to change the path to instead of being relative from src, to be relative from the directory of lib.path (bin.path?).

Another is to take a page from the JS world and use "anchored" paths: #[grammar = "@/src/pest/grammar.pest"], where @ is basically a "path macro" for %CARGO_MANIFEST_DIR%.

And another (poor?) solution is just to make the path %CARGO_MANIFEST_DIR%/src/../{} work even when src doesn't exist (and "just" collapsing isn't necessarily correct in the face of symlinks, thus why Rust's canonicalize currently uses the filesystem instead of being "simple").

@cramertj
Copy link

cramertj commented Jan 7, 2019

It's worth mentioning that #360 was necessary because not all users are using cargo (and are missing the env vars relied upon here). IMO what this really wants is the ability to use include! and have it pre-expand before proc macros run. I'd consider opening an issue on rust-lang/rust, since this seems like a pretty reasonable thing to want and AFAIK there aren't a lot of great options at the moment.

@dragostis
Copy link
Contributor

@cramertj, expending include!s before procedural macros are run would probably not be a stop-gap solution for our Cargo dependency. IMO the needed API is just being able to get the path of the file where the procedural macro resides. I've thought about opening an issue on Rust a while ago, but the procedural macros API was still heavily discussed.

@cramertj
Copy link

cramertj commented Jan 7, 2019

@dragostis

IMO the needed API is just being able to get the path of the file where the procedural macro resides.

Where the macro resides? What are you referring to here? The path of file that invokes the macro? The reason I suggested include! is that I was imagining a sort of two-stage expansion where the first expands to an include! invocation that pulls in the grammar source and the second processes that grammar, generating the resulting Rust code. This would remove the need for the extra include_str! in order to hint to cargo about the dep-tracking, but it does involve extra steps. A proc_macro API for "resolve this Span containing a path to a TokenStream from the pointed-to-file" would perhaps be a more direct way of achieving this.

@dragostis
Copy link
Contributor

Sorry, yes, I meant the invocation.

The API can be as simple as to only resolve the Span to string or a byte buffer.

@cramertj
Copy link

cramertj commented Jan 7, 2019

string or a byte buffer.

I'd think you'd still want a TokenStream so that you can refer to Spans in the file and do error reporting using the normal API.

@seeekr
Copy link

seeekr commented Jun 21, 2019

Just want to leave a message here: Would be really nice to be able to use a path that does not start from the src/ dir. Use case: I'm using pest from a build.rs and thus don't really want my grammar file(s) be located in src/, which should contain only runtime code.

@brson
Copy link

brson commented Oct 10, 2019

I've hit this problem as well in a project where I'm not using the default cargo src/ directory structure. I'd expect this file to be loaded relative to the directory the source file is contained in, similar to all the Rust builtin include-style macros.

I'm guessing its the way it is because plugins don't have access to the source location.

@dragostis
Copy link
Contributor

@brson, precisely. I've thought about making a push for a way to get that in the procedural macro world, but I haven't really had time for it. FYI, you can also inline pest grammars right inside of the macro by providing the grammar inline. #360

@dwtj
Copy link

dwtj commented Mar 22, 2020

It also initially surprised and confused me that relative paths in #[grammar = "..."] are implicitly prefixed with src/.

I am using Pest in a project with a non-Cargo build system: Bazel (with rules_rust to define crates and cargo-raze to manage dependencies). Bazel Rust builds don't need to follow Cargo directory structure conventions like the use of src/.

Fortunately, similar to others, I can easily work around the issue by moving my grammar file to an otherwise empty src/ subdirectory.

@Nukesor
Copy link
Contributor

Nukesor commented Jul 6, 2022

Hey :)

I just stumbled upon this issue and wanted to put in my 2 cents.

One of my projects has the issue at hand.
The project doesn't have a src folder, as it's split into two different binaries/namespaces that shouldn't have any knowledge about each other. However, they should still belong to the same crate, and thereby shouldn't be put into separate crates in a workspace.

Cargo also fully supports this scenario and using src is just a rough guideline.

I would really like to have the possibility to set the path to the parser file, without having to restructure the whole project.

In case you don't want to change the root of the path to CARGO_MANIFEST_DIR due to backward compatibility issues, I have an alternative proposal.

What do you think about adding a new parameter which discovers from the root path: gramar_from_root (it's a stupid name, but just to give an idea)

tomtau pushed a commit that referenced this issue Aug 23, 2022
closes  #325.

The new logic looks for the [grammar = "$path"] path in CARGO_MANIFEST_DIR/ before using the fallback of CARGO_MANIFEST_DIR/src.

This allows projects that don't have a src folder to use a grammar file, while staying backward compatible.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants