New flatten buildpacks/builder implementation - Part 1 - removing depth

internal/builder/builder.go

@@ -104,7 +104,6 @@
 
 type options struct {
 	flatten bool
-	depth   int
 	exclude []string
 }
 
@@ -151,8 +150,8 @@
 		env:                      map[string]string{},
 		buildConfigEnv:           map[string]string{},
 		validateMixins:           true,
-		additionalBuildpacks:     *buildpack.NewModuleManager(opts.flatten, opts.depth),
-		additionalExtensions:     *buildpack.NewModuleManager(opts.flatten, opts.depth),
+		additionalBuildpacks:     *buildpack.NewModuleManager(opts.flatten),
+		additionalExtensions:     *buildpack.NewModuleManager(opts.flatten),
 		flattenExcludeBuildpacks: opts.exclude,
 	}
 
@@ -167,10 +166,16 @@
 	return bldr, nil
 }
 
-func WithFlatten(depth int, exclude []string) BuilderOption {
+func WithFlatten() BuilderOption {
+	return func(o *options) error {
+		o.flatten = true
+		return nil
+	}
+}
+
+func WithFlattenExclude(exclude []string) BuilderOption {
 	return func(o *options) error {
 		o.flatten = true
-		o.depth = depth
 		o.exclude = exclude
 		return nil
 	}

internal/builder/builder_test.go

@@ -1874,7 +1874,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) {
 		when("all", func() {
 			it.Before(func() {
 				var err error
-				bldr, err = builder.New(builderImage, "some-builder", builder.WithFlatten(-1, nil))
+				bldr, err = builder.New(builderImage, "some-builder", builder.WithFlatten())
 				h.AssertNil(t, err)
 
 				// Let's add some buildpacks
@@ -1903,7 +1903,6 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) {
 				})
 			})
 		})
-		// TODO add tests for flatten with depth
 	})
 }
 

internal/commands/builder_create.go

@@ -24,7 +24,6 @@ type BuilderCreateFlags struct {
 	Registry        string
 	Policy          string
 	FlattenExclude  []string
-	Depth           int
 }
 
 // CreateBuilder creates a builder image, based on a builder config
@@ -94,7 +93,6 @@ Creating a custom builder allows you to control what buildpacks are used and wha
 				PullPolicy:      pullPolicy,
 				Flatten:         flags.Flatten,
 				FlattenExclude:  flags.FlattenExclude,
-				Depth:           flags.Depth,
 			}); err != nil {
 				return err
 			}
@@ -113,7 +111,6 @@ Creating a custom builder allows you to control what buildpacks are used and wha
 	cmd.Flags().StringVar(&flags.Policy, "pull-policy", "", "Pull policy to use. Accepted values are always, never, and if-not-present. The default is always")
 	cmd.Flags().BoolVar(&flags.Flatten, "flatten", false, "Flatten each composite buildpack into a single layer")
 	cmd.Flags().StringSliceVarP(&flags.FlattenExclude, "flatten-exclude", "e", nil, "Buildpacks to exclude from flattening, in the form of '<buildpack-id>@<buildpack-version>'")
-	cmd.Flags().IntVar(&flags.Depth, "depth", -1, "Max depth to flatten each composite buildpack.\nOmission of this flag or values < 0 will flatten the entire tree.")
 
 	AddHelpFlag(cmd, "create")
 	return cmd

internal/commands/buildpack_package.go

@@ -27,7 +27,6 @@ type BuildpackPackageFlags struct {
 	Label             map[string]string
 	Publish           bool
 	Flatten           bool
-	Depth             int
 }
 
 // BuildpackPackager packages buildpacks
@@ -111,7 +110,6 @@ func BuildpackPackage(logger logging.Logger, cfg config.Config, packager Buildpa
 				Registry:        flags.BuildpackRegistry,
 				Flatten:         flags.Flatten,
 				FlattenExclude:  flags.FlattenExclude,
-				Depth:           flags.Depth,
 				Labels:          flags.Label,
 			}); err != nil {
 				return err
@@ -139,11 +137,9 @@ func BuildpackPackage(logger logging.Logger, cfg config.Config, packager Buildpa
 	cmd.Flags().StringVarP(&flags.BuildpackRegistry, "buildpack-registry", "r", "", "Buildpack Registry name")
 	cmd.Flags().BoolVar(&flags.Flatten, "flatten", false, "Flatten the buildpack into a single layer")
 	cmd.Flags().StringSliceVarP(&flags.FlattenExclude, "flatten-exclude", "e", nil, "Buildpacks to exclude from flattening, in the form of '<buildpack-id>@<buildpack-version>'")
-	cmd.Flags().IntVar(&flags.Depth, "depth", -1, "Max depth to flatten.\nOmission of this flag or values < 0 will flatten the entire tree.")
 	cmd.Flags().StringToStringVarP(&flags.Label, "label", "l", nil, "Labels to add to packaged Buildpack, in the form of '<name>=<value>'")
 	if !cfg.Experimental {
 		cmd.Flags().MarkHidden("flatten")
-		cmd.Flags().MarkHidden("depth")
 		cmd.Flags().MarkHidden("flatten-exclude")
 	}
 	AddHelpFlag(cmd, "package")

pkg/buildpack/builder.go

@@ -75,7 +75,6 @@ type PackageBuilderOption func(*options) error
 
 type options struct {
 	flatten bool
-	depth   int
 	exclude []string
 	logger  logging.Logger
 	factory archive.TarWriterFactory
@@ -100,21 +99,27 @@ func NewBuilder(imageFactory ImageFactory, ops ...PackageBuilderOption) *Package
 			return nil
 		}
 	}
-	moduleManager := NewModuleManager(opts.flatten, opts.depth)
+	moduleManager := NewModuleManager(opts.flatten)
 	return &PackageBuilder{
 		imageFactory:             imageFactory,
 		dependencies:             *moduleManager,
-		flattenAllBuildpacks:     opts.flatten && opts.depth < 0,
+		flattenAllBuildpacks:     opts.flatten,
 		flattenExcludeBuildpacks: opts.exclude,
 		logger:                   opts.logger,
 		layerWriterFactory:       opts.factory,
 	}
 }
 
-func WithFlatten(depth int, exclude []string) PackageBuilderOption {
+func WithFlatten() PackageBuilderOption {
+	return func(o *options) error {
+		o.flatten = true
+		return nil
+	}
+}
+
+func WithFlattenExclude(exclude []string) PackageBuilderOption {
 	return func(o *options) error {
 		o.flatten = true
-		o.depth = depth
 		o.exclude = exclude
 		return nil
 	}

pkg/buildpack/builder_test.go

@@ -881,7 +881,7 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) {
 				when("no exclusions", func() {
 					it.Before(func() {
 						builder = buildpack.NewBuilder(mockImageFactory("linux"),
-							buildpack.WithFlatten(-1, nil),
+							buildpack.WithFlatten(),
 							buildpack.WithLogger(logger),
 							buildpack.WithLayerWriterFactory(archive.DefaultTarWriterFactory()))
 					})
@@ -904,7 +904,7 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) {
 						excluded := []string{bp31.Descriptor().Info().FullName()}
 
 						builder = buildpack.NewBuilder(mockImageFactory("linux"),
-							buildpack.WithFlatten(-1, excluded),
+							buildpack.WithFlattenExclude(excluded),
 							buildpack.WithLogger(logger),
 							buildpack.WithLayerWriterFactory(archive.DefaultTarWriterFactory()))
 					})

pkg/buildpack/managed_collection.go

@@ -1,25 +1,18 @@
 package buildpack
 
-import (
-	"github.com/buildpacks/pack/pkg/dist"
-)
-
 const (
-	FlattenMaxDepth = -1
-	FlattenNone     = 0
+	FlattenNone = 0
 )
 
 type ManagedCollection struct {
 	explodedModules  []BuildModule
 	flattenedModules [][]BuildModule
 	flatten          bool
-	maxDepth         int
 }
 
-func NewModuleManager(flatten bool, maxDepth int) *ManagedCollection {
+func NewModuleManager(flatten bool) *ManagedCollection {
 	return &ManagedCollection{
 		flatten:          flatten,
-		maxDepth:         maxDepth,
 		explodedModules:  []BuildModule{},
 		flattenedModules: [][]BuildModule{},
 	}
@@ -47,30 +40,17 @@ func (f *ManagedCollection) FlattenedModules() [][]BuildModule {
 	return nil
 }
 
-// AddModules determines whether the explodedModules must be added as flattened or not. It uses
-// flatten and maxDepth configuration given during initialization of the manager.
+// AddModules determines whether the explodedModules must be added as flattened or not.
 func (f *ManagedCollection) AddModules(main BuildModule, deps ...BuildModule) {
 	if !f.flatten {
 		// default behavior
 		f.explodedModules = append(f.explodedModules, append([]BuildModule{main}, deps...)...)
 	} else {
-		if f.maxDepth <= FlattenMaxDepth {
-			// flatten all
-			if len(f.flattenedModules) == 1 {
-				f.flattenedModules[0] = append(f.flattenedModules[0], append([]BuildModule{main}, deps...)...)
-			} else {
-				f.flattenedModules = append(f.flattenedModules, append([]BuildModule{main}, deps...))
-			}
+		// flatten all
+		if len(f.flattenedModules) == 1 {
+			f.flattenedModules[0] = append(f.flattenedModules[0], append([]BuildModule{main}, deps...)...)
 		} else {
-			recurser := newFlattenModuleRecurser(f.maxDepth)
-			calculatedModules := recurser.calculateFlattenedModules(main, deps, 0)
-			for _, modules := range calculatedModules {
-				if len(modules) == 1 {
-					f.explodedModules = append(f.explodedModules, modules...)
-				} else {
-					f.flattenedModules = append(f.flattenedModules, modules)
-				}
-			}
+			f.flattenedModules = append(f.flattenedModules, append([]BuildModule{main}, deps...))
 		}
 	}
 }
@@ -88,62 +68,3 @@ func (f *ManagedCollection) ShouldFlatten(module BuildModule) bool {
 	}
 	return false
 }
-
-type flattenModuleRecurser struct {
-	maxDepth int
-}
-
-func newFlattenModuleRecurser(maxDepth int) *flattenModuleRecurser {
-	return &flattenModuleRecurser{
-		maxDepth: maxDepth,
-	}
-}
-
-// calculateFlattenedModules returns groups of modules that will be added to the output artifact as a single layer containing multiple modules.
-// It takes the given main module and its dependencies and based on the depth it will recursively calculate the groups of modules inspecting if the main
-// module is a composited Buildpack or not until it reaches the maxDepth.
-func (f *flattenModuleRecurser) calculateFlattenedModules(main BuildModule, deps []BuildModule, depth int) [][]BuildModule {
-	modules := make([][]BuildModule, 0)
-	groups := main.Descriptor().Order()
-	if len(groups) > 0 {
-		if depth == f.maxDepth {
-			modules = append(modules, append([]BuildModule{main}, deps...))
-		}
-		if depth < f.maxDepth {
-			nextBPs, nextDeps := buildpacksFromGroups(groups, deps)
-			modules = append(modules, []BuildModule{main})
-			for _, bp := range nextBPs {
-				modules = append(modules, f.calculateFlattenedModules(bp, nextDeps, depth+1)...)
-			}
-		}
-	} else {
-		// It is not a composited Buildpack, we add it as a single module
-		modules = append(modules, []BuildModule{main})
-	}
-	return modules
-}
-
-// buildpacksFromGroups
-func buildpacksFromGroups(order dist.Order, deps []BuildModule) ([]BuildModule, []BuildModule) {
-	bps := make([]BuildModule, 0)
-	newDeps := make([]BuildModule, 0)
-
-	type void struct{}
-	var member void
-	set := make(map[string]void)
-	for _, groups := range order {
-		for _, group := range groups.Group {
-			set[group.FullName()] = member
-		}
-	}
-
-	for _, dep := range deps {
-		if _, ok := set[dep.Descriptor().Info().FullName()]; ok {
-			bps = append(bps, dep)
-		} else {
-			newDeps = append(newDeps, dep)
-		}
-	}
-
-	return bps, newDeps
-}

pkg/buildpack/managed_collection_test.go

@@ -139,7 +139,7 @@ func testModuleManager(t *testing.T, when spec.G, it spec.S) {
 	when("manager is configured in flatten mode", func() {
 		when("flatten all", func() {
 			it.Before(func() {
-				moduleManager = buildpack.NewModuleManager(true, buildpack.FlattenMaxDepth)
+				moduleManager = buildpack.NewModuleManager(true)
 				moduleManager.AddModules(compositeBP1, []buildpack.BuildModule{bp1, compositeBP2, bp21, bp22, compositeBP3, bp31}...)
 			})
 
@@ -177,85 +177,11 @@ func testModuleManager(t *testing.T, when spec.G, it spec.S) {
 				})
 			})
 		})
-
-		when("flatten with depth=1", func() {
-			it.Before(func() {
-				moduleManager = buildpack.NewModuleManager(true, 1)
-				moduleManager.AddModules(compositeBP1, []buildpack.BuildModule{bp1, compositeBP2, bp21, bp22, compositeBP3, bp31}...)
-			})
-
-			when("#FlattenedModules", func() {
-				it("returns 1 flatten module with [compositeBP2, bp21, bp22, compositeBP3, bp31]", func() {
-					modules := moduleManager.FlattenedModules()
-					h.AssertEq(t, len(modules), 1)
-					h.AssertEq(t, len(modules[0]), 5)
-				})
-			})
-
-			when("#ShouldFlatten", func() {
-				it("returns true for flatten explodedModules", func() {
-					h.AssertTrue(t, moduleManager.ShouldFlatten(compositeBP2))
-					h.AssertTrue(t, moduleManager.ShouldFlatten(bp21))
-					h.AssertTrue(t, moduleManager.ShouldFlatten(bp22))
-					h.AssertTrue(t, moduleManager.ShouldFlatten(compositeBP3))
-					h.AssertTrue(t, moduleManager.ShouldFlatten(bp31))
-				})
-
-				it("returns false for no flatten explodedModules", func() {
-					h.AssertFalse(t, moduleManager.ShouldFlatten(bp1))
-					h.AssertFalse(t, moduleManager.ShouldFlatten(compositeBP1))
-				})
-			})
-
-			when("#ExplodedModules", func() {
-				it("returns [bp1, compositeBP1]", func() {
-					modules := moduleManager.ExplodedModules()
-					h.AssertEq(t, len(modules), 2)
-				})
-			})
-		})
-
-		when("flatten with depth=2", func() {
-			it.Before(func() {
-				moduleManager = buildpack.NewModuleManager(true, 2)
-				moduleManager.AddModules(compositeBP1, []buildpack.BuildModule{bp1, compositeBP2, bp21, bp22, compositeBP3, bp31}...)
-			})
-
-			when("#FlattenedModules", func() {
-				it("returns 1 flatten module with [compositeBP3, bp31]", func() {
-					modules := moduleManager.FlattenedModules()
-					h.AssertEq(t, len(modules), 1)
-					h.AssertEq(t, len(modules[0]), 2)
-				})
-			})
-
-			when("#ShouldFlatten", func() {
-				it("returns true for flatten explodedModules", func() {
-					h.AssertTrue(t, moduleManager.ShouldFlatten(compositeBP3))
-					h.AssertTrue(t, moduleManager.ShouldFlatten(bp31))
-				})
-
-				it("returns false for no flatten explodedModules", func() {
-					h.AssertFalse(t, moduleManager.ShouldFlatten(compositeBP2))
-					h.AssertFalse(t, moduleManager.ShouldFlatten(bp21))
-					h.AssertFalse(t, moduleManager.ShouldFlatten(bp22))
-					h.AssertFalse(t, moduleManager.ShouldFlatten(bp1))
-					h.AssertFalse(t, moduleManager.ShouldFlatten(compositeBP1))
-				})
-			})
-
-			when("#ExplodedModules", func() {
-				it("returns [compositeBP1, bp1, compositeBP2, bp21, bp22]", func() {
-					modules := moduleManager.ExplodedModules()
-					h.AssertEq(t, len(modules), 5)
-				})
-			})
-		})
 	})
 
 	when("manager is not configured in flatten mode", func() {
 		it.Before(func() {
-			moduleManager = buildpack.NewModuleManager(false, buildpack.FlattenNone)
+			moduleManager = buildpack.NewModuleManager(false)
 		})
 
 		when("#ExplodedModules", func() {