Improve comment stripping errors

Before, these looked like:

compiling main pipelines: compiling Pipeline[0]: stripping runs comments: 14:13: not a valid test operator: -m

Now, these look like:

compiling "hwloc" test pipelines: compiling Pipeline[0]: stripping runs comments: 14:13: not a valid test operator: -m:
> if [[ uname -m == 'x86_64']]; then
              ^

First big change is that we include the package name in the error.
Without this, it was really challenging to find which package actually
had a syntax error.

Second big change is that we use the nice structured error we get back
from the shell parsing library to include the line that failed with a
cursor pointing at the column that failed.

Signed-off-by: Jon Johnson <jon.johnson@chainguard.dev>

pkg/build/build.go

@@ -207,9 +207,6 @@ func New(ctx context.Context, opts ...Option) (*Build, error) {
 	if b.ConfigFileRepositoryCommit == "" {
 		return nil, fmt.Errorf("config file repository commit was not set")
 	}
-	if b.Runner == nil {
-		return nil, fmt.Errorf("no runner was specified")
-	}
 
 	parsedCfg, err := config.ParseConfiguration(ctx,
 		b.ConfigFile,
@@ -717,6 +714,10 @@ func (b *Build) BuildPackage(ctx context.Context) error {
 	ctx, span := otel.Tracer("melange").Start(ctx, "BuildPackage")
 	defer span.End()
 
+	if b.Runner == nil {
+		return fmt.Errorf("no runner was specified")
+	}
+
 	b.summarize(ctx)
 
 	namespace := b.Namespace
@@ -779,7 +780,7 @@ func (b *Build) BuildPackage(ctx context.Context) error {
 
 	log.Infof("evaluating pipelines for package requirements")
 	if err := b.Compile(ctx); err != nil {
-		return fmt.Errorf("compiling build: %w", err)
+		return fmt.Errorf("compiling %s: %w", b.ConfigFile, err)
 	}
 
 	// Filter out any subpackages with false If conditions.

pkg/build/compile.go

@@ -50,7 +50,7 @@ func (t *Test) Compile(ctx context.Context) error {
 
 	// We want to evaluate this but not accumulate its deps.
 	if err := ignore.CompilePipelines(ctx, sm, cfg.Pipeline); err != nil {
-		return fmt.Errorf("compiling main pipelines: %w", err)
+		return fmt.Errorf("compiling package %q pipelines: %w", t.Package, err)
 	}
 
 	for i, sp := range cfg.Subpackages {
@@ -103,7 +103,7 @@ func (t *Test) Compile(ctx context.Context) error {
 		}
 
 		if err := test.CompilePipelines(ctx, sm, cfg.Test.Pipeline); err != nil {
-			return fmt.Errorf("compiling main test pipelines: %w", err)
+			return fmt.Errorf("compiling %q test pipelines: %w", t.Package, err)
 		}
 
 		// Append anything the main package test needs.
@@ -126,7 +126,7 @@ func (b *Build) Compile(ctx context.Context) error {
 	}
 
 	if err := c.CompilePipelines(ctx, sm, cfg.Pipeline); err != nil {
-		return fmt.Errorf("compiling main pipelines: %w", err)
+		return fmt.Errorf("compiling %q pipelines: %w", cfg.Package.Name, err)
 	}
 
 	for i, sp := range cfg.Subpackages {
@@ -135,7 +135,7 @@ func (b *Build) Compile(ctx context.Context) error {
 		if sp.If != "" {
 			sp.If, err = util.MutateAndQuoteStringFromMap(sm.Substitutions, sp.If)
 			if err != nil {
-				return fmt.Errorf("mutating subpackage if: %w", err)
+				return fmt.Errorf("mutating subpackage %q, if: %w", sp.Name, err)
 			}
 		}
 
@@ -172,7 +172,7 @@ func (b *Build) Compile(ctx context.Context) error {
 		}
 
 		if err := tc.CompilePipelines(ctx, sm, cfg.Test.Pipeline); err != nil {
-			return fmt.Errorf("compiling main test pipelines: %w", err)
+			return fmt.Errorf("compiling %q test pipelines: %w", cfg.Package.Name, err)
 		}
 
 		te := &b.Configuration.Test.Environment.Contents
@@ -370,6 +370,27 @@ func (c *Compiled) gatherDeps(ctx context.Context, pipeline *config.Pipeline) er
 	return nil
 }
 
+func maybeIncludeSyntaxError(runs string, err error) error {
+	var perr syntax.ParseError
+	if !errors.As(err, &perr) {
+		return err
+	}
+
+	line := perr.Pos.Line()
+	lines := strings.Split(runs, "\n")
+	if line <= 0 || line > uint(len(lines)) {
+		return err
+	}
+
+	padding := len("> ") + int(perr.Pos.Col())
+
+	// For example...
+	// 14:13: not a valid test operator: -m
+	// > if [[ uname -m == 'x86_64']]; then
+	//               ^
+	return fmt.Errorf("%w:\n> %s\n%*s", err, lines[line-1], padding, "^")
+}
+
 func stripComments(runs string) (string, error) {
 	parser := syntax.NewParser(syntax.KeepComments(false))
 	printer := syntax.NewPrinter()
@@ -391,7 +412,7 @@ func stripComments(runs string) (string, error) {
 		builder.WriteRune('\n')
 		return perr == nil
 	}); err != nil || perr != nil {
-		return "", errors.Join(err, perr)
+		return "", maybeIncludeSyntaxError(runs, errors.Join(err, perr))
 	}
 
 	return builder.String(), nil

pkg/build/compile_test.go

@@ -147,4 +147,15 @@ func Test_stripComments(t *testing.T) {
 			}
 		})
 	}
+
+	wantErr := `1:13: not a valid test operator: -m:
+> if [[ uname -m == 'x86_64']]; then
+              ^`
+
+	got, err := stripComments("if [[ uname -m == 'x86_64']]; then")
+	if err == nil {
+		t.Errorf("expected error, got %q", got)
+	} else if err.Error() != wantErr {
+		t.Errorf("want:\n%s\ngot:\n%s", wantErr, err)
+	}
 }

pkg/build/test.go

@@ -343,7 +343,7 @@ func (t *Test) TestPackage(ctx context.Context) error {
 
 	log.Infof("evaluating pipelines for package requirements")
 	if err := t.Compile(ctx); err != nil {
-		return fmt.Errorf("compiling test pipelines: %w", err)
+		return fmt.Errorf("compiling %s tests: %w", t.ConfigFile, err)
 	}
 
 	if t.Runner.Name() == container.QemuName {

pkg/cli/compile.go

@@ -25,6 +25,7 @@ import (
 
 	apko_types "chainguard.dev/apko/pkg/build/types"
 	"chainguard.dev/melange/pkg/build"
+	"github.com/chainguard-dev/clog"
 	"github.com/spf13/cobra"
 	"go.opentelemetry.io/otel"
 )
@@ -63,6 +64,9 @@ func compile() *cobra.Command {
 	var cpu, memory string
 	var timeout time.Duration
 	var extraPackages []string
+	var configFileGitCommit string
+	var configFileGitRepoURL string
+	var configFileLicense string
 
 	cmd := &cobra.Command{
 		Use:     "compile",
@@ -72,6 +76,31 @@ func compile() *cobra.Command {
 		Args:    cobra.MinimumNArgs(0),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			ctx := cmd.Context()
+			log := clog.FromContext(ctx)
+
+			var buildConfigFilePath string
+			if len(args) > 0 {
+				buildConfigFilePath = args[0] // e.g. "crane.yaml"
+			}
+
+			// Favor explicit, user-provided information for the git provenance of the
+			// melange build definition. As a fallback, detect this from local git state.
+			// Git auto-detection should be "best effort" and not fail the build if it
+			// fails.
+			if configFileGitCommit == "" {
+				log.Infof("git commit for build config not provided, attempting to detect automatically")
+				commit, err := detectGitHead(ctx, buildConfigFilePath)
+				if err != nil {
+					log.Warnf("unable to detect commit for build config file: %v", err)
+					configFileGitCommit = "unknown"
+				} else {
+					configFileGitCommit = commit
+				}
+			}
+			if configFileGitRepoURL == "" {
+				log.Warnf("git repository URL for build config not provided")
+				configFileGitRepoURL = "https://unknown/unknown/unknown"
+			}
 
 			arch := apko_types.ParseArchitecture(archstr)
 			options := []build.Option{
@@ -108,6 +137,9 @@ func compile() *cobra.Command {
 				build.WithCPU(cpu),
 				build.WithMemory(memory),
 				build.WithTimeout(timeout),
+				build.WithConfigFileRepositoryCommit(configFileGitCommit),
+				build.WithConfigFileRepositoryURL(configFileGitRepoURL),
+				build.WithConfigFileLicense(configFileLicense),
 			}
 
 			if len(args) > 0 {
@@ -176,6 +208,10 @@ func compile() *cobra.Command {
 	cmd.Flags().StringVar(&memory, "memory", "", "default memory resources to use for builds")
 	cmd.Flags().DurationVar(&timeout, "timeout", 0, "default timeout for builds")
 
+	cmd.Flags().StringVar(&configFileGitCommit, "git-commit", "", "commit hash of the git repository containing the build config file (defaults to detecting HEAD)")
+	cmd.Flags().StringVar(&configFileGitRepoURL, "git-repo-url", "", "URL of the git repository containing the build config file (defaults to detecting from configured git remotes)")
+	cmd.Flags().StringVar(&configFileLicense, "license", "NOASSERTION", "license to use for the build config file itself")
+
 	return cmd
 }
 
@@ -191,7 +227,7 @@ func CompileCmd(ctx context.Context, opts ...build.Option) error {
 	defer bc.Close(ctx)
 
 	if err := bc.Compile(ctx); err != nil {
-		return fmt.Errorf("failed to compile package: %w", err)
+		return fmt.Errorf("failed to compile %s: %w", bc.ConfigFile, err)
 	}
 
 	return json.NewEncoder(os.Stdout).Encode(bc.Configuration)