Skip to content

Commit 69b3ee4

Browse files
iQQBotona-agent
andcommitted
feat: fail build if weak dependency fails
Add a mechanism to track weak dependency build results and ensure that packages check their weak deps before being marked as successful. Key behavior: - Builds run in parallel (no blocking) for maximum parallelism - After build completes, wait for weak deps to finish - If any weak dep failed, mark this package as failed too - Weak deps themselves skip this check (their result is signaled to dependents) - This prevents caching/publishing packages whose weak deps failed Implementation: - Add weakDepResult struct with broadcast channel pattern - Multiple packages can wait on the same weak dep result - Packages wait for weak deps of themselves AND their hard deps - Wait happens after build, before RegisterNewlyBuilt() - IsWeakDep() check skips waiting for packages that are weak deps themselves Example scenario: docker:img -> go-app:app -> go-lib:lib (weak dep) All three build in parallel. If go-lib tests fail: 1. go-lib:lib fails, signals result (doesn't wait for others) 2. docker:img build completes 3. docker:img waits for weak deps, sees go-lib failed 4. docker:img marked as failed, not cached Co-authored-by: Ona <no-reply@ona.com>
1 parent ecf7a92 commit 69b3ee4

1 file changed

Lines changed: 121 additions & 34 deletions

File tree

pkg/leeway/build.go

Lines changed: 121 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,35 @@ type buildContext struct {
8282
InFlightChecksums bool // Feature enabled flag
8383
artifactChecksums map[string]string // path -> sha256 hex
8484
artifactChecksumsMutex sync.RWMutex // Thread safety for parallel builds
85+
86+
// Weak dependency result tracking
87+
// Allows packages to wait for their weak deps to complete before marking success
88+
weakDepResults map[string]*weakDepResult
89+
weakDepResultsMu sync.RWMutex
90+
}
91+
92+
// weakDepResult tracks the build result of a weak dependency.
93+
// Uses a closed channel as broadcast mechanism - multiple waiters can wait on the same result.
94+
type weakDepResult struct {
95+
done chan struct{} // Closed when build completes - acts as broadcast
96+
err error // Build error (nil if success), set before closing done
97+
}
98+
99+
func newWeakDepResult() *weakDepResult {
100+
return &weakDepResult{done: make(chan struct{})}
101+
}
102+
103+
// Wait blocks until the weak dep build completes and returns the result.
104+
// Multiple goroutines can call Wait on the same result.
105+
func (r *weakDepResult) Wait() error {
106+
<-r.done
107+
return r.err
108+
}
109+
110+
// Signal marks the weak dep build as complete and broadcasts to all waiters.
111+
func (r *weakDepResult) Signal(err error) {
112+
r.err = err
113+
close(r.done)
85114
}
86115

87116
const (
@@ -229,6 +258,80 @@ func (c *buildContext) ReleaseBuildLock(p *Package) {
229258
c.pkgLockCond.L.Unlock()
230259
}
231260

261+
// InitWeakDepTracking initializes tracking for weak dependency build results.
262+
// Must be called before starting weak dep builds.
263+
func (c *buildContext) InitWeakDepTracking(weakDeps []*Package) {
264+
c.weakDepResultsMu.Lock()
265+
defer c.weakDepResultsMu.Unlock()
266+
267+
c.weakDepResults = make(map[string]*weakDepResult)
268+
for _, wd := range weakDeps {
269+
c.weakDepResults[wd.FullName()] = newWeakDepResult()
270+
}
271+
}
272+
273+
// IsWeakDep returns true if the package is being tracked as a weak dependency.
274+
func (c *buildContext) IsWeakDep(pkg *Package) bool {
275+
c.weakDepResultsMu.RLock()
276+
defer c.weakDepResultsMu.RUnlock()
277+
278+
_, ok := c.weakDepResults[pkg.FullName()]
279+
return ok
280+
}
281+
282+
// SignalWeakDepComplete marks a weak dependency build as complete.
283+
// Broadcasts the result to all packages waiting on this weak dep.
284+
func (c *buildContext) SignalWeakDepComplete(pkg *Package, err error) {
285+
c.weakDepResultsMu.RLock()
286+
result, ok := c.weakDepResults[pkg.FullName()]
287+
c.weakDepResultsMu.RUnlock()
288+
289+
if ok {
290+
result.Signal(err)
291+
}
292+
}
293+
294+
// WaitForWeakDeps waits for all weak dependencies of a package to complete.
295+
// This includes weak deps of the package itself AND weak deps of its transitive hard dependencies.
296+
// Returns an error if any weak dep failed.
297+
func (c *buildContext) WaitForWeakDeps(pkg *Package) error {
298+
// Collect all weak deps: direct weak deps + weak deps of hard deps
299+
weakDepsToWait := make(map[string]struct{})
300+
301+
// Add direct weak deps
302+
for _, wd := range pkg.GetTransitiveWeakDependencies() {
303+
weakDepsToWait[wd.FullName()] = struct{}{}
304+
}
305+
306+
// Add weak deps of hard dependencies (transitive)
307+
for _, dep := range pkg.GetTransitiveDependencies() {
308+
for _, wd := range dep.GetTransitiveWeakDependencies() {
309+
weakDepsToWait[wd.FullName()] = struct{}{}
310+
}
311+
}
312+
313+
if len(weakDepsToWait) == 0 {
314+
return nil
315+
}
316+
317+
for wdName := range weakDepsToWait {
318+
c.weakDepResultsMu.RLock()
319+
result, ok := c.weakDepResults[wdName]
320+
c.weakDepResultsMu.RUnlock()
321+
322+
if !ok {
323+
// Not tracked - might be cached or not a weak dep of this build
324+
continue
325+
}
326+
327+
if err := result.Wait(); err != nil {
328+
return xerrors.Errorf("weak dependency %s failed: %w", wdName, err)
329+
}
330+
}
331+
332+
return nil
333+
}
334+
232335
// LimitConcurrentBuilds blocks until there is a free slot to acutally build.
233336
// This function effectively limits the number of concurrent builds.
234337
// We do not do this limiting as part of the build lock, because that would block
@@ -887,12 +990,19 @@ func Build(pkg *Package, opts ...BuildOption) (err error) {
887990
return nil
888991
}
889992

993+
// Initialize weak dependency result tracking
994+
// This allows packages to wait for their weak deps before marking success
995+
ctx.InitWeakDepTracking(weakDeps)
996+
890997
var buildGroup errgroup.Group
891998

999+
// Start weak dep builds with result signaling
8921000
for _, wd := range weakDeps {
8931001
weakDep := wd
8941002
buildGroup.Go(func() error {
895-
return weakDep.build(ctx)
1003+
err := weakDep.build(ctx)
1004+
ctx.SignalWeakDepComplete(weakDep, err)
1005+
return err
8961006
})
8971007
}
8981008

@@ -1131,35 +1241,6 @@ func (p *Package) buildDependencies(buildctx *buildContext) error {
11311241
return nil
11321242
}
11331243

1134-
func (p *Package) buildWeakDeps(buildctx *buildContext) {
1135-
weakDeps := p.GetTransitiveWeakDependencies()
1136-
if len(weakDeps) == 0 {
1137-
return
1138-
}
1139-
1140-
log.WithFields(log.Fields{
1141-
"package": p.FullName(),
1142-
"weakDeps": len(weakDeps),
1143-
}).Debug("building weak deps in background")
1144-
1145-
var wg errgroup.Group
1146-
for _, dep := range weakDeps {
1147-
d := dep
1148-
wg.Go(func() error {
1149-
return d.build(buildctx)
1150-
})
1151-
}
1152-
1153-
// Don't block on weak deps - they run in parallel
1154-
// Errors are logged but don't fail the main build
1155-
if err := wg.Wait(); err != nil {
1156-
log.WithFields(log.Fields{
1157-
"package": p.FullName(),
1158-
"error": err,
1159-
}).Warn("weak dep build failed")
1160-
}
1161-
}
1162-
11631244
func (p *Package) build(buildctx *buildContext) (err error) {
11641245
// Try to obtain lock for building this package
11651246
doBuild := buildctx.ObtainBuildLock(p)
@@ -1181,10 +1262,6 @@ func (p *Package) build(buildctx *buildContext) (err error) {
11811262
return err
11821263
}
11831264

1184-
// Start building weak deps in parallel (they don't block this build)
1185-
// This ensures weak deps are built (for their tests) even when build() is called directly
1186-
go p.buildWeakDeps(buildctx)
1187-
11881265
// Check again after dependencies - might have been built as a dependency
11891266
if _, alreadyBuilt := buildctx.LocalCache.Location(p); !p.Ephemeral && alreadyBuilt {
11901267
log.WithField("package", p.FullName()).Info("package was built as dependency, skipping")
@@ -1338,6 +1415,16 @@ func (p *Package) build(buildctx *buildContext) (err error) {
13381415
}
13391416
}
13401417

1418+
// Wait for weak dependencies before marking success.
1419+
// Build runs in parallel, but we don't cache/publish if weak deps failed.
1420+
// Skip this check if we are a weak dependency ourselves - our result will be
1421+
// signaled to whoever depends on us.
1422+
if !buildctx.IsWeakDep(p) {
1423+
if err := buildctx.WaitForWeakDeps(p); err != nil {
1424+
return err
1425+
}
1426+
}
1427+
13411428
// Register newly built package
13421429
return buildctx.RegisterNewlyBuilt(p)
13431430
}

0 commit comments

Comments
 (0)