feat: allow specifying buildenv retention policy when building components (#328)

Adds new command-line argument to `azldev component build` to control whether (and when) build environment(s) created during the build operation are retained.

Author: reubeno

.github/copilot-instructions.md

@@ -1,7 +1,7 @@
 # azldev-preview - AZL Dev Tool Preview
 
 Follow the existing code style and conventions for similar code. Refer to all of the documents listed below:
-- [CONTRIBUTING.md](./CONTRIBUTING.md) for general human and AI agent guidelines.
+- [CONTRIBUTING.md](../CONTRIBUTING.md) for general human and AI agent guidelines.
 - `.github/instructions/*.instructions.md` instructions files for language-specific AI agent instructions.
 
 **CRITICAL**: If these instructions are outdated or misleading, FIX THEM IMMEDIATELY! This document (`.github/copilot-instructions.md`) is to help coding agents (like you) to work efficiently and effectively. If you are lead astray or get confused, it is likely it will happen again to others. Help them out by improving this document (keep it SHORT AND CONCISE).

internal/app/azldev/cmds/component/build.go

@@ -13,10 +13,12 @@ import (
 	"github.com/microsoft/azldev/internal/app/azldev/core/components"
 	"github.com/microsoft/azldev/internal/app/azldev/core/sources"
 	"github.com/microsoft/azldev/internal/app/azldev/core/workdir"
+	"github.com/microsoft/azldev/internal/buildenv"
 	"github.com/microsoft/azldev/internal/orchestration/artifacts"
 	"github.com/microsoft/azldev/internal/orchestration/orchestrator"
 	"github.com/microsoft/azldev/internal/projectconfig"
 	"github.com/microsoft/azldev/internal/providers/sourceproviders"
+	"github.com/microsoft/azldev/internal/utils/defers"
 	"github.com/samber/lo"
 	"github.com/spf13/cobra"
 )
@@ -27,6 +29,7 @@ type ComponentBuildOptions struct {
 	ContinueOnError   bool
 	RunChecks         bool
 	SourcePackageOnly bool
+	BuildEnvPolicy    BuildEnvPreservePolicy
 }
 
 // ComponentBuildResults summarizes the results of building a single component.
@@ -46,15 +49,18 @@ func buildOnAppInit(_ *azldev.App, parent *cobra.Command) {
 }
 
 func NewBuildCmd() *cobra.Command {
-	var options ComponentBuildOptions
+	// Fill out options defaults.
+	options := &ComponentBuildOptions{
+		BuildEnvPolicy: BuildEnvPreserveOnFailure,
+	}
 
 	cmd := &cobra.Command{
 		Use:   "build",
 		Short: "Build packages for components",
 		RunE: azldev.RunFuncWithExtraArgs(func(env *azldev.Env, args []string) (interface{}, error) {
 			options.ComponentFilter.ComponentNamePatterns = append(options.ComponentFilter.ComponentNamePatterns, args...)
 
-			return SelectAndBuildComponents(env, &options)
+			return SelectAndBuildComponents(env, options)
 		}),
 		ValidArgsFunction: components.GenerateComponentNameCompletions,
 	}
@@ -64,6 +70,12 @@ func NewBuildCmd() *cobra.Command {
 		"Continue building when some components fail")
 	cmd.Flags().BoolVarP(&options.RunChecks, "check", "c", false, "Run package %check tests")
 	cmd.Flags().BoolVar(&options.SourcePackageOnly, "srpm-only", false, "Build SRPM (source RPM) *only*")
+	cmd.Flags().Var(&options.BuildEnvPolicy, "preserve-buildenv",
+		fmt.Sprintf("Preserve build environment {%s, %s, %s}",
+			BuildEnvPreserveOnFailure,
+			BuildEnvPreserveAlways,
+			BuildEnvPreserveNever,
+		))
 
 	return cmd
 }
@@ -136,11 +148,8 @@ func BuildComponent(
 	component components.Component,
 	workDirFactory *workdir.Factory,
 	options *ComponentBuildOptions,
-) (ComponentBuildResults, error) {
-	var (
-		rpmContentsProvider sourceproviders.RPMContentsProvider
-		err                 error
-	)
+) (results ComponentBuildResults, err error) {
+	var rpmContentsProvider sourceproviders.RPMContentsProvider
 
 	// If the component has external sources, then we build an RPM contents provider;
 	// otherwise, we pass nil to the preparer.
@@ -151,12 +160,23 @@ func BuildComponent(
 		}
 	}
 
-	buildEnv, err := workdir.MkComponentBuildEnvironment(env, workDirFactory, component.GetConfig(), "build")
+	var buildEnv buildenv.RPMAwareBuildEnv
+
+	buildEnv, err = workdir.MkComponentBuildEnvironment(env, workDirFactory, component.GetConfig(), "build")
 	if err != nil {
 		return ComponentBuildResults{},
 			fmt.Errorf("failed to create build environment for component %q: %w", component.GetName(), err)
 	}
 
+	// Clean up the build environment before we return (unless we were asked not to do so).
+	defer defers.HandleDeferError(func() error {
+		if !options.BuildEnvPolicy.ShouldPreserve(err == nil) {
+			return buildEnv.Destroy(env)
+		}
+
+		return nil
+	}, &err)
+
 	sourcePreparer := sources.NewPreparer(env, env, env.FS(), rpmContentsProvider)
 	builder := componentbuilder.New(env, env.FS(), env, sourcePreparer, buildEnv, workDirFactory)
 

internal/app/azldev/cmds/component/buildenvpreservepolicy.go

@@ -0,0 +1,70 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+package component
+
+import (
+	"fmt"
+
+	"github.com/spf13/pflag"
+)
+
+// BuildEnvPreservePolicy defines policy for when to preserve auto-created build environments.
+type BuildEnvPreservePolicy string
+
+const (
+	// BuildEnvPreserveOnFailure indicates build environments should only be preserved for failed builds.
+	BuildEnvPreserveOnFailure BuildEnvPreservePolicy = "on-failure"
+	// BuildEnvPreserveAlways indicates all build environments should be preserved.
+	BuildEnvPreserveAlways BuildEnvPreservePolicy = "always"
+	// BuildEnvPreserveNever indicates build environments should *never* be preserved (i.e., always destroyed).
+	BuildEnvPreserveNever BuildEnvPreservePolicy = "never"
+)
+
+// Assert that BuildEnvPreservePolicy implements the [pflag.Value] interface.
+var _ pflag.Value = (*BuildEnvPreservePolicy)(nil)
+
+func (f *BuildEnvPreservePolicy) String() string {
+	return string(*f)
+}
+
+// Set parses the format from a string; used by command-line parser.
+func (f *BuildEnvPreservePolicy) Set(value string) error {
+	switch value {
+	case string(BuildEnvPreserveOnFailure):
+		*f = BuildEnvPreserveOnFailure
+	case string(BuildEnvPreserveAlways):
+		*f = BuildEnvPreserveAlways
+	case string(BuildEnvPreserveNever):
+		*f = BuildEnvPreserveNever
+	case "":
+		*f = BuildEnvPreserveOnFailure
+	default:
+		// Default to "on failure" but still return an error.
+		*f = BuildEnvPreserveOnFailure
+
+		return fmt.Errorf("unsupported build environment preserve policy: %s", value)
+	}
+
+	return nil
+}
+
+// Type returns a descriptive string used in command-line help.
+func (f *BuildEnvPreservePolicy) Type() string {
+	return "policy"
+}
+
+// ShouldPreserve is a helper to decide whether to preserve a build environment, based on the policy
+// and the actual results of the build.
+func (f *BuildEnvPreservePolicy) ShouldPreserve(buildSucceeded bool) bool {
+	switch *f {
+	case BuildEnvPreserveOnFailure:
+		return !buildSucceeded
+	case BuildEnvPreserveAlways:
+		return true
+	case BuildEnvPreserveNever:
+		return false
+	default:
+		return false
+	}
+}

internal/app/azldev/cmds/component/buildenvpreservepolicy_test.go

@@ -0,0 +1,71 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+package component_test
+
+import (
+	"testing"
+
+	"github.com/microsoft/azldev/internal/app/azldev/cmds/component"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestStringification(t *testing.T) {
+	policy := component.BuildEnvPreserveAlways
+	assert.Equal(t, "always", policy.String())
+
+	policy = component.BuildEnvPreserveNever
+	assert.Equal(t, "never", policy.String())
+
+	policy = component.BuildEnvPreserveOnFailure
+	assert.Equal(t, "on-failure", policy.String())
+}
+
+func TestSet(t *testing.T) {
+	var policy component.BuildEnvPreservePolicy
+
+	err := policy.Set("always")
+	require.NoError(t, err)
+	assert.Equal(t, component.BuildEnvPreserveAlways, policy)
+
+	err = policy.Set("never")
+	require.NoError(t, err)
+	assert.Equal(t, component.BuildEnvPreserveNever, policy)
+
+	err = policy.Set("on-failure")
+	require.NoError(t, err)
+	assert.Equal(t, component.BuildEnvPreserveOnFailure, policy)
+
+	err = policy.Set("")
+	require.NoError(t, err)
+	assert.Equal(t, component.BuildEnvPreserveOnFailure, policy)
+
+	err = policy.Set("unsupported-value")
+	require.Error(t, err)
+	assert.Equal(t, component.BuildEnvPreserveOnFailure, policy)
+}
+
+func TestType(t *testing.T) {
+	var policy component.BuildEnvPreservePolicy
+
+	assert.Equal(t, "policy", policy.Type())
+}
+
+func TestShouldPreserve(t *testing.T) {
+	policy := component.BuildEnvPreserveAlways
+	assert.True(t, policy.ShouldPreserve(true))
+	assert.True(t, policy.ShouldPreserve(false))
+
+	policy = component.BuildEnvPreserveNever
+	assert.False(t, policy.ShouldPreserve(true))
+	assert.False(t, policy.ShouldPreserve(false))
+
+	policy = component.BuildEnvPreserveOnFailure
+	assert.False(t, policy.ShouldPreserve(true))
+	assert.True(t, policy.ShouldPreserve(false))
+
+	policy = "unsupported-value"
+	assert.False(t, policy.ShouldPreserve(true))
+	assert.False(t, policy.ShouldPreserve(false))
+}