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

[6.2.0] Remove actionId from RemoteFileArtifactValue. #17724

Merged
merged 1 commit into from
Mar 10, 2023

Conversation

tjgq
Copy link
Contributor

@tjgq tjgq commented Mar 10, 2023

actionId was added by #11236 to track download requests for input prefetches.

However, according to the REAPI spec, action_id is

An identifier that ties multiple requests to the same action. For example, multiple requests to the CAS, Action Cache, and Execution API are used in order to compile foo.cc.

The input prefetches are requested by local actions that depend on outputs of remote executed action (which is identified by the action_id). These requests are not part of generating action hence shouldn't be bound with the action_id.

We could include ActionExecutionMetadata of the local action for the requests for the tracking purpose, but that desires another CL.

PiperOrigin-RevId: 510430853
Change-Id: I8d64a4f7033320a394e02a2b11498f09a3d15310

actionId was added by bazelbuild#11236 to track download requests for input prefetches.

However, according to the REAPI spec, action_id is

> An identifier that ties multiple requests to the same action. For example, multiple requests to the CAS, Action Cache, and Execution API are used in order to compile foo.cc.

The input prefetches are requested by local actions that depend on outputs of remote executed action (which is identified by the action_id). These requests are not part of generating action hence shouldn't be bound with the action_id.

We could include ActionExecutionMetadata of the local action for the requests for the tracking purpose, but that desires another CL.

PiperOrigin-RevId: 510430853
Change-Id: I8d64a4f7033320a394e02a2b11498f09a3d15310
@tjgq tjgq requested a review from ShreeM01 as a code owner March 10, 2023 12:37
@sgowroji sgowroji added the team-Remote-Exec Issues and PRs for the Execution (Remote) team label Mar 10, 2023
@ShreeM01 ShreeM01 merged commit 60749d5 into bazelbuild:release-6.2.0 Mar 10, 2023
copybara-service bot pushed a commit that referenced this pull request May 9, 2023
Baseline: d60ce2c

Cherry picks:

   + 490f8ba:
     kshyanashree is the release manager of 6.0.0
   + 4e66d93:
     Automated rollback of commit
     2a28909.
   + 48a8d01:
     Allow C/C++ coverage collection for external targets
   + ec7be34:
     Temporarily set parent directory of the input to writable if it
     is not.
   + b098434:
     Infer CPUs for tvOS and watchOS when on Apple Silicon
   + 5cea7dd:
     Improve the documentation for ctx.actions.symlink.
   + a82d26f:
     Add quotes to "Tip"
   + 75b0f3a:
     Write a repo mapping manifest in the runfiles directory (#16555)
   + 07c5c1a:
     Ensure repository names don't start with `~`
   + 30f6c82:
     Escape tilde more gracefully
   + cf3f48c:
     Relax `Label` repo visibility validation
   + 4477823:
     Use "apparent name" instead of "local name" in Bzlmod docs
   + 55f4f48:
     Bazel DevSite: Add required css file.
   + a706994:
     Remove logic that increases delay between progress updates over
     time
   + 1d514ab:
     Remove references to io_bazel repository
   + b0357bd:
     Relnotes python script
   + 8f95651:
     Allow Java coverage collection for external targets
   + bddb191:
     Fix race condition in package-bazel.sh
   + a757852:
     Decanonicalize labels emitted by {a,c,}query if possible
   + 9d250ed:
     Add uniquify parameter to TemplateDict.add_joined
   + f02bcf8:
     Fix identical gcov json file name problem
   + a159330:
     Add `$(rlocationpath(s) ...)` expansion
   + 56f54da:
     Rollup of SBOM correctness fixes (#16655)
   + 72e6e94:
     [cc] Add cc toolchain to starlark cc_proto_library (#16661)
   + 8f28513:
     Make C++ runfiles library repo mapping aware
   + 95f9adc:
     Always collect FileProvider's filesToBuild as data runfiles
   + 4959d34:
     fix: export constraints.bzl file from @local_config_platform so
     it can be used in downstream bzl_library targets
   + 4919d4a:
     Add --host_per_file_copt (#16695)
   + 0a23d46:
     Moving some tests for `RemoteActionFileSystem` of BwoB to a base
     class.
   + 9296068:
     [remote/downloader] Don't include headers in `FetchBlobRequest`
   + 128d833:
     [remote] Respect whether the server supports action cache updates
   + 38c5019:
     [remote/downloader] Migrate `Downloader` to take `Credentials`
     (#16732)
   + 5929cb7:
     Stage repository mapping manifest as a root symlink (#16733)
   + 455454a:
     Expose current repository name to Java with @AutoBazelRepository
   + 97dea59:
     Implement getDirectoryEntries and readdir for
     RemoteActionFileSystem.
   + aa45f5f:
     Move integration tests for BwoB to a base class and add more
     tests there.
   + 1b073ac:
     Make Java runfiles library repo mapping aware
   + 148bbb1:
     Use `_repo_mapping` in C++ runfiles library (#16752)
   + 25558ad:
     Add --experimental_action_cache_store_output_metadata to the
     expansio…
   + 6e945e8:
     Treat `DEBUG` events as progress-like.
   + 1940c5d:
     redact 'token' strings from logging
   + 0b64525:
     Make Bash runfiles library repo mapping aware (#16753)
   + 4caae75:
     Automated rollback of commit
     8f95651.
   + 13ff6d9:
     Fix Bash rlocation failure with stricter Bash options
   + bd88c7e:
     Make bazel Android remote deps compatible with bzlmod (#16772)
   + 6f6d4cc:
     Flip `--incompatible_always_include_files_in_data`
   + 94355b1:
     Add required `--add-opens` server JVM args also with
     non-embedded JDK
   + 8349c95:
     Also collect clang resource directory with
     `-no-canonical-prefixes`
   + dce6ed7:
     Make `bazel run` works with minimal mode
   + ca8674c:
     Include source files with `cquery --output=files` (#16826)
   + 9cb5e0a:
     Fsync before rename after copy in DiskCacheClient
   + 23ffce5:
     Update GetActionResult for disk cache to check referenced files
     when …
   + 0bc0ff5:
     Make Java runfiles library compilable with JDK 8
   + fe16965:
     [6.0.0] Default --incompatible_strict_conflict_checks to true.
     (#16864)
   + 99ca86f:
     Revert "[6.0.0] Default --incompatible_strict_conflict_checks to
     true. (#16864)" (#16872)
   + 312238f:
     Make C++ runfiles library build with `-Werror=shadow`
   + 2baa5a4:
     Keep credentials cached across build commands.
   + 8818a57:
     [6.0.0] Only inject metadata for outputs that cannot be
     reconstructed by skyf… (#16879)
   + 31e4bf4:
     Update java_tools v11.9
   + cd40666:
     replace _get_canonical_form(label) with str(label)
   + e2bc237:
     Avoid exceptions from hermetic sandbox for unsupported artifact
     subclasses
   + b564d14:
     Remove warnings about ignored starlark options
   + 7ccc661:
     [6.0.0] Emit Tree objects in topological order (#16904)
   + 86dee6d:
     Correctly match regex with tree artifact
   + 14925b5:
     Always use target's attributes to set Python version (#16959)
   + a247502:
     Add 'toolchain' parameter to actions.{run,run_shell}
   + 497b7e4:
     Fix Bash `runfiles_current_repository` with RBE
   + 0a2c4ed:
     Fix corner cases in Bash runfiles library
   + 33b514b:
     Fix runfiles creation with MANIFEST when building without the
     bytes
   + 3a13af4:
     Remove LCOV merger dependency of `cc_test` without coverage
     (#17004)
   + 25ba76c:
     Include full tree artifact in inputs when prefetcher doesn't
     support partial tree artifacts.
   + 06deebf:
     Release 6.0.0 (2022-12-19)
   + c2bfb2e:
     Enable git_submodules tests and merge git_repository and
     starlark_git_repository (#17247)
   + e8182ce:
     have 'bazel test' non-test targets depend on
     --remote_download_output
   + c3245cd:
     Add `SpellChecker` suggestions for common Bzlmod errors
   + 8ec8743:
     Use xcrun to invoke install_name_tool
   + 2b2bea4:
     Extra resources
   + 5b4de12:
     Do not clear `--platforms` on no-op change to `--cpu`
   + db3c013:
     Report `AspectCreationException` to the user
   + 53e9fea:
     Use long executable path instead of `argv[0]` in all launchers
   + abae5ca:
     Add sanitizer features to unix_cc_toolchain_config
   + db68419:
     Allow `map_each` to return `None` in `TemplateDict#add_joined`
   + 66b58ee:
     Always emit trailing zero in human-readable download progress
   + 17b8e44:
     Upload all logs in BEP even with minimal upload
   + 28d2daa:
     Set `--experimental_action_listeners` to default in `exec` config
   + 16427c9:
     Do not count tests as failed that have not started
   + 094a0aa:
     Accept tildes in --override_module
   + 5f2866f:
     Do the AC integrity check for disk part of the combined cache.
   + cd10d50:
     Fix `--nozip_undeclared_test_outputs` on Windows
   + 073f54b:
     Allow pyd in extensions of dynamic libraries
   + ac504cb:
     Register JavaCompileActionContext in Bazel.
   + f090433:
     Rollback #14510 because it causes remote test execution to fail
   + 942ddda:
     Prepare backwards compatible usage of optional C++ toolchain
   + 88b51f5:
     Emit LLVM coverage for source file paths with a `tmp` segment
   + bb0b886:
     Enable archive_param_file feature by default for Windows
   + baf97c0:
     Allow `TemplateDict#map_each` callback to return a list of
     strings (#17306)
   + fcb0077:
     Only try to create groups of test actions in the ui.
   + 3c47b47:
     Find `libtool` when using `BAZEL_USE_CPP_ONLY_TOOLCHAIN`.
   + 1da8a82:
     Add -dead_strip in default opt link flags for darwin
   + b0c5eb3:
     Dont query remote cache but always use bytestream protocol
   + 00e9af1:
     Allow Java coverage collection for external targets.
   + dd24a00:
     Test and fix root symlink edge case in runfiles library
   + af97d98:
     [6.1.0] Fix broken CI (#17422)
   + b859571:
     Add `native.package_relative_label` function
   + 82168d4:
     Make Bazel more responsive and use less memory when --jobs is
     high (#17398)
   + 84c1ed4:
     Multiplex worker fixes and tests for Android busybox tools
     (#17371)
   + 0759081:
     Improve error message for concurrent modifications (#17396)
   + 27bc896:
     [6.1.0] Make bazel coverage work with minimal mode (#17397)
   + 544b816:
     [6.1.0] Fix cc_binary bug related to cc_shared_library on
     Windows and prepare for future removal of
     --experimental_cc_shared_library flag (#17445)
   + 9a7aa21:
     Fix Bash `runfiles_current_repository` for tools
   + 911eedc:
     Fix label unambiguous canonical form to correctly report
     non-visible repo names
   + e132653:
     Remove O1 from sanitizer feature flag defaults
   + ba9e2f8:
     Remove usage of gRPC Context cancellation in the remote
     execution client.
   + de03a23:
     Allow -v to libtool
   + 4e35c02:
     Prettify labels in action progress messages with Bzlmod (#17278)
   + 1be0ac3:
     Expand tree outputs before eagerly prefetching them for local
     actions. (#17494)
   + c266651:
     Fix Java coverage collection with Java 8 runtime
   + 1a6ffe6:
     Add a flag to disable execution log sorting.
   + fbec8e2:
     Reduce flakiness on Windows for BwoB tests
   + 420659a:
     Make AutoBazelRepositoryProcessor compatible with Java 8
   + dde6d20:
     Do not recommend `shallow_since` for `git_repository`
   + f76fc61:
     Allow the timeout value for execute calls on a mac to be set via
     an environment variable
   + 773d232:
     Fix RPATHs for cc toolchain solib when sibling layout is used
   + 5932b3b:
     [6.1.0] Add --host_features (#17528)
   + a0fa77c:
     Exit with code 39 if remote cache evicted blobs that Bazel need
     during an invocation (#17496)
   + 1a438b4:
     Only fetch @remote_coverage_tools when collecting coverage
     (#17512)
   + ee1daaf:
     [6.1.0]Only fetch @remote_coverage_tools when collecting
     coverage (#17287)
   + 350e329:
     [6.1.0]Fix symlink file creation overhead (#17488)
   + 5406c95:
     [6.1.0] Cleanup stale state when remote cache evicted (#17538)
   + 2d1b570:
     Bump version number in runfiles.bash init code
   + 3ab8a0a:
     [6.1.0]Let `aquery` print effective environment for all
     `CommandAction`s (#17274)
   + 51e6c38:
     Properly report repo fetch progress during main repo mapping
     computation
   + 744108e:
     [docs] Update migrating to Xcode tutorial (#17563)
   + 9dc7a7e:
     Update //third_party:jsr305 (#17557)
   + 43dadb2:
     Bump minimum supported macOS versions to 10.13
   + 7d9d23c:
     Correctly set default subprocess factory when loading class
     `SubprocessBuilder`.
   + 77f82f4:
     Add an --incompatible_strict_conflict_checks alias for
     --experimental_strict_conflict_checks.
   + e05345d:
     Add support for wrapping system streams in WorkRequestHandler
   + 68e1924:
     Also handle remote cache eviction for tree artifacts. (#17601)
   + 4a6d056:
     Add host transition to lcov_merger in Java version of py_test
   + 546e9e2:
     Fix Bazel 6.0 crash regression (#17613)
   + 7e328bb:
     Include Bazel version information in profile metadata. (#17616)
   + aafe123:
     [6.1.0] Handle remote cache eviction when uploading inputs for
     remote actions. (#17605)
   + 938e348:
     [6.1.0] Rerun the artifact conflict check when
     --incompatible_strict_conflict_checks changes. (#17592)
   + 034a281:
     Report background download for BwoB (#17619)
   + 5afbce5:
     [6.1.0] Flag for writable outputs (experimental) (#17617)
   + d4105e6:
     [6.1.0] Allow .wasm cc executable and dynamic library extension
     (#17440)
   + a306f4f:
     make desugar dependencies deterministic
   + 37953c5:
     Apply exec transition to lcov_merger in sh_test and cc_test
   + 1d73d72:
     [6.1.0]Fix --host_features with multiple transitions (#17641)
   + 755cd4b:
     Release 6.1.0 (2023-03-06)
   + 885ae7e:
     Revert "Add -dead_strip in default opt link flags for darwin
     (#17312)" (#17696)
   + 94c519b:
     Skip empty directories instead of throwing in prefetcher.
     (#17718)
   + 60749d5:
     [6.2.0] Remove actionId from RemoteFileArtifactValue. (#17724)
   + 52deefe:
     Correctly handle templated tree artifacts in the prefetcher.
   + 10587b1:
     Silence setlocale warnings in Java stub
   + 3ad3927:
     Force the Bazel server Java runtime to use the root locale
   + 7c235ff:
     [credentialhelper] Ignore failing to provide request
   + daa3dbe:
     [remote] upload: treat `ALREADY_EXISTS` as success
   + e0cdace:
     Fix data race in prefetcher.
   + c9e3eeb:
     [6.2.0] Update java_tools v11.12 (#17761)
   + 6c89303:
     python: Remove temporary module space created for zip-based
     binaries (#17764)
   + b874e5f:
     [6.2.0]Add test coverage support to android_local_test (#17467)
   + 6fc9bed:
     Fix wasm dynamic library extension crash (#17765)
   + 38ece3c:
     Handle `SIGWINCH` under `bazel run //some:test` (i.e. test
     rules) correctly.
   + 49a9502:
     Clear all remote metadata if any of them are evicted from remote
     cache
   + 8fab22e:
     Include build-tools/X.Y.Z/{lib,lib64}/** in the minimal set of
     SDK files used by the Android integration tests.
   + 3a7236b:
     Allow WORKSPACE and WORKSPACE-loaded .bzl files to see Bzlmod
     root module's mappings (#17818)
   + a87b8e0:
     [6.2.0]Add support for alias targets to cquery's `providers`
     (#17786)
   + ee32eff:
     [6.2.0] Allows --query_file to be used for cquery and aquery
     too. (#17823)
   + cfef67d:
     Fix seeking of empty chunkers.
   + 50ec6bb:
     Rework ByteStreamUploader early return logic.
   + 745ca28:
     Suggest alternatives for typoed rule attribute names
   + 8548e20:
     Relax JSON parser, ensure duplicate keys are overridden
   + c04f0d4:
     Use less subshells and `tee`s in running tests with `bazel run`.
   + 8c6d02e:
     Upgrade Azul JDK 17 archives. (#17852)
   + 3c1c0c0:
     Add suggestions to invalid tag class errors
   + 99b3f38:
     [6.2.0] Add native.module_{name,version} (#17893)
   + f7627e0:
     Support (workspace) relative paths in --override_module closes
     #17551
   + 109b290:
     Fail early if `use_extension` has a bad label
   + f63ce79:
     Avoid unnecessary copying when building Merkle trees.
   + 3ea18cc:
     [6.2.0]Add `module_ctx.is_dev_dependency` (#17934)
   + 2d04c91:
     [6.2.0] Add version to JavaRuntimeInfo (#17913)
   + 2c1a63b:
     Fix CI breakages for release-6.2.0 (#17969)
   + 788801a:
     Enable C++ deps pruning on Windows when PARSE_SHOWINCLUDES is
     available.
   + fb4a0c2:
     [6.2.0] TargetPattern parsing fixes (#17945)
   + 70ce837:
     Add attribute 'provides = [CcInfo]' to '_cc_proto_aspect'
   + 6e18829:
     [6.2.0]Expose cc_proto_aspect as a toplevel symbol. (#17980)
   + 0f55d12:
     Cache Merkle trees for tree artifacts.
   + 6d6fa81:
     Deduplicate concurrent computations of the same Merkle tree.
   + 1f2b3ed:
     Patch zlib to fix compatibility with latest Xcode
   + 27e4c62:
     Add exception message to 'failed to create output directory'
   + 4050120:
     Prevent failures creating output directories
   + 1474b5b:
     Allow multiple matching select branches if they resolve to the
     same value (#18066)
   + 5ddef47:
     Turn applicable_licenses on platform() into a no-op. (#18089)
   + e00509b:
     Use ctime in file digest cache key (#18105)
   + ca30372:
     Gracefully handle output symlinks with BwoB
   + a0cb57f:
     [6.2.0] profile: add profile_finish_ts (#18129)
   + 1a60fad:
     fix(python): Set envvar for runfiles manifest, not runfiles dir,
     when using a manifest (#18133)
   + 5e9fa39:
     Add attribute validation to IncompatibleTargetChecker. (#18135)
   + 97312f3:
     [6.2.0] Update CODEOWNERS (#18149)
   + 76ad4a9:
     [6.2.0]Fix worker and multiplex workers for DexBuilder and
     Desugar actions (#17965)
   + 5afb8b6:
     Lockfile cherry picks (#18143)
   + 1940dfb:
     Automatically retry the build if encountered remote cache
     eviction error (#18171)
   + 755cf95:
     [6.2.0]Allow remote retry max delay to be user configurable
     (#18061)
   + 6c61110:
     Add `module_ctx.extension_metadata` (#18174)
   + c1fea13:
     Introduce max_compatibility_level for bazel_dep (#18178)
   + f95b80d:
     Include cause when reporting `ActionExecutionException`
   + 8a41be9:
     Check for the nullness of AspectValue.
   + 2715120:
     Rename `--experimental_remote_grpc_log` to `--remote_grpc_log`
   + e4682f6:
     [6.2.0] Update java_tools v12.1 (#18197)
   + d94dee2:
     Construct TreeArtifactValues on multiple threads. (#18194)
   + 519eef4:
     Fix crash on multiple output symlinks with BwoB
   + 2442a2e:
     [6.2.0] default_java_toolchain.bzl cherry-picks to fix
     regression (#18225)
   + e4f3d4d:
     Fix message generation of `ActionExecutionException`
   + f39454f:
     Add `dev_dependency` to
     `register_{execution_platforms,toolchains}`
   + bf188c1:
     Fine tune the number of test jobs running in parallel to avoid
     timeout on Intel macOS platform
   + e024247:
     Update java_tools v12.2
   + 2dca982:
     Update java_tools v12.3

Incompatible changes:

  - Bazel no longer increases the delay between progress updates when
    there is no cursor control.
  - `--incompatible_always_include_files_in_data` is flipped
    to true. See #16654 for
    details.
  - `cquery --output=files` also outputs source files.

    Closes #16602.
  - `--incompatible_strict_conflict_checks` is flipped to true. See
    #16729 for details.
  - This changes the behavior of Python version in exec/host
    configuration. Mitigation is to set Python version on the targets.
  - `--features` only applies to targets built in the target
    configuration, and `--host_features` is used for the host / exec
    configuration (gated behind `--incompatible_use_host_features`)

    Fixes #13839

    Closes #16626.

New features:

  - The aquery and cquery commands now respect the --query_file flag
    just like the query command.

Important changes:

  - The new path variable `$(rlocationpath ...)` and its plural form
    `$(rlocationpaths ...)` can be used to expand labels to the paths
    accepted by the `Rlocation` function of runfiles libraries. This
    is the preferred way to access data dependencies at runtime and
    works on all platforms, even when runfiles are not enabled (e.g.,
    on Windows by default).

    Work towards #16124
    Fixes #10923

    Closes #16428.
  - Starlark `print()` statements are now emitted iff the line of
    code is executed. They are no longer replayed on subsequent
    invocations unless the Starlark code is re-executed.
    Additionally, multiple identical `print()` statements (same
    string from the same line of code, e.g. from a loop) are all
    emitted and no longer deduplicated.
  - Fixes a bug where some compilation flags would not be applied to
    a cc_test
  - Added a `native.package_relative_label()` function, which
    converts a label string to a Label object in the context of the
    calling package, in contrast to `Label()`, which does so in the
    context of the current .bzl file. Both functions now also accept
    relative labels such as `:foo`, and are idempotent.
  - Adds coverage metric support to android_local_test
  - Fixed an issue where WORKSPACE and WORKSPACE-loaded .bzl files
    couldn't see the Bzlmod root module's mappings when Bzlmod is
    enabled.
  - Added `native.module_name()` and `native.module_version()` to
    allow BUILD macro authors to acquire information about which
    Bazel module the current repo is associated with.
  - `@foo` labels can now be used on the command line as the
    top-level target (that is, `bazel build @foo` now works).
    Double-dot syntax is now forbidden (`bazel build ../foo` will no
    longer work).
  - Added preliminary support for a lockfile for Bzlmod. It's
    disabled by default; use `--lockfile_mode=update` to enable it.
    This lockfile contains only Bazel module information; it does not
    involve module extensions.
  - Added a new `max_compatibility_level` attribute to the
    `bazel_dep` directive, which allows version selection to upgrade
    a dependency up to the specified compatibility level.

    Co-authored-by: Brentley Jones <github@brentleyjones.com>
  - `--experimental_remote_grpc_log` has been renamed to
    `--remote_grpc_log`

    Closes #18180.

This release contains contributions from many people at Google, as well as Ben Lee, Brentley Jones, Ezekiel Warren, Fabian Meumertzheim, keertk, Keith Smiley, Krzysztof Naglik, kshyanashree, lunch-glide-pepper, oquenchil, Patrick Balestra, Son Luong Ngoc, Ted Kaplan, Ted Kaplan, UebelAndre, Xùdōng Yáng, Yannic, yuzhy8701.
@sluongng
Copy link
Contributor

FYI @coeuvre this change broke a feature on our end that would enable our users to track the origin of the remote cache requests. It's unfortunate that it made into 6.2.0 release without the mentioned change

We could include ActionExecutionMetadata of the local action for the requests for the tracking purpose, but that desires another CL.

Could you please share the idea behind including ActionExecutionMetadata to the request? 🤔

sluongng pushed a commit to sluongng/bazel that referenced this pull request May 10, 2023
Baseline: d60ce2c

Cherry picks:

   + 490f8ba:
     kshyanashree is the release manager of 6.0.0
   + 4e66d93:
     Automated rollback of commit
     2a28909.
   + 48a8d01:
     Allow C/C++ coverage collection for external targets
   + ec7be34:
     Temporarily set parent directory of the input to writable if it
     is not.
   + b098434:
     Infer CPUs for tvOS and watchOS when on Apple Silicon
   + 5cea7dd:
     Improve the documentation for ctx.actions.symlink.
   + a82d26f:
     Add quotes to "Tip"
   + 75b0f3a:
     Write a repo mapping manifest in the runfiles directory (bazelbuild#16555)
   + 07c5c1a:
     Ensure repository names don't start with `~`
   + 30f6c82:
     Escape tilde more gracefully
   + cf3f48c:
     Relax `Label` repo visibility validation
   + 4477823:
     Use "apparent name" instead of "local name" in Bzlmod docs
   + 55f4f48:
     Bazel DevSite: Add required css file.
   + a706994:
     Remove logic that increases delay between progress updates over
     time
   + 1d514ab:
     Remove references to io_bazel repository
   + b0357bd:
     Relnotes python script
   + 8f95651:
     Allow Java coverage collection for external targets
   + bddb191:
     Fix race condition in package-bazel.sh
   + a757852:
     Decanonicalize labels emitted by {a,c,}query if possible
   + 9d250ed:
     Add uniquify parameter to TemplateDict.add_joined
   + f02bcf8:
     Fix identical gcov json file name problem
   + a159330:
     Add `$(rlocationpath(s) ...)` expansion
   + 56f54da:
     Rollup of SBOM correctness fixes (bazelbuild#16655)
   + 72e6e94:
     [cc] Add cc toolchain to starlark cc_proto_library (bazelbuild#16661)
   + 8f28513:
     Make C++ runfiles library repo mapping aware
   + 95f9adc:
     Always collect FileProvider's filesToBuild as data runfiles
   + 4959d34:
     fix: export constraints.bzl file from @local_config_platform so
     it can be used in downstream bzl_library targets
   + 4919d4a:
     Add --host_per_file_copt (bazelbuild#16695)
   + 0a23d46:
     Moving some tests for `RemoteActionFileSystem` of BwoB to a base
     class.
   + 9296068:
     [remote/downloader] Don't include headers in `FetchBlobRequest`
   + 128d833:
     [remote] Respect whether the server supports action cache updates
   + 38c5019:
     [remote/downloader] Migrate `Downloader` to take `Credentials`
     (bazelbuild#16732)
   + 5929cb7:
     Stage repository mapping manifest as a root symlink (bazelbuild#16733)
   + 455454a:
     Expose current repository name to Java with @AutoBazelRepository
   + 97dea59:
     Implement getDirectoryEntries and readdir for
     RemoteActionFileSystem.
   + aa45f5f:
     Move integration tests for BwoB to a base class and add more
     tests there.
   + 1b073ac:
     Make Java runfiles library repo mapping aware
   + 148bbb1:
     Use `_repo_mapping` in C++ runfiles library (bazelbuild#16752)
   + 25558ad:
     Add --experimental_action_cache_store_output_metadata to the
     expansio…
   + 6e945e8:
     Treat `DEBUG` events as progress-like.
   + 1940c5d:
     redact 'token' strings from logging
   + 0b64525:
     Make Bash runfiles library repo mapping aware (bazelbuild#16753)
   + 4caae75:
     Automated rollback of commit
     8f95651.
   + 13ff6d9:
     Fix Bash rlocation failure with stricter Bash options
   + bd88c7e:
     Make bazel Android remote deps compatible with bzlmod (bazelbuild#16772)
   + 6f6d4cc:
     Flip `--incompatible_always_include_files_in_data`
   + 94355b1:
     Add required `--add-opens` server JVM args also with
     non-embedded JDK
   + 8349c95:
     Also collect clang resource directory with
     `-no-canonical-prefixes`
   + dce6ed7:
     Make `bazel run` works with minimal mode
   + ca8674c:
     Include source files with `cquery --output=files` (bazelbuild#16826)
   + 9cb5e0a:
     Fsync before rename after copy in DiskCacheClient
   + 23ffce5:
     Update GetActionResult for disk cache to check referenced files
     when …
   + 0bc0ff5:
     Make Java runfiles library compilable with JDK 8
   + fe16965:
     [6.0.0] Default --incompatible_strict_conflict_checks to true.
     (bazelbuild#16864)
   + 99ca86f:
     Revert "[6.0.0] Default --incompatible_strict_conflict_checks to
     true. (bazelbuild#16864)" (bazelbuild#16872)
   + 312238f:
     Make C++ runfiles library build with `-Werror=shadow`
   + 2baa5a4:
     Keep credentials cached across build commands.
   + 8818a57:
     [6.0.0] Only inject metadata for outputs that cannot be
     reconstructed by skyf… (bazelbuild#16879)
   + 31e4bf4:
     Update java_tools v11.9
   + cd40666:
     replace _get_canonical_form(label) with str(label)
   + e2bc237:
     Avoid exceptions from hermetic sandbox for unsupported artifact
     subclasses
   + b564d14:
     Remove warnings about ignored starlark options
   + 7ccc661:
     [6.0.0] Emit Tree objects in topological order (bazelbuild#16904)
   + 86dee6d:
     Correctly match regex with tree artifact
   + 14925b5:
     Always use target's attributes to set Python version (bazelbuild#16959)
   + a247502:
     Add 'toolchain' parameter to actions.{run,run_shell}
   + 497b7e4:
     Fix Bash `runfiles_current_repository` with RBE
   + 0a2c4ed:
     Fix corner cases in Bash runfiles library
   + 33b514b:
     Fix runfiles creation with MANIFEST when building without the
     bytes
   + 3a13af4:
     Remove LCOV merger dependency of `cc_test` without coverage
     (bazelbuild#17004)
   + 25ba76c:
     Include full tree artifact in inputs when prefetcher doesn't
     support partial tree artifacts.
   + 06deebf:
     Release 6.0.0 (2022-12-19)
   + c2bfb2e:
     Enable git_submodules tests and merge git_repository and
     starlark_git_repository (bazelbuild#17247)
   + e8182ce:
     have 'bazel test' non-test targets depend on
     --remote_download_output
   + c3245cd:
     Add `SpellChecker` suggestions for common Bzlmod errors
   + 8ec8743:
     Use xcrun to invoke install_name_tool
   + 2b2bea4:
     Extra resources
   + 5b4de12:
     Do not clear `--platforms` on no-op change to `--cpu`
   + db3c013:
     Report `AspectCreationException` to the user
   + 53e9fea:
     Use long executable path instead of `argv[0]` in all launchers
   + abae5ca:
     Add sanitizer features to unix_cc_toolchain_config
   + db68419:
     Allow `map_each` to return `None` in `TemplateDict#add_joined`
   + 66b58ee:
     Always emit trailing zero in human-readable download progress
   + 17b8e44:
     Upload all logs in BEP even with minimal upload
   + 28d2daa:
     Set `--experimental_action_listeners` to default in `exec` config
   + 16427c9:
     Do not count tests as failed that have not started
   + 094a0aa:
     Accept tildes in --override_module
   + 5f2866f:
     Do the AC integrity check for disk part of the combined cache.
   + cd10d50:
     Fix `--nozip_undeclared_test_outputs` on Windows
   + 073f54b:
     Allow pyd in extensions of dynamic libraries
   + ac504cb:
     Register JavaCompileActionContext in Bazel.
   + f090433:
     Rollback bazelbuild#14510 because it causes remote test execution to fail
   + 942ddda:
     Prepare backwards compatible usage of optional C++ toolchain
   + 88b51f5:
     Emit LLVM coverage for source file paths with a `tmp` segment
   + bb0b886:
     Enable archive_param_file feature by default for Windows
   + baf97c0:
     Allow `TemplateDict#map_each` callback to return a list of
     strings (bazelbuild#17306)
   + fcb0077:
     Only try to create groups of test actions in the ui.
   + 3c47b47:
     Find `libtool` when using `BAZEL_USE_CPP_ONLY_TOOLCHAIN`.
   + 1da8a82:
     Add -dead_strip in default opt link flags for darwin
   + b0c5eb3:
     Dont query remote cache but always use bytestream protocol
   + 00e9af1:
     Allow Java coverage collection for external targets.
   + dd24a00:
     Test and fix root symlink edge case in runfiles library
   + af97d98:
     [6.1.0] Fix broken CI (bazelbuild#17422)
   + b859571:
     Add `native.package_relative_label` function
   + 82168d4:
     Make Bazel more responsive and use less memory when --jobs is
     high (bazelbuild#17398)
   + 84c1ed4:
     Multiplex worker fixes and tests for Android busybox tools
     (bazelbuild#17371)
   + 0759081:
     Improve error message for concurrent modifications (bazelbuild#17396)
   + 27bc896:
     [6.1.0] Make bazel coverage work with minimal mode (bazelbuild#17397)
   + 544b816:
     [6.1.0] Fix cc_binary bug related to cc_shared_library on
     Windows and prepare for future removal of
     --experimental_cc_shared_library flag (bazelbuild#17445)
   + 9a7aa21:
     Fix Bash `runfiles_current_repository` for tools
   + 911eedc:
     Fix label unambiguous canonical form to correctly report
     non-visible repo names
   + e132653:
     Remove O1 from sanitizer feature flag defaults
   + ba9e2f8:
     Remove usage of gRPC Context cancellation in the remote
     execution client.
   + de03a23:
     Allow -v to libtool
   + 4e35c02:
     Prettify labels in action progress messages with Bzlmod (bazelbuild#17278)
   + 1be0ac3:
     Expand tree outputs before eagerly prefetching them for local
     actions. (bazelbuild#17494)
   + c266651:
     Fix Java coverage collection with Java 8 runtime
   + 1a6ffe6:
     Add a flag to disable execution log sorting.
   + fbec8e2:
     Reduce flakiness on Windows for BwoB tests
   + 420659a:
     Make AutoBazelRepositoryProcessor compatible with Java 8
   + dde6d20:
     Do not recommend `shallow_since` for `git_repository`
   + f76fc61:
     Allow the timeout value for execute calls on a mac to be set via
     an environment variable
   + 773d232:
     Fix RPATHs for cc toolchain solib when sibling layout is used
   + 5932b3b:
     [6.1.0] Add --host_features (bazelbuild#17528)
   + a0fa77c:
     Exit with code 39 if remote cache evicted blobs that Bazel need
     during an invocation (bazelbuild#17496)
   + 1a438b4:
     Only fetch @remote_coverage_tools when collecting coverage
     (bazelbuild#17512)
   + ee1daaf:
     [6.1.0]Only fetch @remote_coverage_tools when collecting
     coverage (bazelbuild#17287)
   + 350e329:
     [6.1.0]Fix symlink file creation overhead (bazelbuild#17488)
   + 5406c95:
     [6.1.0] Cleanup stale state when remote cache evicted (bazelbuild#17538)
   + 2d1b570:
     Bump version number in runfiles.bash init code
   + 3ab8a0a:
     [6.1.0]Let `aquery` print effective environment for all
     `CommandAction`s (bazelbuild#17274)
   + 51e6c38:
     Properly report repo fetch progress during main repo mapping
     computation
   + 744108e:
     [docs] Update migrating to Xcode tutorial (bazelbuild#17563)
   + 9dc7a7e:
     Update //third_party:jsr305 (bazelbuild#17557)
   + 43dadb2:
     Bump minimum supported macOS versions to 10.13
   + 7d9d23c:
     Correctly set default subprocess factory when loading class
     `SubprocessBuilder`.
   + 77f82f4:
     Add an --incompatible_strict_conflict_checks alias for
     --experimental_strict_conflict_checks.
   + e05345d:
     Add support for wrapping system streams in WorkRequestHandler
   + 68e1924:
     Also handle remote cache eviction for tree artifacts. (bazelbuild#17601)
   + 4a6d056:
     Add host transition to lcov_merger in Java version of py_test
   + 546e9e2:
     Fix Bazel 6.0 crash regression (bazelbuild#17613)
   + 7e328bb:
     Include Bazel version information in profile metadata. (bazelbuild#17616)
   + aafe123:
     [6.1.0] Handle remote cache eviction when uploading inputs for
     remote actions. (bazelbuild#17605)
   + 938e348:
     [6.1.0] Rerun the artifact conflict check when
     --incompatible_strict_conflict_checks changes. (bazelbuild#17592)
   + 034a281:
     Report background download for BwoB (bazelbuild#17619)
   + 5afbce5:
     [6.1.0] Flag for writable outputs (experimental) (bazelbuild#17617)
   + d4105e6:
     [6.1.0] Allow .wasm cc executable and dynamic library extension
     (bazelbuild#17440)
   + a306f4f:
     make desugar dependencies deterministic
   + 37953c5:
     Apply exec transition to lcov_merger in sh_test and cc_test
   + 1d73d72:
     [6.1.0]Fix --host_features with multiple transitions (bazelbuild#17641)
   + 755cd4b:
     Release 6.1.0 (2023-03-06)
   + 885ae7e:
     Revert "Add -dead_strip in default opt link flags for darwin
     (bazelbuild#17312)" (bazelbuild#17696)
   + 94c519b:
     Skip empty directories instead of throwing in prefetcher.
     (bazelbuild#17718)
   + 60749d5:
     [6.2.0] Remove actionId from RemoteFileArtifactValue. (bazelbuild#17724)
   + 52deefe:
     Correctly handle templated tree artifacts in the prefetcher.
   + 10587b1:
     Silence setlocale warnings in Java stub
   + 3ad3927:
     Force the Bazel server Java runtime to use the root locale
   + 7c235ff:
     [credentialhelper] Ignore failing to provide request
   + daa3dbe:
     [remote] upload: treat `ALREADY_EXISTS` as success
   + e0cdace:
     Fix data race in prefetcher.
   + c9e3eeb:
     [6.2.0] Update java_tools v11.12 (bazelbuild#17761)
   + 6c89303:
     python: Remove temporary module space created for zip-based
     binaries (bazelbuild#17764)
   + b874e5f:
     [6.2.0]Add test coverage support to android_local_test (bazelbuild#17467)
   + 6fc9bed:
     Fix wasm dynamic library extension crash (bazelbuild#17765)
   + 38ece3c:
     Handle `SIGWINCH` under `bazel run //some:test` (i.e. test
     rules) correctly.
   + 49a9502:
     Clear all remote metadata if any of them are evicted from remote
     cache
   + 8fab22e:
     Include build-tools/X.Y.Z/{lib,lib64}/** in the minimal set of
     SDK files used by the Android integration tests.
   + 3a7236b:
     Allow WORKSPACE and WORKSPACE-loaded .bzl files to see Bzlmod
     root module's mappings (bazelbuild#17818)
   + a87b8e0:
     [6.2.0]Add support for alias targets to cquery's `providers`
     (bazelbuild#17786)
   + ee32eff:
     [6.2.0] Allows --query_file to be used for cquery and aquery
     too. (bazelbuild#17823)
   + cfef67d:
     Fix seeking of empty chunkers.
   + 50ec6bb:
     Rework ByteStreamUploader early return logic.
   + 745ca28:
     Suggest alternatives for typoed rule attribute names
   + 8548e20:
     Relax JSON parser, ensure duplicate keys are overridden
   + c04f0d4:
     Use less subshells and `tee`s in running tests with `bazel run`.
   + 8c6d02e:
     Upgrade Azul JDK 17 archives. (bazelbuild#17852)
   + 3c1c0c0:
     Add suggestions to invalid tag class errors
   + 99b3f38:
     [6.2.0] Add native.module_{name,version} (bazelbuild#17893)
   + f7627e0:
     Support (workspace) relative paths in --override_module closes
     bazelbuild#17551
   + 109b290:
     Fail early if `use_extension` has a bad label
   + f63ce79:
     Avoid unnecessary copying when building Merkle trees.
   + 3ea18cc:
     [6.2.0]Add `module_ctx.is_dev_dependency` (bazelbuild#17934)
   + 2d04c91:
     [6.2.0] Add version to JavaRuntimeInfo (bazelbuild#17913)
   + 2c1a63b:
     Fix CI breakages for release-6.2.0 (bazelbuild#17969)
   + 788801a:
     Enable C++ deps pruning on Windows when PARSE_SHOWINCLUDES is
     available.
   + fb4a0c2:
     [6.2.0] TargetPattern parsing fixes (bazelbuild#17945)
   + 70ce837:
     Add attribute 'provides = [CcInfo]' to '_cc_proto_aspect'
   + 6e18829:
     [6.2.0]Expose cc_proto_aspect as a toplevel symbol. (bazelbuild#17980)
   + 0f55d12:
     Cache Merkle trees for tree artifacts.
   + 6d6fa81:
     Deduplicate concurrent computations of the same Merkle tree.
   + 1f2b3ed:
     Patch zlib to fix compatibility with latest Xcode
   + 27e4c62:
     Add exception message to 'failed to create output directory'
   + 4050120:
     Prevent failures creating output directories
   + 1474b5b:
     Allow multiple matching select branches if they resolve to the
     same value (bazelbuild#18066)
   + 5ddef47:
     Turn applicable_licenses on platform() into a no-op. (bazelbuild#18089)
   + e00509b:
     Use ctime in file digest cache key (bazelbuild#18105)
   + ca30372:
     Gracefully handle output symlinks with BwoB
   + a0cb57f:
     [6.2.0] profile: add profile_finish_ts (bazelbuild#18129)
   + 1a60fad:
     fix(python): Set envvar for runfiles manifest, not runfiles dir,
     when using a manifest (bazelbuild#18133)
   + 5e9fa39:
     Add attribute validation to IncompatibleTargetChecker. (bazelbuild#18135)
   + 97312f3:
     [6.2.0] Update CODEOWNERS (bazelbuild#18149)
   + 76ad4a9:
     [6.2.0]Fix worker and multiplex workers for DexBuilder and
     Desugar actions (bazelbuild#17965)
   + 5afb8b6:
     Lockfile cherry picks (bazelbuild#18143)
   + 1940dfb:
     Automatically retry the build if encountered remote cache
     eviction error (bazelbuild#18171)
   + 755cf95:
     [6.2.0]Allow remote retry max delay to be user configurable
     (bazelbuild#18061)
   + 6c61110:
     Add `module_ctx.extension_metadata` (bazelbuild#18174)
   + c1fea13:
     Introduce max_compatibility_level for bazel_dep (bazelbuild#18178)
   + f95b80d:
     Include cause when reporting `ActionExecutionException`
   + 8a41be9:
     Check for the nullness of AspectValue.
   + 2715120:
     Rename `--experimental_remote_grpc_log` to `--remote_grpc_log`
   + e4682f6:
     [6.2.0] Update java_tools v12.1 (bazelbuild#18197)
   + d94dee2:
     Construct TreeArtifactValues on multiple threads. (bazelbuild#18194)
   + 519eef4:
     Fix crash on multiple output symlinks with BwoB
   + 2442a2e:
     [6.2.0] default_java_toolchain.bzl cherry-picks to fix
     regression (bazelbuild#18225)
   + e4f3d4d:
     Fix message generation of `ActionExecutionException`
   + f39454f:
     Add `dev_dependency` to
     `register_{execution_platforms,toolchains}`
   + bf188c1:
     Fine tune the number of test jobs running in parallel to avoid
     timeout on Intel macOS platform
   + e024247:
     Update java_tools v12.2
   + 2dca982:
     Update java_tools v12.3

Incompatible changes:

  - Bazel no longer increases the delay between progress updates when
    there is no cursor control.
  - `--incompatible_always_include_files_in_data` is flipped
    to true. See bazelbuild#16654 for
    details.
  - `cquery --output=files` also outputs source files.

    Closes bazelbuild#16602.
  - `--incompatible_strict_conflict_checks` is flipped to true. See
    bazelbuild#16729 for details.
  - This changes the behavior of Python version in exec/host
    configuration. Mitigation is to set Python version on the targets.
  - `--features` only applies to targets built in the target
    configuration, and `--host_features` is used for the host / exec
    configuration (gated behind `--incompatible_use_host_features`)

    Fixes bazelbuild#13839

    Closes bazelbuild#16626.

New features:

  - The aquery and cquery commands now respect the --query_file flag
    just like the query command.

Important changes:

  - The new path variable `$(rlocationpath ...)` and its plural form
    `$(rlocationpaths ...)` can be used to expand labels to the paths
    accepted by the `Rlocation` function of runfiles libraries. This
    is the preferred way to access data dependencies at runtime and
    works on all platforms, even when runfiles are not enabled (e.g.,
    on Windows by default).

    Work towards bazelbuild#16124
    Fixes bazelbuild#10923

    Closes bazelbuild#16428.
  - Starlark `print()` statements are now emitted iff the line of
    code is executed. They are no longer replayed on subsequent
    invocations unless the Starlark code is re-executed.
    Additionally, multiple identical `print()` statements (same
    string from the same line of code, e.g. from a loop) are all
    emitted and no longer deduplicated.
  - Fixes a bug where some compilation flags would not be applied to
    a cc_test
  - Added a `native.package_relative_label()` function, which
    converts a label string to a Label object in the context of the
    calling package, in contrast to `Label()`, which does so in the
    context of the current .bzl file. Both functions now also accept
    relative labels such as `:foo`, and are idempotent.
  - Adds coverage metric support to android_local_test
  - Fixed an issue where WORKSPACE and WORKSPACE-loaded .bzl files
    couldn't see the Bzlmod root module's mappings when Bzlmod is
    enabled.
  - Added `native.module_name()` and `native.module_version()` to
    allow BUILD macro authors to acquire information about which
    Bazel module the current repo is associated with.
  - `@foo` labels can now be used on the command line as the
    top-level target (that is, `bazel build @foo` now works).
    Double-dot syntax is now forbidden (`bazel build ../foo` will no
    longer work).
  - Added preliminary support for a lockfile for Bzlmod. It's
    disabled by default; use `--lockfile_mode=update` to enable it.
    This lockfile contains only Bazel module information; it does not
    involve module extensions.
  - Added a new `max_compatibility_level` attribute to the
    `bazel_dep` directive, which allows version selection to upgrade
    a dependency up to the specified compatibility level.

    Co-authored-by: Brentley Jones <github@brentleyjones.com>
  - `--experimental_remote_grpc_log` has been renamed to
    `--remote_grpc_log`

    Closes bazelbuild#18180.

This release contains contributions from many people at Google, as well as Ben Lee, Brentley Jones, Ezekiel Warren, Fabian Meumertzheim, keertk, Keith Smiley, Krzysztof Naglik, kshyanashree, lunch-glide-pepper, oquenchil, Patrick Balestra, Son Luong Ngoc, Ted Kaplan, Ted Kaplan, UebelAndre, Xùdōng Yáng, Yannic, yuzhy8701.
@sluongng
Copy link
Contributor

sluongng commented May 10, 2023

To clarify a bit further, our CAS and AC implementation use this action_id metadata to help correlate the request to the action / build target. This let our users to troubleshoot cache misses issue a lot easier by inspecting write requests' action_id.

image

image

Even if an action were to be executed locally, its result could still be cached remotely in AC and thus, could be used to correlate at the end. I can see this change matter for a very small subset of local actions where remote caching is disabled, but in those cases, I don't think there is much harm in just keeping the action ID as-is.

@coeuvre If possible, I would urge reverting this change for a 6.2.1 patch release 🙏

@coeuvre
Copy link
Member

coeuvre commented May 11, 2023

The idea is, instead of using action_id of generating action when fetch the blobs, we use action_id of local actions that need the blobs as inputs.

I tend to make the latter case work and cherrypick it into 6.2.1. Does that work for you?

fweikert pushed a commit to fweikert/bazel that referenced this pull request May 25, 2023
Baseline: d60ce2c

Cherry picks:

   + 490f8ba:
     kshyanashree is the release manager of 6.0.0
   + 4e66d93:
     Automated rollback of commit
     2a28909.
   + 48a8d01:
     Allow C/C++ coverage collection for external targets
   + ec7be34:
     Temporarily set parent directory of the input to writable if it
     is not.
   + b098434:
     Infer CPUs for tvOS and watchOS when on Apple Silicon
   + 5cea7dd:
     Improve the documentation for ctx.actions.symlink.
   + a82d26f:
     Add quotes to "Tip"
   + 75b0f3a:
     Write a repo mapping manifest in the runfiles directory (bazelbuild#16555)
   + 07c5c1a:
     Ensure repository names don't start with `~`
   + 30f6c82:
     Escape tilde more gracefully
   + cf3f48c:
     Relax `Label` repo visibility validation
   + 4477823:
     Use "apparent name" instead of "local name" in Bzlmod docs
   + 55f4f48:
     Bazel DevSite: Add required css file.
   + a706994:
     Remove logic that increases delay between progress updates over
     time
   + 1d514ab:
     Remove references to io_bazel repository
   + b0357bd:
     Relnotes python script
   + 8f95651:
     Allow Java coverage collection for external targets
   + bddb191:
     Fix race condition in package-bazel.sh
   + a757852:
     Decanonicalize labels emitted by {a,c,}query if possible
   + 9d250ed:
     Add uniquify parameter to TemplateDict.add_joined
   + f02bcf8:
     Fix identical gcov json file name problem
   + a159330:
     Add `$(rlocationpath(s) ...)` expansion
   + 56f54da:
     Rollup of SBOM correctness fixes (bazelbuild#16655)
   + 72e6e94:
     [cc] Add cc toolchain to starlark cc_proto_library (bazelbuild#16661)
   + 8f28513:
     Make C++ runfiles library repo mapping aware
   + 95f9adc:
     Always collect FileProvider's filesToBuild as data runfiles
   + 4959d34:
     fix: export constraints.bzl file from @local_config_platform so
     it can be used in downstream bzl_library targets
   + 4919d4a:
     Add --host_per_file_copt (bazelbuild#16695)
   + 0a23d46:
     Moving some tests for `RemoteActionFileSystem` of BwoB to a base
     class.
   + 9296068:
     [remote/downloader] Don't include headers in `FetchBlobRequest`
   + 128d833:
     [remote] Respect whether the server supports action cache updates
   + 38c5019:
     [remote/downloader] Migrate `Downloader` to take `Credentials`
     (bazelbuild#16732)
   + 5929cb7:
     Stage repository mapping manifest as a root symlink (bazelbuild#16733)
   + 455454a:
     Expose current repository name to Java with @AutoBazelRepository
   + 97dea59:
     Implement getDirectoryEntries and readdir for
     RemoteActionFileSystem.
   + aa45f5f:
     Move integration tests for BwoB to a base class and add more
     tests there.
   + 1b073ac:
     Make Java runfiles library repo mapping aware
   + 148bbb1:
     Use `_repo_mapping` in C++ runfiles library (bazelbuild#16752)
   + 25558ad:
     Add --experimental_action_cache_store_output_metadata to the
     expansio…
   + 6e945e8:
     Treat `DEBUG` events as progress-like.
   + 1940c5d:
     redact 'token' strings from logging
   + 0b64525:
     Make Bash runfiles library repo mapping aware (bazelbuild#16753)
   + 4caae75:
     Automated rollback of commit
     8f95651.
   + 13ff6d9:
     Fix Bash rlocation failure with stricter Bash options
   + bd88c7e:
     Make bazel Android remote deps compatible with bzlmod (bazelbuild#16772)
   + 6f6d4cc:
     Flip `--incompatible_always_include_files_in_data`
   + 94355b1:
     Add required `--add-opens` server JVM args also with
     non-embedded JDK
   + 8349c95:
     Also collect clang resource directory with
     `-no-canonical-prefixes`
   + dce6ed7:
     Make `bazel run` works with minimal mode
   + ca8674c:
     Include source files with `cquery --output=files` (bazelbuild#16826)
   + 9cb5e0a:
     Fsync before rename after copy in DiskCacheClient
   + 23ffce5:
     Update GetActionResult for disk cache to check referenced files
     when …
   + 0bc0ff5:
     Make Java runfiles library compilable with JDK 8
   + fe16965:
     [6.0.0] Default --incompatible_strict_conflict_checks to true.
     (bazelbuild#16864)
   + 99ca86f:
     Revert "[6.0.0] Default --incompatible_strict_conflict_checks to
     true. (bazelbuild#16864)" (bazelbuild#16872)
   + 312238f:
     Make C++ runfiles library build with `-Werror=shadow`
   + 2baa5a4:
     Keep credentials cached across build commands.
   + 8818a57:
     [6.0.0] Only inject metadata for outputs that cannot be
     reconstructed by skyf… (bazelbuild#16879)
   + 31e4bf4:
     Update java_tools v11.9
   + cd40666:
     replace _get_canonical_form(label) with str(label)
   + e2bc237:
     Avoid exceptions from hermetic sandbox for unsupported artifact
     subclasses
   + b564d14:
     Remove warnings about ignored starlark options
   + 7ccc661:
     [6.0.0] Emit Tree objects in topological order (bazelbuild#16904)
   + 86dee6d:
     Correctly match regex with tree artifact
   + 14925b5:
     Always use target's attributes to set Python version (bazelbuild#16959)
   + a247502:
     Add 'toolchain' parameter to actions.{run,run_shell}
   + 497b7e4:
     Fix Bash `runfiles_current_repository` with RBE
   + 0a2c4ed:
     Fix corner cases in Bash runfiles library
   + 33b514b:
     Fix runfiles creation with MANIFEST when building without the
     bytes
   + 3a13af4:
     Remove LCOV merger dependency of `cc_test` without coverage
     (bazelbuild#17004)
   + 25ba76c:
     Include full tree artifact in inputs when prefetcher doesn't
     support partial tree artifacts.
   + 06deebf:
     Release 6.0.0 (2022-12-19)
   + c2bfb2e:
     Enable git_submodules tests and merge git_repository and
     starlark_git_repository (bazelbuild#17247)
   + e8182ce:
     have 'bazel test' non-test targets depend on
     --remote_download_output
   + c3245cd:
     Add `SpellChecker` suggestions for common Bzlmod errors
   + 8ec8743:
     Use xcrun to invoke install_name_tool
   + 2b2bea4:
     Extra resources
   + 5b4de12:
     Do not clear `--platforms` on no-op change to `--cpu`
   + db3c013:
     Report `AspectCreationException` to the user
   + 53e9fea:
     Use long executable path instead of `argv[0]` in all launchers
   + abae5ca:
     Add sanitizer features to unix_cc_toolchain_config
   + db68419:
     Allow `map_each` to return `None` in `TemplateDict#add_joined`
   + 66b58ee:
     Always emit trailing zero in human-readable download progress
   + 17b8e44:
     Upload all logs in BEP even with minimal upload
   + 28d2daa:
     Set `--experimental_action_listeners` to default in `exec` config
   + 16427c9:
     Do not count tests as failed that have not started
   + 094a0aa:
     Accept tildes in --override_module
   + 5f2866f:
     Do the AC integrity check for disk part of the combined cache.
   + cd10d50:
     Fix `--nozip_undeclared_test_outputs` on Windows
   + 073f54b:
     Allow pyd in extensions of dynamic libraries
   + ac504cb:
     Register JavaCompileActionContext in Bazel.
   + f090433:
     Rollback bazelbuild#14510 because it causes remote test execution to fail
   + 942ddda:
     Prepare backwards compatible usage of optional C++ toolchain
   + 88b51f5:
     Emit LLVM coverage for source file paths with a `tmp` segment
   + bb0b886:
     Enable archive_param_file feature by default for Windows
   + baf97c0:
     Allow `TemplateDict#map_each` callback to return a list of
     strings (bazelbuild#17306)
   + fcb0077:
     Only try to create groups of test actions in the ui.
   + 3c47b47:
     Find `libtool` when using `BAZEL_USE_CPP_ONLY_TOOLCHAIN`.
   + 1da8a82:
     Add -dead_strip in default opt link flags for darwin
   + b0c5eb3:
     Dont query remote cache but always use bytestream protocol
   + 00e9af1:
     Allow Java coverage collection for external targets.
   + dd24a00:
     Test and fix root symlink edge case in runfiles library
   + af97d98:
     [6.1.0] Fix broken CI (bazelbuild#17422)
   + b859571:
     Add `native.package_relative_label` function
   + 82168d4:
     Make Bazel more responsive and use less memory when --jobs is
     high (bazelbuild#17398)
   + 84c1ed4:
     Multiplex worker fixes and tests for Android busybox tools
     (bazelbuild#17371)
   + 0759081:
     Improve error message for concurrent modifications (bazelbuild#17396)
   + 27bc896:
     [6.1.0] Make bazel coverage work with minimal mode (bazelbuild#17397)
   + 544b816:
     [6.1.0] Fix cc_binary bug related to cc_shared_library on
     Windows and prepare for future removal of
     --experimental_cc_shared_library flag (bazelbuild#17445)
   + 9a7aa21:
     Fix Bash `runfiles_current_repository` for tools
   + 911eedc:
     Fix label unambiguous canonical form to correctly report
     non-visible repo names
   + e132653:
     Remove O1 from sanitizer feature flag defaults
   + ba9e2f8:
     Remove usage of gRPC Context cancellation in the remote
     execution client.
   + de03a23:
     Allow -v to libtool
   + 4e35c02:
     Prettify labels in action progress messages with Bzlmod (bazelbuild#17278)
   + 1be0ac3:
     Expand tree outputs before eagerly prefetching them for local
     actions. (bazelbuild#17494)
   + c266651:
     Fix Java coverage collection with Java 8 runtime
   + 1a6ffe6:
     Add a flag to disable execution log sorting.
   + fbec8e2:
     Reduce flakiness on Windows for BwoB tests
   + 420659a:
     Make AutoBazelRepositoryProcessor compatible with Java 8
   + dde6d20:
     Do not recommend `shallow_since` for `git_repository`
   + f76fc61:
     Allow the timeout value for execute calls on a mac to be set via
     an environment variable
   + 773d232:
     Fix RPATHs for cc toolchain solib when sibling layout is used
   + 5932b3b:
     [6.1.0] Add --host_features (bazelbuild#17528)
   + a0fa77c:
     Exit with code 39 if remote cache evicted blobs that Bazel need
     during an invocation (bazelbuild#17496)
   + 1a438b4:
     Only fetch @remote_coverage_tools when collecting coverage
     (bazelbuild#17512)
   + ee1daaf:
     [6.1.0]Only fetch @remote_coverage_tools when collecting
     coverage (bazelbuild#17287)
   + 350e329:
     [6.1.0]Fix symlink file creation overhead (bazelbuild#17488)
   + 5406c95:
     [6.1.0] Cleanup stale state when remote cache evicted (bazelbuild#17538)
   + 2d1b570:
     Bump version number in runfiles.bash init code
   + 3ab8a0a:
     [6.1.0]Let `aquery` print effective environment for all
     `CommandAction`s (bazelbuild#17274)
   + 51e6c38:
     Properly report repo fetch progress during main repo mapping
     computation
   + 744108e:
     [docs] Update migrating to Xcode tutorial (bazelbuild#17563)
   + 9dc7a7e:
     Update //third_party:jsr305 (bazelbuild#17557)
   + 43dadb2:
     Bump minimum supported macOS versions to 10.13
   + 7d9d23c:
     Correctly set default subprocess factory when loading class
     `SubprocessBuilder`.
   + 77f82f4:
     Add an --incompatible_strict_conflict_checks alias for
     --experimental_strict_conflict_checks.
   + e05345d:
     Add support for wrapping system streams in WorkRequestHandler
   + 68e1924:
     Also handle remote cache eviction for tree artifacts. (bazelbuild#17601)
   + 4a6d056:
     Add host transition to lcov_merger in Java version of py_test
   + 546e9e2:
     Fix Bazel 6.0 crash regression (bazelbuild#17613)
   + 7e328bb:
     Include Bazel version information in profile metadata. (bazelbuild#17616)
   + aafe123:
     [6.1.0] Handle remote cache eviction when uploading inputs for
     remote actions. (bazelbuild#17605)
   + 938e348:
     [6.1.0] Rerun the artifact conflict check when
     --incompatible_strict_conflict_checks changes. (bazelbuild#17592)
   + 034a281:
     Report background download for BwoB (bazelbuild#17619)
   + 5afbce5:
     [6.1.0] Flag for writable outputs (experimental) (bazelbuild#17617)
   + d4105e6:
     [6.1.0] Allow .wasm cc executable and dynamic library extension
     (bazelbuild#17440)
   + a306f4f:
     make desugar dependencies deterministic
   + 37953c5:
     Apply exec transition to lcov_merger in sh_test and cc_test
   + 1d73d72:
     [6.1.0]Fix --host_features with multiple transitions (bazelbuild#17641)
   + 755cd4b:
     Release 6.1.0 (2023-03-06)
   + 885ae7e:
     Revert "Add -dead_strip in default opt link flags for darwin
     (bazelbuild#17312)" (bazelbuild#17696)
   + 94c519b:
     Skip empty directories instead of throwing in prefetcher.
     (bazelbuild#17718)
   + 60749d5:
     [6.2.0] Remove actionId from RemoteFileArtifactValue. (bazelbuild#17724)
   + 52deefe:
     Correctly handle templated tree artifacts in the prefetcher.
   + 10587b1:
     Silence setlocale warnings in Java stub
   + 3ad3927:
     Force the Bazel server Java runtime to use the root locale
   + 7c235ff:
     [credentialhelper] Ignore failing to provide request
   + daa3dbe:
     [remote] upload: treat `ALREADY_EXISTS` as success
   + e0cdace:
     Fix data race in prefetcher.
   + c9e3eeb:
     [6.2.0] Update java_tools v11.12 (bazelbuild#17761)
   + 6c89303:
     python: Remove temporary module space created for zip-based
     binaries (bazelbuild#17764)
   + b874e5f:
     [6.2.0]Add test coverage support to android_local_test (bazelbuild#17467)
   + 6fc9bed:
     Fix wasm dynamic library extension crash (bazelbuild#17765)
   + 38ece3c:
     Handle `SIGWINCH` under `bazel run //some:test` (i.e. test
     rules) correctly.
   + 49a9502:
     Clear all remote metadata if any of them are evicted from remote
     cache
   + 8fab22e:
     Include build-tools/X.Y.Z/{lib,lib64}/** in the minimal set of
     SDK files used by the Android integration tests.
   + 3a7236b:
     Allow WORKSPACE and WORKSPACE-loaded .bzl files to see Bzlmod
     root module's mappings (bazelbuild#17818)
   + a87b8e0:
     [6.2.0]Add support for alias targets to cquery's `providers`
     (bazelbuild#17786)
   + ee32eff:
     [6.2.0] Allows --query_file to be used for cquery and aquery
     too. (bazelbuild#17823)
   + cfef67d:
     Fix seeking of empty chunkers.
   + 50ec6bb:
     Rework ByteStreamUploader early return logic.
   + 745ca28:
     Suggest alternatives for typoed rule attribute names
   + 8548e20:
     Relax JSON parser, ensure duplicate keys are overridden
   + c04f0d4:
     Use less subshells and `tee`s in running tests with `bazel run`.
   + 8c6d02e:
     Upgrade Azul JDK 17 archives. (bazelbuild#17852)
   + 3c1c0c0:
     Add suggestions to invalid tag class errors
   + 99b3f38:
     [6.2.0] Add native.module_{name,version} (bazelbuild#17893)
   + f7627e0:
     Support (workspace) relative paths in --override_module closes
     bazelbuild#17551
   + 109b290:
     Fail early if `use_extension` has a bad label
   + f63ce79:
     Avoid unnecessary copying when building Merkle trees.
   + 3ea18cc:
     [6.2.0]Add `module_ctx.is_dev_dependency` (bazelbuild#17934)
   + 2d04c91:
     [6.2.0] Add version to JavaRuntimeInfo (bazelbuild#17913)
   + 2c1a63b:
     Fix CI breakages for release-6.2.0 (bazelbuild#17969)
   + 788801a:
     Enable C++ deps pruning on Windows when PARSE_SHOWINCLUDES is
     available.
   + fb4a0c2:
     [6.2.0] TargetPattern parsing fixes (bazelbuild#17945)
   + 70ce837:
     Add attribute 'provides = [CcInfo]' to '_cc_proto_aspect'
   + 6e18829:
     [6.2.0]Expose cc_proto_aspect as a toplevel symbol. (bazelbuild#17980)
   + 0f55d12:
     Cache Merkle trees for tree artifacts.
   + 6d6fa81:
     Deduplicate concurrent computations of the same Merkle tree.
   + 1f2b3ed:
     Patch zlib to fix compatibility with latest Xcode
   + 27e4c62:
     Add exception message to 'failed to create output directory'
   + 4050120:
     Prevent failures creating output directories
   + 1474b5b:
     Allow multiple matching select branches if they resolve to the
     same value (bazelbuild#18066)
   + 5ddef47:
     Turn applicable_licenses on platform() into a no-op. (bazelbuild#18089)
   + e00509b:
     Use ctime in file digest cache key (bazelbuild#18105)
   + ca30372:
     Gracefully handle output symlinks with BwoB
   + a0cb57f:
     [6.2.0] profile: add profile_finish_ts (bazelbuild#18129)
   + 1a60fad:
     fix(python): Set envvar for runfiles manifest, not runfiles dir,
     when using a manifest (bazelbuild#18133)
   + 5e9fa39:
     Add attribute validation to IncompatibleTargetChecker. (bazelbuild#18135)
   + 97312f3:
     [6.2.0] Update CODEOWNERS (bazelbuild#18149)
   + 76ad4a9:
     [6.2.0]Fix worker and multiplex workers for DexBuilder and
     Desugar actions (bazelbuild#17965)
   + 5afb8b6:
     Lockfile cherry picks (bazelbuild#18143)
   + 1940dfb:
     Automatically retry the build if encountered remote cache
     eviction error (bazelbuild#18171)
   + 755cf95:
     [6.2.0]Allow remote retry max delay to be user configurable
     (bazelbuild#18061)
   + 6c61110:
     Add `module_ctx.extension_metadata` (bazelbuild#18174)
   + c1fea13:
     Introduce max_compatibility_level for bazel_dep (bazelbuild#18178)
   + f95b80d:
     Include cause when reporting `ActionExecutionException`
   + 8a41be9:
     Check for the nullness of AspectValue.
   + 2715120:
     Rename `--experimental_remote_grpc_log` to `--remote_grpc_log`
   + e4682f6:
     [6.2.0] Update java_tools v12.1 (bazelbuild#18197)
   + d94dee2:
     Construct TreeArtifactValues on multiple threads. (bazelbuild#18194)
   + 519eef4:
     Fix crash on multiple output symlinks with BwoB
   + 2442a2e:
     [6.2.0] default_java_toolchain.bzl cherry-picks to fix
     regression (bazelbuild#18225)
   + e4f3d4d:
     Fix message generation of `ActionExecutionException`
   + f39454f:
     Add `dev_dependency` to
     `register_{execution_platforms,toolchains}`
   + bf188c1:
     Fine tune the number of test jobs running in parallel to avoid
     timeout on Intel macOS platform
   + e024247:
     Update java_tools v12.2
   + 2dca982:
     Update java_tools v12.3

Incompatible changes:

  - Bazel no longer increases the delay between progress updates when
    there is no cursor control.
  - `--incompatible_always_include_files_in_data` is flipped
    to true. See bazelbuild#16654 for
    details.
  - `cquery --output=files` also outputs source files.

    Closes bazelbuild#16602.
  - `--incompatible_strict_conflict_checks` is flipped to true. See
    bazelbuild#16729 for details.
  - This changes the behavior of Python version in exec/host
    configuration. Mitigation is to set Python version on the targets.
  - `--features` only applies to targets built in the target
    configuration, and `--host_features` is used for the host / exec
    configuration (gated behind `--incompatible_use_host_features`)

    Fixes bazelbuild#13839

    Closes bazelbuild#16626.

New features:

  - The aquery and cquery commands now respect the --query_file flag
    just like the query command.

Important changes:

  - The new path variable `$(rlocationpath ...)` and its plural form
    `$(rlocationpaths ...)` can be used to expand labels to the paths
    accepted by the `Rlocation` function of runfiles libraries. This
    is the preferred way to access data dependencies at runtime and
    works on all platforms, even when runfiles are not enabled (e.g.,
    on Windows by default).

    Work towards bazelbuild#16124
    Fixes bazelbuild#10923

    Closes bazelbuild#16428.
  - Starlark `print()` statements are now emitted iff the line of
    code is executed. They are no longer replayed on subsequent
    invocations unless the Starlark code is re-executed.
    Additionally, multiple identical `print()` statements (same
    string from the same line of code, e.g. from a loop) are all
    emitted and no longer deduplicated.
  - Fixes a bug where some compilation flags would not be applied to
    a cc_test
  - Added a `native.package_relative_label()` function, which
    converts a label string to a Label object in the context of the
    calling package, in contrast to `Label()`, which does so in the
    context of the current .bzl file. Both functions now also accept
    relative labels such as `:foo`, and are idempotent.
  - Adds coverage metric support to android_local_test
  - Fixed an issue where WORKSPACE and WORKSPACE-loaded .bzl files
    couldn't see the Bzlmod root module's mappings when Bzlmod is
    enabled.
  - Added `native.module_name()` and `native.module_version()` to
    allow BUILD macro authors to acquire information about which
    Bazel module the current repo is associated with.
  - `@foo` labels can now be used on the command line as the
    top-level target (that is, `bazel build @foo` now works).
    Double-dot syntax is now forbidden (`bazel build ../foo` will no
    longer work).
  - Added preliminary support for a lockfile for Bzlmod. It's
    disabled by default; use `--lockfile_mode=update` to enable it.
    This lockfile contains only Bazel module information; it does not
    involve module extensions.
  - Added a new `max_compatibility_level` attribute to the
    `bazel_dep` directive, which allows version selection to upgrade
    a dependency up to the specified compatibility level.

    Co-authored-by: Brentley Jones <github@brentleyjones.com>
  - `--experimental_remote_grpc_log` has been renamed to
    `--remote_grpc_log`

    Closes bazelbuild#18180.

This release contains contributions from many people at Google, as well as Ben Lee, Brentley Jones, Ezekiel Warren, Fabian Meumertzheim, keertk, Keith Smiley, Krzysztof Naglik, kshyanashree, lunch-glide-pepper, oquenchil, Patrick Balestra, Son Luong Ngoc, Ted Kaplan, Ted Kaplan, UebelAndre, Xùdōng Yáng, Yannic, yuzhy8701.
@sluongng
Copy link
Contributor

sluongng commented Jun 9, 2023

@coeuvre We continue to receive feedback from enterprise customers on how this change affected their troubleshooting of remote cache traffic negatively.
So I have been wanting to implement the change you mentioned yourself, but I am a bit hazy on the detail

On a highlevel, the interesting function is AbstractActionInputPrefetcher.doDownloadFile where the prefetcher value is hardcoded.

The call stack looks something like this:

# From master @ f75d14bcd92d537c03e88f1aba9c3fdc260e1463

90: buildMetadata(buildRequestId, commandId, "prefetcher", null)
src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java
  81: doDownloadFile(reporter, tempPath, finalPath.relativeTo(execRoot), metadata, priority)
    src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java
      447: downloadFileNoCheckRx(dirCtx, path, treeRoot, actionInput, metadata, priority)
        src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java
          349: prefetchFile(dirCtx, metadataSupplier, input, priority)
            src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java
              311: prefetchFiles(inputs, metadataSupplier, priority)

                src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java
                  252: prefetchInputs()

                src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java
                  605: finalizeAction(action, outputMetadataStore)

                src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java
                  627: downloadFileIfRemote(path)

So it seems like we are not only using this code path to prefetch inputs for Spawns that are about
to be locally executed, we are also using it to fetch outputs of remotely executed actions that
was built without BwtB?

If that's true, I think the scope of impact of this patch is high and I would prefer either reverting
or a fix in the next minor release.

@coeuvre do you have any pointer on how to obtain the action_id for local Spawn, specifically for AbstractSpawnStrategy.prefetchInputs()?

Should we go through the same dance, as in RemoteExecutionService.java? There we turn a Spawn into an Action to compute the ActionKey and use that as actionId.

@sluongng
Copy link
Contributor

sluongng commented Jun 9, 2023

I am taking a light crack at fixing this in #18625

Very much WIP.

@coeuvre
Copy link
Member

coeuvre commented Jun 9, 2023

I have a CL being review internally to fix this. Should be able to submit it next week.

@sluongng
Copy link
Contributor

sluongng commented Jun 9, 2023

I will drop my terrible attempt 🙇
Thanks for looking into this.

copybara-service bot pushed a commit that referenced this pull request Jun 12, 2023
…r#prefetchFiles`.

Due to recent overhual of `prefetchFiles`, the call sites now always have the reference to `ActionExecutionMetadata`, which makes it possible to pass it to `prefetchFiles`.

The reason why we want to access `ActionExecutionMetadata` inside `prefetchFiles` is to associate downloads with actions. For example:

1. When downloading from remote cache, we want to provide action metadata to the RPCs so that remote server can associate them with actions. `actionId` was removed from remote file metadata by f62a8b9 because it is not the right action to associate with for downloads inside this context. We left the action metadata empty for RPCs until this change. #17724 (comment)
2. When building with the bytes, we are able to report download progress to UI. However, `prefetchFiles` is the major way to download when building without the bytes and we can't report download progress on action basis. #17604 added the download report for background downloads, but now, we no longer download toplevel artifacts in background (a5dde12). With this change, we are able to report download progress for each action, either when they download outputs, or prefetch inputs for building without the bytes.

PiperOrigin-RevId: 539635790
Change-Id: Ic9cc1024f5d3560ac46bc462bd549dd81712d92f
@coeuvre
Copy link
Member

coeuvre commented Jun 12, 2023

Here you go: 3b39ab8

Before you ask, I will try to cherrypick it into 6.3.0.

@sluongng
Copy link
Contributor

Ah ok, the fix was not what I expected initially:

  • The action_id is still set to prefetcher in RequestMetadata

  • Fields action_mnemonic, configuration_id and target_id are now set instead in RequestMetadata

This is much better than previously, we could conditionally check for prefetcher action_id to print more metadata to our users.


I could be wrong, but for AbstractActionInputPrefetcher.finalizeAction(), it's when we download outputs from the remote cache after execution. In such a case, I think there should be a way to propagate the action_id down there still, without having to use prefetcher.

@coeuvre
Copy link
Member

coeuvre commented Jun 12, 2023

Yep, for remote actions, we could find a way to propagate the action_id down to the prefetcher. But there is no action_id by definition for local actions that download remote outputs. Let me fix the former case.

@coeuvre
Copy link
Member

coeuvre commented Jun 12, 2023

I could be wrong, but for AbstractActionInputPrefetcher.finalizeAction(), it's when we download outputs from the remote cache after execution. In such a case, I think there should be a way to propagate the action_id down there still, without having to use prefetcher.

@tjgq is actually removing the finalizeAction and moving the downloads into spawn execution so these downloads will have the action_id automatically. However, that changes probably won't be able to be cherrypicked into 6.3.0.

Since you have to have a special case for "prefetcher" because of local actions, it might be fine to use it for remote actions that download their own outputs, for now?

@sluongng
Copy link
Contributor

I think we could make do without that for now. Thanks so much for these fixes 🙇

coeuvre added a commit to coeuvre/bazel that referenced this pull request Jun 13, 2023
…r#prefetchFiles`.

Due to recent overhual of `prefetchFiles`, the call sites now always have the reference to `ActionExecutionMetadata`, which makes it possible to pass it to `prefetchFiles`.

The reason why we want to access `ActionExecutionMetadata` inside `prefetchFiles` is to associate downloads with actions. For example:

1. When downloading from remote cache, we want to provide action metadata to the RPCs so that remote server can associate them with actions. `actionId` was removed from remote file metadata by f62a8b9 because it is not the right action to associate with for downloads inside this context. We left the action metadata empty for RPCs until this change. bazelbuild#17724 (comment)
2. When building with the bytes, we are able to report download progress to UI. However, `prefetchFiles` is the major way to download when building without the bytes and we can't report download progress on action basis. bazelbuild#17604 added the download report for background downloads, but now, we no longer download toplevel artifacts in background (a5dde12). With this change, we are able to report download progress for each action, either when they download outputs, or prefetch inputs for building without the bytes.

PiperOrigin-RevId: 539635790
Change-Id: Ic9cc1024f5d3560ac46bc462bd549dd81712d92f
coeuvre added a commit to coeuvre/bazel that referenced this pull request Jun 13, 2023
…r#prefetchFiles`.

Due to recent overhual of `prefetchFiles`, the call sites now always have the reference to `ActionExecutionMetadata`, which makes it possible to pass it to `prefetchFiles`.

The reason why we want to access `ActionExecutionMetadata` inside `prefetchFiles` is to associate downloads with actions. For example:

1. When downloading from remote cache, we want to provide action metadata to the RPCs so that remote server can associate them with actions. `actionId` was removed from remote file metadata by f62a8b9 because it is not the right action to associate with for downloads inside this context. We left the action metadata empty for RPCs until this change. bazelbuild#17724 (comment)
2. When building with the bytes, we are able to report download progress to UI. However, `prefetchFiles` is the major way to download when building without the bytes and we can't report download progress on action basis. bazelbuild#17604 added the download report for background downloads, but now, we no longer download toplevel artifacts in background (a5dde12). With this change, we are able to report download progress for each action, either when they download outputs, or prefetch inputs for building without the bytes.

PiperOrigin-RevId: 539635790
Change-Id: Ic9cc1024f5d3560ac46bc462bd549dd81712d92f
iancha1992 added a commit that referenced this pull request Jun 13, 2023
…r#prefetchFiles`. (#18656)

Due to recent overhual of `prefetchFiles`, the call sites now always have the reference to `ActionExecutionMetadata`, which makes it possible to pass it to `prefetchFiles`.

The reason why we want to access `ActionExecutionMetadata` inside `prefetchFiles` is to associate downloads with actions. For example:

1. When downloading from remote cache, we want to provide action metadata to the RPCs so that remote server can associate them with actions. `actionId` was removed from remote file metadata by f62a8b9 because it is not the right action to associate with for downloads inside this context. We left the action metadata empty for RPCs until this change. #17724 (comment)
2. When building with the bytes, we are able to report download progress to UI. However, `prefetchFiles` is the major way to download when building without the bytes and we can't report download progress on action basis. #17604 added the download report for background downloads, but now, we no longer download toplevel artifacts in background (a5dde12). With this change, we are able to report download progress for each action, either when they download outputs, or prefetch inputs for building without the bytes.

PiperOrigin-RevId: 539635790
Change-Id: Ic9cc1024f5d3560ac46bc462bd549dd81712d92f

Co-authored-by: Ian (Hee) Cha <heec@google.com>
@coeuvre
Copy link
Member

coeuvre commented Jun 14, 2023

The fix has been cherry-picked in #18656.

traversaro pushed a commit to traversaro/bazel that referenced this pull request Jun 24, 2023
…r#prefetchFiles`.

Due to recent overhual of `prefetchFiles`, the call sites now always have the reference to `ActionExecutionMetadata`, which makes it possible to pass it to `prefetchFiles`.

The reason why we want to access `ActionExecutionMetadata` inside `prefetchFiles` is to associate downloads with actions. For example:

1. When downloading from remote cache, we want to provide action metadata to the RPCs so that remote server can associate them with actions. `actionId` was removed from remote file metadata by f62a8b9 because it is not the right action to associate with for downloads inside this context. We left the action metadata empty for RPCs until this change. bazelbuild#17724 (comment)
2. When building with the bytes, we are able to report download progress to UI. However, `prefetchFiles` is the major way to download when building without the bytes and we can't report download progress on action basis. bazelbuild#17604 added the download report for background downloads, but now, we no longer download toplevel artifacts in background (a5dde12). With this change, we are able to report download progress for each action, either when they download outputs, or prefetch inputs for building without the bytes.

PiperOrigin-RevId: 539635790
Change-Id: Ic9cc1024f5d3560ac46bc462bd549dd81712d92f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants