Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(copybara): sync commits from Aspect-internal silo #647

Merged
merged 6 commits into from
Mar 7, 2024

Conversation

gregmagolan
Copy link
Member

@gregmagolan gregmagolan commented Mar 7, 2024

Bazel Open Source Support customer waiting on the

fix: handle BEP lint results concurrently

change in this sync.

Commit range https://github.com/aspect-build/silo/compare/b2e57a7be83f5e3a055c1f37287c647c0b363c59..650e805ea86d684dc89bbbdcd74b359b19b7dcf2

… don't bother opening it (#4670)

Improves an issue where CLI tries to read report files before bazel
writes them to disk. This doesn't solve the issue but at least it
reduces the number of failures and it is a principled fix one way or the
other since there is no reason to read an empty report file.

See
https://aspect-build.slack.com/archives/C03LLCZPA3D/p1709017490887659?thread_ts=1708534808.122809&cid=C03LLCZPA3D
for more context on the reading files from disk race condition.

---

### Type of change

- Performance (a code change that improves performance)

### Test plan

- Manual testing; please provide instructions so we can reproduce:

```
pro lint --aspect:lock_version //workflows/scaling/... --config=remote
```

GitOrigin-RevId: 650e805ea86d684dc89bbbdcd74b359b19b7dcf2
…aspect:lock_version flag is set (#4759)

This current output
```
Locking Aspect CLI version to 1.2.3 which differs from the version configured in .bazeliskrc or the Aspect CLI config file
```
isn't really useful and its spammy on all of our CI logs. You'll know if
you set `--aspect:lock_version` so the CLI doesn't have to echo that
back to you.

---

### Type of change

- Refactor

**For changes visible to end-users**

- Suggested release notes are provided below:

Aspect CLI no longer prints out "Locking Aspect CLI version to..." when
the `--aspect:lock_version` is set and it prevents using the version
specified in `.bazeliskrc` or the Aspect CLI config file.

### Test plan

- Covered by existing test cases

GitOrigin-RevId: 8c6c8d9c5b8161ce6999dceafcdcf270baf2ca21
…n a bazel command fails (#4760)

This additional line to stderr is spammy and unnecessary and ends up
making the stdout/stderr output of commands such as `bazel run
//:gazelle` harder to parse.

---

### Type of change

- Refactor (a code change that neither fixes a bug or adds a new
feature)

**For changes visible to end-users**

- Suggested release notes are provided below:

Aspect CLI will no longer print out `Error: bazel exited with exit code:
1` when the underlying bazel command fails. This additional line to
stderr is spammy and unnecessary and ends up making the stdout/stderr
output of commands such as `bazel run //:gazelle` harder to parse.

### Test plan

- Covered by existing test cases

GitOrigin-RevId: e019efa35724e2615f8d35d0f9567823919d3e4b
Resolves the case where the BEP event comes in for the results file and
the Aspect CLI attempts to read the file _but_ the file is still being
downloaded by bazel's remote cache downloader and has not yet been
written to disk.

---

### Type of change

- Bug fix (change which fixes an issue)

### Test plan

- Manual testing; please provide instructions so we can reproduce:

```
aspect lint //... --config=remote
```

GitOrigin-RevId: 88d14c9d79fe9fbd94893454a84ee527c53ebbae
@jbedard
Copy link
Member

jbedard commented Mar 7, 2024

The "tidy" commit is the extra commit (not from copybara) to get things green? I guess that's all things like lint that fail (because /cli is dropped form the path) and generated files?

Copy link
Member

@jbedard jbedard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the commits look good, as long as you've taken a pass as well to make sure commits aren't pulling in extras that should be in a separate cleanup commit.

@gregmagolan gregmagolan merged commit 660b39c into main Mar 7, 2024
8 of 9 checks passed
@gregmagolan gregmagolan deleted the 9E6ED7960E7C1C7500F98C1F23B6018B branch March 7, 2024 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants