Repository: uber/prototool
Pull request #257 (2018-12-24 21:59:41)

Cache commands
Fixes https://github.com/uber/prototool/issues/202.

See the updates to the README.md and documentation for details.

CLAassistant: [![CLA assistant check](https://cla-assistant.io/pull/badge/not_signed)](https://cla-assistant.io/uber/prototool?pullRequest=257) <br/>Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our [Contributor License Agreement](https://cla-assistant.io/uber/prototool?pullRequest=257) before we can accept your contribution.<br/>**1** out of **2** committers have signed the CLA.<br/><br/>:white_check_mark: peter-edge<br/>:x: konradjniemiec<br/><sub>You have signed the CLA already but the status is still pending? Let us [recheck](https://cla-assistant.io/check/uber/prototool?pullRequest=257) it.</sub>

Patch:
diff --git a/README.md b/README.md
index 1a98d8e..f49ae90 100644
--- a/README.md
+++ b/README.md
@@ -462,35 +462,23 @@ The entire implementation is purposefully under the `internal` package to not ex
 
 *Question:* How do I download `protoc` ahead of time as part of a Docker build/CI pipeline?
 
-*Answer*: We used to have a command that did this, but removed it for simplicity and because the command as implemented did not properly
-read the configuration file to figure out what version of `protoc` to download. We may re-add this command in the future, however
-here is a technique to accomplish this, including as a `RUN` directive for Docker:
+*Answer*: `prototool cache update`.
 
-```bash
-# the first rm -rf call is not needed if this is a RUN directive for Docker
-rm -rf /tmp/prototool-bootstrap && \
-  echo $'protoc\n  version: 3.6.1' > /tmp/prototool-bootstrap/prototool.yaml && \
-  echo 'syntax = "proto3";' > /tmp/prototool-bootstrap/tmp.proto && \
-  prototool compile /tmp/prototool-bootstrap && \
-  rm -rf /tmp/prototool-bootstrap
-```
-
-The better way to do this as part of a bash script would be to use `mktemp -d` i.e.:
+You can pass both `--cache-path` and `--config-data` flags to this command to customize the invocation.
 
 ```bash
-#!/bin/bash
-
-set -euo pipefail
-
-TMPDIR="$(mktemp -d)"
-trap 'rm -rf "${TMPDIR}"' EXIT
-
-echo $'protoc\n  version: 3.6.1' > "${TMPDIR}/prototool.yaml"
-echo 'syntax = "proto3";' > "${TMPDIR}/tmp.proto"
-prototool compile "${TMPDIR}"
+# Basic invocation which will cache using the default behavior. See prototool help cache update for more details.
+prototool cache update
+# Cache to a specific directory path/to/cache
+prototool cache update --cache-path path/to/cache
+# Cache using custom configuration data instead of finding a prototool.yaml file using the file discovery mechanism
+prototool cache update --config-data '{"protoc":{"version":"3.6.1"}}'
 ```
 
-But for Darwin or Linux, the above should work.
+There is also a command `prototool cache delete` which will delete all cached assets of `prototool`,
+however this command does not accept the `--cache-path` flag - if you specify a custom directory, you
+should clean it up on your own, we don't want to effectively call `rm -rf DIR` via a `prototool` command
+on a location we don't know about.
 
 ##### Alpine Linux Issues
 
diff --git a/internal/cmd/cmd.go b/internal/cmd/cmd.go
index dace0d7..585b205 100644
--- a/internal/cmd/cmd.go
+++ b/internal/cmd/cmd.go
@@ -107,11 +107,15 @@ func getRootCommand(exitCodeAddr *int, develMode bool, args []string, stdin io.R
 	rootCmd.AddCommand(formatCmdTemplate.Build(exitCodeAddr, stdin, stdout, stderr, flags))
 	rootCmd.AddCommand(generateCmdTemplate.Build(exitCodeAddr, stdin, stdout, stderr, flags))
 	rootCmd.AddCommand(grpcCmdTemplate.Build(exitCodeAddr, stdin, stdout, stderr, flags))
-	configCmd := &cobra.Command{Use: "config"}
+	configCmd := &cobra.Command{Use: "config", Short: "Interact with configuration files."}
 	configCmd.AddCommand(configInitCmdTemplate.Build(exitCodeAddr, stdin, stdout, stderr, flags))
 	rootCmd.AddCommand(configCmd)
 	rootCmd.AddCommand(lintCmdTemplate.Build(exitCodeAddr, stdin, stdout, stderr, flags))
 	rootCmd.AddCommand(versionCmdTemplate.Build(exitCodeAddr, stdin, stdout, stderr, flags))
+	cacheCmd := &cobra.Command{Use: "cache", Short: "Interact with the cache."}
+	cacheCmd.AddCommand(cacheUpdateCmdTemplate.Build(exitCodeAddr, stdin, stdout, stderr, flags))
+	cacheCmd.AddCommand(cacheDeleteCmdTemplate.Build(exitCodeAddr, stdin, stdout, stderr, flags))
+	rootCmd.AddCommand(cacheCmd)
 	inspectCmd := &cobra.Command{Use: "inspect", Short: "Top-level command for inspection commands."}
 	inspectCmd.AddCommand(inspectPackagesCmdTemplate.Build(exitCodeAddr, stdin, stdout, stderr, flags))
 	inspectCmd.AddCommand(inspectPackageCmdTemplate.Build(exitCodeAddr, stdin, stdout, stderr, flags))
@@ -123,12 +127,6 @@ func getRootCommand(exitCodeAddr *int, develMode bool, args []string, stdin io.R
 	flags.bindDebug(rootCmd.PersistentFlags())
 
 	if develMode {
-		rootCmd.AddCommand(cleanCmdTemplate.Build(exitCodeAddr, stdin, stdout, stderr, flags))
-		rootCmd.AddCommand(downloadCmdTemplate.Build(exitCodeAddr, stdin, stdout, stderr, flags))
-
-		// we may or may not want to expose these to users
-		// but will not build them into the binary for v1.0
-		flags.bindCachePath(rootCmd.PersistentFlags())
 		flags.bindPrintFields(rootCmd.PersistentFlags())
 	}
 
diff --git a/internal/cmd/flags.go b/internal/cmd/flags.go
index 09bf9cb..4e74e86 100644
--- a/internal/cmd/flags.go
+++ b/internal/cmd/flags.go
@@ -62,7 +62,7 @@ func (f *flags) bindAddress(flagSet *pflag.FlagSet) {
 }
 
 func (f *flags) bindCachePath(flagSet *pflag.FlagSet) {
-	flagSet.StringVar(&f.cachePath, "cache-path", "", "The path to use for the cache, otherwise uses the default behavior.")
+	flagSet.StringVar(&f.cachePath, "cache-path", "", "The path to use for the cache, otherwise uses the default behavior. The user is expected to clean and manage this cache path. See prototool help cache update for more details.")
 }
 
 func (f *flags) bindCallTimeout(flagSet *pflag.FlagSet) {
diff --git a/internal/cmd/templates.go b/internal/cmd/templates.go
index f0972b5..2c29c13 100644
--- a/internal/cmd/templates.go
+++ b/internal/cmd/templates.go
@@ -45,6 +45,7 @@ var (
 			return runner.All(args, flags.disableFormat, flags.disableLint, flags.fix)
 		},
 		BindFlags: func(flagSet *pflag.FlagSet, flags *flags) {
+			flags.bindCachePath(flagSet)
 			flags.bindConfigData(flagSet)
 			flags.bindDisableFormat(flagSet)
 			flags.bindDisableLint(flagSet)
@@ -56,12 +57,43 @@ var (
 		},
 	}
 
-	cleanCmdTemplate = &cmdTemplate{
-		Use:   "clean",
-		Short: "Delete the cache.",
-		Args:  cobra.NoArgs,
+	cacheUpdateCmdTemplate = &cmdTemplate{
+		Use:   "update",
+		Short: "Update the cache by downloading all artifacts.",
+		Long: `This will download artifacts to a cache directory before running any commands. Note that calling this command is not necessary, all artifacts are automatically downloaded when required by other commands. This just provides a mechanism to pre-cache artifacts during your build.
+
+Artifacts are downloaded to the following directories based on flags and environment variables:
+
+- If --cache-path is set, then this directory will be used. The user is
+  expected to manually manage this directory, and the "delete" subcommand
+  will have no effect on it.
+- Otherwise, if $XDG_CACHE_HOME is set, then $XDG_CACHE_HOME/prototool
+  will be used.
+- Otherwise, if on Linux, $HOME/.cache/prototool will be used, or on Darwin,
+  $HOME/Library/Caches/prototool will be used.`,
+		Args: cobra.NoArgs,
+		Run: func(runner exec.Runner, args []string, flags *flags) error {
+			return runner.CacheUpdate()
+		},
+		BindFlags: func(flagSet *pflag.FlagSet, flags *flags) {
+			flags.bindCachePath(flagSet)
+			flags.bindConfigData(flagSet)
+		},
+	}
+
+	cacheDeleteCmdTemplate = &cmdTemplate{
+		Use:   "delete",
+		Short: "Delete all artifacts in the default cache.",
+		Long: `The following directory will be deleted based on environment variables:
+
+- If $XDG_CACHE_HOME is set, then $XDG_CACHE_HOME/prototool will be deleted.
+- Otherwise, if on Linux, $HOME/.cache/prototool will be deleted, or on Darwin,
+  $HOME/Library/Caches/prototool will be deleted.
+
+  This will not delete any custom caches created using the --cache-path option.`,
+		Args: cobra.NoArgs,
 		Run: func(runner exec.Runner, args []string, flags *flags) error {
-			return runner.Clean()
+			return runner.CacheDelete()
 		},
 	}
 
@@ -74,6 +106,7 @@ var (
 			return runner.Compile(args, flags.dryRun)
 		},
 		BindFlags: func(flagSet *pflag.FlagSet, flags *flags) {
+			flags.bindCachePath(flagSet)
 			flags.bindConfigData(flagSet)
 			flags.bindDryRun(flagSet)
 			flags.bindJSON(flagSet)
@@ -154,18 +187,6 @@ If Vim integration is set up, files will be generated when you open a new Protob
 		},
 	}
 
-	downloadCmdTemplate = &cmdTemplate{
-		Use:   "download",
-		Short: "Download the protobuf artifacts to a cache.",
-		Args:  cobra.NoArgs,
-		Run: func(runner exec.Runner, args []string, flags *flags) error {
-			return runner.Download()
-		},
-		BindFlags: func(flagSet *pflag.FlagSet, flags *flags) {
-			flags.bindConfigData(flagSet)
-		},
-	}
-
 	filesCmdTemplate = &cmdTemplate{
 		Use:   "files [dirOrFile]",
 		Short: "Print all files that match the input arguments.",
@@ -186,6 +207,7 @@ If Vim integration is set up, files will be generated when you open a new Protob
 			return runner.Format(args, flags.overwrite, flags.diffMode, flags.lintMode, flags.fix)
 		},
 		BindFlags: func(flagSet *pflag.FlagSet, flags *flags) {
+			flags.bindCachePath(flagSet)
 			flags.bindConfigData(flagSet)
 			flags.bindDiffMode(flagSet)
 			flags.bindJSON(flagSet)
@@ -206,6 +228,7 @@ If Vim integration is set up, files will be generated when you open a new Protob
 			return runner.Gen(args, flags.dryRun)
 		},
 		BindFlags: func(flagSet *pflag.FlagSet, flags *flags) {
+			flags.bindCachePath(flagSet)
 			flags.bindConfigData(flagSet)
 			flags.bindDryRun(flagSet)
 			flags.bindJSON(flagSet)
@@ -291,6 +314,7 @@ $ cat input.json | prototool grpc example \
 			return runner.GRPC(args, flags.headers, flags.address, flags.method, flags.data, flags.callTimeout, flags.connectTimeout, flags.keepaliveTime, flags.stdin)
 		},
 		BindFlags: func(flagSet *pflag.FlagSet, flags *flags) {
+			flags.bindCachePath(flagSet)
 			flags.bindConfigData(flagSet)
 			flags.bindAddress(flagSet)
 			flags.bindCallTimeout(flagSet)
@@ -314,6 +338,7 @@ $ cat input.json | prototool grpc example \
 			return runner.InspectPackages(args)
 		},
 		BindFlags: func(flagSet *pflag.FlagSet, flags *flags) {
+			flags.bindCachePath(flagSet)
 			flags.bindConfigData(flagSet)
 			flags.bindJSON(flagSet)
 			flags.bindProtocURL(flagSet)
@@ -330,6 +355,7 @@ $ cat input.json | prototool grpc example \
 			return runner.InspectPackage(args, flags.name)
 		},
 		BindFlags: func(flagSet *pflag.FlagSet, flags *flags) {
+			flags.bindCachePath(flagSet)
 			flags.bindConfigData(flagSet)
 			flags.bindName(flagSet)
 			flags.bindProtocURL(flagSet)
@@ -346,6 +372,7 @@ $ cat input.json | prototool grpc example \
 			return runner.InspectPackageDeps(args, flags.name)
 		},
 		BindFlags: func(flagSet *pflag.FlagSet, flags *flags) {
+			flags.bindCachePath(flagSet)
 			flags.bindConfigData(flagSet)
 			flags.bindName(flagSet)
 			flags.bindProtocURL(flagSet)
@@ -362,6 +389,7 @@ $ cat input.json | prototool grpc example \
 			return runner.InspectPackageImporters(args, flags.name)
 		},
 		BindFlags: func(flagSet *pflag.FlagSet, flags *flags) {
+			flags.bindCachePath(flagSet)
 			flags.bindConfigData(flagSet)
 			flags.bindName(flagSet)
 			flags.bindProtocURL(flagSet)
@@ -428,6 +456,7 @@ sys	0m0.924s`,
 			return runner.Lint(args, flags.listAllLinters, flags.listLinters, flags.listAllLintGroups, flags.listLintGroup)
 		},
 		BindFlags: func(flagSet *pflag.FlagSet, flags *flags) {
+			flags.bindCachePath(flagSet)
 			flags.bindConfigData(flagSet)
 			flags.bindJSON(flagSet)
 			flags.bindListAllLinters(flagSet)
diff --git a/internal/exec/exec.go b/internal/exec/exec.go
index b9efed9..64e1c30 100644
--- a/internal/exec/exec.go
+++ b/internal/exec/exec.go
@@ -48,8 +48,8 @@ type Runner interface {
 	Init(args []string, uncomment bool) error
 	Create(args []string, pkg string) error
 	Version() error
-	Download() error
-	Clean() error
+	CacheUpdate() error
+	CacheDelete() error
 	Files(args []string) error
 	Compile(args []string, dryRun bool) error
 	Gen(args []string, dryRun bool) error
diff --git a/internal/exec/runner.go b/internal/exec/runner.go
index 271083f..4aebece 100644
--- a/internal/exec/runner.go
+++ b/internal/exec/runner.go
@@ -163,7 +163,7 @@ func (r *runner) Create(args []string, pkg string) error {
 	return r.newCreateHandler(pkg).Create(args...)
 }
 
-func (r *runner) Download() error {
+func (r *runner) CacheUpdate() error {
 	config, err := r.getConfig(r.workDirPath)
 	if err != nil {
 		return err
@@ -172,14 +172,11 @@ func (r *runner) Download() error {
 	if err != nil {
 		return err
 	}
-	path, err := d.Download()
-	if err != nil {
-		return err
-	}
-	return r.println(path)
+	_, err = d.Download()
+	return err
 }
 
-func (r *runner) Clean() error {
+func (r *runner) CacheDelete() error {
 	config, err := r.getConfig(r.workDirPath)
 	if err != nil {
 		return err
diff --git a/internal/protoc/downloader.go b/internal/protoc/downloader.go
index 0f5adac..8fc6a4b 100644
--- a/internal/protoc/downloader.go
+++ b/internal/protoc/downloader.go
@@ -141,7 +141,7 @@ func (d *downloader) WellKnownTypesIncludePath() (string, error) {
 }
 
 func (d *downloader) Delete() error {
-	basePath, err := d.getBasePathNoVersion()
+	basePath, err := d.getBasePathNoVersionOSARCH()
 	if err != nil {
 		return err
 	}
@@ -327,6 +327,26 @@ func (d *downloader) getBasePath() (string, error) {
 	return filepath.Join(basePathNoVersion, d.getBasePathVersionPart()), nil
 }
 
+func (d *downloader) getBasePathNoVersionOSARCH() (string, error) {
+	basePath := d.cachePath
+	var err error
+	if basePath == "" {
+		basePath, err = getDefaultBasePathNoOSARCH()
+		if err != nil {
+			return "", err
+		}
+	} else {
+		basePath, err = file.AbsClean(basePath)
+		if err != nil {
+			return "", err
+		}
+	}
+	if err := file.CheckAbs(basePath); err != nil {
+		return "", err
+	}
+	return basePath, nil
+}
+
 func (d *downloader) getBasePathNoVersion() (string, error) {
 	basePath := d.cachePath
 	var err error
@@ -362,13 +382,29 @@ func getDefaultBasePath() (string, error) {
 }
 
 func getDefaultBasePathInternal(goos string, goarch string, getenvFunc func(string) string) (string, error) {
+	basePathNoOSARCH, err := getDefaultBasePathInternalNoOSARCH(goos, goarch, getenvFunc)
+	if err != nil {
+		return "", err
+	}
 	unameS, unameM, err := getUnameSUnameMPaths(goos, goarch)
 	if err != nil {
 		return "", err
 	}
+	return filepath.Join(basePathNoOSARCH, unameS, unameM), nil
+}
+
+func getDefaultBasePathNoOSARCH() (string, error) {
+	return getDefaultBasePathInternalNoOSARCH(runtime.GOOS, runtime.GOARCH, os.Getenv)
+}
+
+func getDefaultBasePathInternalNoOSARCH(goos string, goarch string, getenvFunc func(string) string) (string, error) {
+	unameS, _, err := getUnameSUnameMPaths(goos, goarch)
+	if err != nil {
+		return "", err
+	}
 	xdgCacheHome := getenvFunc("XDG_CACHE_HOME")
 	if xdgCacheHome != "" {
-		return filepath.Join(xdgCacheHome, "prototool", unameS, unameM), nil
+		return filepath.Join(xdgCacheHome, "prototool"), nil
 	}
 	home := getenvFunc("HOME")
 	if home == "" {
@@ -376,9 +412,9 @@ func getDefaultBasePathInternal(goos string, goarch string, getenvFunc func(stri
 	}
 	switch unameS {
 	case "Darwin":
-		return filepath.Join(home, "Library", "Caches", "prototool", unameS, unameM), nil
+		return filepath.Join(home, "Library", "Caches", "prototool"), nil
 	case "Linux":
-		return filepath.Join(home, ".cache", "prototool", unameS, unameM), nil
+		return filepath.Join(home, ".cache", "prototool"), nil
 	default:
 		return "", fmt.Errorf("invalid value for uname -s: %v", unameS)
 	}


Test patch:
diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go
index 924e457..c57d996 100644
--- a/internal/cmd/cmd_test.go
+++ b/internal/cmd/cmd_test.go
@@ -1025,7 +1025,7 @@ func testDownload(t *testing.T) {
 	// download checks if protoc is already downloaded to the cache location
 	// if it is, then this is effectively a no-op
 	// if it isn't, then this downloads to the cache
-	stdout, exitCode := testDoInternal(nil, "download")
+	stdout, exitCode := testDoInternal(nil, "cache", "update")
 	require.Equal(t, 0, exitCode, "had non-zero exit code when downloading: %s", stdout)
 }