Merge pull request #222 from gitpod-io/wv/fixed-build-dir

Add option to build in the same directory, to better use caches

Author: WVerlaek

cmd/build.go

@@ -181,10 +181,12 @@ func addBuildFlags(cmd *cobra.Command) {
 	cmd.Flags().Bool("jailed-execution", false, "Run all build commands using runc (defaults to false)")
 	cmd.Flags().UintP("max-concurrent-tasks", "j", uint(cpus), "Limit the number of max concurrent build tasks - set to 0 to disable the limit")
 	cmd.Flags().String("coverage-output-path", "", "Output path where test coverage file will be copied after running tests")
+	cmd.Flags().Bool("disable-coverage", false, "Disable test coverage collection (defaults to false)")
 	cmd.Flags().StringToString("docker-build-options", nil, "Options passed to all 'docker build' commands")
 	cmd.Flags().String("report", "", "Generate a HTML report after the build has finished. (e.g. --report myreport.html)")
 	cmd.Flags().String("report-segment", os.Getenv("LEEWAY_SEGMENT_KEY"), "Report build events to segment using the segment key (defaults to $LEEWAY_SEGMENT_KEY)")
 	cmd.Flags().Bool("report-github", os.Getenv("GITHUB_OUTPUT") != "", "Report package build success/failure to GitHub Actions using the GITHUB_OUTPUT environment variable")
+	cmd.Flags().Bool("fixed-build-dir", false, "Use a fixed build directory for each package, instead of based on the package version, to better utilize caches based on absolute paths (defaults to false)")
 }
 
 func getBuildOpts(cmd *cobra.Command) ([]leeway.BuildOption, cache.LocalCache) {
@@ -291,6 +293,8 @@ func getBuildOpts(cmd *cobra.Command) ([]leeway.BuildOption, cache.LocalCache) {
 		_ = os.MkdirAll(coverageOutputPath, 0644)
 	}
 
+	disableCoverage, _ := cmd.Flags().GetBool("disable-coverage")
+
 	var dockerBuildOptions leeway.DockerBuildOptions
 	dockerBuildOptions, err = cmd.Flags().GetStringToString("docker-build-options")
 	if err != nil {
@@ -307,6 +311,11 @@ func getBuildOpts(cmd *cobra.Command) ([]leeway.BuildOption, cache.LocalCache) {
 		log.Fatal(err)
 	}
 
+	fixedBuildDir, err := cmd.Flags().GetBool("fixed-build-dir")
+	if err != nil {
+		log.Fatal(err)
+	}
+
 	return []leeway.BuildOption{
 		leeway.WithLocalCache(localCache),
 		leeway.WithRemoteCache(remoteCache),
@@ -319,6 +328,8 @@ func getBuildOpts(cmd *cobra.Command) ([]leeway.BuildOption, cache.LocalCache) {
 		leeway.WithDockerBuildOptions(&dockerBuildOptions),
 		leeway.WithJailedExecution(jailedExecution),
 		leeway.WithCompressionDisabled(dontCompress),
+		leeway.WithFixedBuildDir(fixedBuildDir),
+		leeway.WithDisableCoverage(disableCoverage),
 	}, localCache
 }
 

pkg/leeway/build.go

@@ -264,6 +264,8 @@ type buildOptions struct {
 	CoverageOutputPath     string
 	DockerBuildOptions     *DockerBuildOptions
 	JailedExecution        bool
+	UseFixedBuildDir       bool
+	DisableCoverage        bool
 
 	context *buildContext
 }
@@ -365,6 +367,20 @@ func WithCompressionDisabled(dontCompress bool) BuildOption {
 	}
 }
 
+func WithFixedBuildDir(fixedBuildDir bool) BuildOption {
+	return func(opts *buildOptions) error {
+		opts.UseFixedBuildDir = fixedBuildDir
+		return nil
+	}
+}
+
+func WithDisableCoverage(disableCoverage bool) BuildOption {
+	return func(opts *buildOptions) error {
+		opts.DisableCoverage = disableCoverage
+		return nil
+	}
+}
+
 func withBuildContext(ctx *buildContext) BuildOption {
 	return func(opts *buildOptions) error {
 		opts.context = ctx
@@ -624,12 +640,6 @@ func (p *Package) build(buildctx *buildContext) error {
 	}
 	defer buildctx.ReleaseBuildLock(p)
 
-	// Get package version
-	version, err := p.Version()
-	if err != nil {
-		return err
-	}
-
 	// Build dependencies first
 	if err := p.buildDependencies(buildctx); err != nil {
 		return err
@@ -653,13 +663,24 @@ func (p *Package) build(buildctx *buildContext) error {
 	buildctx.Reporter.PackageBuildStarted(p)
 
 	// Ensure we notify reporter when build finishes
+	var err error
 	defer func() {
 		pkgRep.Error = err
 		buildctx.Reporter.PackageBuildFinished(p, pkgRep)
 	}()
 
 	// Prepare build directory
-	builddir := filepath.Join(buildctx.BuildDir(), p.FilesystemSafeName()+"."+version)
+	builddir, err := createBuildDir(buildctx, p)
+	if err != nil {
+		return err
+	}
+	defer func() {
+		// Clean up build directory after build, such that the next build of the same component
+		// can use the fixed build directory again.
+		os.RemoveAll(builddir)
+	}()
+
+	log.WithField("package", p.FullName()).WithField("builddir", builddir).Info("using build directory")
 	if err := prepareDirectory(builddir); err != nil {
 		return err
 	}
@@ -756,6 +777,48 @@ func (p *Package) build(buildctx *buildContext) error {
 	return buildctx.RegisterNewlyBuilt(p)
 }
 
+func createBuildDir(buildctx *buildContext, p *Package) (string, error) {
+	if !buildctx.UseFixedBuildDir {
+		// Original behavior: use version as suffix to ensure a unique build directory for each package version.
+		version, err := p.Version()
+		if err != nil {
+			return "", err
+		}
+		return filepath.Join(buildctx.BuildDir(), p.FilesystemSafeName()+"."+version), nil
+	}
+
+	// New behavior: use the package name as the build directory.
+	// This ensures the same directory is used for each new build of the package,
+	// which is more cache-friendly. Allows e.g. Go build/test caching to work,
+	// as well as golangci-lint caching.
+	//
+	// Note: This directoy is only used if it doesn't already exist, otherwise
+	// we'll fall back to the version as suffix.
+	// It is possible that the directory exists because the package is already
+	// being built with a different version (e.g. different args).
+	builddir := filepath.Join(buildctx.BuildDir(), p.FilesystemSafeName())
+
+	err := os.MkdirAll(filepath.Dir(builddir), 0755)
+	if err != nil {
+		return "", fmt.Errorf("failed to create parent directory for build directory: %w", err)
+	}
+
+	err = os.Mkdir(builddir, 0755)
+	if err == nil {
+		return builddir, nil
+	}
+	if !os.IsExist(err) {
+		return "", fmt.Errorf("failed to create build directory: %w", err)
+	}
+
+	// Already exists, use version as suffix
+	version, err := p.Version()
+	if err != nil {
+		return "", fmt.Errorf("failed to get version for package %s: %w", p.FullName(), err)
+	}
+	return filepath.Join(buildctx.BuildDir(), p.FilesystemSafeName()+"."+version), nil
+}
+
 func prepareDirectory(dir string) error {
 	if _, err := os.Stat(dir); !os.IsNotExist(err) {
 		if err := os.RemoveAll(dir); err != nil {
@@ -1345,11 +1408,15 @@ func (p *Package) buildGo(buildctx *buildContext, wd, result string) (res *packa
 			testCommand = append(testCommand, "-v")
 		}
 
-		if buildctx.buildOptions.CoverageOutputPath != "" {
-			testCommand = append(testCommand, fmt.Sprintf("-coverprofile=%v", codecovComponentName(p.FullName())))
-		} else {
-			testCommand = append(testCommand, "-coverprofile=testcoverage.out")
-			reportCoverage = collectGoTestCoverage(filepath.Join(wd, "testcoverage.out"))
+		// Added as an option to disable test coverage until Go 1.25 is released https://github.com/golang/go/commit/6a4bc8d17eb6703baf0c483fb40e0d3e1f0f6af3
+		// Running with coverage stops the test cache from being used, which will be fixed in Go 1.25.
+		if !buildctx.DisableCoverage {
+			if buildctx.buildOptions.CoverageOutputPath != "" {
+				testCommand = append(testCommand, fmt.Sprintf("-coverprofile=%v", codecovComponentName(p.FullName())))
+			} else {
+				testCommand = append(testCommand, "-coverprofile=testcoverage.out")
+				reportCoverage = collectGoTestCoverage(filepath.Join(wd, "testcoverage.out"))
+			}
 		}
 		testCommand = append(testCommand, "./...")