From 72d32a248b06d1f1b67f31fa327c5656921c4faa Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Wed, 11 Sep 2024 15:56:20 +0200 Subject: [PATCH] flux diff artifact: Compute a unified diff internally by default. Signed-off-by: Florian Forster --- cmd/flux/diff_artifact.go | 169 ++++++++++++++++++++++++++++++-------- go.mod | 2 + go.sum | 4 + 3 files changed, 139 insertions(+), 36 deletions(-) diff --git a/cmd/flux/diff_artifact.go b/cmd/flux/diff_artifact.go index d5f94aab..b19849aa 100644 --- a/cmd/flux/diff_artifact.go +++ b/cmd/flux/diff_artifact.go @@ -22,17 +22,22 @@ import ( "errors" "fmt" "io" + "io/fs" "os" "os/exec" "path/filepath" "sort" "strings" + "bitbucket.org/creachadair/stringset" oci "github.com/fluxcd/pkg/oci/client" "github.com/fluxcd/pkg/tar" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" "github.com/gonvenience/ytbx" "github.com/google/shlex" + "github.com/hexops/gotextdiff" + "github.com/hexops/gotextdiff/myers" + "github.com/hexops/gotextdiff/span" "github.com/homeport/dyff/pkg/dyff" "github.com/spf13/cobra" "golang.org/x/exp/maps" @@ -61,29 +66,33 @@ type diffArtifactFlags struct { provider flags.SourceOCIProvider ignorePaths []string brief bool - differ *semanticDiffFlag + differ *differFlag } var diffArtifactArgs = newDiffArtifactArgs() func newDiffArtifactArgs() diffArtifactFlags { - defaultDiffer := mustExternalDiff() - return diffArtifactFlags{ provider: flags.SourceOCIProvider(sourcev1.GenericOCIProvider), - differ: &semanticDiffFlag{ + differ: &differFlag{ options: map[string]differ{ - "yaml": dyffBuiltin{ + "dyff": dyffBuiltin{ opts: []dyff.CompareOption{ dyff.IgnoreOrderChanges(false), dyff.KubernetesEntityDetection(true), }, }, - "false": defaultDiffer, + "external": externalDiff{}, + "unified": unifiedDiff{}, + }, + description: map[string]string{ + "dyff": `semantic diff for YAML inputs`, + "external": `execute the command in the "` + externalDiffVar + `" environment variable`, + "unified": "generic unified diff for arbitrary text inputs", }, - value: "false", - differ: defaultDiffer, + value: "unified", + differ: unifiedDiff{}, }, } } @@ -94,7 +103,7 @@ func init() { diffArtifactCmd.Flags().Var(&diffArtifactArgs.provider, "provider", sourceOCIRepositoryArgs.provider.Description()) diffArtifactCmd.Flags().StringSliceVar(&diffArtifactArgs.ignorePaths, "ignore-paths", excludeOCI, "set paths to ignore in .gitignore format") diffArtifactCmd.Flags().BoolVarP(&diffArtifactArgs.brief, "brief", "q", false, "just print a line when the resources differ; does not output a list of changes") - diffArtifactCmd.Flags().Var(diffArtifactArgs.differ, "semantic-diff", "use a semantic diffing algorithm") + diffArtifactCmd.Flags().Var(diffArtifactArgs.differ, "differ", diffArtifactArgs.differ.usage()) diffCmd.AddCommand(diffArtifactCmd) } @@ -297,53 +306,125 @@ type differ interface { Diff(ctx context.Context, from, to string) (string, error) } -// externalDiffCommand implements the differ interface using an external diff command. -type externalDiffCommand struct { - name string - flags []string +type unifiedDiff struct{} + +func (d unifiedDiff) Diff(_ context.Context, fromDir, toDir string) (string, error) { + fromFiles, err := filesInDir(fromDir) + if err != nil { + return "", err + } + + toFiles, err := filesInDir(toDir) + if err != nil { + return "", err + } + + allFiles := fromFiles.Union(toFiles) + + var sb strings.Builder + + for _, relPath := range allFiles.Elements() { + diff, err := d.diffFiles(fromDir, toDir, relPath) + if err != nil { + return "", err + } + + fmt.Fprint(&sb, diff) + } + + return sb.String(), nil +} + +func (d unifiedDiff) diffFiles(fromDir, toDir, relPath string) (string, error) { + fromPath := filepath.Join(fromDir, relPath) + fromData, err := d.readFile(fromPath) + if err != nil { + return "", fmt.Errorf("readFile(%q): %w", fromPath, err) + } + + toPath := filepath.Join(toDir, relPath) + toData, err := d.readFile(toPath) + if err != nil { + return "", fmt.Errorf("readFile(%q): %w", toPath, err) + } + + edits := myers.ComputeEdits(span.URIFromPath(fromPath), string(fromData), string(toData)) + return fmt.Sprint(gotextdiff.ToUnified(fromPath, toPath, string(fromData), edits)), nil } +func (d unifiedDiff) readFile(path string) ([]byte, error) { + file, err := os.Open(path) + if err != nil { + return nil, fmt.Errorf("os.Open(%q): %w", path, err) + } + defer file.Close() + + return io.ReadAll(file) +} + +func filesInDir(root string) (stringset.Set, error) { + var files stringset.Set + + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + + if !d.Type().IsRegular() { + return nil + } + + relPath, err := filepath.Rel(root, path) + if err != nil { + return fmt.Errorf("filepath.Rel(%q, %q): %w", root, path, err) + } + + files.Add(relPath) + return nil + }) + if err != nil { + return nil, err + } + + return files, err +} + +// externalDiff implements the differ interface using an external diff command. +type externalDiff struct{} + // externalDiffVar is the environment variable users can use to overwrite the external diff command. const externalDiffVar = "FLUX_EXTERNAL_DIFF" -// mustExternalDiff initializes an externalDiffCommand using the externalDiffVar environment variable. -func mustExternalDiff() externalDiffCommand { +func (externalDiff) Diff(ctx context.Context, fromDir, toDir string) (string, error) { cmdline := os.Getenv(externalDiffVar) if cmdline == "" { - cmdline = "diff -ur" + return "", fmt.Errorf("the required %q environment variable is unset", externalDiffVar) } args, err := shlex.Split(cmdline) if err != nil { - panic(fmt.Sprintf("shlex.Split(%q): %v", cmdline, err)) + return "", fmt.Errorf("shlex.Split(%q): %w", cmdline, err) } - return externalDiffCommand{ - name: args[0], - flags: args[1:], - } -} + var executable string + executable, args = args[0], args[1:] -func (c externalDiffCommand) Diff(ctx context.Context, fromDir, toDir string) (string, error) { - var args []string - - args = append(args, c.flags...) args = append(args, fromDir, toDir) - cmd := exec.CommandContext(ctx, c.name, args...) + cmd := exec.CommandContext(ctx, executable, args...) var stdout bytes.Buffer cmd.Stdout = &stdout cmd.Stderr = os.Stderr - err := cmd.Run() + err = cmd.Run() var exitErr *exec.ExitError if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 { // exit code 1 only means there was a difference => ignore } else if err != nil { - return "", fmt.Errorf("executing %q: %w", c.name, err) + return "", fmt.Errorf("executing %q: %w", executable, err) } return stdout.String(), nil @@ -383,14 +464,15 @@ func (d dyffBuiltin) Diff(ctx context.Context, fromDir, toDir string) (string, e return buf.String(), nil } -// semanticDiffFlag implements pflag.Value for choosing a semantic diffing algorithm. -type semanticDiffFlag struct { - options map[string]differ - value string +// differFlag implements pflag.Value for choosing a diffing implementation. +type differFlag struct { + options map[string]differ + description map[string]string + value string differ } -func (f *semanticDiffFlag) Set(s string) error { +func (f *differFlag) Set(s string) error { d, ok := f.options[s] if !ok { return fmt.Errorf("invalid value: %q", s) @@ -402,14 +484,29 @@ func (f *semanticDiffFlag) Set(s string) error { return nil } -func (f *semanticDiffFlag) String() string { +func (f *differFlag) String() string { return f.value } -func (f *semanticDiffFlag) Type() string { +func (f *differFlag) Type() string { keys := maps.Keys(f.options) sort.Strings(keys) return strings.Join(keys, "|") } + +func (f *differFlag) usage() string { + var b strings.Builder + fmt.Fprint(&b, "how the diff is generated:") + + keys := maps.Keys(f.options) + + sort.Strings(keys) + + for _, key := range keys { + fmt.Fprintf(&b, "\n %q: %s", key, f.description[key]) + } + + return b.String() +} diff --git a/go.mod b/go.mod index fe76555a..222ecf9c 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ go 1.22.4 replace gopkg.in/yaml.v3 => gopkg.in/yaml.v3 v3.0.1 require ( + bitbucket.org/creachadair/stringset v0.0.14 github.com/Masterminds/semver/v3 v3.2.1 github.com/ProtonMail/go-crypto v1.0.0 github.com/cyphar/filepath-securejoin v0.3.1 @@ -158,6 +159,7 @@ require ( github.com/hashicorp/go-version v1.7.0 // indirect github.com/hashicorp/golang-lru/arc/v2 v2.0.5 // indirect github.com/hashicorp/golang-lru/v2 v2.0.5 // indirect + github.com/hexops/gotextdiff v1.0.3 github.com/imdario/mergo v0.3.16 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect diff --git a/go.sum b/go.sum index 53c00fd1..c8216353 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,5 @@ +bitbucket.org/creachadair/stringset v0.0.14 h1:t1ejQyf8utS4GZV/4fM+1gvYucggZkfhb+tMobDxYOE= +bitbucket.org/creachadair/stringset v0.0.14/go.mod h1:Ej8fsr6rQvmeMDf6CCWMWGb14H9mz8kmDgPPTdiVT0w= code.gitea.io/sdk/gitea v0.19.0 h1:8I6s1s4RHgzxiPHhOQdgim1RWIRcr0LVMbHBjBFXq4Y= code.gitea.io/sdk/gitea v0.19.0/go.mod h1:IG9xZJoltDNeDSW0qiF2Vqx5orMWa7OhVWrjvrd5NpI= dario.cat/mergo v1.0.0 h1:AGCNq9Evsj31mOgNPcLyXc+4PNABt905YmuqPYYpBWk= @@ -323,6 +325,8 @@ github.com/hashicorp/golang-lru/arc/v2 v2.0.5 h1:l2zaLDubNhW4XO3LnliVj0GXO3+/CGN github.com/hashicorp/golang-lru/arc/v2 v2.0.5/go.mod h1:ny6zBSQZi2JxIeYcv7kt2sH2PXJtirBN7RDhRpxPkxU= github.com/hashicorp/golang-lru/v2 v2.0.5 h1:wW7h1TG88eUIJ2i69gaE3uNVtEPIagzhGvHgwfx2Vm4= github.com/hashicorp/golang-lru/v2 v2.0.5/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM= +github.com/hexops/gotextdiff v1.0.3 h1:gitA9+qJrrTCsiCl7+kh75nPqQt1cx4ZkudSTLoUqJM= +github.com/hexops/gotextdiff v1.0.3/go.mod h1:pSWU5MAI3yDq+fZBTazCSJysOMbxWL1BSow5/V2vxeg= github.com/homeport/dyff v1.7.1 h1:B3KJUtnU53H2UryxGcfYKQPrde8VjjbwlHZbczH3giQ= github.com/homeport/dyff v1.7.1/go.mod h1:iLe5b3ymc9xmHZNuJlNVKERE8L2isQMBLxFiTXcwZY0= github.com/imdario/mergo v0.3.16 h1:wwQJbIsHYGMUyLSPrEq1CT16AhnhNJQ51+4fdHUnCl4=