diff --git a/internal/builder/builder.go b/internal/builder/builder.go index fa7dd6e9d..6b09d0378 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -104,7 +104,6 @@ type BuilderOption func(*options) error type options struct { flatten bool - depth int exclude []string } @@ -151,8 +150,8 @@ func constructBuilder(img imgutil.Image, newName string, errOnMissingLabel bool, 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 @@ func constructBuilder(img imgutil.Image, newName string, errOnMissingLabel bool, 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 } diff --git a/internal/builder/builder_test.go b/internal/builder/builder_test.go index 37124533d..f6aee5080 100644 --- a/internal/builder/builder_test.go +++ b/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 }) } diff --git a/internal/commands/builder_create.go b/internal/commands/builder_create.go index c3c1ccb90..89e915acc 100644 --- a/internal/commands/builder_create.go +++ b/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 '@'") - 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 diff --git a/internal/commands/buildpack_package.go b/internal/commands/buildpack_package.go index dff2bf398..b4c49893a 100644 --- a/internal/commands/buildpack_package.go +++ b/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 '@'") - 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 '='") if !cfg.Experimental { cmd.Flags().MarkHidden("flatten") - cmd.Flags().MarkHidden("depth") cmd.Flags().MarkHidden("flatten-exclude") } AddHelpFlag(cmd, "package") diff --git a/pkg/buildpack/builder.go b/pkg/buildpack/builder.go index 93724a3e6..8ece27bbd 100644 --- a/pkg/buildpack/builder.go +++ b/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 } diff --git a/pkg/buildpack/builder_test.go b/pkg/buildpack/builder_test.go index 7181e08bc..a487b0323 100644 --- a/pkg/buildpack/builder_test.go +++ b/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())) }) diff --git a/pkg/buildpack/managed_collection.go b/pkg/buildpack/managed_collection.go index 6f6043372..0013a211b 100644 --- a/pkg/buildpack/managed_collection.go +++ b/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 -} diff --git a/pkg/buildpack/managed_collection_test.go b/pkg/buildpack/managed_collection_test.go index 9fb6ff3d6..eeae8feda 100644 --- a/pkg/buildpack/managed_collection_test.go +++ b/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() { diff --git a/pkg/client/create_builder.go b/pkg/client/create_builder.go index b697313b3..1e1a8f5ca 100644 --- a/pkg/client/create_builder.go +++ b/pkg/client/create_builder.go @@ -48,9 +48,6 @@ type CreateBuilderOptions struct { // Flatten layers Flatten bool - // Max depth for flattening compose buildpacks. - Depth int - // List of buildpack images to exclude from the package been flatten. FlattenExclude []string } @@ -158,7 +155,7 @@ func (c *Client) createBaseBuilder(ctx context.Context, opts CreateBuilderOption var builderOpts []builder.BuilderOption if opts.Flatten { - builderOpts = append(builderOpts, builder.WithFlatten(opts.Depth, opts.FlattenExclude)) + builderOpts = append(builderOpts, builder.WithFlattenExclude(opts.FlattenExclude)) } bldr, err := builder.New(baseImage, opts.BuilderName, builderOpts...) if err != nil { diff --git a/pkg/client/create_builder_test.go b/pkg/client/create_builder_test.go index ea0c95a93..a8b1d190c 100644 --- a/pkg/client/create_builder_test.go +++ b/pkg/client/create_builder_test.go @@ -1092,36 +1092,6 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { }) }) }) - - when("with depth", func() { - when("depth = 1", func() { - it("creates 3 layers [1,2,[3,4,5,6,7]]", func() { - prepareFetcherWithRunImages() - opts.Flatten = true - opts.Depth = 1 - - successfullyCreateFlattenBuilder() - - layers := fakeLayerImage.AddedLayersOrder() - - h.AssertEq(t, len(layers), 3) - }) - }) - - when("depth = 2", func() { - it("creates 5 layers [1,2,3,4,[5,6,7]]", func() { - prepareFetcherWithRunImages() - opts.Flatten = true - opts.Depth = 2 - - successfullyCreateFlattenBuilder() - - layers := fakeLayerImage.AddedLayersOrder() - - h.AssertEq(t, len(layers), 5) - }) - }) - }) }) }) } diff --git a/pkg/client/package_buildpack.go b/pkg/client/package_buildpack.go index 502564d2f..96b514f09 100644 --- a/pkg/client/package_buildpack.go +++ b/pkg/client/package_buildpack.go @@ -54,9 +54,6 @@ type PackageBuildpackOptions struct { // Flatten layers Flatten bool - // Max depth for flattening compose buildpacks. - Depth int - // List of buildpack images to exclude from the package been flatten. FlattenExclude []string @@ -86,7 +83,7 @@ func (c *Client) PackageBuildpack(ctx context.Context, opts PackageBuildpackOpti var packageBuilderOpts []buildpack.PackageBuilderOption if opts.Flatten { - packageBuilderOpts = append(packageBuilderOpts, buildpack.WithFlatten(opts.Depth, opts.FlattenExclude), + packageBuilderOpts = append(packageBuilderOpts, buildpack.WithFlattenExclude(opts.FlattenExclude), buildpack.WithLayerWriterFactory(writerFactory), buildpack.WithLogger(c.logger)) } packageBuilder := buildpack.NewBuilder(c.imageFactory, packageBuilderOpts...) diff --git a/pkg/client/package_buildpack_test.go b/pkg/client/package_buildpack_test.go index 0814e6e33..dfeecc98d 100644 --- a/pkg/client/package_buildpack_test.go +++ b/pkg/client/package_buildpack_test.go @@ -504,8 +504,6 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { when("flatten all", func() { it("creates package image with all dependencies", func() { - opts.Depth = -1 - successfullyCreateFlattenPackage() layers := fakeLayerImage.AddedLayersOrder() @@ -514,32 +512,6 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { // TODO add test case for flatten all with --flatten-exclude }) - - when("with depth", func() { - when("depth = 0", func() { - it("creates 3 layers [1,2,[3,4,5,6,7]]", func() { - opts.Depth = 0 - - successfullyCreateFlattenPackage() - - layers := fakeLayerImage.AddedLayersOrder() - h.AssertEq(t, len(layers), 3) - }) - }) - - when("depth = 1", func() { - it("creates 5 layers [1,2,3,4,[5,6,7]]", func() { - opts.Depth = 1 - - successfullyCreateFlattenPackage() - - layers := fakeLayerImage.AddedLayersOrder() - h.AssertEq(t, len(layers), 5) - }) - }) - - // TODO add test case for flatten with --depth AND --flatten-exclude - }) }) })