From 3c7141e80196e95b4fcaf27924b0467079d7768c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Sun, 25 Sep 2022 20:36:27 +0100 Subject: [PATCH] update the state of a few TODOs related to upstream Go The generics issue has been fixed for the upcoming Go 1.20. Include that version as a reminder for when we can drop Go 1.19. The fs.SkipAll proposal is also implemented for Go 1.20. The BinaryContentID comment was a little bit trickier. We did get stamped VCS information some time ago, but it only provides us with the current commit info and a dirty bit. That is not enough for our use of the build cache, because we want any uncommitted changes to garble to cause rebuilds. I don't think we'll get any better than using garble's own build ID. Reword the quasi-TODO to instead explain what we're doing and why. --- internal/literals/obfuscators.go | 2 +- main_test.go | 3 +-- shared.go | 14 +++++++------- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/internal/literals/obfuscators.go b/internal/literals/obfuscators.go index 6948020..3f13f11 100644 --- a/internal/literals/obfuscators.go +++ b/internal/literals/obfuscators.go @@ -21,7 +21,7 @@ var obfuscators = []obfuscator{ swap{}, split{}, shuffle{}, - // seed{}, TODO: re-enable once https://github.com/golang/go/issues/47631 is fixed + // seed{}, TODO: re-enable once https://go.dev/issue/47631 is fixed in Go 1.20 } // If math/rand.Seed() is not called, the generator behaves as if seeded by rand.Seed(1), diff --git a/main_test.go b/main_test.go index e7a5bbc..e04f261 100644 --- a/main_test.go +++ b/main_test.go @@ -288,8 +288,7 @@ func grepfiles(ts *testscript.TestScript, neg bool, args []string) { anyFound := false path, pattern := args[0], args[1] rx := regexp.MustCompile(pattern) - // TODO: use https://github.com/golang/go/issues/47209 when merged, - // hopefully in Go 1.20. + // TODO: use fs.SkipAll in Go 1.20 and later. errSkipAll := fmt.Errorf("sentinel error: stop walking") if err := filepath.WalkDir(path, func(path string, d fs.DirEntry, err error) error { if err != nil { diff --git a/shared.go b/shared.go index f3e37c1..d387339 100644 --- a/shared.go +++ b/shared.go @@ -33,12 +33,12 @@ type sharedCache struct { // useful for type checking of the packages as we obfuscate them. ListedPackages map[string]*listedPackage - // We can't rely on the module version to exist, - // because it's missing in local builds without 'go install'. - // For now, use 'go tool buildid' on the garble binary. - // Just like Go's own cache, we use hex-encoded sha256 sums. - // Once https://github.com/golang/go/issues/37475 is fixed, - // we can likely just use that. + // We can't use garble's own module version, as it may not exist. + // We can't use the stamped VCS information either, + // as uncommitted changes simply show up as "dirty". + // + // The only unique way to identify garble's version without being published + // or committed is to use its content ID from the build cache. BinaryContentID []byte GOGARBLE string @@ -228,7 +228,7 @@ func appendListedPackages(packages []string, withDeps bool) error { pkg.GarbleActionID = addGarbleToHash(actionID) } - // Work around https://golang.org/issue/28749: + // Work around https://go.dev/issue/28749: // cmd/go puts assembly, C, and C++ files in CompiledGoFiles. // // TODO: remove when upstream has fixed the bug.