fix loading tests in Open Source version
This required giving up on the len(patterns) = len(loaded) assumption,
which in turn required refactoring how the concurrency is structured.
I tested that edge cases like -parallel_jobs=1 and -parallel_jobs=35
(for a conversion with 35 packages) work correctly.
Also consider .pb.go files as generated while here.
fixes golang/open2opaque#3
fixes golang/open2opaque#4
PiperOrigin-RevId: 707109446
Change-Id: I2241e4a9a54613e68efee437b691946c66a3bfd3
Reviewed-on: https://go-review.googlesource.com/c/open2opaque/+/637135
Reviewed-by: Michael Stapelberg <stapelberg@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nicolas Hillegeer <aktau@google.com>
Auto-Submit: Michael Stapelberg <stapelberg@google.com>
diff --git a/internal/o2o/loader/loaderblaze.go b/internal/o2o/loader/loaderblaze.go
index 2d6e0e0..6c173ef 100644
--- a/internal/o2o/loader/loaderblaze.go
+++ b/internal/o2o/loader/loaderblaze.go
@@ -8,6 +8,7 @@
"context"
"fmt"
"os"
+ "strings"
"golang.org/x/tools/go/packages"
)
@@ -51,6 +52,7 @@
packages.NeedSyntax |
packages.NeedTypes |
packages.NeedTypesInfo,
+ Tests: true,
}
pkgs, err := packages.Load(cfg, patterns...)
if err != nil {
@@ -58,14 +60,15 @@
return
}
- if got, want := len(pkgs), len(patterns); got != want {
- failBatch(targets, res, fmt.Errorf("BUG: Load(%s) resulted in %d packages, want %d packages", patterns, got, want))
- return
- }
-
// Validate the response: ensure we can associate each returned package with
// a requested target, or fail the entire batch.
for _, pkg := range pkgs {
+ if strings.Contains(pkg.ID, ".test") {
+ // go/packages returns test packages for the provided patterns,
+ // e.g. "google.golang.org/o2o [google.golang.org/o2o.test]"
+ // for pattern google.golang.org/o2o.
+ continue
+ }
if _, ok := targetByID[pkg.ID]; !ok {
failBatch(targets, res, fmt.Errorf("BUG: Load() returned package %s, which was not requested", pkg.ID))
return
@@ -75,6 +78,9 @@
LoadedPackage:
for _, pkg := range pkgs {
t := targetByID[pkg.ID]
+ if t == nil {
+ t = &Target{ID: pkg.ID}
+ }
result := &Package{
Fileset: pkg.Fset,
TypeInfo: pkg.TypesInfo,
@@ -90,11 +96,14 @@
continue LoadedPackage
}
relPath := absPath
+ libraryUnderTest := false
+ generated := strings.HasSuffix(relPath, ".pb.go")
f := &File{
AST: pkg.Syntax[idx],
Path: relPath,
- LibraryUnderTest: t.LibrarySrcs[relPath],
+ LibraryUnderTest: libraryUnderTest,
Code: string(b),
+ Generated: generated,
}
result.Files = append(result.Files, f)
}
diff --git a/internal/o2o/rewrite/rewrite.go b/internal/o2o/rewrite/rewrite.go
index 68163ea..d763a4d 100644
--- a/internal/o2o/rewrite/rewrite.go
+++ b/internal/o2o/rewrite/rewrite.go
@@ -473,14 +473,14 @@
}
fixPackageBatch(ctx, pkgCfg, cfg.targets[idx:end], resc)
}
+ close(resc)
}()
fmt.Printf("Loading packages (in batches of up to %d)...\n", cfg.parallelJobs)
writtenByPath := make(map[string]bool)
var total, fail int
- for range cfg.targets {
- res := <-resc
+ for res := range resc {
profile.Add(res.ctx, "main/gotresp")
total++
@@ -497,7 +497,7 @@
tleft := time.Duration(len(cfg.targets)-total) * tavg
profile.Add(res.ctx, "done")
- fmt.Printf(`PROCESSED %d/%d packages
+ fmt.Printf(`PROCESSED %d packages (total patterns: %d)
Last package: %s
Total time: %s
Package profile: %s
@@ -584,15 +584,16 @@
func fixPackageBatch(ctx context.Context, cfg packageConfig, targets []*loader.Target, resc chan fixResult) {
results := make(chan loader.LoadResult, len(targets))
+ go func() {
+ cfg.loader.LoadPackages(ctx, targets, results)
+ close(results)
+ }()
+
var wg sync.WaitGroup
- wg.Add(len(targets))
- for range targets {
+ for res := range results {
+ wg.Add(1)
go func() {
defer wg.Done()
- res, ok := <-results
- if !ok {
- return // channel closed
- }
ctx := profile.NewContext(ctx)
if err := res.Err; err != nil {
resc <- fixResult{
@@ -604,6 +605,7 @@
}
profile.Add(ctx, "main/scheduled")
+ cfg := cfg // copy so that we can safely modify
cfg.configuredPkg.Testonly = res.Target.Testonly
cfg.configuredPkg.Loader = cfg.loader
cfg.configuredPkg.Pkg = res.Package
@@ -619,8 +621,6 @@
}
}()
}
- cfg.loader.LoadPackages(ctx, targets, results)
- close(results)
wg.Wait()
}
