-
Notifications
You must be signed in to change notification settings - Fork 237
Diff within files #121
Diff within files #121
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,14 +19,14 @@ package cmd | |
import ( | ||
"errors" | ||
"fmt" | ||
"os" | ||
"strings" | ||
"sync" | ||
|
||
"github.com/GoogleCloudPlatform/container-diff/differs" | ||
pkgutil "github.com/GoogleCloudPlatform/container-diff/pkg/util" | ||
"github.com/GoogleCloudPlatform/container-diff/util" | ||
"github.com/sirupsen/logrus" | ||
"github.com/spf13/cobra" | ||
"os" | ||
"strings" | ||
"sync" | ||
) | ||
|
||
var diffCmd = &cobra.Command{ | ||
|
@@ -43,7 +43,8 @@ var diffCmd = &cobra.Command{ | |
return nil | ||
}, | ||
Run: func(cmd *cobra.Command, args []string) { | ||
if err := diffImages(args[0], args[1], strings.Split(types, ",")); err != nil { | ||
typesFlagSet := checkIfTypesFlagSet(cmd) | ||
if err := diffImages(args[0], args[1], strings.Split(types, ","), typesFlagSet); err != nil { | ||
logrus.Error(err) | ||
os.Exit(1) | ||
} | ||
|
@@ -57,7 +58,7 @@ func checkDiffArgNum(args []string) error { | |
return nil | ||
} | ||
|
||
func diffImages(image1Arg, image2Arg string, diffArgs []string) error { | ||
func diffImages(image1Arg, image2Arg string, diffArgs []string, typesFlagSet bool) error { | ||
diffTypes, err := differs.GetAnalyzers(diffArgs) | ||
if err != nil { | ||
return err | ||
|
@@ -71,7 +72,9 @@ func diffImages(image1Arg, image2Arg string, diffArgs []string) error { | |
var wg sync.WaitGroup | ||
wg.Add(2) | ||
|
||
fmt.Fprintf(os.Stderr, "Starting diff on images %s and %s, using differs: %s\n", image1Arg, image2Arg, diffArgs) | ||
if typesFlagSet || filename == "" { | ||
fmt.Fprintf(os.Stderr, "Starting diff on images %s and %s, using differs: %s\n", image1Arg, image2Arg, diffArgs) | ||
} | ||
|
||
imageMap := map[string]*pkgutil.Image{ | ||
image1Arg: {}, | ||
|
@@ -101,6 +104,17 @@ func diffImages(image1Arg, image2Arg string, diffArgs []string) error { | |
defer pkgutil.CleanupImage(*imageMap[image2Arg]) | ||
} | ||
|
||
if filename != "" { | ||
fmt.Fprintln(os.Stderr, "Computing filename diffs") | ||
err := diffFile(imageMap, image1Arg, image2Arg) | ||
if err != nil { | ||
return err | ||
} | ||
if !typesFlagSet { | ||
return nil | ||
} | ||
} | ||
|
||
fmt.Fprintln(os.Stderr, "Computing diffs") | ||
req := differs.DiffRequest{*imageMap[image1Arg], *imageMap[image2Arg], diffTypes} | ||
diffs, err := req.GetDiff() | ||
|
@@ -116,6 +130,20 @@ func diffImages(image1Arg, image2Arg string, diffArgs []string) error { | |
return nil | ||
} | ||
|
||
func diffFile(imageMap map[string]*pkgutil.Image, image1Arg, image2Arg string) error { | ||
|
||
image1FilePath := imageMap[image1Arg].FSPath + filename | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these should probably use something like |
||
image2FilePath := imageMap[image2Arg].FSPath + filename | ||
|
||
diff, err := util.DiffFile(image1FilePath, image2FilePath, image1Arg, image2Arg) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think util.DiffFile can do the path joining actually. You could make func DiffFile(image1, image2, filename string) (FileNameDiff, error) {
// join paths
// run diff
// return full FileNameDiff object
} |
||
if err != nil { | ||
return err | ||
} | ||
diff.Filename = filename | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you pass filename into |
||
util.TemplateOutput(diff, "FileNameDiff") | ||
return nil | ||
} | ||
|
||
func init() { | ||
RootCmd.AddCommand(diffCmd) | ||
addSharedFlags(diffCmd) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,6 +36,7 @@ import ( | |
var json bool | ||
var save bool | ||
var types string | ||
var filename string | ||
|
||
var LogLevel string | ||
|
||
|
@@ -117,6 +118,11 @@ func checkIfValidAnalyzer(flagtypes string) error { | |
return nil | ||
} | ||
|
||
func checkIfTypesFlagSet(cmd *cobra.Command) bool { | ||
flag := cmd.Flags().Lookup("types") | ||
return flag.Changed | ||
} | ||
|
||
func getPrepperForImage(image string) (pkgutil.Prepper, error) { | ||
cli, err := client.NewEnvClient() | ||
if err != nil { | ||
|
@@ -151,4 +157,5 @@ func addSharedFlags(cmd *cobra.Command) { | |
cmd.Flags().StringVarP(&types, "types", "t", "apt", "This flag sets the list of analyzer types to use. It expects a comma separated list of supported analyzers.") | ||
cmd.Flags().BoolVarP(&save, "save", "s", false, "Set this flag to save rather than remove the final image filesystems on exit.") | ||
cmd.Flags().BoolVarP(&util.SortSize, "order", "o", false, "Set this flag to sort any file/package results by descending size. Otherwise, they will be sorted by name.") | ||
cmd.Flags().StringVarP(&filename, "filename", "f", "", "View the diff of a file in both containers (can only be used with container-diff diff)") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should add this flag to the diff subcommand specific flags instead of have it be at the root level. That will let you remove a lot of the flag validation. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ package util | |
|
||
import ( | ||
"bytes" | ||
"errors" | ||
"io/ioutil" | ||
"os" | ||
"path/filepath" | ||
|
@@ -53,6 +54,24 @@ func GetSize(path string) int64 { | |
return stat.Size() | ||
} | ||
|
||
//GetFileContents returns the contents of a file at the specified path | ||
func GetFileContents(path string) ([]string, error) { | ||
stat, err := os.Stat(path) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this checking for the existence of a file? It should be
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it does check for existence, but also checks that the path isn't a directory after, which is why I thought it might be easier to split it up? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess what is the expected behavior when the file doesn't exist? I think you wouldn't want to throw an error? A: "" A: "something" A: no file |
||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
if stat.IsDir() { | ||
return nil, errors.New("--filename is a directory, not a file") | ||
} | ||
contents, err := ioutil.ReadFile(path) | ||
|
||
if err != nil { | ||
return nil, err | ||
} | ||
return []string{string(contents)}, nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why doesn't this return a normal string? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the diff library I'm using wanted this format, but it probably makes more sense to move that to DiffFile |
||
} | ||
|
||
func getDirectorySize(path string) (int64, error) { | ||
var size int64 | ||
err := filepath.Walk(path, func(_ string, info os.FileInfo, err error) error { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,11 @@ type DirDiff struct { | |
Mods []EntryDiff | ||
} | ||
|
||
type FileNameDiff struct { | ||
Filename string | ||
Diff string | ||
} | ||
|
||
type EntryDiff struct { | ||
Name string | ||
Size1 int64 | ||
|
@@ -118,6 +123,36 @@ func DiffDirectory(d1, d2 pkgutil.Directory) (DirDiff, bool) { | |
return DirDiff{addedEntries, deletedEntries, modifiedEntries}, same | ||
} | ||
|
||
//DiffFile diffs within a file | ||
func DiffFile(path1, path2, image1, image2 string) (FileNameDiff, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't satisfy any interface right? In that case, you should probably pass the filename parameter into here too and set it on the FileNameDiff |
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove newline |
||
var result FileNameDiff | ||
|
||
image1Contents, err := pkgutil.GetFileContents(path1) | ||
if err != nil { | ||
return result, err | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when you encounter an error, you should return |
||
} | ||
image2Contents, err := pkgutil.GetFileContents(path2) | ||
|
||
if err != nil { | ||
return result, err | ||
} | ||
|
||
diff := difflib.UnifiedDiff{ | ||
A: image1Contents, | ||
B: image2Contents, | ||
FromFile: image1, | ||
ToFile: image2, | ||
} | ||
|
||
text, err := difflib.GetUnifiedDiffString(diff) | ||
|
||
if err != nil { | ||
return result, err | ||
} | ||
return FileNameDiff{Diff: text}, nil | ||
} | ||
|
||
// Checks for content differences between files of the same name from different directories | ||
func GetModifiedEntries(d1, d2 pkgutil.Directory) []string { | ||
d1files := d1.Content | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this once you've added the flag under
container-diff diff
only