refactor: simplify git_metadata_sha propigation

this refactor is in reponse to the Slack comment "this resilient enough to lots of things that might go wrong?"
reffering to the previous GitMetadataSHA implementation

I agree it was not simple. I hope this is better.

internal/acceptance/bake/bake_test.go

@@ -12,14 +12,14 @@ import (
 	"testing"
 	"time"
 
-	"github.com/pivotal-cf/kiln/internal/builder"
-
 	"github.com/onsi/gomega/gbytes"
 	"github.com/onsi/gomega/gexec"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	. "github.com/pivotal-cf-experimental/gomegamatchers"
+
+	"github.com/pivotal-cf/kiln/internal/builder"
 )
 
 const (
@@ -42,7 +42,7 @@ var _ = BeforeSuite(func() {
 	pathToMain, err = gexec.Build("github.com/pivotal-cf/kiln",
 		"--ldflags", fmt.Sprintf("-X main.version=%s", buildVersion))
 	Expect(err).NotTo(HaveOccurred())
-	tileSourceRevision, err = builder.GitMetadataSHA(".", true)()
+	tileSourceRevision, err = builder.GitMetadataSHA(".", true)
 	Expect(err).NotTo(HaveOccurred())
 })
 

internal/builder/interpolator.go

@@ -48,23 +48,14 @@ type InterpolateInput struct {
 	PropertyBlueprints map[string]any
 	RuntimeConfigs     map[string]any
 	StubReleases       bool
-	MetadataGitSHA     func() (string, error)
+	MetadataGitSHA     string
 }
 
 func NewInterpolator() Interpolator {
 	return Interpolator{}
 }
 
 func (i Interpolator) Interpolate(input InterpolateInput, name string, templateYAML []byte) ([]byte, error) {
-	var gitMetadataSHA string
-	if input.MetadataGitSHA != nil {
-		sha, err := input.MetadataGitSHA()
-		if err != nil {
-			return nil, err
-		}
-		gitMetadataSHA = sha
-	}
-
 	interpolatedYAML, err := i.interpolate(input, name, templateYAML)
 	if err != nil {
 		return nil, err
@@ -77,7 +68,7 @@ func (i Interpolator) Interpolate(input InterpolateInput, name string, templateY
 
 	return setKilnMetadata(prettyMetadata, KilnMetadata{
 		KilnVersion:    input.KilnVersion,
-		MetadataGitSHA: gitMetadataSHA,
+		MetadataGitSHA: input.MetadataGitSHA,
 	})
 }
 
@@ -184,9 +175,7 @@ func (i Interpolator) functions(input InterpolateInput) template.FuncMap {
 			if !ok {
 				switch key {
 				case MetadataGitSHAVariable:
-					if input.MetadataGitSHA != nil {
-						return input.MetadataGitSHA()
-					}
+					return input.MetadataGitSHA, nil
 				case BuildVersionVariable:
 					return versionFunc()
 				}

internal/builder/kiln_metadata.go

@@ -9,8 +9,8 @@ import (
 )
 
 type KilnMetadata struct {
-	MetadataGitSHA string `yaml:"metadata_git_sha,omitempty"`
-	KilnVersion    string `yaml:"kiln_version,omitempty"`
+	MetadataGitSHA string `yaml:"metadata_git_sha"`
+	KilnVersion    string `yaml:"kiln_version"`
 }
 
 func setKilnMetadata(in []byte, kilnMetadata KilnMetadata) ([]byte, error) {

internal/builder/metadata_git_sha.go

@@ -10,34 +10,38 @@ import (
 
 const dirtyStateSHAValue = "DEVELOPMENT"
 
-func GitMetadataSHA(repositoryDirectory string, isDev bool) func() (string, error) {
-	var cache string
-	return func() (s string, err error) {
-		if cache != "" {
-			return cache, nil
-		}
-		if _, err := exec.LookPath("git"); err != nil {
-			return "", fmt.Errorf("could not calculate %q: %w", MetadataGitSHAVariable, err)
-		}
-		gitStatus := exec.Command("git", "status", "--porcelain")
-		gitStatus.Dir = repositoryDirectory
-		err = gitStatus.Run()
-		if err != nil {
-			if gitStatus.ProcessState.ExitCode() == 1 && isDev {
-				_, _ = fmt.Fprintf(os.Stderr, "WARNING: git working directory has un-commited changes: the variable %q has has development only value %q", MetadataGitSHAVariable, dirtyStateSHAValue)
-				return "DEVELOPMENT", nil
-			}
-			return "", fmt.Errorf("failed to run `%s %s`: %w", gitStatus.Path, strings.Join(gitStatus.Args, " "), err)
-		}
-		var out bytes.Buffer
-		gitRevParseHead := exec.Command("git", "rev-parse", "HEAD")
-		gitRevParseHead.Dir = repositoryDirectory
-		gitRevParseHead.Stdout = &out
-		err = gitRevParseHead.Run()
-		if err != nil {
-			return "", fmt.Errorf("failed to get HEAD revision hash: %w", err)
+func GitMetadataSHA(repositoryDirectory string, isDev bool) (string, error) {
+	if err := ensureGitExecutableIsFound(); err != nil {
+		return "", err
+	}
+	gitStatus := exec.Command("git", "status", "--porcelain")
+	gitStatus.Dir = repositoryDirectory
+	err := gitStatus.Run()
+	if err != nil {
+		if gitStatus.ProcessState.ExitCode() == 1 && isDev {
+			_, _ = fmt.Fprintf(os.Stderr, "WARNING: git working directory has un-commited changes: the variable %q has has development only value %q", MetadataGitSHAVariable, dirtyStateSHAValue)
+			return dirtyStateSHAValue, nil
 		}
-		cache = strings.TrimSpace(out.String())
-		return cache, nil
+		return "", fmt.Errorf("failed to run `%s %s`: %w", gitStatus.Path, strings.Join(gitStatus.Args, " "), err)
+	}
+	return gitHeadRevision(repositoryDirectory)
+}
+
+func gitHeadRevision(repositoryDirectory string) (string, error) {
+	var out bytes.Buffer
+	gitRevParseHead := exec.Command("git", "rev-parse", "HEAD")
+	gitRevParseHead.Dir = repositoryDirectory
+	gitRevParseHead.Stdout = &out
+	err := gitRevParseHead.Run()
+	if err != nil {
+		return "", fmt.Errorf("failed to get HEAD revision hash: %w", err)
+	}
+	return strings.TrimSpace(out.String()), nil
+}
+
+func ensureGitExecutableIsFound() error {
+	if _, err := exec.LookPath("git"); err != nil {
+		return fmt.Errorf("could not calculate %q: %w", MetadataGitSHAVariable, err)
 	}
+	return nil
 }

internal/commands/bake.go

@@ -456,6 +456,11 @@ func (b Bake) Execute(args []string) error {
 		return fmt.Errorf("failed to read metadata: %s", err)
 	}
 
+	gitMetadataSHA, err := builder.GitMetadataSHA(filepath.Dir(b.Options.Kilnfile), b.Options.MetadataOnly || b.Options.StubReleases)
+	if err != nil {
+		return fmt.Errorf("failed to read metadata: %s", err)
+	}
+
 	input := builder.InterpolateInput{
 		KilnVersion:        b.KilnVersion,
 		Version:            b.Options.Version,
@@ -471,7 +476,7 @@ func (b Bake) Execute(args []string) error {
 		PropertyBlueprints: propertyBlueprints,
 		RuntimeConfigs:     runtimeConfigs,
 		StubReleases:       b.Options.StubReleases,
-		MetadataGitSHA:     builder.GitMetadataSHA(filepath.Dir(b.Options.Kilnfile), b.Options.MetadataOnly || b.Options.StubReleases),
+		MetadataGitSHA:     gitMetadataSHA,
 	}
 	interpolatedMetadata, err := b.interpolator.Interpolate(input, b.Options.Metadata, metadata)
 	if err != nil {

internal/commands/bake_test.go

@@ -258,11 +258,14 @@ var _ = Describe("Bake", func() {
 
 			Expect(fakeInterpolator.InterpolateCallCount()).To(Equal(1))
 
+			metadataGitSHA, err := builder.GitMetadataSHA(".", true)
+			Expect(err).NotTo(HaveOccurred())
+
 			input, interpolateName, metadata := fakeInterpolator.InterpolateArgsForCall(0)
-			Expect(input.MetadataGitSHA).NotTo(BeNil())
-			input.MetadataGitSHA = nil // func pointers are not comparable unless they are both nil
+			Expect(input.MetadataGitSHA).NotTo(BeEmpty())
 			Expect(input).To(Equal(builder.InterpolateInput{
-				Version: "1.2.3",
+				MetadataGitSHA: metadataGitSHA,
+				Version:        "1.2.3",
 				BOSHVariables: map[string]any{
 					"some-secret": builder.Metadata{
 						"name": "some-secret",