Skip to content

Commit 23883d3

Browse files
committed
fix(codegen): mask stale generated outputs from the loader instead of unlinking them
Root cause of the check-generate CI failures on this PR, found by running `go generate ./...` locally: base and master pass, every commit of this branch failed deterministically (the "different type each run" in CI was map-iteration order picking which missing type to report first — the failure itself was 100% reproducible). The old upfront `syscall.Unlink` of the exec + model outputs was not redundant — it was LOAD-BEARING for every config that autobinds the package its models are generated into (all codegen/testserver fixtures, the federation examples): 1. old: unlink first → stale models file GONE at schema-load time → autobind finds nothing → modelgen generates all types → OK. 2. this branch (before this commit): stale models file VISIBLE at load → autobind binds the previously-generated types as if they were user-written models → modelgen SKIPS them → the freshly written models file loses them → the exec build reload fails with "unable to find type: <pkg>.<Type>". But the unlink is also exactly what #2345/#3505 are about: delete-then- interrupted leaves the user with no generated file at all. Both properties need to hold, so replace the destructive unlink with a NON-destructive loader mask: - internal/code.Packages gains an Overlay (packages.Config.Overlay) threaded into every Load, plus MaskFile/UnmaskFile. - api.generate masks each existing output (exec + model) with a package-clause-only stub read from the file's own package clause (empty bytes would be a parse error and break loading the rest of the package). Missing/unparseable files are left alone — nothing stale to bind, matching the old unlink's no-op there. - config.Config owns the overlay map (MaskGeneratedFile) because LoadSchema RECREATES c.Packages mid-generation — an overlay set only on the first instance would silently vanish. The map is shared by reference with every instance the Config creates. - templates.write unmasks each file once its new contents are on disk (including the unchanged-content short-circuit path): from that moment disk is truth, and the exec build's reload right after modelgen must see the just-generated types, not the stub. Disk state is now only ever changed by the atomic rename — kill generation at any instant and the previous outputs are intact — while the loader sees the same "outputs don't exist yet" world the old unlink provided. Verified: - `go generate ./...` (the exact check-generate CI command): failed with 22 "unable to find type" errors before, exits 0 with this commit, run twice back-to-back with a clean tree both times (regenerated output identical to committed). - The deterministic single-package repro (codegen/testserver/ followschema: `rm -f resolver.go && go run testdata/gqlgen.go -config gqlgen.yml -stub stub.go` on a clean tree) fails without this commit and passes with it; models-gen.go regenerates at full 436 lines instead of being emptied to a bare package clause. - api, codegen/templates, internal/code, codegen/config suites green; gofmt/vet/golines/gci clean. No standalone regression test is added: a minimal self-autobind fixture does NOT reproduce (the trigger needs the testserver fixtures' richer interface/implementor topology), and the check-generate CI step already runs the full corpus — it is the deterministic gate that caught this.
1 parent 614e973 commit 23883d3

4 files changed

Lines changed: 131 additions & 0 deletions

File tree

api/generate.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ package api
22

33
import (
44
"fmt"
5+
"go/parser"
6+
"go/token"
7+
"os"
58
"path/filepath"
69
"regexp"
710
"strings"
@@ -26,6 +29,32 @@ var (
2629
) // regex to grab the version number from a url
2730
)
2831

32+
// maskGeneratedOutput hides an existing generated output file from the type
33+
// loader for the duration of this generation run (see the comment at the top
34+
// of generate). The mask is a package-clause-only stub — NOT empty bytes,
35+
// which would be a Go parse error and break loading the rest of the package —
36+
// so the package name is read from the real file's own package clause. A
37+
// missing/unparseable file needs no mask (there is nothing stale to bind to;
38+
// generation will (re)create it), matching how the old unlink was a no-op on
39+
// a missing file.
40+
func maskGeneratedOutput(cfg *config.Config, filename string) {
41+
if filename == "" {
42+
return
43+
}
44+
abs, err := filepath.Abs(filename)
45+
if err != nil {
46+
return
47+
}
48+
if _, err := os.Stat(abs); err != nil {
49+
return // nothing on disk — nothing stale to mask
50+
}
51+
f, err := parser.ParseFile(token.NewFileSet(), abs, nil, parser.PackageClauseOnly)
52+
if err != nil || f.Name == nil {
53+
return // can't determine the package — leave it visible rather than corrupt the load
54+
}
55+
cfg.MaskGeneratedFile(abs, "package "+f.Name.Name+"\n")
56+
}
57+
2958
// Generate generates GraphQL code based on the provided config.
3059
func Generate(cfg *config.Config, option ...Option) error {
3160
return generate(cfg, nil, option...)
@@ -54,6 +83,24 @@ func generate(
5483
incrementalOpts *codegen.IncrementalOptions,
5584
option ...Option,
5685
) error {
86+
// MASK gqlgen's own previous outputs from the type loader, WITHOUT deleting
87+
// them from disk. If a stale generated model file is visible while the
88+
// schema loads, autobind finds the previously-generated types in it and
89+
// binds them as if they were user-written models — so modelgen skips
90+
// (re)generating them, the freshly-written model file comes out (near-)empty,
91+
// and the exec build then fails with "unable to find type" (every testserver
92+
// config that autobinds its own model package hits this). Before this
93+
// change, api.Generate handled that by syscall.Unlink-ing the outputs up
94+
// front — but a deleted-then-interrupted generation left the user with NO
95+
// generated file at all (#2345, #3505). An overlay gives the loader the
96+
// same "these files don't exist yet" view with no destructive disk write:
97+
// the real files stay intact until the atomic rename replaces them, and
98+
// templates.write unmasks each file once its new contents are on disk.
99+
maskGeneratedOutput(cfg, cfg.Exec.Filename)
100+
if cfg.Model.IsDefined() {
101+
maskGeneratedOutput(cfg, cfg.Model.Filename)
102+
}
103+
57104
plugins := []plugin.Plugin{}
58105
if cfg.Model.IsDefined() {
59106
plugins = append(plugins, modelgen.New())

codegen/config/config.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,13 @@ type Config struct {
7878
Sources []*ast.Source `yaml:"-"`
7979
Packages *code.Packages `yaml:"-"`
8080
Schema *ast.Schema `yaml:"-"`
81+
82+
// packagesOverlay is the loader overlay shared by every Packages instance
83+
// this Config creates (LoadSchema recreates c.Packages, so the overlay must
84+
// outlive any single instance). Entries mask gqlgen's own stale generated
85+
// outputs from autobind during a generation run — see MaskGeneratedFile and
86+
// api.Generate. Keyed by absolute file path.
87+
packagesOverlay map[string][]byte
8188
}
8289

8390
// boolOrFalse returns the value of a *bool pointer, or false if nil.
@@ -337,12 +344,29 @@ func CompleteConfig(config *Config) error {
337344
return nil
338345
}
339346

347+
// MaskGeneratedFile registers a loader overlay masking absPath with the given
348+
// stub contents for every Packages instance this Config creates — including
349+
// the one LoadSchema recreates — so gqlgen's own stale outputs can be hidden
350+
// from autobind for the whole generation run (see api.Generate). Held on the
351+
// Config (not just the current Packages) because LoadSchema rebuilds
352+
// c.Packages, which would otherwise silently drop masks set before it.
353+
func (c *Config) MaskGeneratedFile(absPath, contents string) {
354+
if c.packagesOverlay == nil {
355+
c.packagesOverlay = map[string][]byte{}
356+
}
357+
c.packagesOverlay[absPath] = []byte(contents)
358+
if c.Packages != nil {
359+
c.Packages.MaskFile(absPath, contents)
360+
}
361+
}
362+
340363
func (c *Config) Init() error {
341364
if c.Packages == nil {
342365
c.Packages = code.NewPackages(
343366
code.WithBuildTags(c.GoBuildTags...),
344367
code.PackagePrefixToCache("github.com/99designs/gqlgen/graphql"),
345368
code.WithPreloadNames(templatePackageNames...),
369+
code.WithOverlay(c.packagesOverlay),
346370
)
347371
}
348372

@@ -1168,6 +1192,7 @@ func (c *Config) LoadSchema() error {
11681192
code.WithBuildTags(c.GoBuildTags...),
11691193
code.PackagePrefixToCache("github.com/99designs/gqlgen/graphql"),
11701194
code.WithPreloadNames(templatePackageNames...),
1195+
code.WithOverlay(c.packagesOverlay),
11711196
)
11721197
}
11731198

codegen/templates/templates.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -737,6 +737,8 @@ func write(filename string, b []byte, packages *code.Packages, opts imports.Prun
737737
// Skip write if content is unchanged - preserves mtime for Go build cache
738738
existing, readErr := os.ReadFile(filename)
739739
if readErr == nil && bytes.Equal(existing, formatted) {
740+
// The on-disk file IS the current output — later loads must see it.
741+
unmask(packages, filename)
740742
return nil
741743
}
742744

@@ -806,9 +808,22 @@ func write(filename string, b []byte, packages *code.Packages, opts imports.Prun
806808
cleanup()
807809
return fmt.Errorf("failed to rename temp file: %w", err)
808810
}
811+
// The new contents are on disk — remove any loader mask api.Generate placed
812+
// over this output (see maskGeneratedOutput), so later loads in this same
813+
// run (e.g. the exec build reloading the model package after modelgen wrote
814+
// it) see the just-generated types instead of the empty stub.
815+
unmask(packages, filename)
809816
return nil
810817
}
811818

819+
// unmask lifts api.Generate's loader mask (maskGeneratedOutput) for a just-
820+
// written (or confirmed-current) output file — from here on, disk is truth.
821+
func unmask(packages *code.Packages, filename string) {
822+
if abs, err := filepath.Abs(filename); err == nil {
823+
packages.UnmaskFile(abs)
824+
}
825+
}
826+
812827
// renameWithRetry wraps os.Rename with a few short retries on Windows. Go's
813828
// os.Rename on Windows already calls MoveFileEx with MOVEFILE_REPLACE_EXISTING
814829
// — the same underlying API natefinch/atomic's ReplaceFile wraps — so it is

internal/code/packages.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ type (
2929
loadErrors []error
3030
buildFlags []string
3131
packagesToCachePrefix string
32+
overlay map[string][]byte
3233

3334
numLoadCalls int // stupid test steam. ignore.
3435
numNameCalls int // stupid test steam. ignore.
@@ -58,6 +59,18 @@ func PackagePrefixToCache(prefixPath string) func(p *Packages) {
5859
}
5960
}
6061

62+
// WithOverlay option for NewPackages supplies the packages.Config.Overlay map
63+
// used by every Load: files present in the map are read from it INSTEAD of
64+
// the file system. The map is held by REFERENCE (not copied) so a caller —
65+
// config.Config, which recreates its Packages instance during LoadSchema —
66+
// can keep one overlay alive across recreations, and MaskFile/UnmaskFile
67+
// mutations remain visible to whichever Packages instance currently holds it.
68+
func WithOverlay(overlay map[string][]byte) func(p *Packages) {
69+
return func(p *Packages) {
70+
p.overlay = overlay
71+
}
72+
}
73+
6174
// NewPackages creates a new packages cache
6275
// It will load all packages in the current module, and any packages that are passed to Load or
6376
// LoadAll
@@ -69,6 +82,35 @@ func NewPackages(opts ...Option) *Packages {
6982
return p
7083
}
7184

85+
// MaskFile makes every subsequent Load treat the file at absPath as if it
86+
// contained only the given contents (a package-clause-only stub), WITHOUT
87+
// touching the file on disk — a packages.Config.Overlay entry. gqlgen uses
88+
// this to hide its OWN previously-generated outputs (the model file) from the
89+
// type loader during generation: if a stale models_gen.go is visible while
90+
// the schema loads, autobind finds the previously-generated types in it and
91+
// binds them as if they were user-written models, so modelgen skips
92+
// (re)generating them and the freshly-written model file comes out empty —
93+
// the types vanish and the exec build fails with "unable to find type".
94+
// Historically api.Generate prevented that by DELETING the outputs up front
95+
// (syscall.Unlink), but that is exactly what left users with a missing
96+
// generated.go when generation was interrupted (#2345, #3505): masking at the
97+
// loader gives the same load semantics with no destructive disk write.
98+
func (p *Packages) MaskFile(absPath, contents string) {
99+
if p.overlay == nil {
100+
p.overlay = map[string][]byte{}
101+
}
102+
p.overlay[absPath] = []byte(contents)
103+
}
104+
105+
// UnmaskFile removes a MaskFile entry so subsequent Loads read the real file
106+
// from disk again — called right after gqlgen atomically writes that file
107+
// (see codegen/templates.write): once the new contents are on disk, disk is
108+
// the truth and later reloads (e.g. the exec build after modelgen runs) must
109+
// see the just-generated types, not the mask.
110+
func (p *Packages) UnmaskFile(absPath string) {
111+
delete(p.overlay, absPath)
112+
}
113+
72114
func dedupPackages(packages []string) []string {
73115
packageMap := make(map[string]struct{})
74116
dedupedPackages := make([]string, 0, len(packageMap))
@@ -132,6 +174,7 @@ func (p *Packages) LoadAll(importPaths ...string) []*packages.Package {
132174
pkgs, err := packages.Load(&packages.Config{
133175
Mode: mode,
134176
BuildFlags: p.buildFlags,
177+
Overlay: p.overlay,
135178
}, missing...)
136179
if err != nil {
137180
p.loadErrors = append(p.loadErrors, err)
@@ -208,6 +251,7 @@ func (p *Packages) LoadWithTypes(importPath string) *packages.Package {
208251
pkgs, err := packages.Load(&packages.Config{
209252
Mode: mode,
210253
BuildFlags: p.buildFlags,
254+
Overlay: p.overlay,
211255
}, importPath)
212256
if err != nil {
213257
p.loadErrors = append(p.loadErrors, err)

0 commit comments

Comments
 (0)