Cleanup flag parsing.

README.md

@@ -41,48 +41,37 @@ To use `container-diff analyze` to perform analysis on a single image, you need
 
 ```
 container-diff analyze <img>     [Run default analyzers]
-container-diff analyze <img> --types=history  [History]
-container-diff analyze <img> --types=file  [File System]
-container-diff analyze <img> --types=pip  [Pip]
-container-diff analyze <img> --types=apt  [Apt]
-container-diff analyze <img> --types=node  [Node]
-container-diff analyze <img> --types=apt,node  [Apt and Node]
-# --types=<analyzer1>,<analyzer2>,<analyzer3>,...
+container-diff analyze <img> --type=history  [History]
+container-diff analyze <img> --type=file  [File System]
+container-diff analyze <img> --type=pip  [Pip]
+container-diff analyze <img> --type=apt  [Apt]
+container-diff analyze <img> --type=node  [Node]
+container-diff analyze <img> --type=apt --type=node  [Apt and Node]
+# --type=<analyzer1> --type=<analyzer2> --type=<analyzer3>,...
 ```
 
 To use container-diff to perform a diff analysis on two images, you need two Docker images (in the form of an ID, tarball, or URL from a repo). Once you have those images, you can run any of the following differs:
 ```
 container-diff diff <img1> <img2>     [Run all differs]
-container-diff diff <img1> <img2> --types=history  [History]
-container-diff diff <img1> <img2> --types=file  [File System]
-container-diff diff <img1> <img2> --types=pip  [Pip]
-container-diff diff <img1> <img2> --types=apt  [Apt]
-container-diff diff <img1> <img2> --types=node  [Node]
+container-diff diff <img1> <img2> --type=history  [History]
+container-diff diff <img1> <img2> --type=file  [File System]
+container-diff diff <img1> <img2> --type=pip  [Pip]
+container-diff diff <img1> <img2> --type=apt  [Apt]
+container-diff diff <img1> <img2> --type=node  [Node]
 ```
 
 You can similarly run many analyzers at once:
 
 ```
-container-diff diff <img1> <img2> --types=history,apt,node
+container-diff diff <img1> <img2> --type=history --type=apt --type=node
 ```
 
 To view the diff of an individual file in two different images, you can use the filename flag in conjuction with the file system diff analyzer.
 
 ```
-container-diff diff <img1> <img2> --types=file --filename=/path/to/file
+container-diff diff <img1> <img2> --type=file --filename=/path/to/file
 ```
 
-All of the analyzer flags with their long versions can be seen below:
-
-| Differ                    | Short flag | Long Flag  |
-| ------------------------- |:----------:| ----------:|
-| File system diff          | -f         | --file     |
-| History                   | -d 	 | --history  |
-| npm installed packages    | -n 	 | --node     |
-| pip installed packages    | -p 	 | --pip      |
-| apt-get installed packages| -a 	 | --apt      |
-
-
 ## Image Sources
 
 container-diff supports Docker images located in both a local Docker daemon and a remote registry. To explicitly specify a local image, use the `daemon://` prefix on the image name; similarly, for an explicitly remote image, use the `remote://` prefix.
@@ -242,7 +231,7 @@ Tarballs provided directly to the tool must be in the Docker format (i.e. have a
 ## Example Run
 
 ```
-$ container-diff diff gcr.io/google-appengine/python:2017-07-21-123058 gcr.io/google-appengine/python:2017-06-29-190410 --types=apt,node,pip
+$ container-diff diff gcr.io/google-appengine/python:2017-07-21-123058 gcr.io/google-appengine/python:2017-06-29-190410 --type=apt --type=node --type=pip
 
 -----AptDiffer-----
 

cmd/analyze.go

@@ -20,7 +20,6 @@ import (
 	"errors"
 	"fmt"
 	"os"
-	"strings"
 
 	"github.com/GoogleCloudPlatform/container-diff/differs"
 	pkgutil "github.com/GoogleCloudPlatform/container-diff/pkg/util"
@@ -33,16 +32,13 @@ var analyzeCmd = &cobra.Command{
 	Short: "Analyzes an image: [image]",
 	Long:  `Analyzes an image using the specifed analyzers as indicated via flags (see documentation for available ones).`,
 	Args: func(cmd *cobra.Command, args []string) error {
-		if err := validateArgs(args, checkAnalyzeArgNum); err != nil {
-			return err
-		}
-		if err := checkIfValidAnalyzer(types); err != nil {
+		if err := validateArgs(args, checkAnalyzeArgNum, checkIfValidAnalyzer); err != nil {
 			return err
 		}
 		return nil
 	},
 	Run: func(cmd *cobra.Command, args []string) {
-		if err := analyzeImage(args[0], strings.Split(types, ",")); err != nil {
+		if err := analyzeImage(args[0], types); err != nil {
 			logrus.Error(err)
 			os.Exit(1)
 		}

cmd/diff.go

@@ -19,14 +19,14 @@ package cmd
 import (
 	"errors"
 	"fmt"
+	"os"
+	"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 filename string
@@ -36,19 +36,13 @@ var diffCmd = &cobra.Command{
 	Short: "Compare two images: [image1] [image2]",
 	Long:  `Compares two images using the specifed analyzers as indicated via flags (see documentation for available ones).`,
 	Args: func(cmd *cobra.Command, args []string) error {
-		if err := validateArgs(args, checkDiffArgNum); err != nil {
-			return err
-		}
-		if err := checkIfValidAnalyzer(types); err != nil {
-			return err
-		}
-		if err := checkFilenameFlag(types); err != nil {
+		if err := validateArgs(args, checkDiffArgNum, checkIfValidAnalyzer, checkFilenameFlag); err != nil {
 			return err
 		}
 		return nil
 	},
 	Run: func(cmd *cobra.Command, args []string) {
-		if err := diffImages(args[0], args[1], strings.Split(types, ",")); err != nil {
+		if err := diffImages(args[0], args[1], types); err != nil {
 			logrus.Error(err)
 			os.Exit(1)
 		}
@@ -62,13 +56,16 @@ func checkDiffArgNum(args []string) error {
 	return nil
 }
 
-func checkFilenameFlag(types string) error {
-	if filename != "" {
-		if !strings.Contains(types, "file") {
-			return errors.New("Please include --types=file with the --filename flag")
+func checkFilenameFlag(_ []string) error {
+	if filename == "" {
+		return nil
+	}
+	for _, t := range types {
+		if t == "file" {
+			return nil
 		}
 	}
-	return nil
+	return errors.New("Please include --types=file with the --filename flag")
 }
 
 func diffImages(image1Arg, image2Arg string, diffArgs []string) error {

cmd/root.go

@@ -17,7 +17,6 @@ limitations under the License.
 package cmd
 
 import (
-	"errors"
 	goflag "flag"
 	"fmt"
 	"os"
@@ -35,7 +34,7 @@ import (
 
 var json bool
 var save bool
-var types string
+var types diffTypes
 
 var LogLevel string
 
@@ -104,12 +103,11 @@ func validateArgs(args []string, validatefxns ...validatefxn) error {
 	return nil
 }
 
-func checkIfValidAnalyzer(flagtypes string) error {
-	if flagtypes == "" {
-		return errors.New("Please provide at least one analyzer to run")
+func checkIfValidAnalyzer(_ []string) error {
+	if len(types) == 0 {
+		types = []string{"apt"}
 	}
-	analyzers := strings.Split(flagtypes, ",")
-	for _, name := range analyzers {
+	for _, name := range types {
 		if _, exists := differs.Analyzers[name]; !exists {
 			return fmt.Errorf("Argument %s is not a valid analyzer", name)
 		}
@@ -146,9 +144,35 @@ func init() {
 	pflag.CommandLine.AddGoFlagSet(goflag.CommandLine)
 }
 
+// Define a type named "diffSlice" as a slice of strings
+type diffTypes []string
+
+// Now, for our new type, implement the two methods of
+// the flag.Value interface...
+// The first method is String() string
+func (d *diffTypes) String() string {
+	return strings.Join(*d, ",")
+}
+
+// The second method is Set(value string) error
+func (d *diffTypes) Set(value string) error {
+	// Dedupe repeated elements.
+	for _, t := range *d {
+		if t == value {
+			return nil
+		}
+	}
+	*d = append(*d, value)
+	return nil
+}
+
+func (d *diffTypes) Type() string {
+	return "Diff Types"
+}
+
 func addSharedFlags(cmd *cobra.Command) {
 	cmd.Flags().BoolVarP(&json, "json", "j", false, "JSON Output defines if the diff should be returned in a human readable format (false) or a JSON (true).")
-	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().VarP(&types, "type", "t", "This flag sets the list of analyzer types to use. Set it repeatedly to use multiple 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.")
 }

docker_diff.bzl

@@ -23,8 +23,7 @@ def _impl(ctx):
 #!/bin/bash
 set -e
 ./%s diff %s %s""" % (container_diff_loction, image_location, ctx.attr.diff_base)
-  if ctx.attr.diff_types:
-      content += " --types=%s" % (",".join(ctx.attr.diff_types))
+  content += " ".join(["--type=%s" % t for t in ctx.attr.diff_types])
 
   ctx.file_action(
       output = ctx.outputs.executable,

tests/integration_test.go

@@ -97,83 +97,83 @@ func TestDiffAndAnalysis(t *testing.T) {
 			subcommand:   "diff",
 			imageA:       diffBase,
 			imageB:       diffModified,
-			differFlags:  []string{"--types=file"},
+			differFlags:  []string{"--type=file"},
 			expectedFile: "file_diff_expected.json",
 		},
 		{
 			description:  "apt differ",
 			subcommand:   "diff",
 			imageA:       aptBase,
 			imageB:       aptModified,
-			differFlags:  []string{"--types=apt"},
+			differFlags:  []string{"--type=apt"},
 			expectedFile: "apt_diff_expected.json",
 		},
 		{
 			description:  "node differ",
 			subcommand:   "diff",
 			imageA:       nodeBase,
 			imageB:       nodeModified,
-			differFlags:  []string{"--types=node"},
+			differFlags:  []string{"--type=node"},
 			expectedFile: "node_diff_order_expected.json",
 		},
 		{
 			description:  "multi differ",
 			subcommand:   "diff",
 			imageA:       multiBase,
 			imageB:       multiModified,
-			differFlags:  []string{"--types=node,pip,apt"},
+			differFlags:  []string{"--type=node", "--type=pip", "--type=apt"},
 			expectedFile: "multi_diff_expected.json",
 		},
 		{
 			description:  "multi differ local",
 			subcommand:   "diff",
 			imageA:       multiBaseLocal,
 			imageB:       multiModifiedLocal,
-			differFlags:  []string{"--types=node,pip,apt"},
+			differFlags:  []string{"--type=node", "--type=pip", "--type=apt"},
 			expectedFile: "multi_diff_expected.json",
 		},
 		{
 			description:  "history differ",
 			subcommand:   "diff",
 			imageA:       diffBase,
 			imageB:       diffModified,
-			differFlags:  []string{"--types=history"},
+			differFlags:  []string{"--type=history"},
 			expectedFile: "hist_diff_expected.json",
 		},
 		{
 			description:  "apt sorted differ",
 			subcommand:   "diff",
 			imageA:       aptBase,
 			imageB:       aptModified,
-			differFlags:  []string{"--types=apt", "-o"},
+			differFlags:  []string{"--type=apt", "-o"},
 			expectedFile: "apt_sorted_diff_expected.json",
 		},
 		{
 			description:  "apt analysis",
 			subcommand:   "analyze",
 			imageA:       aptModified,
-			differFlags:  []string{"--types=apt"},
+			differFlags:  []string{"--type=apt"},
 			expectedFile: "apt_analysis_expected.json",
 		},
 		{
 			description:  "file sorted analysis",
 			subcommand:   "analyze",
 			imageA:       diffModified,
-			differFlags:  []string{"--types=file", "-o"},
+			differFlags:  []string{"--type=file", "-o"},
 			expectedFile: "file_sorted_analysis_expected.json",
 		},
 		{
 			description:  "pip analysis",
 			subcommand:   "analyze",
 			imageA:       pipModified,
-			differFlags:  []string{"--types=pip"},
+			differFlags:  []string{"--type=pip"},
 			expectedFile: "pip_analysis_expected.json",
 		},
 		{
 			description:  "node analysis",
 			subcommand:   "analyze",
 			imageA:       nodeModified,
-			differFlags:  []string{"--types=node"},
+			differFlags:  []string{"--type=node"},
 			expectedFile: "node_analysis_expected.json",
 		},
 	}