-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
RFC - cargo templates #2922
RFC - cargo templates #2922
Conversation
Create 0000-cargo-templates.md
This comment has been minimized.
This comment has been minimized.
Co-authored-by: XAMPPRocky <4464295+XAMPPRocky@users.noreply.github.com>
Co-authored-by: XAMPPRocky <4464295+XAMPPRocky@users.noreply.github.com>
Co-authored-by: XAMPPRocky <4464295+XAMPPRocky@users.noreply.github.com>
Thanks for the fixes @XAMPPRocky ❤️ I copied this from the internals forum (hence why it had some funky markdown syntax) |
@jsjoeio It looks like a good start! I'm assuming that you're planning on fleshing out the details before the RFC is accepted, correct? E.g., a more precise description of what a template is, and how it works. |
(Moving our discussion to the main comment thread so that it's more visible). My apologies @ckaran - now I see the hole you're pointing out. I think this RFC does not sufficiently answer this requirement from the template:
Which seems to be around what you're encouraging we do. Thanks for calling that out. (I naively overlooked that thinking the implementation details could be resolved after acceptance of the RFC). How would this be implementedI think I'd like to import a comment from the Pre-RFC by @ckaran which answers the question, "but how do we implement this?" My goal: import it here to surface it and start a discussion around what we agree about this proposed implementation and what we would like to further discuss. Once we reach an agreement, I'll add it to the RFC so that it is part of the final proposal. Implementation Solution 1This solution was proposed by @ckaran in the Pre-RFC in the comments here: "I want to turn attention to how templates are supposed to function for a moment. Basically, what is the underlying engine going to do, and going to be able to do. Is the engine going to be allowed to do arbitrary things? Or is it going to be sandboxed? How will it be sandboxed? Are we going to develop a new domain specific language (DSL) for templates alone? Can we use a language that is already available? My vote is to use the absolute simplest method I can think of; copy/paste/text substitution. This would involve making the convention that each child of the template directory is a complete template. Thus:
[package]
name = "A"
version = "0.1.0"
authors = ["Alice B Caring <abc@place.com>"]
edition = "2018"
description = """A small, complete template for the [foo-bar-baz](https://crates.io/crates/foo-bar-baz) crate.
This template will create a small, but complete, project that uses the [foo-bar-baz](https://crates.io/crates/foo-bar-baz) crate.
"""
keywords = ["foo", "bar", "baz"]
categories = ["template"]
license = "MIT OR Apache-2.0" # It would probably be better to require that templates use the same license as the original crate.
[badges]
maintenance = {status = "experimental"}
[dependencies]
foo-bar-baz = {version = ">= 0.4, <= 0.6", features = ["serde"], uuid = "B23901D91F984C39888E468F531E0F97"}
[keys]
authors = {description = "A list of strings of authors like 'Alice B Caring <abc@place.com>'", default = ""}
date = {description = "The date that the file was generated. Can be any form you want, it will be directly copied into the template.", default = ""} Most of those keys are directly copied from the Cargo docs, and so should already be familiar to users of the tool. The only new keys are When using cargo --template, you might have subcommands like:
The generate command would have two required arguments; the first is the name of the template to use (A) in the example above, and the location where you want the template to be copied to (
but if I used template B, I'd see something like this:
This actually solves the 'what is a template?' question; to use template B correctly, I would need to use The engine would be a simple text substitution engine. It would search for text strings of the form This solves the major security problems because the template isn't being executed, and you can't really do a DOS attack on text substitution (notice how the method above doesn't permit re-evaluation of the generated text, which means you don't get a billion-laughs style attack). This leaves two issues; how do template authors indicate that they really want the string {{ or }}, and how do you reuse a template for the same destination path multiple times in a row?
I think that something like the above would be sufficient for most templates; can anyone think of something where it wouldn't work?" |
Should urls might be used for sources as well? I think it's quite handy to fetch template directly from git source (e.g. from yet unpublished crate or private repo). Example usage might be |
I think it could be, but I would strongly prefer it not to be. The issue was covered in the RFC; your specific example means that we're tied to both That said, I agree that this would be a useful ability. Quickly scanning the cargo subcommands wiki page, it looks like cargo clone would be a good starting point for this. Is there a way of tying the commands together? E.g., if you have cargo clone installed already, could |
@jsjoeio The motivation should contain a background on how granular you want to make things on practical examples ie 1. what is the difference to example code 2. why are automatic substitutions/code production useful. The main points of disagreemant between community and cargo team should be listed and resolved in the rfc. Personally I dont like template as name due to the usage in c++, where code is derived from code (vs here code is created as starting point), but I think it is okayish. |
@matu3ba I can't answer for @jsjoeio , but I can describe what I see this being useful for; perhaps @jsjoeio can provide further explanations. The main use I see for such a feature is for creating templates for large frameworks where the example requires a significant amount of customization before it is ready to use. For example, amethyst templates might require the same key in numerous places (e.g.,
As for using text substitution, I'll admit that I came up with that idea. Other methods are possible, but they are more difficult to implement, and could leave the end user open to attacks by bad actors. Text substitution seemed like a good balance between expressive power, danger, and ease of integrating with |
Regarding literal In my view, anything not built around whitelisting file patterns is a footgun of a nature comparable to the C preprocessor's approach to macros. (I say this as someone who had a I also find it a little odd to push everything except pulling from crates.io off to a later date when it seems most sensible to test with local files and/or git repos so you're not encouraging people to pollute crates.io with crates, just to test an experimental feature. |
As @ckaran said, we covered this in the Pre-RFC. I think we should should leave it out and keep this MVP as small as possible. |
@matu3ba those are fair points! I was hoping the Pre-RFC + RFC would serve as discussions places for the community to decide "yes, we want this" vs. "no, we don't want this" rather than using a survey (unless that's how other RFCs do it?) @ckaran gave some thorough and detailed examples so I won't repeat that here. |
As far as I can tell, I don't see any comments on here yet from members of the Cargo team. Would love to get their feedback and see if they think this is a worthy addition to |
This is a really good point. You should be able to test it somehow locally so that you don't have to push to crates.io. @ckaran thoughts? |
I want to redirect the conversation to the "Implementation Solution 1" by @ckaran. I feel like we can simplify it even more.
And for this "how do you reuse a template for the same destination path multiple times in a row?" maybe the v1 doesn't account for this. If we implement it and people request this feature, then we add it in. I feel like those last two issues can be considered out of scope. Trying to keep this as small as possible. @ckaran thoughts? |
I think that testing locally is going to be a must-have feature; without it, crates.io will rapidly fill up with terrible half-baked templates. It would also make automation a little bit easier, especially when the template is in a different crate than the crate the template is for; you can run a cron job that pulls the latest crate, then tests the templates locally against the latest pull. If all tests of the template pass, then the script can bump up the range of versions that the template works with automatically, and publish that to crates.io. If any tests fail, then the template will not have been published (leaving crates.io in a clean state), and the template author(s) can correct as needed. |
I think that doing this would oversimplify the An alternative to creating an RFC that includes both the command and the template directory structure/substitution strings might be to specify the template directory structure only. Other tools could then be created as cargo extensions that are aware of and understand the directory structure/substitution strings. Once the community decides which tool is the best one to use, it could be incorporated into core |
How do we show list of templates for a crate like |
The command will likely need to be run within a project to get a listing. That would mean that you would only see the templates defined within that particular project. To get more templates, you'd need to search crates.io for template crates specific to your needs, download them, and then use them. It would be better to have the tool do this for you, but I'm starting to lean away from an RFC that specifies the command, and lean towards an RFC that specifies the directory structure and precise format of a template (e.g., |
Ah, so it is something like a protocol. Sounds like the ability to support |
I want to thank @jsjoeio for putting this together, and for everyone's comments. Per the comment at #2922 (comment), the author has expressed that they will not be able to continue with it, so I am going to move to close this as postponed. There is still interest in a feature like this, so if someone is willing to pick it up again, starting with this and trying to address the comments here might be a good starting point. Also, such a feature can be developed as a plugin, so working in that direction is always a possibility. @rfcbot fcp postpone |
Team member @ehuss has proposed to postpone this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
I have one question which I haven't seen addressed yet (with just a quick search through the comments.) When looking at the rocket example, for a while rocket had to main branches, If rocket's async branch also updates the templates, would there be a way to use the new templates before they get published? So on rocket's readme, they can say: "Hey, you can try the async branch.
Ah, I see it's likely to be postponed, so well, nvm I guess. |
@tbelaire Yes it's been postponed because nobody has time to finish it up yet (are you volunteering? 😉), but the likely process would be fairly simple:
As a summary, the discussion moved away from creating tools to creating a standard directory structure & format for templates. The last one discussed wouldn't care about branches at all, so the above would probably be sufficient. |
The final comment period, with a disposition to postpone, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. The RFC is now postponed. |
@ckaran, It looks like this RFC nearly made it to the finish line and sat down. It was mentioned above that "nobody has time to finish it". If someone would point me at the next steps to push it over the finish line, I'd love to contribute if possible. I'm...shudders...volunteering. |
@thor314 Honestly, it's been so long since I looked at this that I completely forgot about it. Skimming it, it looks like what needs to be done is to write an RFC outlining the directory structure, what templating is (e.g., the use of I can give you more hints here and there, but I'm kind of swamped at work right now, so my responses will be late and sporadic. Sorry! :( |
...and I would like to re-raise my concern about blacklisting vs. whitelisting which files get templated rather than just passed through verbatim. I would consider it a dereliction of my duty as a responsible package maintainer to use the standard templating tool rather than providing my own if the standard templating tool uses blacklist-based templating prone to what I experienced with the previous attempt. Too much of a footgun to be consistent with the rest of a Rust codebase. (I misunderstood the docs and wound up mangling my justfile because it was blacklist-based and both the templating and just used |
@ssokolow I understand your concerns, but I think this would be an issue regardless of the templating scheme used. Blacklisting/whitelisting require your to maintain the lists appropriately. Using external tools that convert all files in a directory to a template format (whatever format might be chosen) might not be used, or might be inappropriately used1. We might be able to add some metadata to a Footnotes
|
I'd need to see the fine details of a given approach to discuss them but, generally, I think the approach most in line with Rust's philosophy elsewhere is to have something like a That way, you don't wind up accidentally corrupting an un-checksummed file that happens to contain the relevant metacharacters. (eg. a As for defaults, there could be discussion about whether things like It would also resolve the idempotency issue by providing a signifier that the output of a template is not a valid template. (Whether it's possible to specify that a template generates a
I'm curious what issues you foresee with that approach. To me, that feels like pushing back against mandatory seatbelt-wearing in vehicles because an improperly used shoulder belt can cause injury to a child in a crash. |
Sorry, I wasn't being clear enough.
Strangely enough, I was coming at it from the opposite direction; such files ARE processed by the external tool to turn them into templates, even if they contain those sequences. The templatized equivalents are no longer valid files, but when the template engine is run to create the template instances, all of the
I only see the following tradeoffs, but I haven't thought deeply about this:
Neither of these should be used as an excuse NOT to do something smart though! Seat belts are a GOOD thing! We just need to be aware that they aren't perfect, and try to minimize the possibility of errors, that's all. |
This is sounding less like you want a Cargo template system MVP that actually stands a chance of getting approved and more like you want to jump right to something straight out of CMake, to be honest.
I still don't see why I like that, as long as I don't touch the contents of For a templating MVP:
This approach should be trivial to make forward-compatible with something that requires a This "make an MVP and then incrementally add stuff as proven necessary, rather than trying to compete with every feature of a competing build system" approach is characteristic of Cargo. |
OK, so if I understand your goals correctly, you assume that the engine runs exactly once; nobody tries to pull in updated templates and rerun an engine, which might mung things up on the output side, correct? If that's so, then your design is not only fine, it's preferable. I agree with you that |
Exactly. I think interoperability dependent on buy-in from other tools and support for re-running the engine is something that shouldn't be spec'd out now when we don't even have a simple "like |
Works for me. |
🖼️ Rendered
📝 Summary
RFC for the following new feature:
Example using a template from the
rocket
crate:💁 Additional Notes
This RFC went through two Pre-RFCs, which can be viewed here: