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

Old NEAR contracts won't compile with the new borsh re-exported from near-sdk-rs ("Could not find borsh") #277

Closed
fadeevab opened this issue Jan 12, 2024 · 5 comments

Comments

@fadeevab
Copy link

I hit the issue with the new near-sdk-rs: near/near-sdk-rs#1129

I tried to compile my project with near-sdk-rs 5.0.0-alpha.1 because of this issue near/near-sdk-rs#1119, however, build fails with the errors:

error: proc-macro derive panicked
 --> src/main.rs
  |
6 | #[derive(BorshSerialize, BorshDeserialize)]
  |          ^^^^^^^^^^^^^^
  |
  = help: message: called `Result::unwrap()` on an `Err` value: Could not find `borsh` in `dependencies` or `dev-dependencies` in `/home/xxxx/yyyyy/Cargo.toml`!

BEFORE, a simple import of near_sdk::borsh::self was enough to make derive macro work.
NOW, either a direct borsh import is needed (thus, re-export from near-sdk becomes useless), or #[borsh(crate = "near_sdk::borsh")] is necessary for every structure (as suggested by @frol, near/near-sdk-rs#1129 (comment))

@frol
Copy link
Collaborator

frol commented Jan 12, 2024

This new behavior is by design and serde does the same.

@frol frol closed this as completed Jan 12, 2024
@fadeevab
Copy link
Author

@frol in this case, borsh re-export loses its sense, because using the borsh crate directly will be more convenient. I thought that re-export had to make things more convenient, not more complicated. I have 20 structures, I don't wanna add annotations to each one.

@frol
Copy link
Collaborator

frol commented Feb 6, 2024

@fadeevab Re-export also keeps things consistent. Ideally, I don't want to derive neither serde nor borsh manually, so I at the back of my mind I am thinking how to achieve that without introducing too much magic to it.

@fadeevab
Copy link
Author

fadeevab commented Feb 6, 2024

@frol :) One quick workaround came to my mind: falling back to near_sdk::borsh reexport if near_sdk has found - somewhere around these lines:

let name = &crate_name(BORSH).unwrap();

Little bit dirty :) However, intuitively, if I already use near_sdk::borsh::BorshDeserialize, I expect that #derive just works... Also, another justification to do so: borsh is already under the near (GitHub) account and ecosystem.

All that is not critical of course.

@frol
Copy link
Collaborator

frol commented Feb 10, 2024

@fadeevab Given that there are more than just borsh that needs to be configured near/near-sdk-rs#1142, I don't think this change is worth it.

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