-
Notifications
You must be signed in to change notification settings - Fork 45
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
Add env.json #613
Add env.json #613
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's really nice for the SDK interop, thanks! The only thing that comes to mind is that given that env.json
is meant to be used across SDKs it could be a better idea to store it in a similar repo (which is one more repo to maintain, but makes a lot more sense for the SDK developers). I don't think the decision on that should be tied to this PR though.
@dmkozh :nod: That's a great point. I guess this is somewhat similar to XDR. For the longest time XDR lived in the stellar-core repo, and for the most part that was fine, but it was inconvenient at times. I'm inclined to keep the file here in the short term, because there are advantages – process optimizations mostly – to keeping things that change together, together. But over time it will be even more important that this file is discoverable, and versionable really clearly. It's that last aspect of versioning that I think was our big win with moving XDR into its own repo, and I expect it'll be similar here. I'm happy to move it into a new repo now if folks think that's a good idea now, or we can wait a little while and do it down the track. |
+1 on that, I would be inclined to move it away from env closer to the release, as the env interface should've stabilized enough at that point. |
What
Add env.json and use it for generating the macro_rule table that is used for generating the environment host interface and interface implementations.
Why
There are a lot of host functions.
There are so many that we use macro_rules to code generate most of the code relating to them in our Rust crates.
Unfortunately macro_rules doesn't help us code generate things that aren't Rust, like documentation, or non-Rust SDKs, or tools.
If we have any hope of developers building alternative non-Rust SDKs, we need to provide some things like the functions that SDKs need to extern reference in a way that will allow them to code generate. In a similar way we code generate today in the Rust crates.
JSON is accessible to everyone, and there are plenty of tools for it.
I considered a couple approaches to this change:
I initially wanted to do (1), however macro_rules cannot do things like write files. Only proc-macros can. So it was more practical to do (2) and generate the existing macro_rules from a .json file.
This makes the JSON file the source of truth. If we're adding new host functions, we should add them to the JSON file and rebuild.
This proc-macro generates the existing macro_rule, the same as what used to be there. Here is a snapshot of the generated code:
There are some downside to loading files in proc-macros.
The Rust compiler will not always determine if the files need to trigger a rebuild of the proc-macro. Because of this you might edit the JSON file and not see a change. Running cargo clean will always resolve this. This is a bit inconvenient, but given that we don't add host functions everyday, I think this is an okay tradeoff.