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

io.Writer panic: name Write not declared #788

Closed
schollz opened this issue Apr 28, 2023 · 7 comments
Closed

io.Writer panic: name Write not declared #788

schollz opened this issue Apr 28, 2023 · 7 comments
Assignees
Labels
🐞 bug Something isn't working 📦 🤖 gnovm Issues or PRs gnovm related

Comments

@schollz
Copy link
Contributor

schollz commented Apr 28, 2023

I'm a little stumped. I have valid Go code (tested by converting to .go and running tests). But it seems not to be valid Gno code. It is using io and doesn't like using the .Write() even though I can see that io and that function seem to be integrated into Gno. I wonder if I'm not importing thing correctly? I could use help figuring out whats going on.

Here is my Gno package + test: d8d6297

Its just a small piece of a .wav file encoder (I'm working on a music application).

I'm running

gno test --verbose examples/gno.land/p/demo/wav

And getting the following error:

panic: name Write not declared [recovered]
        panic: ./examples/gno.land/p/demo/wav/wav.gno:16: name Write not declared

But if I change .gno to .go I have no errors:

go test -v
=== RUN   TestWav
--- PASS: TestWav (0.00s)
PASS
ok      github.com/gnolang/gno/examples/gno.land/p/demo/wav     0.003s

If something needs to be added to the Gno library, I'm happy to work on that. I thought asking here would be the fastest way to find out if its just me doing something I'm not supposed to..

@moul
Copy link
Member

moul commented Apr 28, 2023

encoding/binary isn't ported yet, so we have two modes for running gnovm: one with Golang packages like encoding/binary and fmt, and a restricted mode with only ported stdlibs and published library contracts.

To fix this, we can try:

  1. Port encoding/binary - can someone give it a try?
  2. Update tools to display clearer errors when a package is missing - @harry-hov @thehowl, could one of you take a look since you worked on related features recently?
  3. Add alternative contexts to the gno test CLI to temporarily bypass this limitation - I'll try it out now. (feat: add 'gno test -use-native-fallback' #789)

@schollz: consider working on another PoC while waiting for 1. or 3. to be addressed.

Edit: the port will probably be harder than expected since encoding/binary official implementation relies on reflection.
Edit 2: Our focus is currently on markdown rendering. We could explore creating a custom function to automatically encode binaries in base64/hex for sharing within markdown. Let's pause this exploration for now and discuss our approach to managing binaries in general.

@moul
Copy link
Member

moul commented Apr 29, 2023

I'll explore a related topic to support additional content-types, while maintaining our markdown-first approach. This includes investigating the implementation of a secure SVG image generation or simple PNG alternatives for the gno blog. However, it's important to note that while these features are of interest to me and can push the limits of Gno, they are not critical to the mainnet and will be a secondary priority or potentially blocked to keep the mainnet as streamlined as possible.

See #439: rendering extensions
See #690: initial generative ascii art

@schollz
Copy link
Contributor Author

schollz commented Apr 30, 2023

Thanks Manfred.

automatically encode binaries in base64/hex for sharing within markdown

This is the approach I was going towards, but for now I'll leave this and move onto other PoCs!

@thehowl
Copy link
Member

thehowl commented May 5, 2023

I've assigned myself to the issue to try improving the error message. Though I think I'll also consider working on reflection as a larger thing I'll want to try work on (or at least make a PoC to start a discussion on inclusion and implementation).

@schollz as for your POC, I think you could actually add encoding/binary to our stdlibs for now without the reflection parts. Reflection in encoding/binary is only used as a fallback and for complex data structures (slices and structs), as most others work simply on type switches.

@schollz
Copy link
Contributor Author

schollz commented May 5, 2023

I think you could actually add encoding/binary to our stdlibs for now without the reflection parts

Good idea, I will take a stab at that.

@schollz
Copy link
Contributor Author

schollz commented May 7, 2023

Okay to circle back to this and to make a signpost for future travelers:

I did try to port encoding/binary but it uses a lot of reflection and in the end, all I needed was writes for uint16 and uint32 in little-endian. So I made a workaround that should work for a lot of people encountering this.

My workaround is simply to create a custom io.Writer function that has explicit functions for writing the variables I need (in my case WriteUint32 and WriteUint16). The implementation of those functions is taken directly from the encoding/binary source - for example: little endian PutUint32.

Here's my gno working example for reference: https://github.com/schollz/gno/blob/bytebeat/examples/gno.land/p/demo/audio/riff/riff.gno

This is not as easy as importing encoding/binary but I personally like these solutions to copy-paste only the code you need. If gno adopts proverbs, I would put up the classic "A little copying is better than a little dependency.".

@ajnavarro ajnavarro added 🐞 bug Something isn't working 📦 🤖 gnovm Issues or PRs gnovm related and removed bug labels May 15, 2023
@moul moul moved this to 🏆 Needed for Launch in 🚀 The Launch [DEPRECATED] Sep 5, 2023
@moul moul added this to the 🚀 main.gno.land milestone Sep 6, 2023
@Kouteki Kouteki removed this from the 🚀 Mainnet launch milestone Oct 16, 2024
@thehowl
Copy link
Member

thehowl commented Oct 22, 2024

We have support for encoding/binary limited to BigEndian/LittleEndian; we don't have Write support at the moment because it requires reflection.

I'm closing this issue in favour of #1267 for tracking changes to the standard libraries. The error message is hard to improve; I'm more interested in unifying the standard libraries and providing no-op shims in the stdlibs for methods like this one instead, so that the error doesn't show up.

@thehowl thehowl closed this as not planned Won't fix, can't repro, duplicate, stale Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working 📦 🤖 gnovm Issues or PRs gnovm related
Projects
Status: 🚀 Needed for Launch
Development

No branches or pull requests

5 participants