Creating example directory with books and hashes examples from wiki

[frumioj]

No description provided.

[lthibault]

John, thanks a million for this contribution! I've been meaning to do something similar, but never quite found the time. This really helps a lot! 🙂

I've flagged a couple of minor formatting issues. Otherwise LGTM.

(EDIT: I did note that this is currently a copy/paste of the non-building examples. But I appreciate the initiative. We'll get this fixed!)

[zenhack]

I opened a pr on this pr: frumioj#1

Can confirm that after fixing the build issues it just hangs. Here's an interesting observation: If I apply this additional patch, it works:

diff --git a/example/hashes/hashesserver.go b/example/hashes/hashesserver.go
index f09b444..24ba1c1 100644
--- a/example/hashes/hashesserver.go
+++ b/example/hashes/hashesserver.go
@@ -124,8 +124,25 @@ func client(ctx context.Context, rwc io.ReadWriteCloser) error {
 }
 
 func main() {
+	l, err := net.Listen("unix", "/tmp/fff")
+	chkfatal(err)
+
 	ctx := context.Background()
-	c1, c2 := net.Pipe()
-	go serveHash(ctx, c1)
+
+	go func() {
+		c1, err := l.Accept()
+		chkfatal(err)
+		serveHash(ctx, c1)
+	}()
+
+	c2, err := net.Dial("unix", "/tmp/fff")
+	chkfatal(err)
+
 	client(ctx, c2)
 }
+
+func chkfatal(err error) {
+	if err != nil {
+		panic(err)
+	}
+}

i.e. it works fine if you use a real network connection, instead of net.Pipe(). So I suspect this example is suffering from #189.

[lthibault]

i.e. it works fine if you use a real network connection, instead of net.Pipe(). So I suspect this example is suffering from #189.

@frumioj The current work-around for this is to increase the MaxConcurrentCalls field in server.Policy. Note that this will deadlock if this number of concurrent calls is exceeded, so you'll need to set it to a sufficiently high value (and maybe track client-side connections)?

We're working on a fix for this. ETA unknown, but it's basically our highest-priority item.

[lthibault]

Is this ready to merge?

[frumioj]

Is this ready to merge?

I had planned to:

1. Get the books example done
2. Put documentation comments in my code

before merge. Can also do either/both as a new PR? WDYT?

[zenhack]

Let's wait on these tasks before merging; it's not like we're likely to have conflicts with other ongoing work. Quoting John Kemp (2021-12-18 13:22:55)

…

Is this ready to merge? I had planned to: 1. Get the books example done 2. Put documentation comments in my code before merge. Can also do either/both as a new PR? WDYT? -- Reply to this email directly, [1]view it on GitHub, or [2]unsubscribe. Triage notifications on the go with GitHub Mobile for [3]iOS or [4]Android. You are receiving this because you commented. Message ID: ***@***.***> Verweise 1. #204 (comment) 2. https://github.com/notifications/unsubscribe-auth/AAGXYPX6AFBNUDN7X37KRJ3URTGP7ANCNFSM5KJC5YIA 3. https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 4. https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub

[frumioj]

In the books example code, I get a panic when reading the books schema into the decoder: panic: capnp: decode: too many segments to decode - when looking at the code that makes the error, I find it difficult to believe that this file results in > 512 segments? There is a newline at the end of the file now. Is this the correct way to call the exe?

johnk@REGEN:~/src/go-capnproto2/example/books/ex2$ cat ../books/books.capnp
using Go = import "/go.capnp";
@0x85d3acc39d94e0f8;
$Go.package("books");
$Go.import("foo/books");

struct Book {
    title @0 :Text;
    # Title of the book.

    pageCount @1 :Int32;
    # Number of pages in the book.
}
johnk@REGEN:~/src/go-capnproto2/example/books/ex2$ cat ../books/books.capnp | ./bookstest2 
panic: capnp: decode: too many segments to decode

goroutine 1 [running]:
main.main()
	/home/johnk/src/go-capnproto2/example/books/ex2/books2.go:15 +0x266
johnk@REGEN:~/src/go-capnproto2/example/books/ex2$ cat books2.go
package main

import (
    "fmt"
    "os"

    "books"
    "capnproto.org/go/capnp/v3"
)

func main() {
    // Read the message from stdin.
    msg, err := capnp.NewDecoder(os.Stdin).Decode()
    if err != nil {
        panic(err)
    }

    // Extract the root struct from the message.
    book, err := books.ReadRootBook(msg)
    if err != nil {
        panic(err)
    }

    // Access fields from the struct.
    title, err := book.Title()
    if err != nil {
        panic(err)
    }
    pageCount := book.PageCount()
    fmt.Printf("%q has %d pages\n", title, pageCount)
}

[zenhack]

The example program is expecting to read a capnproto encoded message from stdin, containing a Book struct. Instead, you're feeding it the text of the book schema. So it's interpreting the first 4 bytes of that as the segment count for the message, and ascii @0x8 interpreted as a little-endian integer is indeed greater than 512.

If you want to construct messages from text for testing purposes, look at the capnp tool's convert and eval commands.

[frumioj]

The example program is expecting to read a capnproto encoded message from stdin, containing a Book struct. Instead, you're feeding it the text of the book schema. So it's interpreting the first 4 bytes of that as the segment count for the message, and ascii @0x8 interpreted as a little-endian integer is indeed greater than 512.

If you want to construct messages from text for testing purposes, look at the capnp tool's convert and eval commands.

Thanks. I created a book.txt with a cleartext message, and included instruction for creating the encoded value and reading it into the ex2 books example. I think it's getting pretty close!

[frumioj]

Hi, I think I addressed the review changes (review is also outdated) but github prevents this from being merged. Is this PR OK @lthibault, or what else remains?

[zenhack]

This looks mostly reasonable, a couple comments in-line.

The one other thing I would suggest is that we at least build these in CI (if not actually run them), so we'll notice if the examples get out of date again. Actually I think the existing CI should run it, since it just does ./...; nevermind.

Thanks for the patch!

[frumioj]

LGTM

[zenhack]

LGTM. I'll wait for CI to run and @lthibault to sign off before merging.

[lthibault]

LGTM.

[lthibault]

Bump @frumioj

[frumioj]

Bump @frumioj

Apologies - I somehow missed these comments (may have been on holiday I guess?) - will address ASAP!

[lthibault]

LGTM 🎉

[lthibault]

Many thanks once again for your help, John!

Out of curiosity, what are you building?

[frumioj]

I'm working on building a distributed cryptographic key management system (https://github.com/frumioj/capnkeys). I'd like to use capnproto for the interface to that if possible.

[lthibault]

Wow, that's serendipitous! Believe it or not, I have a need for such a thing for Wetware.

Are you able to chat on Matrix by any chance? You can find me here.