Grackle adoptation for gokv tutorial

[mjschwenne]

Hello!

I'm a PhD student working with Tej this semester on grackle, a tool which generates marshaling code and proofs from protobuf files. It's still a work in progress, but a lot of the basic features are complete and I've applied it to the gokv tutorial. Note that there are other proto files in the PR. Many of them needs features like slices which we're currently working on but aren't ready yet.

There is a new script update-grackle.sh which can run grackle and update the output. I've added a check to the CI workflow to ensure that the grackle output is kept up to date.

Please let me know if you have any questions or suggestions to improve this!

[upamanyus]

Would this be nicer as a python script (especially if this gets more complex in the future)? E.g. something like Perennial's etc/update-goose.py.

[tchajed]

I also would prefer to migrate this to python now rather than later. It's already at my personal limit for bash script complexity.

[mjschwenne]

I would also prefer that, although it was @tchajed who suggested bash in the first place. I will work on this shortly.

[tchajed]

Sorry for flip flopping on that, I thought this would be simpler than it turned out.

[upamanyus]

Cool! One thought: would it make sense to add (some variant of) ./update-grackle.sh to the default make target? Right now, I rely on running make before committing changes to make to make sure that nothing is broken, and it would be nice if that covered grackle-generated files as well.

One possibility is simply adding ./update-grackle.sh to the check target in the top-level Makefile, which would actually update the grackle-generated files. This seems nice since it's simple to implement and automatically fixes out-of-date files.

Another possibility would be to do some sort of read-only check in make check instead of updating the files. So if foo_gk.go is out of date, the read-only check would fail with an error instead of overwriting it with updated output. This seems nice since make check currently only "checks" stuff rather than modifying anything, and it would keep CI as simple as running make check.

[tchajed]

I also would prefer to migrate this to python now rather than later. It's already at my personal limit for bash script complexity.

[tchajed]

There should be a comment that this is auto-generated at the top of the file.

[tchajed]

I think adding a read-only check feature to grackle is a bit more complexity than necessary to do upstream.

I would like to integrate it into your make workflow. My suggestion would be to add an update-grackle target that runs the script; if you are okay with depending on grackle's dependencies (which I believe is to have protoc installed) then we can also make that part of the default target. We can do a little more work in CI to call make update-grackle and check afterward that git status is empty; we use that approach in other places and I think adding a little more complexity to the CI checks isn't a big deal.

[mjschwenne]

if you are okay with depending on grackle's dependencies (which I believe is to have protoc installed)

Correct, that is the only external dependency for grackle.

I'd be happy to integrate grackle checking into the makefile. As Tej mentioned, grackle doesn't currently support checking output without writing it, although it may be possible to bootstrap the --debug flag which prints the grackle output to standard out for this purpose.

The easiest option at the moment is probably to add ./update-grackle.sh to the fix make rule and utilize the new CI job to check that the grackle output is up to date if you would like to avoid make check updating files. Otherwise, we could add the grackle updates to make check as well.

Something else to keep in mind is that grackle is still being developed and it's possible that my changes to upstream grackle will cause the CI job to fail even if no other changes to the grackled portion of gokv are made. We may want to change the @latest tag to some fixed commit but that runs the risk of missing new features.

[tchajed]

Looks like you forgot to add the python script (update-gracke.py). Also the github workflow should reference the new file and not the sh script.

I agree that using a tagged version is safer, to avoid CI failures while grackle is rapidly changing. We can still bump that version regularly.