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

feat: return JSON-based ABI generation #73

Closed
wants to merge 2 commits into from

Conversation

itegulov
Copy link
Contributor

Fixes #61

Basically reverts #35 with some refactoring and better error messages

Also, this whole invoke_cargo shebang is getting out of hand. I am surprised there is no crate that encapsulates this tbh

Comment on lines +54 to +63
Args: IntoIterator<Item = Arg>,
Arg: AsRef<OsStr>,
TPath: AsRef<Path>,
Env: IntoIterator<Item = (EnvKey, EnvVal)>,
EnvKey: AsRef<OsStr>,
EnvVal: AsRef<OsStr>,
StdoutFn: FnOnce(ChildStdout) -> Result<StdoutResult, std::io::Error> + Send + 'static,
StdoutResult: Send + 'static,
StderrFn: FnOnce(ChildStderr) -> Result<StderrResult, std::io::Error> + Send + 'static,
StderrResult: Send + 'static,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've seen some crates use cargo as a direct dependency, apparently it has a public facing API separate from the CLI, just never used it directly.

https://docs.rs/cargo

cargo-near/src/abi/mod.rs Outdated Show resolved Hide resolved

fn main() -> Result<(), std::io::Error> {
let abi_entries: Vec<near_sdk::__private::ChunkedAbiEntry> = vec![#(#near_abi_function_invocations),*];
let contents = serde_json::to_string_pretty(&abi_entries)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can actually move this bit to the SDK, simplifying things. See alt: #74

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really, it may look similar but there is an important difference that I have touched upon here: #74 (review)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was thinking a Rust String might be safe, but since we don't have that guarantee, it's just best to switch to the C ABI.

Co-authored-by: Miraculous Owonubi <[email protected]>
@itegulov
Copy link
Contributor Author

Closed in favor of #74

@itegulov itegulov closed this Oct 27, 2022
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

Successfully merging this pull request may close these issues.

Add a check for near-abi version
2 participants