Revert #3111 (#3122)

* Revert "Only download child dirs for filegroups where necessary (#3111)"

This reverts commit eb90915.

* version

ChangeLog

@@ -1,3 +1,7 @@
+Version 17.8.5
+--------------
+ * Revert #3111
+
 Version 17.8.4
 --------------
  * Fix a nil pointer when querying targets with the `--arch` flag (#3113)

VERSION

@@ -1 +1 @@
-17.8.4
+17.8.5

src/remote/action.go

@@ -193,7 +193,7 @@ func (c *Client) uploadInputs(ch chan<- *uploadinfo.Entry, target *core.BuildTar
  if target.IsRemoteFile {
  return &pb.Directory{}, nil
  }
- b, err := c.uploadInputDir(ch, target, isTest, false)
+ b, err := c.uploadInputDir(ch, target, isTest)
  if err != nil {
  return nil, err
  }
@@ -202,7 +202,7 @@ func (c *Client) uploadInputs(ch chan<- *uploadinfo.Entry, target *core.BuildTar
 
 // uploadInputDir uploads the inputs to the build rule. It returns an un-finalised directory builder representing the
 // directory structure of the input dir. The caller is expected to finalise this by calling Build().
-func (c *Client) uploadInputDir(ch chan<- *uploadinfo.Entry, target *core.BuildTarget, isTest, needChildDirs bool) (*dirBuilder, error) {
+func (c *Client) uploadInputDir(ch chan<- *uploadinfo.Entry, target *core.BuildTarget, isTest bool) (*dirBuilder, error) {
  b := newDirBuilder(c)
  for input := range c.state.IterInputs(target, isTest) {
  if l, ok := input.Label(); ok {
@@ -243,7 +243,7 @@ func (c *Client) uploadInputDir(ch chan<- *uploadinfo.Entry, target *core.BuildT
  Name: filepath.Base(d.Name),
  Digest: d.Digest,
  })
- if needChildDirs && target.IsFilegroup {
+ if target.IsFilegroup {
  if err := c.addChildDirs(b, filepath.Join(pkgName, d.Name), d.Digest); err != nil {
  return b, err
  }

src/remote/remote.go

@@ -6,7 +6,6 @@ package remote
 import (
  "context"
  "encoding/hex"
- "errors"
  "fmt"
  iofs "io/fs"
  "os"
@@ -55,8 +54,6 @@ var remoteCacheReadDuration = metrics.NewHistogram(
  metrics.ExponentialBuckets(0.1, 2, 12), // 12 buckets, starting at 0.1ms and doubling in width.
 )
 
-var errMissingOutputFromFilegroup = errors.New("Missing output from filegroup")
-
 // A Client is the interface to the remote API.
 //
 // It provides a higher-level interface over the specific RPCs available.
@@ -904,14 +901,7 @@ func (c *Client) fetchRemoteFile(target *core.BuildTarget, actionDigest *pb.Dige
 
 // buildFilegroup "builds" a single filegroup target.
 func (c *Client) buildFilegroup(target *core.BuildTarget, command *pb.Command, actionDigest *pb.Digest) (*core.BuildMetadata, *pb.ActionResult, error) {
- if metadata, ar, err := c.buildFilegroupWithChildDirs(target, command, actionDigest, false); err == nil || !errors.Is(err, errMissingOutputFromFilegroup) {
- return metadata, ar, err
- }
- return c.buildFilegroupWithChildDirs(target, command, actionDigest, true)
-}
-
-func (c *Client) buildFilegroupWithChildDirs(target *core.BuildTarget, command *pb.Command, actionDigest *pb.Digest, needChildDirs bool) (*core.BuildMetadata, *pb.ActionResult, error) {
- inputDir, err := c.uploadInputDir(nil, target, false, needChildDirs) // We don't need to actually upload the inputs here, that is already done.
+ inputDir, err := c.uploadInputDir(nil, target, false) // We don't need to actually upload the inputs here, that is already done.
  if err != nil {
  return nil, nil, err
  }
@@ -934,8 +924,8 @@ func (c *Client) buildFilegroupWithChildDirs(target *core.BuildTarget, command *
  IsExecutable: f.IsExecutable,
  })
  } else {
- // This might happen with needChildDirs=false, it shouldn't happen otherwise.
- return fmt.Errorf("%w: %s", errMissingOutputFromFilegroup, out)
+ // Of course, we should not get here (classic developer things...)
+ return fmt.Errorf("Missing output from filegroup: %s", out)
  }
  }
  return nil