Skip to content

Commit

Permalink
preprocessor: do not include snippets in hash key calc
Browse files Browse the repository at this point in the history
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 <[email protected]>
Change-Id: Ia18d4d3ae8378a6c268f1ab364cc941fd3a1f0bb
Dispatch-Trailer: {"type":"trybot","CL":1171202,"patchset":2,"ref":"refs/changes/02/1171202/2","targetBranch":"alpha"}
  • Loading branch information
myitcv authored and cueckoo committed Oct 25, 2023
1 parent 32f02f2 commit 6c0e007
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 8 deletions.
1 change: 0 additions & 1 deletion gen_preprocessembed.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 20 additions & 7 deletions internal/cmd/genpreprocessorembed/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"os"
"os/exec"
"path/filepath"
"strings"
)

const target = "gen_preprocessembed.go"
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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)
Expand Down

0 comments on commit 6c0e007

Please sign in to comment.