Add Path field to patch spec to specify file in the patch source

This allows context or other directory based sources to be used as
patches.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

docs/spec.schema.json

@@ -522,6 +522,10 @@
  "strip": {
  "type": "integer",
  "description": "Strip is the number of leading path components to strip from the patch.\nThe default is 1 which is typical of a git diff."
+ },
+ "path": {
+ "type": "string",
+ "description": "Optional subpath to the patch file inside the source\nThis is only useful for directory-backed sources."
  }
  },
  "additionalProperties": false,

frontend/rpm/template.go

@@ -268,34 +268,28 @@ func (w *specWrapper) PrepareSources() (fmt.Stringer, error) {
  fmt.Fprintf(b, "tar -C \"%%{_builddir}/%s\" -xzf \"%%{_sourcedir}/%s.tar.gz\"\n", gomodsName, gomodsName)
  })
 
- for _, name := range keys {
- src := w.Spec.Sources[name]
-
- err := func(name string, src dalec.Source) error {
- if patches[name] {
- // This source is a patch so we don't need to set anything up
- return nil
- }
-
- isDir := dalec.SourceIsDir(src)
-
- if !isDir {
- fmt.Fprintf(b, "cp -a \"%%{_sourcedir}/%s\" .\n", name)
- return nil
- }
-
- fmt.Fprintf(b, "mkdir -p \"%%{_builddir}/%s\"\n", name)
- fmt.Fprintf(b, "tar -C \"%%{_builddir}/%s\" -xzf \"%%{_sourcedir}/%s.tar.gz\"\n", name, name)
-
- for _, patch := range w.Spec.Patches[name] {
- fmt.Fprintf(b, "patch -d %q -p%d -s < \"%%{_sourcedir}/%s\"\n", name, *patch.Strip, patch.Source)
- }
-
- prepareGomods()
- return nil
- }(name, src)
- if err != nil {
- return nil, fmt.Errorf("error preparing source %s: %w", name, err)
+ // Extract all the sources from the rpm source dir
+ for _, key := range keys {
+ if !dalec.SourceIsDir(w.Spec.Sources[key]) {
+ // This is a file, nothing to extract, but we need to put it into place
+ // in the rpm build dir
+ fmt.Fprintf(b, "cp -a \"%%{_sourcedir}/%s\" .\n", key)
+ continue
+ }
+ // This is a directory source so it needs to be untarred into the rpm build dir.
+ fmt.Fprintf(b, "mkdir -p \"%%{_builddir}/%s\"\n", key)
+ fmt.Fprintf(b, "tar -C \"%%{_builddir}/%s\" -xzf \"%%{_sourcedir}/%s.tar.gz\"\n", key, key)
+ }
+ prepareGomods()
+
+ // Apply patches to all sources.
+ // Note: These are applied based on the key sorting algorithm (lexicographic).
+ // Using one patch to patch another patch is not supported, except that it may
+ // occur if they happen to be sorted lexicographically.
+ patchKeys := dalec.SortMapKeys(w.Spec.Patches)
+ for _, key := range patchKeys {
+ for _, patch := range w.Spec.Patches[key] {
+ fmt.Fprintf(b, "patch -d %q -p%d -s --input \"%%{_builddir}/%s\"\n", key, *patch.Strip, filepath.Join(patch.Source, patch.Path))
  }
  }
 

load.go

@@ -407,6 +407,42 @@ func (s Spec) Validate() error {
  }
  }
 
+ var patchErr error
+ for src, patches := range s.Patches {
+ for _, patch := range patches {
+ patchSrc, ok := s.Sources[patch.Source]
+ if !ok {
+ patchErr = goerrors.Join(patchErr, &InvalidPatchError{Source: src, PatchSpec: &patch, Err: errMissingSource})
+ continue
+ }
+
+ if err := validatePatch(patch, patchSrc); err != nil {
+ patchErr = goerrors.Join(patchErr, &InvalidPatchError{Source: src, PatchSpec: &patch, Err: err})
+ }
+ }
+ }
+ if patchErr != nil {
+ return patchErr
+ }
+
+ return nil
+}
+
+func validatePatch(patch PatchSpec, patchSrc Source) error {
+ if SourceIsDir(patchSrc) {
+ // Patch sources that use directory-backed sources require a subpath in the
+ // patch spec.
+ if isRoot(patch.Path) {
+ return errPatchRequiresSubpath
+ }
+ return nil
+ }
+
+ // File backed sources with a subpath in the patch spec is invalid since it is
+ // already a file, not a directory.
+ if !isRoot(patch.Path) {
+ return errPatchFileNoSubpath
+ }
  return nil
 }
 

load_test.go

@@ -603,3 +603,81 @@ func TestSpec_SubstituteBuildArgs(t *testing.T) {
  assert.Check(t, cmp.Equal(spec.Targets["t2"].PackageConfig.Signer.Args["WHATEVER"], argWithDefault))
  assert.Check(t, cmp.Equal(spec.Targets["t2"].PackageConfig.Signer.Args["REGULAR"], plainOleValue))
 }
+
+func Test_validatePatch(t *testing.T) {
+ type testCase struct {
+ name string
+ patchSrc Source
+ subpath bool
+ }
+
+ // Create a test case for each source type.
+ // For each type we need to specify if it should have a subpath or not.
+ cases := []testCase{
+ {
+ name: "ineline file",
+ patchSrc: Source{Inline: &SourceInline{File: &SourceInlineFile{}}},
+ subpath: false,
+ },
+ {
+ name: "inline dir",
+ patchSrc: Source{Inline: &SourceInline{Dir: &SourceInlineDir{}}},
+ subpath: true,
+ },
+ {
+ name: "git",
+ patchSrc: Source{Git: &SourceGit{}},
+ subpath: true,
+ },
+ {
+ name: "image",
+ patchSrc: Source{DockerImage: &SourceDockerImage{}},
+ subpath: true,
+ },
+ {
+ name: "HTTP",
+ patchSrc: Source{HTTP: &SourceHTTP{}},
+ subpath: false,
+ },
+ {
+ name: "context",
+ patchSrc: Source{Context: &SourceContext{}},
+ subpath: true,
+ },
+ {
+ name: "build",
+ patchSrc: Source{Build: &SourceBuild{}},
+ subpath: true,
+ },
+ }
+
+ // For each case generate 2 tests: 1 with a subpath and 1 without
+ // Use the subpath field in the test case to determine if the validation
+ // should return an error.
+ //
+ // If subpath is false in the testcase but the test is passing in a subpath then
+ // an error is expected.
+ // Likewise when subpath is true but no subpath is given.
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ t.Run("subpath=true", func(t *testing.T) {
+ ps := PatchSpec{Path: "/test"}
+ err := validatePatch(ps, tc.patchSrc)
+ if tc.subpath {
+ assert.NilError(t, err)
+ return
+ }
+ assert.ErrorIs(t, err, errPatchFileNoSubpath)
+ })
+ t.Run("subpath=false", func(t *testing.T) {
+ ps := PatchSpec{}
+ err := validatePatch(ps, tc.patchSrc)
+ if tc.subpath {
+ assert.ErrorIs(t, err, errPatchRequiresSubpath)
+ return
+ }
+ assert.NilError(t, err)
+ })
+ })
+ }
+}

source.go

@@ -163,7 +163,27 @@ func (s *InvalidSourceError) Unwrap() error {
  return s.Err
 }
 
-var sourceNamePathSeparatorError = errors.New("source name must not container path separator")
+type InvalidPatchError struct {
+ Source string
+ PatchSpec *PatchSpec
+ Err error
+}
+
+func (s *InvalidPatchError) Error() string {
+ return fmt.Sprintf("invalid patch for source %q, patch source: %q: %v", s.Source, s.PatchSpec.Source, s.Err)
+}
+
+func (s *InvalidPatchError) Unwrap() error {
+ return s.Err
+}
+
+var (
+ sourceNamePathSeparatorError = errors.New("source name must not contain path separator")
+ errMissingSource = errors.New("source is missing from sources list")
+
+ errPatchRequiresSubpath = errors.New("patch source refers to a directory source without a subpath to the patch file to use")
+ errPatchFileNoSubpath = errors.New("patch source refers to a file source but patch spec specifies a subpath")
+)
 
 type LLBGetter func(sOpts SourceOpts, opts ...llb.ConstraintsOpt) (llb.State, error)
 
@@ -508,7 +528,7 @@ func patchSource(worker, sourceState llb.State, sourceToState map[string]llb.Sta
  // on each iteration, mount source state to /src to run `patch`, and
  // set the state under /src to be the source state for the next iteration
  sourceState = worker.Run(
- llb.AddMount("/patch", patchState, llb.Readonly, llb.SourcePath(p.Source)),
+ llb.AddMount("/patch", patchState, llb.Readonly, llb.SourcePath(filepath.Join(p.Source, p.Path))),
  llb.Dir("src"),
  ShArgs(fmt.Sprintf("patch -p%d < /patch", *p.Strip)),
  WithConstraints(opts...),

spec.go

@@ -107,6 +107,9 @@ type PatchSpec struct {
  // Strip is the number of leading path components to strip from the patch.
  // The default is 1 which is typical of a git diff.
  Strip *int `yaml:"strip,omitempty" json:"strip,omitempty"`
+ // Optional subpath to the patch file inside the source
+ // This is only useful for directory-backed sources.
+ Path string `yaml:"path,omitempty" json:"path,omitempty"`
 }
 
 // ChangelogEntry is an entry in the changelog.

test/azlinux_test.go

@@ -10,6 +10,7 @@ import (
 
  "github.com/Azure/dalec"
  "github.com/Azure/dalec/test/testenv"
+ "github.com/moby/buildkit/client/llb"
  gwclient "github.com/moby/buildkit/frontend/gateway/client"
  moby_buildkit_v1_frontend "github.com/moby/buildkit/frontend/gateway/pb"
 )
@@ -128,6 +129,23 @@ func testLinuxDistro(ctx context.Context, t *testing.T, testConfig testLinuxConf
  })
 
  t.Run("container", func(t *testing.T) {
+ const src2Patch3File = "patch3"
+ src2Patch3Content := []byte(`
+diff --git a/file3 b/file3
+new file mode 100700
+index 0000000..5260cb1
+--- /dev/null
++++ b/file3
+@@ -0,0 +1,3 @@
++#!/usr/bin/env bash
++
++echo "Added another new file"
+`)
+ src2Patch3Context := llb.Scratch().File(
+ llb.Mkfile(src2Patch3File, 0o600, src2Patch3Content),
+ )
+ src2Patch3ContextName := "patch-context"
+
  spec := dalec.Spec{
  Name: "test-container-build",
  Version: "0.0.1",
@@ -172,8 +190,10 @@ index 84d55c5..22b9b11 100644
  },
  "src2-patch2": {
  Inline: &dalec.SourceInline{
- File: &dalec.SourceInlineFile{
- Contents: `
+ Dir: &dalec.SourceInlineDir{
+ Files: map[string]*dalec.SourceInlineFile{
+ "the-patch": {
+ Contents: `
 diff --git a/file2 b/file2
 new file mode 100700
 index 0000000..5260cb1
@@ -184,14 +204,22 @@ index 0000000..5260cb1
 +
 +echo "Added a new file"
 `,
+ },
+ },
  },
  },
  },
+ "src2-patch3": {
+ Context: &dalec.SourceContext{
+ Name: src2Patch3ContextName,
+ },
+ },
  },
  Patches: map[string][]dalec.PatchSpec{
  "src2": {
  {Source: "src2-patch1"},
- {Source: "src2-patch2"},
+ {Source: "src2-patch2", Path: "the-patch"},
+ {Source: "src2-patch3", Path: src2Patch3File},
  },
  },
 
@@ -234,6 +262,17 @@ index 0000000..5260cb1
  {
  Command: "grep 'Added a new file' ./src2/file2",
  },
+ {
+ // file added by patch
+ Command: "test -f ./src2/file3",
+ },
+ {
+ // file added by patch
+ Command: "test -x ./src2/file3",
+ },
+ {
+ Command: "grep 'Added another new file' ./src2/file3",
+ },
  {
  // Test that a multiline command works with env vars
  Env: map[string]string{
@@ -308,7 +347,11 @@ echo "$BAR" > bar.txt
  }
 
  testEnv.RunTest(ctx, t, func(ctx context.Context, gwc gwclient.Client) {
- sr := newSolveRequest(withSpec(ctx, t, &spec), withBuildTarget(testConfig.BuildTarget))
+ sr := newSolveRequest(
+ withSpec(ctx, t, &spec),
+ withBuildTarget(testConfig.BuildTarget),
+ withBuildContext(ctx, t, src2Patch3ContextName, src2Patch3Context),
+ )
  sr.Evaluate = true
 
  solveT(ctx, t, gwc, sr)
@@ -322,8 +365,8 @@ echo "$BAR" > bar.txt
  },
  })
 
- sr = newSolveRequest(withSpec(ctx, t, &spec), withBuildTarget(testConfig.BuildTarget))
- sr.Evaluate = true
+ // update the spec in the solve reuqest
+ withSpec(ctx, t, &spec)(&sr)
 
  if _, err := gwc.Solve(ctx, sr); err == nil {
  t.Fatal("expected test spec to run with error but got none")

test/helpers_test.go

@@ -229,3 +229,22 @@ func solveT(ctx context.Context, t *testing.T, gwc gwclient.Client, req gwclient
  }
  return res
 }
+
+func withBuildContext(ctx context.Context, t *testing.T, name string, st llb.State) srOpt {
+ return func(sr *gwclient.SolveRequest) {
+ if sr.FrontendOpt == nil {
+ sr.FrontendOpt = make(map[string]string)
+ }
+ if sr.FrontendInputs == nil {
+ sr.FrontendInputs = make(map[string]*pb.Definition)
+ }
+
+ def, err := st.Marshal(ctx)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ sr.FrontendOpt["context:"+name] = "input:" + name
+ sr.FrontendInputs[name] = def.ToPB()
+ }
+}