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

Restrict generate! paths to paths within crate boundaries #555

Open
rvolosatovs opened this issue Apr 7, 2023 · 6 comments
Open

Restrict generate! paths to paths within crate boundaries #555

rvolosatovs opened this issue Apr 7, 2023 · 6 comments

Comments

@rvolosatovs
Copy link
Member

Crates using generate! with paths leading outside the crate boundary cannot be dependent upon in cargo vendor use cases.

E.g. see https://github.com/rvolosatovs/wasi-vendor-repro for a reproducer

Refs bytecodealliance/preview2-prototyping#133

A draft PR will follow

@rvolosatovs
Copy link
Member Author

Note, that the fix enabling cargo vendor use cases is actually trivial - symlinks bytecodealliance/preview2-prototyping#134

@alexcrichton
Copy link
Member

Thanks for the report! I'm not sure if there's much that can be done within this crate, though, about that?

@rvolosatovs
Copy link
Member Author

Thanks for the report! I'm not sure if there's much that can be done within this crate, though, about that?

The proposal here is to disallow path references like this https://github.com/bytecodealliance/preview2-prototyping/blob/083879cb955d7cc719eb7fa1b59c6096fcc97bbf/wasi/src/lib.rs#L7, which lead outside the crate

generate! would fail with a descriptive error in this case. I have a WIP PR ready for it, but adapting tests, which rely heavily on this feature in this repository is quite time-consuming

@alexcrichton
Copy link
Member

I'm not sure that disallowing outside-root paths is a way to handle this, though, since not all crates will make their way to crates.io which would cause this to be an issue. Personally I feel like it's better to be more flexible with the macro, but still have facilities available for those publishing to crates.io.

@rvolosatovs
Copy link
Member Author

I'm not sure that disallowing outside-root paths is a way to handle this, though, since not all crates will make their way to crates.io which would cause this to be an issue. Personally I feel like it's better to be more flexible with the macro, but still have facilities available for those publishing to crates.io.

Well, this does not break the crates being published, rather it breaks all downstream consumers of these crates using the standard cargo functionality: cargo vendor. Whether those crates downstream consumers depend upon are published to a particular crate registry or just consumed via git is irrelevant here.

@alexcrichton
Copy link
Member

I don't think I quite agree with that because if a crate was cargo publish'd using out-of-tree paths then it would still be broken on crates.io. If depended on with Cargo via a git dependency it would still work, which I say falls under the umbrell of "standard cargo functionality".

I don't dispute that the vendor use case is broken, but what I'm saying is that I don't think it's the right solution to require all uses to be vendor-ready. It should definitely be possible to be vendor-ready, but I don't think it's worthwhile to require that of everyone.

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

2 participants