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

Optional CARGO_MANIFEST_DIR #477

Open
Ahmed-Ali opened this issue Apr 22, 2021 · 10 comments
Open

Optional CARGO_MANIFEST_DIR #477

Ahmed-Ali opened this issue Apr 22, 2021 · 10 comments

Comments

@Ahmed-Ali
Copy link

In askama_shared/src/lib.rs#L36 this will cause the Template macro to fail if the CARGO_MANIFEST_DIR doesn't exist;
This works well with the default rustc tool chain; however, this also means if for whatever reason CARGO_MANIFEST_DIR doesn't exist, the crate won't work. (proc-macro panic)

Any chance this can be optional instead?

@djc
Copy link
Owner

djc commented Apr 22, 2021

What do you think it should do if the manifest dit is not available?

@Ahmed-Ali
Copy link
Author

Looking at the code now, doesn't seem like it is reasonable to be able to have optional "Root" path; but maybe we can provide it through a template parameter or something; currently we can provide path relative to the manifest root dir, but maybe alternatively we can provide an absolute path?

which I guess will be way less portable -_-

@djc
Copy link
Owner

djc commented Apr 22, 2021

What is your use case anyway? What build system are you using if not Cargo? It doesn't seem too big of a deal to just set CARGO_MANIFEST_DIR in your environment in some other way.

@Ahmed-Ali
Copy link
Author

We are using BUCK; there is away to set the env variables in there; but it felt bit hacky; yet I can't think for better solution; the absolute path doesn't seem like an improvement

@djc
Copy link
Owner

djc commented Apr 22, 2021

If there is some other environment variable in a Buck context that we can work off of, I'd be open to having that as a fallback.

@Ahmed-Ali
Copy link
Author

Ahmed-Ali commented Apr 22, 2021

doesn't seem like we have an equivalent really, for some reason. The only way that worked just fine, is to create a filegroup buck target, set its location is the value of CARGO_MANIFEST_DIR and place the templates there. Yet the challenge here is any other project uses askama (directly or transitively, might be required to do some additional work arounds).

@Ahmed-Ali
Copy link
Author

few folks suggested to use include_str instead. Also another approach might be to avoid invoking this code path if one uses source instead of path in the macro; with some document mentioning that path depends on CARGO_MANIFEST_DIR

@djc
Copy link
Owner

djc commented Apr 23, 2021

Now I'm wondering if include_str!() works in attribute values...

@vallentin
Copy link
Collaborator

I'm not entirely up to date on this, but as far as I recall using e.g. include_str!() in an attribute is possible, but it is unstable and requires enabling the extended_key_value_attributes feature. See rust-lang/rust#78835 and rust-lang/rust#78837.

@sunshowers
Copy link

sunshowers commented Apr 26, 2021

You can use include_str in attributes on stable through a declarative macro, I believe. It's a bit of a hack but it works well. See https://github.com/facebookincubator/cargo-guppy/blob/main/tools/determinator/src/rules.rs#L159-L187 for an example, and https://docs.rs/determinator/0.4.0/determinator/rules/struct.DeterminatorRules.html#associatedconstant.DEFAULT_RULES_TOML as the result.

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

No branches or pull requests

4 participants