Don't crash reading the config of a subrepo with no remote tree (#3577)

Building a Rust plugin against a repo with ~100 crate subrepos, we hit
an intermittent segfault under remote execution whenever Please read a
subrepo's config:

```
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18]
please/src/remote/fs.New({...}, 0x0, {...})     fs.go:67
please/src/remote.(*Client).SubrepoFS(...)      remote.go:330
please/src/core.(*Subrepo).FS.func1()           subrepo.go:58
please/src/core.readSubrepoConfig(...)          subrepo.go:119
```

Note the literal `0x0` argument. `SubrepoFS` looks the subrepo's output
tree up by label and passes whatever it finds straight to
`remotefs.New`:

```go
tree := c.subrepoTrees[target.Label]
return remotefs.New(c.remoteFSClient, tree, root)
```

`subrepoTrees` is only ever populated in one place (`utils.go`, when a
target's outputs are set from a *remote* build), so a subrepo that was
built locally, served from the cache, or not built yet has no entry, and
`tree` is nil. `New` then does `append(tree.Children, tree.Root)` and
dies. `readSubrepoConfig` calls `Subrepo.FS()` unconditionally — it has
to, in order to look for `.plzconfig` — so any repo whose subrepo
targets can come from cache rather than a fresh remote build can hit
this.

That matches what we saw: it was sensitive to cache state and flipped
between otherwise identical runs.

This makes a nil tree (or one with no root) produce an empty filesystem,
so the config lookup reports "does not exist" and parsing continues —
which is already the handling for a subrepo that has no `.plzconfig`.
The added test panics at the same `addr=0x18` without the change and
passes with it.

`plz test //src/remote/...` is green.

Author: jackmarsh

src/remote/fs/fs.go

@@ -63,7 +63,16 @@ func (fs *CASFileSystem) Stat(name string) (iofs.FileInfo, error) {
 }
 
 // New creates a new filesystem on top of the given proto, using client to download files from the CAS on demand.
+// A nil tree (or one with no root) yields an empty filesystem: callers such as SubrepoFS look trees up by label and
+// find nothing for a subrepo that wasn't built remotely during this invocation, and an empty filesystem reports that
+// as "does not exist" rather than crashing.
 func New(c Client, tree *pb.Tree, workingDir string) *CASFileSystem {
+	if tree == nil {
+		tree = &pb.Tree{}
+	}
+	if tree.Root == nil {
+		tree.Root = &pb.Directory{}
+	}
 	directories := make(map[digest.Digest]*pb.Directory, len(tree.Children))
 	for _, child := range append(tree.Children, tree.Root) {
 		dg, err := digest.NewFromMessage(child)

src/remote/fs/fs_test.go

@@ -298,3 +298,26 @@ func TestStat(t *testing.T) {
 		})
 	}
 }
+
+func TestNilTree(t *testing.T) {
+	// SubrepoFS looks trees up by label and gets nothing for a subrepo that
+	// wasn't built remotely in this invocation (a cache hit, say). Reading
+	// that subrepo's config then reached here with a nil tree and panicked.
+	fc := &fakeClient{}
+	for _, tc := range []struct {
+		name string
+		tree *pb.Tree
+	}{
+		{name: "nil tree", tree: nil},
+		{name: "nil root", tree: &pb.Tree{}},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			fs := New(fc, tc.tree, ".")
+			require.NotNil(t, fs)
+			_, err := fs.Open(".plzconfig")
+			assert.True(t, os.IsNotExist(err), "expected not-exist, got %v", err)
+			_, err = iofs.Stat(fs, "anything")
+			assert.True(t, os.IsNotExist(err), "expected not-exist, got %v", err)
+		})
+	}
+}