Skip to content

Commit 37bc783

Browse files
geroplona-agent
andcommitted
fix(cache): upload successfully built packages even when build fails
When building packages with dependencies, if any dependency fails, the entire build would fail without uploading successfully built packages to remote cache. This caused unnecessary rebuilds after cache cleanup. Changes: - Upload packages to remote cache regardless of overall build outcome - Add CacheUploadFailed reporter method for visibility - Cache upload failures are non-fatal warnings - Build errors still take precedence in return values This ensures partial build results are cached, reducing rebuild time in complex dependency graphs. Co-authored-by: Ona <no-reply@ona.com>
1 parent 4eca8ad commit 37bc783

File tree

2 files changed

+69
-6
lines changed

2 files changed

+69
-6
lines changed

pkg/leeway/build.go

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -537,21 +537,29 @@ func Build(pkg *Package, opts ...BuildOption) (err error) {
537537

538538
buildErr := pkg.build(ctx)
539539

540-
// Check for build errors immediately and return if there are any
541-
if buildErr != nil {
542-
// We deliberately swallow the target package build error as that will have already been reported using the reporter.
543-
return xerrors.Errorf("build failed")
540+
// Upload successfully built packages to remote cache regardless of build outcome
541+
// This ensures partial build results are cached for future builds
542+
pkgsToUpload := ctx.GetNewPackagesForCache()
543+
if len(pkgsToUpload) > 0 {
544+
log.WithField("count", len(pkgsToUpload)).Debug("uploading packages to remote cache")
544545
}
545546

546-
// Only proceed with cache upload if build succeeded
547-
pkgsToUpload := ctx.GetNewPackagesForCache()
548547
// Convert []*Package to []cache.Package
549548
pkgsToUploadCache := make([]cache.Package, len(pkgsToUpload))
550549
for i, p := range pkgsToUpload {
551550
pkgsToUploadCache[i] = p
552551
}
553552

554553
cacheErr := ctx.RemoteCache.Upload(context.Background(), ctx.LocalCache, pkgsToUploadCache)
554+
if cacheErr != nil {
555+
ctx.Reporter.CacheUploadFailed(pkgsToUploadCache, cacheErr)
556+
}
557+
558+
// Check for build errors and return after cache upload
559+
if buildErr != nil {
560+
// We deliberately swallow the target package build error as that will have already been reported using the reporter.
561+
return xerrors.Errorf("build failed")
562+
}
555563
if cacheErr != nil {
556564
return cacheErr
557565
}

pkg/leeway/reporter.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import (
1919
"github.com/gookit/color"
2020
segment "github.com/segmentio/analytics-go/v3"
2121
"github.com/segmentio/textio"
22+
23+
"github.com/gitpod-io/leeway/pkg/leeway/cache"
2224
)
2325

2426
// Reporter provides feedback about the build progress to the user.
@@ -46,6 +48,10 @@ type Reporter interface {
4648
// PackageBuildFinished is called when the package build has finished. If an error is passed in
4749
// the package build was not succesfull.
4850
PackageBuildFinished(pkg *Package, rep *PackageBuildReport)
51+
52+
// CacheUploadFailed is called when uploading build artifacts to remote cache fails.
53+
// This is a non-fatal error that doesn't prevent the build from succeeding.
54+
CacheUploadFailed(pkgs []cache.Package, err error)
4955
}
5056

5157
type PackageBuildReport struct {
@@ -247,6 +253,25 @@ func (r *ConsoleReporter) PackageBuildFinished(pkg *Package, rep *PackageBuildRe
247253
io.WriteString(out, msg)
248254
}
249255

256+
// CacheUploadFailed is called when uploading build artifacts to remote cache fails.
257+
func (r *ConsoleReporter) CacheUploadFailed(pkgs []cache.Package, err error) {
258+
if len(pkgs) == 0 {
259+
return
260+
}
261+
262+
color.Printf("<yellow>⚠️ cache upload failed</> <gray>(built packages remain in local cache)</>\n")
263+
color.Printf("<white>Reason:</> %s\n", err)
264+
265+
if len(pkgs) <= 5 {
266+
color.Printf("<gray>Affected packages:</>\n")
267+
for _, pkg := range pkgs {
268+
color.Printf(" - %s\n", pkg.FullName())
269+
}
270+
} else {
271+
color.Printf("<gray>%d packages affected</>\n", len(pkgs))
272+
}
273+
}
274+
250275
func getRunPrefix(p *Package) string {
251276
return color.Gray.Render(fmt.Sprintf("[%s] ", p.FullName()))
252277
}
@@ -297,6 +322,11 @@ func (r *WerftReporter) PackageBuildFinished(pkg *Package, rep *PackageBuildRepo
297322
fmt.Printf("[%s|%s] %s\n", pkg.FullName(), status, msg)
298323
}
299324

325+
// CacheUploadFailed is called when uploading build artifacts to remote cache fails.
326+
func (r *WerftReporter) CacheUploadFailed(pkgs []cache.Package, err error) {
327+
log.WithError(err).WithField("packageCount", len(pkgs)).Warn("cache upload failed")
328+
}
329+
300330
type HTMLPackageReport struct {
301331
ID string
302332
logs strings.Builder
@@ -425,6 +455,11 @@ func (r *HTMLReporter) PackageBuildFinished(pkg *Package, rep *PackageBuildRepor
425455
}
426456
}
427457

458+
// CacheUploadFailed is called when uploading build artifacts to remote cache fails.
459+
func (r *HTMLReporter) CacheUploadFailed(pkgs []cache.Package, err error) {
460+
// HTML reporter doesn't need to track cache upload failures
461+
}
462+
428463
func (r *HTMLReporter) Report() {
429464
vars := make(map[string]interface{})
430465
vars["Name"] = r.filename
@@ -535,6 +570,13 @@ func (cr CompositeReporter) PackageBuildStarted(pkg *Package) {
535570
}
536571
}
537572

573+
// CacheUploadFailed implements Reporter
574+
func (cr CompositeReporter) CacheUploadFailed(pkgs []cache.Package, err error) {
575+
for _, r := range cr {
576+
r.CacheUploadFailed(pkgs, err)
577+
}
578+
}
579+
538580
var _ Reporter = CompositeReporter{}
539581

540582
type NoopReporter struct{}
@@ -554,6 +596,9 @@ func (*NoopReporter) PackageBuildLog(pkg *Package, isErr bool, buf []byte) {}
554596
// PackageBuildStarted implements Reporter
555597
func (*NoopReporter) PackageBuildStarted(pkg *Package) {}
556598

599+
// CacheUploadFailed implements Reporter
600+
func (*NoopReporter) CacheUploadFailed(pkgs []cache.Package, err error) {}
601+
557602
var _ Reporter = ((*NoopReporter)(nil))
558603

559604
func NewSegmentReporter(key string) *SegmentReporter {
@@ -650,6 +695,11 @@ func (sr *SegmentReporter) track(event string, props segment.Properties) {
650695
}
651696
}
652697

698+
// CacheUploadFailed is called when uploading build artifacts to remote cache fails.
699+
func (sr *SegmentReporter) CacheUploadFailed(pkgs []cache.Package, err error) {
700+
// Could track cache upload failures for analytics in the future
701+
}
702+
653703
func NewGitHubReporter() *GitHubActionReporter {
654704
return &GitHubActionReporter{}
655705
}
@@ -681,3 +731,8 @@ func (sr *GitHubActionReporter) PackageBuildFinished(pkg *Package, rep *PackageB
681731
}
682732
fmt.Fprintf(f, "%s=%v\n", pkg.FilesystemSafeName(), success)
683733
}
734+
735+
// CacheUploadFailed is called when uploading build artifacts to remote cache fails.
736+
func (sr *GitHubActionReporter) CacheUploadFailed(pkgs []cache.Package, err error) {
737+
fmt.Printf("::warning::Cache upload failed: %s (%d packages affected)\n", err, len(pkgs))
738+
}

0 commit comments

Comments
 (0)