FIXME: pass the logger and print the warning inside

buildpack/build.go

@@ -164,24 +164,18 @@ func runBuildCmd(d BpDescriptor, bpLayersDir, planPath string, inputs BuildInput
 func (d BpDescriptor) processLayers(layersDir string, logger log.Logger) (map[string]LayerMetadataFile, error) {
 	if api.MustParse(d.WithAPI).LessThan("0.6") {
 		return eachLayer(layersDir, d.WithAPI, func(path, buildpackAPI string) (LayerMetadataFile, error) {
-			layerMetadataFile, msg, err := DecodeLayerMetadataFile(path+".toml", buildpackAPI)
+			layerMetadataFile, err := DecodeLayerMetadataFile(path+".toml", buildpackAPI, logger)
 			if err != nil {
 				return LayerMetadataFile{}, err
 			}
-			if msg != "" {
-				logger.Warn(msg)
-			}
 			return layerMetadataFile, nil
 		})
 	}
 	return eachLayer(layersDir, d.WithAPI, func(path, buildpackAPI string) (LayerMetadataFile, error) {
-		layerMetadataFile, msg, err := DecodeLayerMetadataFile(path+".toml", buildpackAPI)
+		layerMetadataFile, err := DecodeLayerMetadataFile(path+".toml", buildpackAPI, logger)
 		if err != nil {
 			return LayerMetadataFile{}, err
 		}
-		if msg != "" {
-			return LayerMetadataFile{}, errors.New(msg)
-		}
 		if err := renameLayerDirIfNeeded(layerMetadataFile, path); err != nil {
 			return LayerMetadataFile{}, err
 		}

buildpack/layermetadata.go

@@ -7,6 +7,8 @@ import (
 
 	"github.com/BurntSushi/toml"
 
+	"github.com/buildpacks/lifecycle/log"
+
 	"github.com/buildpacks/lifecycle/api"
 )
 
@@ -34,23 +36,31 @@ func EncodeLayerMetadataFile(lmf LayerMetadataFile, path, buildpackAPI string) e
 	return errors.New("couldn't find an encoder")
 }
 
-func DecodeLayerMetadataFile(path, buildpackAPI string) (LayerMetadataFile, string, error) { // FIXME: pass the logger and print the warning inside (instead of returning a message)
+func DecodeLayerMetadataFile(path string, buildpackAPI string, logger log.Logger) (LayerMetadataFile, error) { // FIXME: pass the logger and print the warning inside (instead of returning a message)
 	fh, err := os.Open(path)
 	if os.IsNotExist(err) {
-		return LayerMetadataFile{}, "", nil
+		return LayerMetadataFile{}, nil
 	} else if err != nil {
-		return LayerMetadataFile{}, "", err
+		return LayerMetadataFile{}, err
 	}
 	defer fh.Close()
 
 	decoders := supportedEncoderDecoders()
 
 	for _, decoder := range decoders {
 		if decoder.IsSupported(buildpackAPI) {
-			return decoder.Decode(path)
+			lmf, str, err := decoder.Decode(path)
+			if str != "" {
+				if api.MustParse(buildpackAPI).LessThan("0.6") {
+					logger.Warn(str)
+				} else {
+					return LayerMetadataFile{}, errors.New(str)
+				}
+			}
+			return lmf, err
 		}
 	}
-	return LayerMetadataFile{}, "", errors.New("couldn't find a decoder")
+	return LayerMetadataFile{}, errors.New("couldn't find a decoder")
 }
 
 type encoderDecoder interface {

buildpack/layermetadata_test.go

@@ -1,16 +1,26 @@
 package buildpack_test
 
 import (
+	"fmt"
 	"os"
 	"testing"
 
+	"github.com/apex/log"
+
+	"github.com/apex/log/handlers/memory"
+
 	"github.com/sclevine/spec"
 	"github.com/sclevine/spec/report"
 
+	llog "github.com/buildpacks/lifecycle/log"
+
 	"github.com/buildpacks/lifecycle/buildpack"
 	h "github.com/buildpacks/lifecycle/testhelpers"
 )
 
+var logHandler *memory.Handler
+var logger llog.Logger
+
 func TestLayerMetadata(t *testing.T) {
 	spec.Run(t, "unit-layermetadata", testLayerMetadata, spec.Report(report.Terminal{}))
 }
@@ -24,6 +34,8 @@ func testLayerMetadata(t *testing.T, when spec.G, it spec.S) {
 			var err error
 			metadataFile, err = os.CreateTemp("", "test")
 			h.AssertNil(t, err)
+			logHandler = memory.New()
+			logger = &log.Logger{Handler: logHandler}
 		})
 		it.After(func() {
 			os.Remove(metadataFile.Name())
@@ -33,12 +45,34 @@ func testLayerMetadata(t *testing.T, when spec.G, it spec.S) {
 			h.AssertNil(t, err)
 
 			var lmf buildpack.LayerMetadataFile
-			lmf, s, err := buildpack.DecodeLayerMetadataFile(metadataFile.Name(), "0.9")
+			lmf, err = buildpack.DecodeLayerMetadataFile(metadataFile.Name(), "0.9", logger)
 			h.AssertNil(t, err)
-			h.AssertEq(t, s, "")
 			h.AssertEq(t, lmf.Cache, true)
 			h.AssertEq(t, lmf.Build, false)
 			h.AssertEq(t, lmf.Launch, false)
 		})
+		it("logs a warning when the metadata file has wrong format (on older apis)", func() {
+			err := os.WriteFile(metadataFile.Name(), []byte("cache = true"), 0400)
+			h.AssertNil(t, err)
+			var lmf buildpack.LayerMetadataFile
+			lmf, err = buildpack.DecodeLayerMetadataFile(metadataFile.Name(), "0.5", logger)
+			h.AssertNil(t, err)
+			h.AssertEq(t, lmf.Cache, false)
+			h.AssertEq(t, lmf.Build, false)
+			h.AssertEq(t, lmf.Launch, false)
+			expected := fmt.Sprintf("the launch, cache and build flags should be in the types table of %s", metadataFile.Name())
+			h.AssertLogEntry(t, logHandler, expected)
+		})
+		it("returns an error when the metadata file has wrong format", func() {
+			err := os.WriteFile(metadataFile.Name(), []byte("cache = true"), 0400)
+			h.AssertNil(t, err)
+
+			var lmf buildpack.LayerMetadataFile
+			lmf, err = buildpack.DecodeLayerMetadataFile(metadataFile.Name(), "0.9", logger)
+			h.AssertNotNil(t, err)
+			h.AssertEq(t, lmf.Cache, false)
+			h.AssertEq(t, lmf.Build, false)
+			h.AssertEq(t, lmf.Launch, false)
+		})
 	})
 }

buildpack/layers.go

@@ -148,17 +148,10 @@ func (l *Layer) Path() string {
 
 func (l *Layer) Read() (LayerMetadata, error) {
 	tomlPath := l.Path() + ".toml"
-	layerMetadataFile, msg, err := DecodeLayerMetadataFile(tomlPath, l.api)
+	layerMetadataFile, err := DecodeLayerMetadataFile(tomlPath, l.api, l.logger)
 	if err != nil {
 		return LayerMetadata{}, err
 	}
-	if msg != "" {
-		if api.MustParse(l.api).LessThan("0.6") {
-			l.logger.Warn(msg)
-		} else {
-			return LayerMetadata{}, errors.New(msg)
-		}
-	}
 	var sha string
 	shaBytes, err := os.ReadFile(l.Path() + ".sha")
 	if err != nil && !os.IsNotExist(err) { // if the sha file doesn't exist, an empty sha will be returned

go.mod

@@ -115,7 +115,7 @@ require (
 	golang.org/x/oauth2 v0.7.0 // indirect
 	golang.org/x/text v0.9.0 // indirect
 	golang.org/x/time v0.3.0 // indirect
-	golang.org/x/tools v0.7.0 // indirect
+	golang.org/x/tools v0.8.0 // indirect
 	google.golang.org/appengine v1.6.7 // indirect
 	google.golang.org/genproto v0.0.0-20230306155012-7f2fa6fef1f4 // indirect
 	google.golang.org/grpc v1.53.0 // indirect

go.sum

@@ -735,6 +735,8 @@ golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0=
 golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
 golang.org/x/tools v0.7.0 h1:W4OVu8VVOaIO0yzWMNdepAulS7YfoS3Zabrm8DOXXU4=
 golang.org/x/tools v0.7.0/go.mod h1:4pg6aUX35JBAogB10C9AtvVL+qowtN4pT3CGSQex14s=
+golang.org/x/tools v0.8.0 h1:vSDcovVPld282ceKgDimkRSC8kpaH1dgyc9UMzlt84Y=
+golang.org/x/tools v0.8.0/go.mod h1:JxBZ99ISMI5ViVkT1tr6tdNmXeTrcpVSD3vZ1RsRdN4=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=