Fixing an parsing error with the buildpacks to be flattened

Signed-off-by: Juan Bustamante <[email protected]>

internal/builder/builder_test.go

@@ -1907,7 +1907,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) {
 		when("buildpacks to be flattened are defined", func() {
 			it.Before(func() {
 				var err error
-				flattenModules, err := buildpack.ParseFlattenBuildModules([]string{"buildpack-1-id@buildpack-1-version-1,buildpack-1-id@buildpack-1-version-2,buildpack-2-id@buildpack-2-version-1"})
+				flattenModules, err := buildpack.ParseFlattenBuildModules([]string{"buildpack-1-id@buildpack-1-version-1;buildpack-1-id@buildpack-1-version-2;buildpack-2-id@buildpack-2-version-1"}, ";")
 				h.AssertNil(t, err)
 
 				bldr, err = builder.New(builderImage, "some-builder", builder.WithFlattened(flattenModules))

internal/commands/builder_create.go

@@ -82,7 +82,7 @@ Creating a custom builder allows you to control what buildpacks are used and wha
 				return err
 			}
 
-			toFlatten, err := buildpack.ParseFlattenBuildModules(flags.Flatten)
+			toFlatten, err := buildpack.ParseFlattenBuildModules(flags.Flatten, ";")
 			if err != nil {
 				return err
 			}
@@ -114,7 +114,7 @@ Creating a custom builder allows you to control what buildpacks are used and wha
 	cmd.Flags().StringVarP(&flags.BuilderTomlPath, "config", "c", "", "Path to builder TOML file (required)")
 	cmd.Flags().BoolVar(&flags.Publish, "publish", false, "Publish the builder directly to the container registry specified in <image-name>, instead of the daemon.")
 	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().StringSliceVar(&flags.Flatten, "flatten", nil, "List of buildpacks to flatten together into a single layer (format: '<buildpack-id>@<buildpack-version>,<buildpack-id>@<buildpack-version>'")
+	cmd.Flags().StringSliceVar(&flags.Flatten, "flatten", nil, "List of buildpacks to flatten together into a single layer (format: '<buildpack-id>@<buildpack-version>;<buildpack-id>@<buildpack-version>'")
 	cmd.Flags().StringToStringVarP(&flags.Label, "label", "l", nil, "Labels to add to the builder image, in the form of '<name>=<value>'")
 
 	AddHelpFlag(cmd, "create")

pkg/buildpack/build_module_info.go

@@ -32,10 +32,17 @@ func (b *buildModuleInfosImpl) BuildModule() []dist.ModuleInfo {
 	return b.modules
 }
 
-func ParseFlattenBuildModules(buildpacksID []string) (FlattenModuleInfos, error) {
+// ParseFlattenBuildModules parses the given list of buildpacks IDs into a group of modules to be flattened
+func ParseFlattenBuildModules(buildpacksID []string, sep string) (FlattenModuleInfos, error) {
 	var buildModuleInfos []ModuleInfos
 	for _, ids := range buildpacksID {
-		modules, err := parseBuildpackName(ids)
+		var names []string
+		if sep != "" {
+			names = strings.Split(ids, sep)
+		} else {
+			names = []string{ids}
+		}
+		modules, err := parseBuildpackNames(names)
 		if err != nil {
 			return nil, err
 		}
@@ -44,10 +51,9 @@ func ParseFlattenBuildModules(buildpacksID []string) (FlattenModuleInfos, error)
 	return &flattenModules{modules: buildModuleInfos}, nil
 }
 
-func parseBuildpackName(names string) (ModuleInfos, error) {
+func parseBuildpackNames(names []string) (ModuleInfos, error) {
 	var buildModuleInfos []dist.ModuleInfo
-	ids := strings.Split(names, ",")
-	for _, id := range ids {
+	for _, id := range names {
 		if strings.Count(id, "@") != 1 {
 			return nil, errors.Errorf("invalid format %s; please use '<buildpack-id>@<buildpack-version>' to add buildpacks to be flattened", id)
 		}

pkg/buildpack/build_module_info_test.go

@@ -28,7 +28,7 @@ func testBuildModuleInfo(t *testing.T, when spec.G, it spec.S) {
 				})
 
 				it("parses successfully", func() {
-					flattenModuleInfos, err := buildpack.ParseFlattenBuildModules(buildModules)
+					flattenModuleInfos, err := buildpack.ParseFlattenBuildModules(buildModules, "")
 					h.AssertNil(t, err)
 					h.AssertNotNil(t, flattenModuleInfos)
 					h.AssertTrue(t, len(flattenModuleInfos.FlattenModules()) == 1)
@@ -39,11 +39,11 @@ func testBuildModuleInfo(t *testing.T, when spec.G, it spec.S) {
 
 			when("more than one buildpackID is provided", func() {
 				it.Before(func() {
-					buildModules = []string{"some-buildpack@version-1, another-buildpack@version-2"}
+					buildModules = []string{"some-buildpack@version-1; another-buildpack@version-2"}
 				})
 
 				it("parses multiple buildpackIDs", func() {
-					flattenModuleInfos, err := buildpack.ParseFlattenBuildModules(buildModules)
+					flattenModuleInfos, err := buildpack.ParseFlattenBuildModules(buildModules, ";")
 					h.AssertNil(t, err)
 					h.AssertNotNil(t, flattenModuleInfos)
 					h.AssertTrue(t, len(flattenModuleInfos.FlattenModules()) == 1)
@@ -59,31 +59,31 @@ func testBuildModuleInfo(t *testing.T, when spec.G, it spec.S) {
 		when("buildpacksID don't have format <buildpack>@<version>", func() {
 			when("@<version> is missing", func() {
 				it("errors with a descriptive message", func() {
-					_, err := buildpack.ParseFlattenBuildModules([]string{"some-buildpack"})
+					_, err := buildpack.ParseFlattenBuildModules([]string{"some-buildpack"}, "")
 					h.AssertNotNil(t, err)
 					h.AssertError(t, err, fmt.Sprintf("invalid format %s; please use '<buildpack-id>@<buildpack-version>' to add buildpacks to be flattened", "some-buildpack"))
 				})
 			})
 
 			when("<version> is missing", func() {
 				it("errors with a descriptive message", func() {
-					_, err := buildpack.ParseFlattenBuildModules([]string{"some-buildpack@"})
+					_, err := buildpack.ParseFlattenBuildModules([]string{"some-buildpack@"}, "")
 					h.AssertNotNil(t, err)
 					h.AssertError(t, err, fmt.Sprintf("invalid format %s; '<buildpack-id>' and '<buildpack-version>' must be specified", "some-buildpack@"))
 				})
 			})
 
 			when("<buildpack> is missing", func() {
 				it("errors with a descriptive message", func() {
-					_, err := buildpack.ParseFlattenBuildModules([]string{"@version-1"})
+					_, err := buildpack.ParseFlattenBuildModules([]string{"@version-1"}, "")
 					h.AssertNotNil(t, err)
 					h.AssertError(t, err, fmt.Sprintf("invalid format %s; '<buildpack-id>' and '<buildpack-version>' must be specified", "@version-1"))
 				})
 			})
 
 			when("multiple @ are used", func() {
 				it("errors with a descriptive message", func() {
-					_, err := buildpack.ParseFlattenBuildModules([]string{"some-buildpack@@version-1"})
+					_, err := buildpack.ParseFlattenBuildModules([]string{"some-buildpack@@version-1"}, "")
 					h.AssertNotNil(t, err)
 					h.AssertError(t, err, fmt.Sprintf("invalid format %s; please use '<buildpack-id>@<buildpack-version>' to add buildpacks to be flattened", "some-buildpack@@version-1"))
 				})

pkg/buildpack/managed_collection.go

@@ -1,5 +1,7 @@
 package buildpack
 
+import "fmt"
+
 // ManagedCollection keeps track of build modules and the manner in which they should be added to an OCI image (as flattened or exploded).
 type ManagedCollection interface {
 	// AllModules returns all build modules handled by the manager.
@@ -121,6 +123,7 @@ func (ff *managedCollectionV2) AddModules(main BuildModule, deps ...BuildModule)
 	for _, module := range allModules {
 		if ff.flattenModuleInfos != nil && len(ff.flattenGroups()) > 0 {
 			pos := ff.flattenedLayerFor(module)
+			fmt.Printf("position %d for module %s", pos, module.Descriptor().Info().FullName())
 			if pos >= 0 {
 				ff.flattenedModules[pos] = append(ff.flattenedModules[pos], module)
 			} else {

pkg/buildpack/managed_collection_test.go

@@ -184,8 +184,9 @@ func testModuleManager(t *testing.T, when spec.G, it spec.S) {
 		when("V2 is used", func() {
 			when("flattened build modules are provided", func() {
 				it.Before(func() {
-					flattenBuildModules, err = buildpack.ParseFlattenBuildModules([]string{"composite-buildpack-3-id@composite-buildpack-3-version,buildpack-31-id@buildpack-31-version", "composite-buildpack-2-id@composite-buildpack-2-version,buildpack-21-id@buildpack-21-version,buildpack-22-id@buildpack-22-version"})
+					flattenBuildModules, err = buildpack.ParseFlattenBuildModules([]string{"composite-buildpack-3-id@composite-buildpack-3-version;buildpack-31-id@buildpack-31-version", "composite-buildpack-2-id@composite-buildpack-2-version;buildpack-21-id@buildpack-21-version;buildpack-22-id@buildpack-22-version"}, ";")
 					h.AssertNil(t, err)
+					h.AssertTrue(t, len(flattenBuildModules.FlattenModules()) == 2)
 
 					moduleManager = buildpack.NewManagedCollectionV2(flattenBuildModules)
 					moduleManager.AddModules(compositeBP1, []buildpack.BuildModule{bp1, compositeBP2, bp21, bp22, compositeBP3, bp31}...)

pkg/client/create_builder_test.go

@@ -1085,7 +1085,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) {
 			when("flatten all", func() {
 				it("creates 1 layer for all buildpacks", func() {
 					prepareFetcherWithRunImages()
-					opts.Flatten, err = buildpack.ParseFlattenBuildModules([]string{"flatten/bp-1@1,flatten/bp-2@2,flatten/bp-4@4,flatten/bp-6@6,flatten/bp-7@7,flatten/bp-3@3,flatten/bp-5@5"})
+					opts.Flatten, err = buildpack.ParseFlattenBuildModules([]string{"flatten/bp-1@1;flatten/bp-2@2;flatten/bp-4@4;flatten/bp-6@6;flatten/bp-7@7;flatten/bp-3@3;flatten/bp-5@5"}, ";")
 					h.AssertNil(t, err)
 
 					successfullyCreateFlattenBuilder()
@@ -1099,7 +1099,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) {
 			when("only some modules are flattened", func() {
 				it("creates 1 layer for buildpacks [1,2,3,4,5,6] and 1 layer for buildpack [7]", func() {
 					prepareFetcherWithRunImages()
-					opts.Flatten, err = buildpack.ParseFlattenBuildModules([]string{"flatten/bp-1@1,flatten/bp-2@2,flatten/bp-4@4,flatten/bp-6@6,flatten/bp-3@3,flatten/bp-5@5"})
+					opts.Flatten, err = buildpack.ParseFlattenBuildModules([]string{"flatten/bp-1@1;flatten/bp-2@2;flatten/bp-4@4;flatten/bp-6@6;flatten/bp-3@3;flatten/bp-5@5"}, ";")
 					h.AssertNil(t, err)
 
 					successfullyCreateFlattenBuilder()
@@ -1110,7 +1110,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) {
 
 				it("creates 1 layer for buildpacks [1,2,3] and 1 layer for [4,5,6] and 1 layer for [7]", func() {
 					prepareFetcherWithRunImages()
-					opts.Flatten, err = buildpack.ParseFlattenBuildModules([]string{"flatten/bp-1@1,flatten/bp-2@2,flatten/bp-3@3", "flatten/bp-4@4,flatten/bp-6@6,flatten/bp-5@5"})
+					opts.Flatten, err = buildpack.ParseFlattenBuildModules([]string{"flatten/bp-1@1;flatten/bp-2@2;flatten/bp-3@3", "flatten/bp-4@4;flatten/bp-6@6;flatten/bp-5@5"}, ";")
 					h.AssertNil(t, err)
 
 					successfullyCreateFlattenBuilder()