Skip to content

Commit a74dd82

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 wait for their weak deps to complete before running build commands. This prevents publishing packages (e.g., docker images) when their weak dependencies (e.g., Go library tests) have 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 before build commands run (prevents docker push, etc.) Example scenario that now fails correctly: docker:img -> go-app:app -> go-lib:lib (weak dep) If go-lib tests fail, docker:img build will fail before pushing. Co-authored-by: Ona <no-reply@ona.com>
1 parent ecf7a92 commit a74dd82

1 file changed

Lines changed: 108 additions & 34 deletions

File tree

pkg/leeway/build.go

Lines changed: 108 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,71 @@ 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+
// SignalWeakDepComplete marks a weak dependency build as complete.
274+
// Broadcasts the result to all packages waiting on this weak dep.
275+
func (c *buildContext) SignalWeakDepComplete(pkg *Package, err error) {
276+
c.weakDepResultsMu.RLock()
277+
result, ok := c.weakDepResults[pkg.FullName()]
278+
c.weakDepResultsMu.RUnlock()
279+
280+
if ok {
281+
result.Signal(err)
282+
}
283+
}
284+
285+
// WaitForWeakDeps waits for all weak dependencies of a package to complete.
286+
// This includes weak deps of the package itself AND weak deps of its transitive hard dependencies.
287+
// Returns an error if any weak dep failed.
288+
func (c *buildContext) WaitForWeakDeps(pkg *Package) error {
289+
// Collect all weak deps: direct weak deps + weak deps of hard deps
290+
weakDepsToWait := make(map[string]struct{})
291+
292+
// Add direct weak deps
293+
for _, wd := range pkg.GetTransitiveWeakDependencies() {
294+
weakDepsToWait[wd.FullName()] = struct{}{}
295+
}
296+
297+
// Add weak deps of hard dependencies (transitive)
298+
for _, dep := range pkg.GetTransitiveDependencies() {
299+
for _, wd := range dep.GetTransitiveWeakDependencies() {
300+
weakDepsToWait[wd.FullName()] = struct{}{}
301+
}
302+
}
303+
304+
if len(weakDepsToWait) == 0 {
305+
return nil
306+
}
307+
308+
for wdName := range weakDepsToWait {
309+
c.weakDepResultsMu.RLock()
310+
result, ok := c.weakDepResults[wdName]
311+
c.weakDepResultsMu.RUnlock()
312+
313+
if !ok {
314+
// Not tracked - might be cached or not a weak dep of this build
315+
continue
316+
}
317+
318+
if err := result.Wait(); err != nil {
319+
return xerrors.Errorf("weak dependency %s failed: %w", wdName, err)
320+
}
321+
}
322+
323+
return nil
324+
}
325+
232326
// LimitConcurrentBuilds blocks until there is a free slot to acutally build.
233327
// This function effectively limits the number of concurrent builds.
234328
// We do not do this limiting as part of the build lock, because that would block
@@ -887,12 +981,19 @@ func Build(pkg *Package, opts ...BuildOption) (err error) {
887981
return nil
888982
}
889983

984+
// Initialize weak dependency result tracking
985+
// This allows packages to wait for their weak deps before marking success
986+
ctx.InitWeakDepTracking(weakDeps)
987+
890988
var buildGroup errgroup.Group
891989

990+
// Start weak dep builds with result signaling
892991
for _, wd := range weakDeps {
893992
weakDep := wd
894993
buildGroup.Go(func() error {
895-
return weakDep.build(ctx)
994+
err := weakDep.build(ctx)
995+
ctx.SignalWeakDepComplete(weakDep, err)
996+
return err
896997
})
897998
}
898999

@@ -1131,35 +1232,6 @@ func (p *Package) buildDependencies(buildctx *buildContext) error {
11311232
return nil
11321233
}
11331234

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-
11631235
func (p *Package) build(buildctx *buildContext) (err error) {
11641236
// Try to obtain lock for building this package
11651237
doBuild := buildctx.ObtainBuildLock(p)
@@ -1181,10 +1253,6 @@ func (p *Package) build(buildctx *buildContext) (err error) {
11811253
return err
11821254
}
11831255

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-
11881256
// Check again after dependencies - might have been built as a dependency
11891257
if _, alreadyBuilt := buildctx.LocalCache.Location(p); !p.Ephemeral && alreadyBuilt {
11901258
log.WithField("package", p.FullName()).Info("package was built as dependency, skipping")
@@ -1233,6 +1301,12 @@ func (p *Package) build(buildctx *buildContext) (err error) {
12331301
buildctx.LimitConcurrentBuilds()
12341302
defer buildctx.ReleaseConcurrentBuild()
12351303

1304+
// Wait for weak dependencies before running build commands.
1305+
// This ensures we don't do work with side effects (like docker push) if a weak dep failed.
1306+
if err := buildctx.WaitForWeakDeps(p); err != nil {
1307+
return err
1308+
}
1309+
12361310
// Build the package based on its type
12371311
var (
12381312
result, _ = buildctx.LocalCache.Location(p)

0 commit comments

Comments
 (0)