Skip to content

Commit

Permalink
fix(cli): use localExecRoot if possible when constructing path to res…
Browse files Browse the repository at this point in the history
…ults files (#7299)

Synced from #768 by
withered-magic:
use `localExecRoot` if possible when constructing path to results files
during `aspect lint`

---

Currently, when remote caching is enabled and `readBEPFile` needs to
handle `bytestream` URIs, it constructs the path to the corresponding
results file using the workspace root, such that the resulting path
looks something like
`/path/to/workspace/bazel-out/k8-fastbuild/bin/path/to/file`.

However, this only works if the convenience symlinks are in place; if
the convenience symlinks have been disabled, e.g. if
`--experimental_convenience_symlinks=ignore` is specified in `.bazelrc`,
then such paths are no longer valid, and `aspect lint` fails with the
following sort of error:

```
Error: failed to find lint results file /path/to/workspace/bazel-out/k8-fastbuild/bin/file1.AspectRulesLintESLint.out.exit_code: stat /path/to/workspace/bazel-out/k8-fastbuild/bin/file1.AspectRulesLintESLint.out.exit_code: no such file or directory
```

Therefore, in the case when the convenience symlinks are missing, the
path to the results file must be constructed using the execroot as the
base. Fortunately, since we are already processing BEP events, we can
simply use the `workspaceInfo` event to record the current execroot and
later use it to construct paths as needed.

---

### Changes are visible to end-users: yes

- Searched for relevant documentation and updated as needed: yes
- Breaking change (forces users to change their own code or config): no
- Suggested release notes appear below: yes

### Test plan

- Manual testing; please provide instructions so we can reproduce: Added
a repro with instructions here:
https://github.com/withered-magic/aspect-cli-repro

Closes
[#768](#768)

Co-authored-by: withered-magic <[email protected]>
GitOrigin-RevId: 9141bac4333aeb052c0f9aec1e6b3683a45d2adc
  • Loading branch information
jbedard and withered-magic committed Nov 8, 2024
1 parent f53c08f commit e270b50
Showing 1 changed file with 12 additions and 1 deletion.
13 changes: 12 additions & 1 deletion pkg/aspect/lint/bep.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ type ResultForLabel struct {
type LintBEPHandler struct {
namedSets map[string]*buildeventstream.NamedSetOfFiles
workspaceRoot string
localExecRoot string
besCompleted chan<- struct{}
resultsByLabel map[string]*ResultForLabel
}
Expand Down Expand Up @@ -71,7 +72,14 @@ func (runner *LintBEPHandler) readBEPFile(file *buildeventstream.File) ([]byte,
}
// Because we set --experimental_remote_download_regex, we can depend on the results file being
// in the output tree even when using a remote cache with build without the bytes.
resultsFile = path.Join(runner.workspaceRoot, path.Join(file.PathPrefix...), file.Name)
// If possible, we use the localExecRoot from the workspaceInfo event when constructing the path
// to the results file in case the convenience symlinks are not present, e.g. if
// --experimental_convenience_symlinks=ignore is specified.
root := runner.workspaceRoot
if runner.localExecRoot != "" {
root = runner.localExecRoot
}
resultsFile = path.Join(root, path.Join(file.PathPrefix...), file.Name)
} else {
return nil, fmt.Errorf("unsupported BES file uri %v", f.Uri)
}
Expand Down Expand Up @@ -122,6 +130,9 @@ func parseLinterMnemonicFromFilename(filename string) string {
func (runner *LintBEPHandler) bepEventCallback(event *buildeventstream.BuildEvent) error {
switch event.Payload.(type) {

case *buildeventstream.BuildEvent_WorkspaceInfo:
runner.localExecRoot = event.GetWorkspaceInfo().GetLocalExecRoot()

case *buildeventstream.BuildEvent_NamedSetOfFiles:
runner.namedSets[event.Id.GetNamedSet().Id] = event.GetNamedSetOfFiles()

Expand Down

0 comments on commit e270b50

Please sign in to comment.