v.io/jiri/profiles: bug fix, ensure that a version is always specified.

In order to be able to unambiguosly remove profiles, its essential that
the manifest records the default version # that was used at install time.
This was ensured when using v23-profile install, but not when using
profiles.EnsureProfileIsInstalled. This CL fixes that, but this
requires fixing up the existing installations. To do so, I introduce
a 'sanitize' command that can perform various cleanups like this.
It is intended for use by end users and from presubmits.

Whilst doing this, I folded cleanup into the new sanitize command and
similarly folded the 'recreate' command into list --info.

MultiPart: 1/2

Change-Id: I78de04fe773c0c95a9a9873a42013f2c1941ce65
diff --git a/profiles/commandline/driver.go b/profiles/commandline/driver.go
index 51436b9..1b26baf 100644
--- a/profiles/commandline/driver.go
+++ b/profiles/commandline/driver.go
@@ -38,7 +38,7 @@
 		cmdEnv,
 		cmdUninstall,
 		cmdUpdate,
-		cmdRecreate,
+		cmdCleanup,
 	},
 }
 
@@ -90,26 +90,16 @@
 	ArgsLong: "<profiles> is a list of profiles to update, if omitted all profiles are updated.",
 }
 
-// cmdCleanup represents the "profile cleanup" command.
+// cmdCleanup represents the "profile Cleanup" command.
 var cmdCleanup = &cmdline.Command{
 	Runner:   cmdline.RunnerFunc(runCleanup),
 	Name:     "cleanup",
-	Short:    "Uninstall versions of the given profiles that are older than the default",
-	Long:     "Uninstall versions of the given profiles that are older than the default.",
+	Short:    "Cleanup the locally installed profiles",
+	Long:     "Cleanup the locally installed profiles. This is generally required when recovering from earlier bugs or when preparing for a subsequent change to the profiles implementation.",
 	ArgsName: "<profiles>",
 	ArgsLong: "<profiles> is a list of profiles to cleanup, if omitted all profiles are cleaned.",
 }
 
-// cmdRecreate represents the "profile recreate" command.
-var cmdRecreate = &cmdline.Command{
-	Runner:   cmdline.RunnerFunc(runRecreate),
-	Name:     "recreate",
-	Short:    "Display a list of commands that will recreate the currently installed profiles",
-	Long:     "Display a list of commands that will recreate the currently installed profiles.",
-	ArgsName: "<profiles>",
-	ArgsLong: "<profiles> is a list of profiles to be recreated, if omitted commands to recreate all profiles are displayed.",
-}
-
 // cmdUninstall represents the "profile uninstall" command.
 var cmdUninstall = &cmdline.Command{
 	Runner:   cmdline.RunnerFunc(runUninstall),
@@ -121,16 +111,18 @@
 }
 
 var (
-	targetFlag        profiles.Target
-	manifestFlag      string
-	showManifestFlag  bool
-	profilesFlag      string
-	rootDir           string
-	availableFlag     bool
-	verboseFlag       bool
-	allFlag           bool
-	infoFlag          string
-	mergePoliciesFlag profiles.MergePolicies
+	targetFlag           profiles.Target
+	manifestFlag         string
+	showManifestFlag     bool
+	profilesFlag         string
+	rootDir              string
+	availableFlag        bool
+	verboseFlag          bool
+	allFlag              bool
+	infoFlag             string
+	mergePoliciesFlag    profiles.MergePolicies
+	specificVersionsFlag bool
+	cleanupFlag          bool
 )
 
 func Main(name string) {
@@ -152,11 +144,10 @@
 	for _, fs := range []*flag.FlagSet{
 		&cmdInstall.Flags,
 		&cmdUpdate.Flags,
-		&cmdCleanup.Flags,
 		&cmdUninstall.Flags,
+		&cmdCleanup.Flags,
 		&cmdEnv.Flags,
-		&cmdList.Flags,
-		&cmdRecreate.Flags} {
+		&cmdList.Flags} {
 		profiles.RegisterManifestFlag(fs, &manifestFlag, defaultManifestFilename)
 	}
 
@@ -175,10 +166,11 @@
 	profiles.RegisterProfilesFlag(&cmdEnv.Flags, &profilesFlag)
 	profiles.RegisterMergePoliciesFlag(&cmdEnv.Flags, &mergePoliciesFlag)
 
-	// uninstall, list and env accept: --v
+	// uninstall, list, env and cleanup accept: --v
 	for _, fs := range []*flag.FlagSet{
 		&cmdUpdate.Flags,
 		&cmdList.Flags,
+		&cmdCleanup.Flags,
 		&cmdEnv.Flags} {
 		fs.BoolVar(&verboseFlag, "v", false, "print more detailed information")
 	}
@@ -195,6 +187,10 @@
 		profiles.LookupManager(mgr).AddFlags(&cmdInstall.Flags, profiles.Install)
 		profiles.LookupManager(mgr).AddFlags(&cmdUninstall.Flags, profiles.Uninstall)
 	}
+
+	// cleanup accepts the following flags:
+	cmdCleanup.Flags.BoolVar(&cleanupFlag, "gc", false, "uninstall profile targets that are older than the current default")
+	cmdCleanup.Flags.BoolVar(&specificVersionsFlag, "ensure-specific-versions-are-set", false, "ensure that profile targets have a specific version set")
 }
 
 func runList(env *cmdline.Env, args []string) error {
@@ -263,7 +259,7 @@
 					out.WriteString(fmtHeader(name, target))
 					out.WriteString(" ")
 				}
-				r, err := fmtInfo(mgr, profile, target)
+				r, err := fmtInfo(ctx, mgr, profile, target)
 				if err != nil {
 					return err
 				}
@@ -272,7 +268,7 @@
 					out.WriteString("\n")
 				}
 			}
-			fmt.Fprint(ctx.Stdout(), fmtOutput(ctx, out.String()))
+			fmt.Fprint(ctx.Stdout(), out.String())
 		}
 	}
 	return nil
@@ -291,6 +287,7 @@
 		InstallationDir string
 		CommandLineEnv  []string
 		Env             []string
+		Command         string
 	}
 	Profile struct {
 		Description    string
@@ -307,13 +304,14 @@
 	Target.InstallationDir - the installation directory of the requested profile.
 	Target.CommandLineEnv - the environment variables specified via the command line when installing this profile target.
 	Target.Env - the environment variables computed by the profile installation process for this target.
-	Note: if no --target is specified then metadata for all targets will be displayed.
+	Target.Command - a command that can be used to create this profile.
+	Note: if no --target is specified then the requested field will be displayed for all targets.
 	Profile.Description - description of the requested profile.
 	Profile.Root - the root directory of the requested profile.
 	Profile.Versions - the set of supported versions for this profile.
 	Profile.DefaultVersion - the default version of the requested profile.
 	Profile.LatestVersion - the latest version available for the requested profile.
-	Note: if no profiles are specified then metadata for all profiles will be displayed.`
+	Note: if no profiles are specified then the requested field will be displayed for all profiles.`
 }
 
 func fmtOutput(ctx *tool.Context, o string) string {
@@ -331,27 +329,39 @@
 	return out.String()
 }
 
-func fmtInfo(mgr profiles.Manager, profile *profiles.Profile, target *profiles.Target) (string, error) {
+func fmtInfo(ctx *tool.Context, mgr profiles.Manager, profile *profiles.Profile, target *profiles.Target) (string, error) {
 	if len(infoFlag) > 0 {
+		// Populate an instance listInfo
 		info := &listInfo{}
-		info.SchemaVersion = profiles.SchemaVersion()
-		if target != nil {
-			info.Target.InstallationDir = target.InstallationDir
-			info.Target.CommandLineEnv = target.CommandLineEnv().Vars
-			info.Target.Env = target.Env.Vars
-		}
-		if profile != nil {
-			info.Profile.Root = profile.Root
-		}
+		name := ""
 		if mgr != nil {
-			info.Profile.Description = mgr.Info()
+			// Format the description on its own, without any preceeding
+			// text so that the line breaks work correctly.
+			info.Profile.Description = "\n" + fmtOutput(ctx, mgr.Info()) + "\n"
 			vi := mgr.VersionInfo()
 			if supported := vi.Supported(); len(supported) > 0 {
 				info.Profile.Versions = supported
 				info.Profile.LatestVersion = supported[0]
 			}
 			info.Profile.DefaultVersion = vi.Default()
+			name = mgr.Name()
 		}
+		info.SchemaVersion = profiles.SchemaVersion()
+		if target != nil {
+			info.Target.InstallationDir = target.InstallationDir
+			info.Target.CommandLineEnv = target.CommandLineEnv().Vars
+			info.Target.Env = target.Env.Vars
+			clenv := ""
+			if len(info.Target.CommandLineEnv) > 0 {
+				clenv = fmt.Sprintf(" --env=\"%s\" ", strings.Join(info.Target.CommandLineEnv, ","))
+			}
+			info.Target.Command = fmt.Sprintf("jiri v23-profile install --target=%s %s%s", target, clenv, name)
+		}
+		if profile != nil {
+			info.Profile.Root = profile.Root
+		}
+
+		// Use a template to print out any field in our instance of listInfo.
 		tmpl, err := template.New("list").Parse("{{ ." + infoFlag + "}}")
 		if err != nil {
 			return "", err
@@ -365,34 +375,6 @@
 	return "", nil
 }
 
-func runRecreate(env *cmdline.Env, args []string) error {
-	ctx := tool.NewContextFromEnv(env)
-	if err := profiles.Read(ctx, manifestFlag); err != nil {
-		fmt.Fprintf(ctx.Stderr(), "Failed to read manifest: %v", err)
-		return err
-	}
-	profileNames := args
-	if len(args) == 0 {
-		profileNames = profiles.Profiles()
-	}
-	prefix := "jiri v23-profile install"
-	for _, name := range profileNames {
-		profile := profiles.LookupProfile(name)
-		if profile == nil {
-			return fmt.Errorf("Profile %v is not installed", name)
-		}
-		for _, target := range profile.Targets() {
-			fmt.Fprintf(ctx.Stdout(), "%s --target=%s", prefix, target)
-			cmdEnv := target.CommandLineEnv()
-			if len(cmdEnv.Vars) > 0 {
-				fmt.Fprintf(ctx.Stdout(), " --env=\"%s\"", strings.Join(cmdEnv.Vars, ","))
-			}
-			fmt.Fprintf(ctx.Stdout(), " %s\n", name)
-		}
-	}
-	return nil
-}
-
 func runEnv(env *cmdline.Env, args []string) error {
 	if len(profilesFlag) == 0 {
 		return fmt.Errorf("no profiles were specified using --profiles")
@@ -494,14 +476,7 @@
 	return profiles.Write(ctx, manifestFlag)
 }
 
-func runCleanup(env *cmdline.Env, args []string) error {
-	ctx := tool.NewContextFromEnv(env)
-	if len(args) == 0 {
-		args = profiles.Managers()
-	}
-	if err := initCommand(ctx, args); err != nil {
-		return err
-	}
+func runGC(ctx *tool.Context, args []string) error {
 	for _, n := range args {
 		mgr := profiles.LookupManager(n)
 		vi := mgr.VersionInfo()
@@ -509,13 +484,72 @@
 		for _, target := range profile.Targets() {
 			if vi.IsOlderThanDefault(target.Version()) {
 				err := mgr.Uninstall(ctx, *target)
-				logResult(ctx, "Cleanup", mgr, *target, err)
+				logResult(ctx, "gc", mgr, *target, err)
 				if err != nil {
 					return err
 				}
 			}
 		}
 	}
+	return nil
+}
+
+func runEnsureVersionsAreSet(ctx *tool.Context, args []string) error {
+	for _, name := range args {
+		profile := profiles.LookupProfile(name)
+		mgr := profiles.LookupManager(name)
+		if mgr == nil {
+			fmt.Fprintf(ctx.Stderr(), "%s is not linked into this binary", name)
+			continue
+		}
+		for _, target := range profile.Targets() {
+			if len(target.Version()) == 0 {
+				prior := target
+				version, err := mgr.VersionInfo().Select(target.Version())
+				if err != nil {
+					return err
+				}
+				target.SetVersion(version)
+				if verboseFlag {
+					fmt.Fprintf(ctx.Stdout(), "%s %s had no version, now set to: %s\n", name, prior, target)
+				}
+			}
+		}
+	}
+	return nil
+}
+
+func runCleanup(env *cmdline.Env, args []string) error {
+	ctx := tool.NewContextFromEnv(env)
+	if err := profiles.Read(ctx, manifestFlag); err != nil {
+		fmt.Fprintf(ctx.Stderr(), "Failed to read manifest: %v", err)
+		return err
+	}
+	if len(args) == 0 {
+		args = profiles.Profiles()
+	}
+	dirty := false
+	if specificVersionsFlag {
+		if verboseFlag {
+			fmt.Fprintf(ctx.Stdout(), "Ensuring that all targets have a specific version set for %s\n", args)
+		}
+		if err := runEnsureVersionsAreSet(ctx, args); err != nil {
+			return fmt.Errorf("ensure-specific-versions-are-set: %v", err)
+		}
+		dirty = true
+	}
+	if cleanupFlag {
+		if verboseFlag {
+			fmt.Fprintf(ctx.Stdout(), "Removing targets older than the default version for %s\n", args)
+		}
+		if err := runGC(ctx, args); err != nil {
+			return fmt.Errorf("gc: %v", err)
+		}
+		dirty = true
+	}
+	if !dirty {
+		return fmt.Errorf("at least one option must be specified")
+	}
 	return profiles.Write(ctx, manifestFlag)
 }
 
diff --git a/profiles/env_test.go b/profiles/env_test.go
index d1975f3..85154bf 100644
--- a/profiles/env_test.go
+++ b/profiles/env_test.go
@@ -46,9 +46,9 @@
 	profiles.InstallProfile("a", "root")
 	profiles.InstallProfile("b", "root")
 	t1, t2 := &profiles.Target{}, &profiles.Target{}
-	t1.Set("t1=cpu1-os1")
+	t1.Set("t1=cpu1-os1@1")
 	t1.Env.Set("A=B C=D,B=C Z=Z")
-	t2.Set("t1=cpu1-os1")
+	t2.Set("t1=cpu1-os1@1")
 	t2.Env.Set("A=Z,B=Z,Z=Z1")
 	profiles.AddProfileTarget("a", *t1)
 	profiles.AddProfileTarget("b", *t2)
diff --git a/profiles/manager_test.go b/profiles/manager_test.go
index 167dcbd..04e53cb 100644
--- a/profiles/manager_test.go
+++ b/profiles/manager_test.go
@@ -83,7 +83,7 @@
 		profiles.Register(myProfile, newProfile(myProfile))
 		flags := flag.NewFlagSet("example", flag.ContinueOnError)
 		profiles.RegisterTargetAndEnvFlags(flags, &target)
-		flags.Parse([]string{"--target=myTarget=arm-linux", "--env=A=B,C=D", "--env=E=F"})
+		flags.Parse([]string{"--target=myTarget=arm-linux@1", "--env=A=B,C=D", "--env=E=F"})
 	}
 	init()
 
@@ -127,10 +127,10 @@
 
 	// Output:
 	// Profile: myNewProfile: installed
-	// [myTarget=arm-linux@]
+	// [myTarget=arm-linux@1]
 	//
 	// Profile: myNewProfile: installed
-	// [myTarget=arm-linux@]
+	// [myTarget=arm-linux@1]
 	//
 	// Profile: myNewProfile: uninstalled
 	//
diff --git a/profiles/manifest.go b/profiles/manifest.go
index da6e0e5..4378a14 100644
--- a/profiles/manifest.go
+++ b/profiles/manifest.go
@@ -282,6 +282,9 @@
 
 		for _, target := range profile.targets {
 			sort.Strings(target.Env.Vars)
+			if len(target.version) == 0 {
+				return fmt.Errorf("missing version for profile %s target: %s", name, target)
+			}
 			schema.Profiles[i].Targets = append(schema.Profiles[i].Targets,
 				&targetSchema{
 					Tag:             target.tag,
diff --git a/profiles/manifest_test.go b/profiles/manifest_test.go
index 1dbfb36..c6eae49 100644
--- a/profiles/manifest_test.go
+++ b/profiles/manifest_test.go
@@ -20,7 +20,7 @@
 )
 
 func addProfileAndTargets(t *testing.T, name string) {
-	t1, _ := profiles.NewTargetWithEnv("t1=cpu1-os1", "A=B,C=D")
+	t1, _ := profiles.NewTargetWithEnv("t1=cpu1-os1@1", "A=B,C=D")
 	t2, _ := profiles.NewTargetWithEnv("t2=cpu2-os2@bar", "A=B,C=D")
 	if err := profiles.AddProfileTarget(name, t1); err != nil {
 		t.Fatal(err)
@@ -67,9 +67,21 @@
 	defer os.RemoveAll(filepath.Dir(filename))
 	ctx := tool.NewDefaultContext()
 
+	// test for no version being set.
+	t1, _ := profiles.NewTargetWithEnv("t1=cpu1-os1", "A=B,C=D")
+	if err := profiles.AddProfileTarget("b", t1); err != nil {
+		t.Fatal(err)
+	}
+	if err := profiles.Write(ctx, filename); err == nil || !strings.HasPrefix(err.Error(), "missing version for profile") {
+		t.Fatalf("was expecing a missing version error, but got %v", err)
+	}
+	profiles.RemoveProfileTarget("b", t1)
+
 	addProfileAndTargets(t, "b")
 	addProfileAndTargets(t, "a")
-	profiles.Write(ctx, filename)
+	if err := profiles.Write(ctx, filename); err != nil {
+		t.Fatal(err)
+	}
 
 	g, _ := ioutil.ReadFile(filename)
 	w, _ := ioutil.ReadFile("./testdata/m1.xml")
@@ -153,7 +165,7 @@
 	defer os.RemoveAll(filepath.Dir(filename))
 
 	var t1 profiles.Target
-	t1.Set("tag=cpu,os")
+	t1.Set("tag=cpu-os@1")
 	profiles.AddProfileTarget("__first", t1)
 
 	if err := profiles.Write(ctx, filename); err != nil {
diff --git a/profiles/testdata/m1.xml b/profiles/testdata/m1.xml
index 45979f8..f86975a 100644
--- a/profiles/testdata/m1.xml
+++ b/profiles/testdata/m1.xml
@@ -1,6 +1,6 @@
 <profiles version="3">
   <profile name="a" root="">
-    <target tag="t1" arch="cpu1" os="os1" installation-directory="" version="">
+    <target tag="t1" arch="cpu1" os="os1" installation-directory="" version="1">
       <envvars></envvars>
       <command-line>
         <var>A=B</var>
@@ -16,7 +16,7 @@
     </target>
   </profile>
   <profile name="b" root="">
-    <target tag="t1" arch="cpu1" os="os1" installation-directory="" version="">
+    <target tag="t1" arch="cpu1" os="os1" installation-directory="" version="1">
       <envvars></envvars>
       <command-line>
         <var>A=B</var>
diff --git a/profiles/util.go b/profiles/util.go
index 4be9a35..2ce3e91 100644
--- a/profiles/util.go
+++ b/profiles/util.go
@@ -222,6 +222,11 @@
 	if mgr == nil {
 		return fmt.Errorf("profile %v is not supported", profile)
 	}
+	version, err := mgr.VersionInfo().Select(target.Version())
+	if err != nil {
+		return err
+	}
+	target.SetVersion(version)
 	mgr.SetRoot(root)
 	if ctx.Run().Opts().Verbose || ctx.Run().Opts().DryRun {
 		fmt.Fprintf(ctx.Stdout(), "install %s %s\n", profile, target.DebugString())
@@ -245,6 +250,11 @@
 	if mgr == nil {
 		return fmt.Errorf("profile %v is not supported", profile)
 	}
+	version, err := mgr.VersionInfo().Select(target.Version())
+	if err != nil {
+		return err
+	}
+	target.SetVersion(version)
 	mgr.SetRoot(root)
 	if ctx.Run().Opts().Verbose || ctx.Run().Opts().DryRun {
 		fmt.Fprintf(ctx.Stdout(), "uninstall %s %s\n", profile, target.DebugString())