jiri: Fix upgrade and update for new-style manifests

The old-style manifests are required to live under
JIRI_ROOT/.manifest/v2, and use the <import name="foo"/> style.

The new-style manifests live where the remote import specifies,
and use <import/> for remote imports, and <fileimport/> for file
imports.

I've already pushed new-style manifests under the top-level
manifest repo directory, changing all old-style name imports to
new-style file imports.  This CL changes the upgrade logic to
write a .jiri_manifest that points to these new-style manifests.
At some point we'll remove the old-style manifests, but in the
interim we'll have both, to allow a gradual transition.

Also fixed a bug in the "jiri update" logic.  The buggy logic
wrote the update_history snapshot *before* doing the update.  But
in order to write a snapshot, the current manifest must be read
to capture the tools, hosts and hooks, since those aren't
available by scanning the filesystem.

The old logic is buggy because the user may have changed which
manifest(s) to use, thus the tools, hosts and hooks would be for
the new manifest(s), not the original one.  This bug was exposed
when using new-style manifests, since after "jiri upgrade v23",
we'd try to run "jiri update", which would fail because the new
manifest repo wasn't cloned yet.

The fix is simple - we just capture the update_history
snapshot *after* doing an update, rather than before.  This is a
better match for the intention of update_history anyways; it
represents the last-known-good snapshot.

In theory we could probably change the update_history snapshot
logic to just write out the in-memory manifest that we've
collected by performing the update.  We can consider that change
in subsequent cleanup.

Change-Id: I601bb70c4c13c2d770dae8e06b7be2d092271aa2
diff --git a/update.go b/update.go
index 9e126ad..05f46dd 100644
--- a/update.go
+++ b/update.go
@@ -53,22 +53,19 @@
 		return err
 	}
 
+	// Update all projects to their latest version.
+	// Attempt <attemptsFlag> times before failing.
+	updateFn := func() error { return project.UpdateUniverse(jirix, gcFlag) }
+	if err := retry.Function(jirix.Context, updateFn, retry.AttemptsOpt(attemptsFlag)); err != nil {
+		return err
+	}
 	// Create a snapshot of the current state of all projects and write it to the
 	// update history directory.
 	snapshotFile := filepath.Join(jirix.UpdateHistoryDir(), time.Now().Format(time.RFC3339))
 	if err := project.CreateSnapshot(jirix, snapshotFile); err != nil {
 		return err
 	}
-
-	// Update all projects to their latest version.
-	// Attempt <attemptsFlag> times before failing.
-	updateFn := func() error {
-		if err := project.UpdateUniverse(jirix, gcFlag); err != nil {
-			return err
-		}
-		// We're careful to only attempt the bin dir transition after the update has
-		// succeeded, to avoid messy partial states.
-		return project.TransitionBinDir(jirix)
-	}
-	return retry.Function(jirix.Context, updateFn, retry.AttemptsOpt(attemptsFlag))
+	// Only attempt the bin dir transition after the update has succeeded, to
+	// avoid messy partial states.
+	return project.TransitionBinDir(jirix)
 }
diff --git a/upgrade.go b/upgrade.go
index 273e83d..1209c4f 100644
--- a/upgrade.go
+++ b/upgrade.go
@@ -81,15 +81,16 @@
 	if len(args) != 1 {
 		return jirix.UsageErrorf("must specify upgrade kind")
 	}
+	kind := args[0]
 	var argRemote, argRoot, argPath, argManifest string
-	switch kind := args[0]; kind {
+	switch kind {
 	case "v23":
 		argRemote = "https://vanadium.googlesource.com/manifest"
-		argRoot, argPath, argManifest = "", "manifest", "v2/default"
+		argRoot, argPath, argManifest = "", "manifest", "public"
 	case "fuchsia":
 		// TODO(toddw): Confirm these choices.
 		argRemote = "https://github.com/effenel/fnl-start.git"
-		argRoot, argPath, argManifest = "", "manifest", "v2/default"
+		argRoot, argPath, argManifest = "", "manifest", "default"
 	default:
 		return jirix.UsageErrorf("unknown upgrade kind %q", kind)
 	}
@@ -104,19 +105,25 @@
 		seenOldImport := false
 		var newImports []project.Import
 		for _, oldImport := range manifest.Imports {
-			switch {
-			case oldImport.Remote != "":
+			if oldImport.Remote != "" {
 				// This is a new-style remote import, carry it over directly.
 				newImports = append(newImports, oldImport)
-			case !seenOldImport:
+				continue
+			}
+			// This is an old-style file import, convert it to the new style.
+			oldName := oldImport.Name
+			if kind == "v23" && oldName == "default" {
+				oldName = "public" // default no longer exists, now it's just public.
+			}
+			if !seenOldImport {
 				// This is the first old import, update the manifest name for the remote
 				// import we'll be adding later.
-				argManifest = filepath.Join("v2", oldImport.Name)
+				argManifest = oldName
 				seenOldImport = true
-			default:
-				// Convert import from name="foo" to file="manifest/v2/foo"
+			} else {
+				// Convert import from name="foo" to file="manifest/foo"
 				manifest.FileImports = append(manifest.FileImports, project.FileImport{
-					File: filepath.Join(argRoot, argPath, "v2", oldImport.Name),
+					File: filepath.Join(argRoot, argPath, oldName),
 				})
 			}
 		}
diff --git a/upgrade_test.go b/upgrade_test.go
index a94e276..f9e5733 100644
--- a/upgrade_test.go
+++ b/upgrade_test.go
@@ -41,7 +41,7 @@
 			Args: []string{"v23"},
 			Want: `<manifest>
   <imports>
-    <import manifest="v2/default" remote="https://vanadium.googlesource.com/manifest"/>
+    <import manifest="public" remote="https://vanadium.googlesource.com/manifest"/>
   </imports>
 </manifest>
 `,
@@ -56,7 +56,7 @@
 `,
 			Want: `<manifest>
   <imports>
-    <import manifest="v2/default" remote="https://vanadium.googlesource.com/manifest"/>
+    <import manifest="public" remote="https://vanadium.googlesource.com/manifest"/>
   </imports>
 </manifest>
 `,
@@ -71,7 +71,7 @@
 `,
 			Want: `<manifest>
   <imports>
-    <import manifest="v2/private" remote="https://vanadium.googlesource.com/manifest"/>
+    <import manifest="private" remote="https://vanadium.googlesource.com/manifest"/>
   </imports>
 </manifest>
 `,
@@ -88,9 +88,9 @@
 `,
 			Want: `<manifest>
   <imports>
-    <import manifest="v2/private" remote="https://vanadium.googlesource.com/manifest"/>
-    <fileimport file="manifest/v2/infrastructure"/>
-    <fileimport file="manifest/v2/default"/>
+    <import manifest="private" remote="https://vanadium.googlesource.com/manifest"/>
+    <fileimport file="manifest/infrastructure"/>
+    <fileimport file="manifest/public"/>
   </imports>
 </manifest>
 `,
@@ -107,9 +107,9 @@
 `,
 			Want: `<manifest>
   <imports>
-    <import manifest="v2/default" remote="https://vanadium.googlesource.com/manifest"/>
-    <fileimport file="manifest/v2/infrastructure"/>
-    <fileimport file="manifest/v2/private"/>
+    <import manifest="public" remote="https://vanadium.googlesource.com/manifest"/>
+    <fileimport file="manifest/infrastructure"/>
+    <fileimport file="manifest/private"/>
   </imports>
 </manifest>
 `,
@@ -124,7 +124,7 @@
 			Args: []string{"fuchsia"},
 			Want: `<manifest>
   <imports>
-    <import manifest="v2/default" remote="https://github.com/effenel/fnl-start.git"/>
+    <import manifest="default" remote="https://github.com/effenel/fnl-start.git"/>
   </imports>
 </manifest>
 `,
@@ -139,7 +139,7 @@
 `,
 			Want: `<manifest>
   <imports>
-    <import manifest="v2/default" remote="https://github.com/effenel/fnl-start.git"/>
+    <import manifest="default" remote="https://github.com/effenel/fnl-start.git"/>
   </imports>
 </manifest>
 `,
@@ -154,7 +154,7 @@
 `,
 			Want: `<manifest>
   <imports>
-    <import manifest="v2/private" remote="https://github.com/effenel/fnl-start.git"/>
+    <import manifest="private" remote="https://github.com/effenel/fnl-start.git"/>
   </imports>
 </manifest>
 `,
@@ -171,9 +171,9 @@
 `,
 			Want: `<manifest>
   <imports>
-    <import manifest="v2/private" remote="https://github.com/effenel/fnl-start.git"/>
-    <fileimport file="manifest/v2/infrastructure"/>
-    <fileimport file="manifest/v2/default"/>
+    <import manifest="private" remote="https://github.com/effenel/fnl-start.git"/>
+    <fileimport file="manifest/infrastructure"/>
+    <fileimport file="manifest/default"/>
   </imports>
 </manifest>
 `,
@@ -190,9 +190,9 @@
 `,
 			Want: `<manifest>
   <imports>
-    <import manifest="v2/default" remote="https://github.com/effenel/fnl-start.git"/>
-    <fileimport file="manifest/v2/infrastructure"/>
-    <fileimport file="manifest/v2/private"/>
+    <import manifest="default" remote="https://github.com/effenel/fnl-start.git"/>
+    <fileimport file="manifest/infrastructure"/>
+    <fileimport file="manifest/private"/>
   </imports>
 </manifest>
 `,
@@ -267,7 +267,7 @@
 	localData := `<manifest/>`
 	jiriData := `<manifest>
   <imports>
-    <import manifest="v2/default" remote="https://vanadium.googlesource.com/manifest"/>
+    <import manifest="public" remote="https://vanadium.googlesource.com/manifest"/>
   </imports>
 </manifest>
 `