deb: Add prepend/append for paths instead of replacing full $PATH

This makes the build a bit more friendly to building outside of DALEC
where we don't mess with *their* $PATH except by adding the paths we
need instead of replacing the whole thing.

Signed-off-by: Brian Goff <[email protected]>

Author: cpuguy83

frontend/deb/debroot.go

@@ -65,6 +65,25 @@ func sourcePatchesDir(sOpt dalec.SourceOpts, base llb.State, dir, name string, s
 	return append(states, series), nil
 }
 
+type SourcePkgConfig struct {
+	// PrependPath is a list of paths to be prepended to the $PATH var in build
+	// scripts.
+	PrependPath []string
+	// APpendPath is a list of paths to be appended to the $PATH var in build
+	// scripts.
+	AppendPath []string
+}
+
+// Addpath creates a SourcePkgConfig where the first argument is sets
+// [SourcePkgConfig.PrependPath] and the 2nd argument sets
+// [SourcePkgConfig.AppendPath]
+func AddPath(pre, post []string) SourcePkgConfig {
+	return SourcePkgConfig{
+		PrependPath: pre,
+		AppendPath:  post,
+	}
+}
+
 // Debroot creates a debian root directory suitable for use with debbuild.
 // This does not include sources in case you want to mount sources (instead of copying them) later.
 //
@@ -75,7 +94,7 @@ func sourcePatchesDir(sOpt dalec.SourceOpts, base llb.State, dir, name string, s
 // an upgrade even if it is technically the same underlying source.
 // It may be left blank but is highly recommended to set this.
 // Use [ReadDistroVersionID] to get a suitable value.
-func Debroot(ctx context.Context, sOpt dalec.SourceOpts, spec *dalec.Spec, worker, in llb.State, target, dir, distroVersionID string, opts ...llb.ConstraintsOpt) (llb.State, error) {
+func Debroot(ctx context.Context, sOpt dalec.SourceOpts, spec *dalec.Spec, worker, in llb.State, target, dir, distroVersionID string, cfg SourcePkgConfig, opts ...llb.ConstraintsOpt) (llb.State, error) {
 	control, err := controlFile(spec, in, target, dir)
 	if err != nil {
 		return llb.Scratch(), errors.Wrap(err, "error generating control file")
@@ -122,15 +141,10 @@ func Debroot(ctx context.Context, sOpt dalec.SourceOpts, spec *dalec.Spec, worke
 	dalecDir := base.
 		File(llb.Mkdir(filepath.Join(dir, "dalec"), 0o755), opts...)
 
-	pathVar, _, err := worker.GetEnv(ctx, "PATH", opts...)
-	if err != nil {
-		return in, fmt.Errorf("error looking up $PATH in worker image: %w", err)
-	}
-
-	states = append(states, dalecDir.File(llb.Mkfile(filepath.Join(dir, "dalec/build.sh"), 0o700, createBuildScript(spec, pathVar)), opts...))
-	states = append(states, dalecDir.File(llb.Mkfile(filepath.Join(dir, "dalec/patch.sh"), 0o700, createPatchScript(spec, pathVar)), opts...))
-	states = append(states, dalecDir.File(llb.Mkfile(filepath.Join(dir, "dalec/fix_sources.sh"), 0o700, fixupSources(spec, pathVar)), opts...))
-	states = append(states, dalecDir.File(llb.Mkfile(filepath.Join(dir, "dalec/fix_perms.sh"), 0o700, fixupArtifactPerms(spec, pathVar)), opts...))
+	states = append(states, dalecDir.File(llb.Mkfile(filepath.Join(dir, "dalec/build.sh"), 0o700, createBuildScript(spec, &cfg)), opts...))
+	states = append(states, dalecDir.File(llb.Mkfile(filepath.Join(dir, "dalec/patch.sh"), 0o700, createPatchScript(spec, &cfg)), opts...))
+	states = append(states, dalecDir.File(llb.Mkfile(filepath.Join(dir, "dalec/fix_sources.sh"), 0o700, fixupSources(spec, &cfg)), opts...))
+	states = append(states, dalecDir.File(llb.Mkfile(filepath.Join(dir, "dalec/fix_perms.sh"), 0o700, fixupArtifactPerms(spec, &cfg)), opts...))
 
 	customEnable, err := customDHInstallSystemdPostinst(spec)
 	if err != nil {
@@ -166,9 +180,9 @@ func Debroot(ctx context.Context, sOpt dalec.SourceOpts, spec *dalec.Spec, worke
 	return dalec.MergeAtPath(in, states, "/"), nil
 }
 
-func fixupArtifactPerms(spec *dalec.Spec, pathVar string) []byte {
+func fixupArtifactPerms(spec *dalec.Spec, cfg *SourcePkgConfig) []byte {
 	buf := bytes.NewBuffer(nil)
-	writeScriptHeader(buf, pathVar)
+	writeScriptHeader(buf, cfg)
 
 	basePath := filepath.Join("debian", spec.Name)
 
@@ -203,9 +217,9 @@ func fixupArtifactPerms(spec *dalec.Spec, pathVar string) []byte {
 //     to bring those back.
 //
 // This is called from `debian/rules` after the source tarball has been extracted.
-func fixupSources(spec *dalec.Spec, pathVar string) []byte {
+func fixupSources(spec *dalec.Spec, cfg *SourcePkgConfig) []byte {
 	buf := bytes.NewBuffer(nil)
-	writeScriptHeader(buf, pathVar)
+	writeScriptHeader(buf, cfg)
 
 	// now, we need to find all the sources that are file-backed and fix them up
 	for name, src := range spec.Sources {
@@ -229,29 +243,41 @@ func fixupSources(spec *dalec.Spec, pathVar string) []byte {
 		// Older go versions did not have support for the `GOMODCACHE` var
 		// This is a hack to try and make the build work by linking the go modules
 		// we've already fetched into to module dir under $GOPATH
-		fmt.Fprintf(buf, `test -n "$(go env GOMODCACHE)" || (GOPATH="$(go env GOPATH)"; mkdir -p "${GOPATH}/pkg" && ln -s "$(pwd)/%s" "${GOPATH}/pkg/mod")`, gomodsName)
+		fmt.Fprintf(buf, `env | grep PATH; test -n "$(go env GOMODCACHE)" || (GOPATH="$(go env GOPATH)"; mkdir -p "${GOPATH}/pkg" && ln -s "$(pwd)/%s" "${GOPATH}/pkg/mod")`, gomodsName)
 		// Above command does not have a newline due to quoting issues, so add that here.
 		fmt.Fprint(buf, "\n")
 	}
 
 	return buf.Bytes()
 }
 
-func writeScriptHeader(buf io.Writer, pathVar string) {
+func setupPathVar(pre, post []string) string {
+	if len(pre) == 0 && len(post) == 0 {
+		return ""
+	}
+
+	full := append(pre, "$PATH")
+	full = append(full, post...)
+	return strings.Join(full, ":")
+}
+
+func writeScriptHeader(buf io.Writer, cfg *SourcePkgConfig) {
 	fmt.Fprintln(buf, "#!/usr/bin/env sh")
 	fmt.Fprintln(buf)
 
 	fmt.Fprintln(buf, "set -ex")
 
-	if pathVar != "" {
-		fmt.Fprintln(buf, `export PATH="`+pathVar+`"`)
+	if cfg != nil {
+		if pathVar := setupPathVar(cfg.PrependPath, cfg.AppendPath); pathVar != "" {
+			fmt.Fprintln(buf, "export PATH="+pathVar)
+		}
 	}
 }
 
-func createPatchScript(spec *dalec.Spec, pathVar string) []byte {
+func createPatchScript(spec *dalec.Spec, cfg *SourcePkgConfig) []byte {
 	buf := bytes.NewBuffer(nil)
 
-	writeScriptHeader(buf, pathVar)
+	writeScriptHeader(buf, cfg)
 
 	for name, patches := range spec.Patches {
 		for _, patch := range patches {
@@ -263,9 +289,9 @@ func createPatchScript(spec *dalec.Spec, pathVar string) []byte {
 	return buf.Bytes()
 }
 
-func createBuildScript(spec *dalec.Spec, pathVar string) []byte {
+func createBuildScript(spec *dalec.Spec, cfg *SourcePkgConfig) []byte {
 	buf := bytes.NewBuffer(nil)
-	writeScriptHeader(buf, pathVar)
+	writeScriptHeader(buf, cfg)
 
 	sorted := dalec.SortMapKeys(spec.Build.Env)
 	for _, k := range sorted {

frontend/deb/distro/install.go

@@ -134,7 +134,7 @@ func (d *Config) InstallBuildDeps(sOpt dalec.SourceOpts, spec *dalec.Spec, targe
 			opts := append(opts, dalec.ProgressGroup("Install build dependencies"))
 			opts = append([]llb.ConstraintsOpt{dalec.WithConstraint(c)}, opts...)
 
-			srcPkg, err := deb.SourcePackage(ctx, sOpt, in, depsSpec, targetKey, "", opts...)
+			srcPkg, err := deb.SourcePackage(ctx, sOpt, in, depsSpec, targetKey, "", deb.SourcePkgConfig{}, opts...)
 			if err != nil {
 				return in, err
 			}

frontend/deb/distro/pkg.go

@@ -30,7 +30,14 @@ func (d *Config) BuildDeb(ctx context.Context, worker llb.State, sOpt dalec.Sour
 	}
 
 	worker = worker.With(d.InstallBuildDeps(sOpt, spec, targetKey))
-	srcPkg, err := deb.SourcePackage(ctx, sOpt, worker.With(ensureGolang(client, spec, targetKey, opts...)), spec, targetKey, versionID, opts...)
+
+	var cfg deb.SourcePkgConfig
+	extraPaths, err := prepareGo(ctx, client, &cfg, worker, spec, targetKey, opts...)
+	if err != nil {
+		return worker, err
+	}
+
+	srcPkg, err := deb.SourcePackage(ctx, sOpt, worker.With(extraPaths), spec, targetKey, versionID, cfg, opts...)
 	if err != nil {
 		return worker, err
 	}
@@ -44,60 +51,83 @@ func (d *Config) BuildDeb(ctx context.Context, worker llb.State, sOpt dalec.Sour
 	return frontend.MaybeSign(ctx, client, st, spec, targetKey, sOpt)
 }
 
-// ensureGolang is a work-around for the case where the base distro golang package
-// is too old, but other packages are provided (e.g. `golang-1.22`) and those
-// other packages don't actually add go tools to $PATH.
-// It assumes if you added one of these go packages and there is no `go` in $PATH
-// that you probably wanted to use that version of go.
-func ensureGolang(client gwclient.Client, spec *dalec.Spec, targetKey string, opts ...llb.ConstraintsOpt) llb.StateOption {
-	return func(in llb.State) llb.State {
-		deps := spec.GetBuildDeps(targetKey)
-		if _, hasNormalGo := deps["golang"]; hasNormalGo {
-			return in
+func noOpStateOpt(in llb.State) llb.State {
+	return in
+}
+
+func prepareGo(ctx context.Context, client gwclient.Client, cfg *deb.SourcePkgConfig, worker llb.State, spec *dalec.Spec, targetKey string, opts ...llb.ConstraintsOpt) (llb.StateOption, error) {
+	goBin, err := searchForAltGolang(ctx, client, spec, targetKey, worker, opts...)
+	if err != nil {
+		return noOpStateOpt, errors.Wrap(err, "error while looking for alternate go bin path")
+	}
+
+	if goBin == "" {
+		return noOpStateOpt, nil
+	}
+	cfg.PrependPath = append(cfg.PrependPath, goBin)
+	return addPaths([]string{goBin}, opts...), nil
+}
+
+func searchForAltGolang(ctx context.Context, client gwclient.Client, spec *dalec.Spec, targetKey string, in llb.State, opts ...llb.ConstraintsOpt) (string, error) {
+	if !spec.HasGomods() {
+		return "", nil
+	}
+	var candidates []string
+
+	deps := spec.GetBuildDeps(targetKey)
+	if _, hasNormalGo := deps["golang"]; hasNormalGo {
+		return "", nil
+	}
+
+	for dep := range deps {
+		if strings.HasPrefix(dep, "golang-") {
+			// Get the base version component
+			_, ver, _ := strings.Cut(dep, "-")
+			// Trim off any potential extra stuff like `golang-1.20-go` (ie the `-go` bit)
+			// This is just for having definitive search paths to check it should
+			// not be an issue if this is not like the above example and its
+			// something else like `-doc` since we are still going to check the
+			// binary exists anyway (plus this would be highly unlikely in any case).
+			ver, _, _ = strings.Cut(ver, "-")
+			candidates = append(candidates, "usr/lib/go-"+ver+"/bin")
 		}
+	}
 
-		return in.Async(func(ctx context.Context, in llb.State, c *llb.Constraints) (llb.State, error) {
-			var candidates []string
-			for dep := range deps {
-				if strings.HasPrefix(dep, "golang-") {
-					// Get the base version component
-					_, ver, _ := strings.Cut(dep, "-")
-					// Trim off any potential extra stuff like `golang-1.20-go` (ie the `-go` bit)
-					// This is just for having definitive search paths to check it should
-					// not be an issue if this is not like the above example and its
-					// something else like `-doc` since we are still going to check the
-					// binary exists anyway (plus this would be highly unlikely in any case).
-					ver, _, _ = strings.Cut(ver, "-")
-					candidates = append(candidates, "usr/lib/go-"+ver+"/bin")
-				}
-			}
+	if len(candidates) == 0 {
+		return "", nil
+	}
 
-			if len(candidates) == 0 {
-				return in, nil
-			}
+	stfs, err := bkfs.FromState(ctx, &in, client, opts...)
+	if err != nil {
+		return "", err
+	}
 
-			opts := []llb.ConstraintsOpt{dalec.WithConstraint(c), dalec.WithConstraints(opts...)}
+	for _, p := range candidates {
+		_, err := fs.Stat(stfs, filepath.Join(p, "go"))
+		if err == nil {
+			// bkfs does not allow a leading `/` in the stat path per spec for [fs.FS]
+			// Add that in here
+			p := "/" + p
+			return p, nil
+		}
+	}
 
-			pathVar, _, err := in.GetEnv(ctx, "PATH", opts...)
-			if err != nil {
-				return in, err
-			}
+	return "", nil
+}
 
-			stfs, err := bkfs.FromState(ctx, &in, client, opts...)
+// prepends the provided values to $PATH
+func addPaths(paths []string, opts ...llb.ConstraintsOpt) llb.StateOption {
+	return func(in llb.State) llb.State {
+		if len(paths) == 0 {
+			return in
+		}
+		return in.Async(func(ctx context.Context, in llb.State, c *llb.Constraints) (llb.State, error) {
+			opts := []llb.ConstraintsOpt{dalec.WithConstraint(c), dalec.WithConstraints(opts...)}
+			pathEnv, _, err := in.GetEnv(ctx, "PATH", opts...)
 			if err != nil {
 				return in, err
 			}
-
-			for _, p := range candidates {
-				_, err := fs.Stat(stfs, filepath.Join(p, "go"))
-				if err == nil {
-					// bkfs does not allow a leading `/` in the stat path per spec for [fs.FS]
-					// Add that in here
-					p := "/" + p
-					return in.AddEnv("PATH", p+":"+pathVar), nil
-				}
-			}
-			return in, nil
+			return in.AddEnv("PATH", strings.Join(append(paths, pathEnv), ":")), nil
 		})
 	}
 }
@@ -203,7 +233,14 @@ func (cfg *Config) HandleSourcePkg(ctx context.Context, client gwclient.Client)
 		}
 
 		worker = worker.With(cfg.InstallBuildDeps(sOpt, spec, targetKey, pg))
-		st, err := deb.SourcePackage(ctx, sOpt, worker.With(ensureGolang(client, spec, targetKey, pg)), spec, targetKey, versionID, pg)
+
+		var cfg deb.SourcePkgConfig
+		extraPaths, err := prepareGo(ctx, client, &cfg, worker, spec, targetKey, pg)
+		if err != nil {
+			return nil, nil, err
+		}
+
+		st, err := deb.SourcePackage(ctx, sOpt, worker.With(extraPaths), spec, targetKey, versionID, cfg, pg)
 		if err != nil {
 			return nil, nil, errors.Wrap(err, "error building source package")
 		}

frontend/deb/pkg.go

@@ -71,11 +71,11 @@ func createPatches(spec *dalec.Spec, sources map[string]llb.State, worker llb.St
 	return patches
 }
 
-func SourcePackage(ctx context.Context, sOpt dalec.SourceOpts, worker llb.State, spec *dalec.Spec, targetKey, distroVersionID string, opts ...llb.ConstraintsOpt) (llb.State, error) {
+func SourcePackage(ctx context.Context, sOpt dalec.SourceOpts, worker llb.State, spec *dalec.Spec, targetKey, distroVersionID string, cfg SourcePkgConfig, opts ...llb.ConstraintsOpt) (llb.State, error) {
 	if err := validateSpec(spec); err != nil {
 		return llb.Scratch(), err
 	}
-	dr, err := Debroot(ctx, sOpt, spec, worker, llb.Scratch(), targetKey, "", distroVersionID)
+	dr, err := Debroot(ctx, sOpt, spec, worker, llb.Scratch(), targetKey, "", distroVersionID, cfg, opts...)
 	if err != nil {
 		return llb.Scratch(), err
 	}