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

hihihihihihi #113

Closed
wants to merge 269 commits into from
Closed

hihihihihihi #113

wants to merge 269 commits into from

Conversation

iancha1992
Copy link
Owner

hiya

fmeum and others added 30 commits November 2, 2022 16:00
The behavior is analogous to that of Args.add_joined.

Closes bazelbuild#16213.

PiperOrigin-RevId: 485571903
Change-Id: Id69de92d703d5202bfc7b50abfbbb4326441f242

Co-authored-by: kshyanashree <[email protected]>
**The problem:**

Bazel moves all `.gcov.json.gz` files to one directory. If in a test target, two source files have identical names, then the second `.gcov.json.gz` overwrites the first one.

**The solution:**

I added `gcno_path` to the move destination in order to distinguish multiple `.gcov.json.gz` with the same name.

**Testing:**

In the `test_cc_test_coverage_gcov` test case I added the `different/a.cc` source file, so currently we have the following source tree:
```
coverage_srcs/a.h
coverage_srcs/a.cc
coverage_srcs/b.h
coverage_srcs/t.cc
coverage_srcs/different/a.h
coverage_srcs/different/a.cc
```

gcda and gcno files are created next to the source files. The final `gcov.json` files are placed in the corresponding paths:
```
$COVERAGE_DIR_VAR/coverage_srcs/*a.gcov.json.gz
$COVERAGE_DIR_VAR/coverage_srcs/*t.gcov.json.gz
$COVERAGE_DIR_VAR/coverage_srcs/different/*a.gcov.json.gz
```

Closes bazelbuild#16527.

PiperOrigin-RevId: 483911427
Change-Id: I1608407e4b7264fb5fd436997bdc344344932b97
The new location expansion pattern `rlocationpath` and its plural version `rlocationpaths` resolve to the path(s) suitable for the Rlocation function offered by runfiles libraries.

Compared to the `rootpath` pattern, which can returns `../repo_name/pkg/file` for a file in an external repository and `pkg/file` for a file in the main repository, the path returned by `rlocationpath` is always of the form `repo_name/pkg/file`.

RELNOTES: 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.

PiperOrigin-RevId: 485930083
Change-Id: I85c0ef29c8332eeabd8473d2611ede546eb24016
* Rename default_applicable_licenses to default_package_metadata.

Leave default_applicable_licenses as an alias.
Don't allow both to be set.

Step 1 of https://docs.google.com/document/d/1uyJjkKbE8kV8EinakaR9q-Un25zCukhoH_dRBkWHSKQ/edit#

PiperOrigin-RevId: 485705150
Change-Id: I5e0012e37e5bca55ed43f83dd9f26a26f78b543d

* Improve the check for being a package metadata rule

This allows the refactoring which will happen after default_applicable_licenses is renamed to default_package_metadata. The current behavior is intended to prevent
`applicable_licenses` from being set on a `license` rule. The required behavior is that we don't set `applicable_licenses` on any of the metadata rules.

Future changes might have to take into account the ability to set the license for a tool rule within `rules_package`.

For background, see https://docs.google.com/document/d/1uyJjkKbE8kV8EinakaR9q-Un25zCukhoH_dRBkWHSKQ/edit#heading=h.izpa22p82m6c

Closes bazelbuild#16596.

PiperOrigin-RevId: 485457037
Change-Id: Ifb105f646ae0c291a6841b3ddb84ed536e9d71e3

* Make constraint_setting / constraint_value non_configurable.

The concept of allowing what is fundamentally a constant to vary by target we are building for is too much to reason about, let along get the code correct.

PiperOrigin-RevId: 483748098
Change-Id: I966b7d21ad8d38de9867c870a0669e2123063809
Fixes bazelbuild#16577

Closes bazelbuild#16585.

PiperOrigin-RevId: 485600476
Change-Id: I3c2985731ff800a2c6136f401ed3c7e5e89518ad

Co-authored-by: Yannic <[email protected]>
Also removes a comment mentioning a `Create` overload that does not exist.

Work towards bazelbuild#16124

Closes bazelbuild#16623.

PiperOrigin-RevId: 486612245
Change-Id: Ib22cadd354c93eb1e113e27b271c639345c20074

Co-authored-by: Fabian Meumertzheim <[email protected]>
…ld#16680)

Guidelines for Starlark rules suggest that `data`-like attributes on
rules should always merge the default outputs of rule targets into the
transitive runfiles. See:
https://docs.bazel.build/versions/main/skylark/rules.html#runfiles-features-to-avoid

As a result, Starlark rules generally don't (and shouldn't) explicitly
include their default outputs into their runfiles. Before this commit,
native rules did not merge these outputs in the same way as idiomatic
Starlark rules, which led to unexpectedly absent runfiles.

Fixes bazelbuild#15043

Closes bazelbuild#15052.

PiperOrigin-RevId: 486663801
Change-Id: I4a30b8b8a4dfc84bf4de27fd7894dd48e795c081
…n be used in downstream bzl_library targets (bazelbuild#16698)

Downstream rule sets may depend on `@local_config_platform//:constraints.bzl` but when they do there is no way easy way to make that load statement compatible with `bzl_library`. This change makes it possible to use `bzl_library` on starlark code that loads from `@local_config_platform//:constraints.bzl`.

For example,

```
bzl_library(
    name = "local_config_platform_constraints",
    srcs = ["@local_config_platform//:constraints.bzl"],
)

bzl_library(
    name = "platform_utils",
    srcs = ["//lib/private:platform_utils.bzl"],
    deps = [":local_config_platform_constraints"],
)
```

Closes bazelbuild#16665.

PiperOrigin-RevId: 486957479
Change-Id: I328b7a3722aea95b3151ed88f23c277ed4154202

Co-authored-by: Greg Magolan <[email protected]>
Fixes bazelbuild#12406

Closes bazelbuild#16618.

PiperOrigin-RevId: 486616830
Change-Id: Icd9774b2d05d09288a34375e229b13e2a74cf3d7

Co-authored-by: Keith Smiley <[email protected]>
Co-authored-by: kshyanashree <[email protected]>
…s. (bazelbuild#16722)

PiperOrigin-RevId: 485588407
Change-Id: Ie9f3919c703c968eb19328539334548238e273b9

Co-authored-by: Googler <[email protected]>
…lbuild#16677)

Including the headers in the request is very inefficient as credentials should never change the content of the downloaded archive. In fact, given that Bazel verifies the checksum of the downloaded file, the credentials cannot possibly used in a way where they influence the outcome of the download (other than deciding whether or not the user is allowed to download the blob at all). Hence, the credentials should not be included in the request.

Users that need to send credentials to the remote downloader should do so by passing the credentials as metadata to the gRPC call.

Note that the remote downloader is behind an experimental flag, so this change does not need to go thorugh the incompatible change process.

Closes bazelbuild#16595.

PiperOrigin-RevId: 485576157
Change-Id: I8afc7c818e4eed63ac1f70c3e4c2115a1d365830
…ates (bazelbuild#16724)

Only a subset of users may be allowed to update the action cache (e.g., only CI but not devs).

Today, there are 2 ways to achive the desired behavior:
  - `GetCapabilities` returning that all users are allowed to update, and `UpdateActionResult` returning an error that Bazel prints and ignores, or
  - have the users that are not allowed to update the action cache set `--remote_upload_local_results=false`.

Why don't we instead respect whether the remote cache supports uploading action results?

Note that this requires support from the remote system to fully work (i.e., it needs to return `update_enabled = false` for users that don't have permission). Otherwise, Bazel's behavior will be the same as before this change: failed `UpdateActionResult` do not cause the build to fail. The only change this introduces is that Bazel will no longer error if `--remote_upload_local_results=true` and `GetCapabilities` returning `update_enabled = false`.

Closes bazelbuild#16624.

PiperOrigin-RevId: 486901751
Change-Id: I0991f6891e21711df1e23ae0998a8bc95e2389bc
…build#16732)

Progress on bazelbuild#15856

Closes bazelbuild#16601.

PiperOrigin-RevId: 485837451
Change-Id: I785882d0ff3e171dcaee6aa6f628bca9232c730a
By adding the repository mapping manifest to runfiles as a root symlink, it is staged as `foo.runfiles/_repo_mapping` with all execution strategies. This includes sandboxed and remote execution, which previously did not stage the manifest at all.

As a side effect, runfiles libraries can now find the repository mapping manifest via `rlocation("_repo_mapping")`.

Fixes bazelbuild#16643
Work towards bazelbuild#16124

Closes bazelbuild#16652.

PiperOrigin-RevId: 487532254
Change-Id: I9774b8930337c5967fce92a861cc0db71dea2f0f
…elbuild#16736)

Java targets depending on `@bazel_tools//tools/java/runfiles` can add the new `@AutoBazelRepository` to a class to have an annotation processor generate a companion class with a `BAZEL_REPOSITORY` constant containing the repository name of the target that compiled the class.

This requires a small addition to JavaBuilder to parse the repository name out of the target label and pass it to javac as a processor option.

Work towards bazelbuild#16124

Closes bazelbuild#16534.

PiperOrigin-RevId: 487573496
Change-Id: Id9b6526ce32268089c91c6d17363d1e7682f64a4
…bazelbuild#16738)

So spawns can read content of directires within action exuection.

Part of bazelbuild#16556.

PiperOrigin-RevId: 486918859
Change-Id: Ida86e4c927093d26f7f96d2f0c2aa0d1d74cc8a4

Co-authored-by: Googler <[email protected]>
…ere. (bazelbuild#16749)

Remove the test parameters since it adds a lot of test time but for most tests we don't need to test these combinations. If we care a specific case (e.g. toplevel, remote cache, etc.), we should have a test case.

PiperOrigin-RevId: 483705367
Change-Id: If4a166d2545bd7aea6fb63ec901a0ec889e88ca0

Co-authored-by: Googler <[email protected]>
Work towards bazelbuild#16124

Closes bazelbuild#16549.

PiperOrigin-RevId: 487797147
Change-Id: Ic8e643898b145b7ea1e72f4a0deedfd4dfd50242
Work towards bazelbuild#16124

Closes bazelbuild#16701.

PiperOrigin-RevId: 487806351
Change-Id: I3b04fef84d817b0875bfd71c65efe6db13468b13
bazelbuild#16740)

…n of flag --remote_download_{toplevel,minimal}.

So users don't need to add it manually.

It is always desired to use this flag along with BwoB.

Closes bazelbuild#13912.

Closes bazelbuild#16720.

PiperOrigin-RevId: 487248820
Change-Id: I94f6486a0a61522dc9000de7e9a595ace65c97e2

Co-authored-by: Chi Wang <[email protected]>
The primary source of `DEBUG` events is the starlark `print()` function. The motivation in making this change is to emit `print()` statements more intelligibly, including:

* Not deduplicating multiple identical prints from the same line of code.
* Not replaying prints when the corresponding skyframe node is cached. For example, we currently replay prints in a `BUILD` file even when the package is up to date and the starlark code is not executed.

A secondary benefit is that we don't spend memory retaining `DEBUG` events in skyframe.

This change makes `DEBUG` events match the semantics of `INFO` and `PROGRESS` events. See the documentation on `Reportable#storeForReplay`. There is some chance that this causes additional spam or breaks something relying on cached print statements in incremental build output, but per [style guide](https://bazel.build/rules/bzl-style#print-for-debugging), `print()` should not be used for production code anyway.

There are a couple other minor sources of `DEBUG` events besides `print()`, and those semantics will be changed as well (for the better, in my opinion).

Fixes bazelbuild#16721.

RELNOTES: 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.
PiperOrigin-RevId: 487645012
Change-Id: I8b13df67febc9f330c864930688bca31c3711276

Co-authored-by: Googler <[email protected]>
It's common for users to set 'TOKEN' as an env var. While this is a little like whack-a-mole and we can't cover everything, this seems like a common string to redact.

Closes bazelbuild#16622.

PiperOrigin-RevId: 488392632
Change-Id: I7b48199cc140d6736cd145df63e03eeda747c7fb
(cherry picked from commit 1940c5d)

Co-authored-by: Matt Mackay <[email protected]>
Work towards bazelbuild#16124

Closes bazelbuild#16693.

PiperOrigin-RevId: 487811689
Change-Id: Iec5ef01ae09394372b87e5cb697ceb728563d2dc
*** Reason for rollback ***

Breaks rules_kotlin android_test() targets.

*** Original change description ***

Allow Java coverage collection for external targets

PiperOrigin-RevId: 488453071
Change-Id: Ib63645058eb956271160c6c2cd205da90be39df6

Co-authored-by: Googler <[email protected]>
When run in the context of the bazel-skylib unit test setup, the new repo mapping logic in the Bash runfiles library failed due to a non-matching grep in a pipe. Fix this by using a wrapper around grep throughout that does not exit with exit code 1 if there is no match.

Fixes bazelbuild/bazel-skylib#411 (comment)

Closes bazelbuild#16755.

PiperOrigin-RevId: 488749744
Change-Id: I087b03d9e95ba27a409c551bdc27d0027919a0fe

Co-authored-by: Fabian Meumertzheim <[email protected]>
* Upgrade bazel's bzlmod rules_java version to 5.3.5

Latest rules_java has fixes for Android build integration.

PiperOrigin-RevId: 488701777
Change-Id: I144f6feac88feaad5419fcdd9e21e5b8e2003b16

* Adds a bzlmod extension for remote android_tools

PiperOrigin-RevId: 488720288
Change-Id: I6e7baa1d87d2d7b5434d8870e726faf362842dac

Co-authored-by: Googler <[email protected]>
RELNOTES[INC]: `--incompatible_always_include_files_in_data` is flipped
to true. See bazelbuild#16654 for
details.
…bazelbuild#16787)

Since the Bazel server requires JDK 11 or higher to run, the `--add-opens` server JVM arg for `java.lang` can now be added unconditionally, which ensures support with JDK 17+.

Also removes the additional opens for `java.nio`, which was only needed to silence a protobuf warning that has since been fixed upstream.

Fixes bazelbuild#16705
Fixes bazelbuild#15831

Closes bazelbuild#16706.

PiperOrigin-RevId: 489372772
Change-Id: I880e2689f59b2d4420b1e2e0517697d7fb03abbc
…azelbuild#16800)

`clang -print-resource-dir` without `-no-canonical-prefixes` returns a different path than is actually used to include `asan_blacklist.txt` on macOS with non-Apple clang.

Closes bazelbuild#16730.

PiperOrigin-RevId: 489475662
Change-Id: If17f347d76f86e0ec5804f9e8789f44f46ab27b4
Always use `ToplevelArtifactsDownloader` when building without the bytes.

It checks the combination of current command (e.g. `build`, `run`, etc.) and download mode (e.g. `toplevel`, `minimal`) to decide whether download outputs for the toplevel targets or not.

Also in the `RunCommand`, we wait for the background downloads before checking the local filesystem.

Fixes bazelbuild#11920.

Closes bazelbuild#16545.

PiperOrigin-RevId: 487181884
Change-Id: I6b1a78a0d032d2cac8093eecf9c4d2e76f90380f
iancha1992 and others added 15 commits June 6, 2023 19:07
…d#18440)

This is a follow-up to bazelbuild#18198

Make Bazel compatible with newer version of Remote Api by setting
output_paths along-side output_directories and output_files. The latter
2 are deprecated in newer REAPI specification.

Closes bazelbuild#18202.

PiperOrigin-RevId: 527560509
Change-Id: I14c80d69aa9a5e9bf29a8c5694412ecd58ea17bf

Co-authored-by: Son Luong Ngoc <[email protected]>
…ility (bazelbuild#18568)

A 1s timeout was introduced in checking whether LinuxSandbox is available, to prevent a complete hangup on broken systems. However, it turned out that it occasionally results in misjudging that linux-sandbox being not available.
`local_termination_grace_seconds` defaults to 15s, which hopefully gives more headroom and configurability in various setups.

Fixes bazelbuild#18071

Closes bazelbuild#18151.

PiperOrigin-RevId: 536953768
Change-Id: I5d344ee5bf06cb9b13a2cba9d077f0981f4430a3

Co-authored-by: Takeo Sawada <[email protected]>
…18598)

Closes bazelbuild#18587.

PiperOrigin-RevId: 538171456
Change-Id: Idd8300c73beb60b101aaf28c4f04843051bae292

Co-authored-by: Fabian Meumertzheim <[email protected]>
Instead of `<name>@<version>/MODULE.bazel`, the actual location of a MODULE.bazel file is now reported in stack traces, either as a file path or a URL.

Closes bazelbuild#18572.

PiperOrigin-RevId: 538597414
Change-Id: I038e070d9bd4397bba2ed491597cadd0b56d6481

Co-authored-by: Fabian Meumertzheim <[email protected]>
This fixes an issue introduced by PR bazelbuild#14005 where .c and .C extensions were handled case-insensitive on Windows so the cxxopt will be passed to C source files.

Closes bazelbuild#15073 .

Closes bazelbuild#18119.

PiperOrigin-RevId: 526001251
Change-Id: I464e5feae397bdac443ddd159309f77071629e01

Co-authored-by: Kai Zhang <[email protected]>
…ild#18638)

(This script is run at different points in the release process, including once after the tag is created to update the final release index page and GitHub release page.)

PiperOrigin-RevId: 537860711
Change-Id: I6bf129c82eb2c83b37993d4ef691d4783c5735b6
…#18649)

The canonical repo name of the `platforms` module is now forced to be `platforms`, which ensures that `@platforms` constraints used by toolchains defined in `WORKSPACE` match those referenced by the auto-configured host platform provided by `local_config_platform`.

Fixes bazelbuild#17289

Closes bazelbuild#18624.

PiperOrigin-RevId: 539710874
Change-Id: I171f308b06e7ec7559641b49b4c8c53dddac0d3c

Co-authored-by: Fabian Meumertzheim <[email protected]>
…r#prefetchFiles`. (bazelbuild#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. 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

Co-authored-by: Ian (Hee) Cha <[email protected]>
…bazelbuild#18559)

* feat: Implement failure circuit breaker

Copy of bazelbuild#18120: I accidentally closed bazelbuild#18120 during rebase and doesn't have permission to reopen.

We have noticed that any problems with the remote cache have a detrimental effect on build times. On investigation we found that the interface for the circuit breaker was left unimplemented.

To address this issue, implemented a failure circuit breaker, which includes three new Bazel flags: 1) experimental_circuitbreaker_strategy, 2) experimental_remote_failure_threshold, and 3) experimental_emote_failure_window.

In this implementation, I have implemented failure strategy for circuit breaker and used failure count to trip the circuit.

Reasoning behind using failure count instead of failure rate : To measure failure rate I also need the success count. While both the failure and success count need to be an AtomicInteger as both will be modified concurrently by multiple threads. Even though getAndIncrement is very light weight operation, at very high request it might contribute to latency.

Reasoning behind using failure circuit breaker : A new instance of Retrier.CircuitBreaker is created for each build. Therefore, if the circuit breaker trips during a build, the remote cache will be disabled for that build. However, it will be enabled again
for the next build as a new instance of Retrier.CircuitBreaker will be created. If needed in the future we may add cool down strategy also. e.g. failure_and_cool_down_startegy.

closes bazelbuild#18136

Closes bazelbuild#18359.

PiperOrigin-RevId: 536349954
Change-Id: I5e1c57d4ad0ce07ddc4808bf1f327bc5df6ce704

* remove target included in cherry-pick by mistake

* Use failure_rate instead of failure count for circuit breaker

---------

Co-authored-by: Ian (Hee) Cha <[email protected]>
Closes bazelbuild#11350.

RELNOTES: Add flag --experimental_collect_code_coverage_for_generated_files.
PiperOrigin-RevId: 539648731
Change-Id: I352de7a74c522db6fbe5e10a21268914d1e39d58
When the digest size exceeds the max configured digest size by remote-cache, an "out_of_range" error is returned. These errors should not be considered as API failures for the circuit breaker logic, as they do not indicate any issues with the remote-cache service.
Similarly there are other non-retriable errors that should not be treated as server failure such as ALREADY_EXISTS.

This change considers non-retriable errors as user/client error and logs them as success. While retriable errors such `DEADLINE_EXCEEDED`, `UNKNOWN` etc are logged as failure.

Related PRs

Closes bazelbuild#18613.

PiperOrigin-RevId: 539948823
Change-Id: I5b51f6a3aecab7c17d73f78b8234d9a6da49fe6c
)

Invocation id (`BUILD_ID` in code) is expected to be different for each command. When retrying the build, we rely on Bazel generating a new invocation id for a new attempt. However, if flag `--invocation_id` is set, Bazel just uses the provided value instead of generating a new one. In this case, invocation id stays the same among multiple attempts which could cause issues like bazelbuild#18547.

This PR fixes that by not retrying the build if the invocation id is same to previous attempt. Also updated the doc to point this requirement out.

Closes bazelbuild#18591.

PiperOrigin-RevId: 539946840
Change-Id: I6ae85ea923b0fdbff97fe2e44e36995f0205f8a1
We report download progress to UI when downloading outputs from remote cache. UI thread keeps track of active downloads. There are two cases the UI thread could leak memory:

1. If we failed to close the output stream, the `reporter.finished()` will never be called, prevent UI thread from releasing the active download. This is fixed by calling `reporter.finished()` in `finally` block.
2. Normally, UI thread stops after `BuildCompleted` event. However, if we have background download after build is completed, UI thread is not stopped to continue printing out download progress. But after all downloads are done, we forgot to stop the UI thread, resulting all referenced objects leaked. This is fixed by calling `checkActivities()` for every download progress.

Fixes bazelbuild#18145.

Closes bazelbuild#18593.

PiperOrigin-RevId: 539923685
Change-Id: I7e2887035e540b39e382ab5fcbc06bad03b10427

Co-authored-by: Chi Wang <[email protected]>
…elbuild#18663)

Closes bazelbuild#18428.

PiperOrigin-RevId: 532793826
Change-Id: I0f63aa7ee341f5181b905c7ba78af60321b90836

Co-authored-by: Tiago Quelhas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.