golang
x/pkgsite: update codec generation for ast changes
In https://go.dev/issue/76031, we added a new field to ast.BasicLit. Meanwhile, pkgsite encodes the AST in binary.
The new field therefore either breaks pkgsite encoding of package source (see internal/unit.Documentation.Source), or makes our codec generation stale (running go generate ./internal/godoc/... results in a diff adding the encoding/decoding of the new field (here)
Many questions to answer:
- How does changing our encoding version work?
- Can we update the codec without regenerating all documentation (a rather expensive endeavor at this point).
- Should we punt on updating the codec, and add a mechanism in generation that skips this field? The new field is
ValueEnd, and if it's 0 it degenerates to the old behavior. - What do we do in the future if there was a nontrivial change to the AST that warranted a new encoding version?
@ethanalee-work would you mind looking into this? I don't think it's urgent (I'll fix the failing test), but we should at the very least update pkgsite so that go generate ./... is clean. Ideally, we'd also brainstorm here what to actually do if we need to update the encoding. (Apologies if this is already documented somewhere and I missed it).
CC @jba @nicholashusin @adonovan
Change https://go.dev/cl/721800 mentions this issue: internal/godoc: fix codec generation test
Gosh, I hope I designed the codec to handle non-breaking changes like this... will look tomorrow.
@jba to be clear I don't know whether you did or not, but didn't have time to check: I wanted to file this issue before I fixed the broken test.
And even if you didn't, we'll figure it out.
This is my solution to adding fields: https://github.com/golang/pkgsite/blob/master/internal/godoc/codec/doc.go#L65-L77
Untested in the field (n.p.i). I hope it works...
@jba nice. Thanks for looking that up.
@ethanalee-work we should probably write a unit test for this specific case and then go generate ./internal/godoc/... and ensure the test passes. I.e. test that some fixed byte slice still decodes to an equivalent BasicLit.
Change https://go.dev/cl/727201 mentions this issue: internal/godoc: add unit test for TestDecodeBasicLit