-
Notifications
You must be signed in to change notification settings - Fork 24
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
process: sync commits from Aspect-internal silo #773
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jbedard
force-pushed
the
788EA150BB0FAAB4A3FF81F26DCBE189
branch
from
November 7, 2024 23:16
1847662
to
9ab16c2
Compare
It is complicated to present these: - markdown doesn't look great in a terminal output - the column width needs artificial limitation for terminal display - it's hard to discover they exist when running 'aspect help' - we want to show them on the docsite also, which makes it harder for non-programmer tech writers to edit - when/if we rewrite the CLI core, we'll probably want a different way - we already have 'aspect docs' that can open things in the browser Replaces #7100 Note that it has a subtle effect of: - removing a copy of these from being Apache 2 OSS in https://github.com/aspect-build/aspect-cli/tree/main/docs/help/topics - These are now versioned at head, rather than with the version of the CLI you're running. --- ### Changes are visible to end-users: no GitOrigin-RevId: d7dcf02fa93dcf8fc1776deb1372b9715ca7b49a
--- ### Changes are visible to end-users: no GitOrigin-RevId: 2d61f9d1e0c67bfe977fe561f018fea91c445753
When the `bazel lint` run finishes and we are waiting for the BES to complete, we had a 60 millisecond timeout. Increase this timeout to 60 seconds. Hit this timeout at a customer when we should not have Customer thread: https://aspect-build.slack.com/archives/C03LLCZPA3D/p1728675490951849 --- ### Changes are visible to end-users: yes - Searched for relevant documentation and updated as needed: no - Breaking change (forces users to change their own code or config): no - Suggested release notes appear below: yes Increase linting BES completion timeout to 60 seconds ### Test plan - Covered by existing test cases GitOrigin-RevId: 851b34257be6577ebf5358ba9c43d6121ccc1698
…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 <withering.magic@gmail.com> GitOrigin-RevId: 9141bac4333aeb052c0f9aec1e6b3683a45d2adc
Close #771 --- ### 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 Fix the use of cli empty-string arguments. ### Test plan - Manual testing; please provide instructions so we can reproduce: passed `""` to various CLI commands and verified it gets passed into the command code GitOrigin-RevId: 08aa6c34918dd9858a4124e77274fd816745c0cd
jbedard
force-pushed
the
788EA150BB0FAAB4A3FF81F26DCBE189
branch
from
November 7, 2024 23:43
22e55f6
to
7e90c45
Compare
alexeagle
approved these changes
Nov 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Commit range https://github.com/aspect-build/silo/compare/fae3a0d76857bdaf185fc30d7953c7da538521ba..08aa6c34918dd9858a4124e77274fd816745c0cd