tailscale / tailscale/tailscale

cmd/cloner, cmd/viewer: relax stale generated code check?

Open
#9,634 9 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
36.5k
Forks
3.2k
Avg merge
1d 23h
Merged PRs (30d)
132

Description

In 2352690bde826aa46eeee01af61d7990dab3a5ea, @josharian added a compile-time check that the generated cloner/viewer code is updated. It works, but it's too aggressive. Many people get bit by inability to modify tailcfg.go, add a field, and re-run `go generate` because then even `go generate` fails, saying the generated code is out of date. You have to then know the trick to go delete the two (cloner + viewer) "A compilation failure here" parts and re-run go generate.

Rather than doing the static compilation check, maybe we move it to be init-time with reflect, like:

```patch
@@@ -344,23 +345,29 @@ func (src *RegisterRequest) Clone() *RegisterRequest {
}

// A compilation failure here means this code must be regenerated, with the command at the top of this file.
-var _RegisterRequestCloneNeedsRegeneration = RegisterRequest(struct {
- _ structs.Incomparable
- Version CapabilityVersion
- NodeKey key.NodePublic
- OldNodeKey key.NodePublic
- NLKey key.NLPublic
- Auth RegisterResponseAuth
- Expiry time.Time
- Followup string
- Hostinfo *Hostinfo
- Ephemeral bool
- NodeKeySignature tkatype.MarshaledSignature
- SignatureType SignatureType
- Timestamp *time.Time
- DeviceCert []byte
- Signature []byte
-}{})
+func init() {
+ t1 := reflect.TypeOf((*RegisterRequest)(nil)).Elem()
+ t2 := reflect.ValueOf(&struct {
+ _ structs.Incomparable
+ Version CapabilityVersion
+ NodeKey key.NodePublic
+ OldNodeKey key.NodePublic
+ NLKey key.NLPublic
+ Auth RegisterResponseAuth
+ Expiry time.Time
+ Followup string
+ Hostinfo *Hostinfo
+ Ephemeral bool
+ NodeKeySignature tkatype.MarshaledSignature
+ SignatureType SignatureType
+ Timestamp *time.Time
+ DeviceCert []byte
+ Signature []byte
+ }{}).Elem().Type()
+ if !t2.ConvertibleTo(t1) {
+ panic("boom")
+ }
+}

// Clone makes a deep copy of DERPHomeParams.
```

That's a bit more expensive, but not a ton. The extra type info making its way into the final binaries is a little bloaty, though. So if we're doing runtime checks anyway, we can probably explode on something cheaper, like a hash of all the field names/types/sizes. But that trades things off towards more CPU instead of less disk.

Ideally we could move those compilation failure lines off into their own files that are build-tag-ignored by "go generate" but I couldn't find a way.

/cc @maisem

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.