From 6c0e007be3879efcbc865b9eddbc239352918f7f Mon Sep 17 00:00:00 2001 From: Paul Jolly Date: Wed, 25 Oct 2023 05:40:50 +0100 Subject: [PATCH] preprocessor: do not include snippets in hash key calc The contents of the snippets function cannot influence the result of the preprocessor on input files. Hence we must exclude the snippets package from the sum that calculates a hash key of the preprocessor itself. The dependency on snippets comes about because of the --serve mode, whereby the preprocessor also acts as the webserver hosting the snippets function. Arguably we shouldn't do that because it conflates the real role of the preprocessor. That's probably fair, but not a change to make right now. There isn't therefore a particularly clean way of carving out the snippets dependency so as not to form part of the hash. We could use build tags to represent the different aspects of the preprocessor, but again that's more work than we care to spend on this right now. So simply hardcode the exclusion for now and add a TODO that this should probably be revisited in a more meaningful way. Preprocessor-No-Write-Cache: true Signed-off-by: Paul Jolly Change-Id: Ia18d4d3ae8378a6c268f1ab364cc941fd3a1f0bb Dispatch-Trailer: {"type":"trybot","CL":1171202,"patchset":2,"ref":"refs/changes/02/1171202/2","targetBranch":"alpha"} --- gen_preprocessembed.go | 1 - internal/cmd/genpreprocessorembed/main.go | 27 +++++++++++++++++------ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/gen_preprocessembed.go b/gen_preprocessembed.go index b13358f793..64e2f576a5 100644 --- a/gen_preprocessembed.go +++ b/gen_preprocessembed.go @@ -7,7 +7,6 @@ import "embed" //go:embed go.mod //go:embed go.sum //go:embed internal/fsnotify/*.go -//go:embed internal/functions/snippets/*.go //go:embed internal/parse/*.go //go:embed internal/cmd/preprocessor/cmd/*.go //go:embed internal/cmd/preprocessor/cmd/schema.cue diff --git a/internal/cmd/genpreprocessorembed/main.go b/internal/cmd/genpreprocessorembed/main.go index f9617bc656..013f5dd4ab 100644 --- a/internal/cmd/genpreprocessorembed/main.go +++ b/internal/cmd/genpreprocessorembed/main.go @@ -25,6 +25,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" ) const target = "gen_preprocessembed.go" @@ -44,10 +45,13 @@ func main() { log.Fatalf("failed to decode main module information: %v", err) } + const thisModulePath = "github.com/cue-lang/cuelang.org" + const preprocessorPath = thisModulePath + "/internal/cmd/preprocessor" + const serverlessFunctionsPrefix = thisModulePath + "/internal/functions/" + // Verify we are in the right module! - const thisModule = "github.com/cue-lang/cuelang.org" - if mainMod.Path != thisModule { - log.Fatalf("main module is %s; expected %s", mainMod.Path, thisModule) + if mainMod.Path != thisModulePath { + log.Fatalf("main module is %s; expected %s", mainMod.Path, thisModulePath) } // Write to gen_pkghash.go for the current pkg @@ -66,7 +70,7 @@ func main() { embed("go.mod") embed("go.sum") - depsCmd := exec.Command("go", "list", "-e", "-deps", "-json", "github.com/cue-lang/cuelang.org/internal/cmd/preprocessor") + depsCmd := exec.Command("go", "list", "-e", "-deps", "-json", preprocessorPath) depsOut, err := depsCmd.CombinedOutput() if err != nil { log.Fatalf("failed to determine deps via [%v]: %v\n%s", depsCmd, err, depsOut) @@ -84,11 +88,20 @@ func main() { log.Fatalf("failed to decode package deps: %v", err) } // We only care about packages in the main module - if p.Module == nil || p.Module.Path != thisModule { + if p.Module == nil || p.Module.Path != thisModulePath { continue } - // We must ignore the package at the root of the module - if p.ImportPath == thisModule { + // Ignore certain packages when it comes to computing the hash of the + // preprocessor. + // + // TODO: ignoring of the module path (which is the target package of this + // generation) is something we cannot avoid (we could be more precise in + // only ignoring the file that is the target of the generation). Ignoring + // the snippets dependency is a bit hacky however. We could think about a + // different way of doing that, either by moving the serving of snippets out + // of the scope of the preprocessor, using build tags for the non-core bits + // of preprocessor... + if p.ImportPath == thisModulePath || strings.HasPrefix(p.ImportPath, serverlessFunctionsPrefix) { continue } relPath, err := filepath.Rel(mainMod.Dir, p.Dir)