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

Cherrypick Dexmapper fixes into Bazel 6.3.0 #18852

Conversation

ahumesky
Copy link
Contributor

@ahumesky ahumesky commented Jul 6, 2023

Cherrypick 3e800a7 into Bazel 6.3.0 to address #16368

aoeui and others added 30 commits June 9, 2023 10:49
This resolves a TODO in the code. Touches up the comments for readability.

The helper will be used in a child CL.

PiperOrigin-RevId: 539122888
Change-Id: Ibe18ad08139c1ea9ef510b51ed7153e62dcefdf7
- Upgraded rules_jvm_external version.
- Fixed the patch file to match the latest version.

Fixes bazelbuild#18596

Closes bazelbuild#18603.

PiperOrigin-RevId: 539142459
Change-Id: Ie2680ae053289e52d7fcfc0cf220138dc7d0f7bb
PiperOrigin-RevId: 539146277
Change-Id: I0b960762cf57de825da57a3c324dcc68030119e5
Baseline: 67446d6

Cherry picks:

   + 4344a03:
     Automated rollback of commit
     00a4fef.

Incompatible changes:

  - `--incompatible_check_sharding_support` is enabled by default.
    Sharded tests with test runners that do not properly advertise
    support for test sharding will fail. Refer to
    bazelbuild#18339 for migration
    advice.

Important changes:

  - Options specified on the pseudo-command `common` in `.rc` files
    are now ignored by commands that do not support them as long as
    they are valid options for *any* Bazel command. Previously,
    commands that did not support all options given for `common`
    would fail to run. These previous semantics of `common` are now
    available via the new `always` pseudo-command.
  - the 'default' param of json.decode can now be used as a keyword
    parameter.
  - As a transitional step in a larger refactoring, rule transitions
    are applied twice. Once during dependency resolution and once
    right before
    analysis of those rules. After the refactoring is complete, rule
    transitions
    will be applied only once.

This release contains contributions from many people at Google, as well as Fabian Meumertzheim, Jimm chja20, Keith Smiley.
PiperOrigin-RevId: 539567296
Change-Id: I5e4945e58adcbee2b42b2a182761c101425d5701
Fixes bazelbuild#11942.

PiperOrigin-RevId: 539589072
Change-Id: I760b842b145d7f88013024ff0dffee414261520f
PiperOrigin-RevId: 539591651
Change-Id: I25580ffd0b51d5db1f755c53523211d5cd377eb2
…preparation for removal.

PiperOrigin-RevId: 539592265
Change-Id: Ib542d5e86660a8959b1c2a521a9bcf217b74d770
PiperOrigin-RevId: 539597529
Change-Id: Ib80822979701c48daa6af889a753483d862618c0
This is unnecessary. We don't have a use case now where this flag is enabled and
the skymeld flag isn't.

bazelbuild#14057

PiperOrigin-RevId: 539598912
Change-Id: I5e2bda47085c606728b3a4a19d38ee0afa214812
Closes bazelbuild#18615.

PiperOrigin-RevId: 539607278
Change-Id: I250250452db923ba132f1445c46b0112b175e505
…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
PiperOrigin-RevId: 539637641
Change-Id: I56e8310503fe61adb7e08a02d3263f89e1d0965f
…ntially run them later on.

Prior to this CL, we'd only _build_ the exclusive targets sequentially right before running them. This was a deviation from the behavior of non-Skymeld blaze, where these targets are first built in parallel and then run sequentially.

PiperOrigin-RevId: 539645460
Change-Id: I28212b6efca7ea84294aac338ecd4c9340e2f701
Closes bazelbuild#11350.

RELNOTES: Add flag --experimental_collect_code_coverage_for_generated_files.
PiperOrigin-RevId: 539648731
Change-Id: I352de7a74c522db6fbe5e10a21268914d1e39d58
In case if we have 2 different keys with same mnemonic this method could return incorrect result. Right now this method is used only in tests.

PiperOrigin-RevId: 539668044
Change-Id: Ie6ee4958708bbed9939614abd201b0b7e6e5b398
This is a first step towards bazelbuild#18629.

Closes bazelbuild#18630.

PiperOrigin-RevId: 539698272
Change-Id: I1584c5fa47b591956609e05c8b55467de0f95e2e
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
- Created a PackageArgs class that stores the values passed to the attrs of the `package()` function, so that they can be applied to a package builder in one go
  - These attr values used to be applied as soon as each attr is processed (i.e. evaluated). We can't do that anymore for repo(), since at the time of REPO.bazel evaluation, the package hasn't started being built yet.
  - Instead of storing the individual fields, Package now just holds on to a PackageArgs object.
  - In a follow-up CL, we store all the repo() values in a PackageArgs class, and apply those before the package() ones.
- Also removed `Package#isDefaultVisibilitySet` as nobody calls it anymore.

Work towards bazelbuild#18077

PiperOrigin-RevId: 539728608
Change-Id: Ie4a081fa0e52e833ac2a6be50033f95728da3a9b
PiperOrigin-RevId: 539736928
Change-Id: Ib44d22ed8e1e21463f594c880f214d4c46a73b36
ExternalDepsException.

Part of bazelbuild#18629.

Closes bazelbuild#18648.

PiperOrigin-RevId: 539754872
Change-Id: I3d31f823b89952eab2d5ce533947bceb3a1bc205
PiperOrigin-RevId: 539791524
Change-Id: Icdf1d0a0e076bf773908881b42fc78c8e49c31e8
PiperOrigin-RevId: 539802907
Change-Id: Ie7d09d396e7104ade9d52d0c98a86c43fefa69c2
Closes bazelbuild#18635.

PiperOrigin-RevId: 539803930
Change-Id: I011311292d96706f7c38852897b21ae7ab888a64
PiperOrigin-RevId: 539808018
Change-Id: I873e5ac4b2ec039bfc683079800eed77eade4bab
… code.

This implementation has full production fidelity minus visibility checking,
which will be switched on when parent-side rule transitions are deleted from
production code.

Fidelity improvements over the current test implementation.
- Handles Starlark transitions.
- Correctly handles transition keys.

PiperOrigin-RevId: 539828037
Change-Id: Id6f788ebfefca2d9361ed30ff34cb91f76a88673
This simplifies the code a little.

PiperOrigin-RevId: 539844214
Change-Id: Idcb66287fc480327d424d16a5a576f0a7140737b
*** Reason for rollback ***

rule.name was deprecated

PiperOrigin-RevId: 539850239
Change-Id: I9a447fefea2200f6e99a40e7c43fc299080e0f24
PiperOrigin-RevId: 539897609
Change-Id: I69563a3fc5e6bf82497ba97adf50b9fe54dc8630
Partial commit for third_party/*, see bazelbuild#18639.

Signed-off-by: Pavan Singh <pavanksingh@google.com>
samshadwell and others added 26 commits June 27, 2023 11:09
Ran into some confusion around this at work. We recently changed our Bazel repo so that everything was compiling/running with Java 17, but some folks would get issues when `bazel run`ing `java_binary` targets. Error messages indicated they were still running on a Java 8 VM despite the `--java_runtime_version` flag.

Looking at where the stub template is filled in, I noticed that JAVABIN environment variable takes precedence over the java_executable: https://cs.opensource.google/bazel/bazel/+/master:src/main/starlark/builtins_bzl/bazel/java/bazel_java_binary.bzl;l=183?q=javabin&ss=bazel%2Fbazel&start=11

The folks running into issues set their `JAVABIN` in `.zshrc` files, so that was taking precedence over the Bazel flags and incorrectly running the Java 17 compiled code on a Java 8 VM. Seems reasonable that `JAVABIN` take precedence, but I figure this should probably be documented somewhere to save other folks the trouble of digging into the guts to figure out what was going on.

Closes bazelbuild#18564.

PiperOrigin-RevId: 543796927
Change-Id: Ia0eaa6af41ae3589874f5810190aedefc39942bd
…e build configuration.

RELNOTES: None.
PiperOrigin-RevId: 543811424
Change-Id: I36f85c1de2b6a64ad4650b1e5601300e7e1a46b1
…e_write_contents

This is so that we can write out the files in bazel for android,
which converts the aquery results to a ninja file.

PiperOrigin-RevId: 543837984
Change-Id: I9926a282214b293eb46ac376f42b21104e6af43b
This adds an `sbom` rule and uses it to produce one for Bazel.

```
bazel build //tools/compliance:all
more bazel-bin/tools/compliance/bazel_sbom.json
```

This is no where near complete. It just establishes the rule and the helper to have an end to end path. One or more followup PRs will add more fields to the SBOM.

Closes bazelbuild#18789.

PiperOrigin-RevId: 543956376
Change-Id: I9ab593561e58cf4e6a4d2ba8ade3b00bcc947fe5
…facts` flag.

PiperOrigin-RevId: 543976073
Change-Id: Ia961518c6bc9f068d225ce9b8289d8ac8eab7b09
PiperOrigin-RevId: 543981692
Change-Id: Ia8412b24b72e7b807883b6a614ea530543b62fa9
*** Reason for rollback ***

bazelbuild#18771

PiperOrigin-RevId: 544025702
Change-Id: I5c036cda4536f86088f259391cdb7c58ef04df6d
Add flag `--experimental_remote_cache_lease_extension`, which when set, Bazel will create a background thread periodically sending `FindMissingBlobs` requests to CAS during the build.

1. All the outputs that were not downloaded are within the scope of lease extension. The outputs are acquired from skyframe by traversing the action graph.
2. Lease extension starts after any action was built and ends after execution phase ended. The frequency is related to `--experimental_remote_cache_ttl`.
3. Lease extensions are performed on action basis, not by collecting all outputs and issue one giant `FindMissingBlobs`.
    - Collecting all outputs might increase memory watermark and cause OOM.
    - Sending one `FindMissingBlobs` request per action may increase the overhead of network roundtrip, but the cost should be saturated given that the lease extension happens at background and is not wall time critical.
4. For an incremental build, the same applies: lease extension starts after any action was executed.
    - We don't want lease extension blocking action execution, nor affecting build performance.
    - Since we have TTL based cache discarding, any expired blobs will be discarded.
    - Leases of blobs that are not downloaded, still used by this build (because they are referenced by skyframe) will be extended as normal.

Part of bazelbuild#16660.

Closes bazelbuild#17944.

PiperOrigin-RevId: 544032753
Change-Id: Iafe8b96c48abbb2e67302cd7a2f06f97ab43f825
…ker_max_multiplex_instances`

The resulting default value won't change since `MultiResourceConverter` map `null` and `auto` to same value.

PiperOrigin-RevId: 544036500
Change-Id: Ib87dffcf2e96ce1fc64f978005ff9aeeffdffe96
To work around [test failure](https://buildkite.com/bazel/bazel-bazel-github-presubmit/builds/16353#018901e5-b0f1-41be-a6f4-e12878711ad3) on bazelbuild#18608. This PR enables Bazel to detect the latest VC build tools installed, but apparently there is a test case which is failing with the new toolchain. Setting --test_env=BAZEL_VC ensures the integrations tests also uses the older VC build tools for now.

PiperOrigin-RevId: 544037250
Change-Id: I4625da17ff2168acbe63813aa7a01e49e0cb459a
by replacing `mock` with `spy` because we rely on the real method and these methods are final.

Before mockito 5, it uses sublcass mockmaker which cannot mock final method/class. So when we call final methods on these mocked classes, we are calling into the real methods.

Now, it uses inline mockmaker which **can** mock final method/class. Calling final methods on these mocked classes will return `null` if we didn't provide the stub.

This change fixes that by using `spy`.

Closes bazelbuild#18800.

PiperOrigin-RevId: 544038459
Change-Id: Id75a5cb3b9d14e38d6bec918449e5aee671471eb
-1 native aspect

PiperOrigin-RevId: 544044482
Change-Id: I3be0bc6fba70e5e46e2c31e46a9b10a13f2f8a5b
Instead of this flag we should use `worker_multiplex`

PiperOrigin-RevId: 544053771
Change-Id: I1d3a5a54b8990271f61b026bca9a15aa3fa5e908
WindowsSubprocess sets its stdoutStream and stderrStream fields to null when output is redirected to a file.
WindowsSubprocess.close should check whether those fields are null before calling ProcessInputStream.close.

Closes bazelbuild#18759.

PiperOrigin-RevId: 544055326
Change-Id: I91488a45997002ede4d94b2ca4da56f058504fe0
Similar to bazelbuild@5637083, use `spy` instead of `mock` for the test that relies on real method.

The tests rely on `FileArtifactValue#addTo` which is a final method. Before Mockito 5, when we call this method on a mocked object, we call into the real method. With Mockito 5, we can mock the final method and now they are doing nothing which breaks the tests.

PiperOrigin-RevId: 544056698
Change-Id: Ie4340ee9cf2615928cc3df4b11be891ef9ddb3d2
Yanked module versions no longer contribute dependency requirements or emit `DEBUG` messages for `print()` statements.

Since the module files of yanked modules are still evaluated to learn their compatibility levels, they can still fail to execute.

Closes bazelbuild#18698.

PiperOrigin-RevId: 544059396
Change-Id: I8a37d5c7975947cd717f6e56d97cce467f22178e
Closes bazelbuild#18783.

PiperOrigin-RevId: 544060603
Change-Id: I1ec59f74f2c85ee7a927024842cd94825d5e211b
I've fixed relevant tests.

I've added and exposed 2 simple Starlark functions relevant for unions of j2objc providers in Native code: j2objc_mapping_file_info_union and j2objc_entry_class_info_union.

PiperOrigin-RevId: 544073703
Change-Id: I7785a13e29b441f075f69cc174882f3250ec238e
It used to be an invariant in the cc_shared_library design that the rule would
only link the dynamic_deps (other cc_shared_libraries) that were exporting
cc_library targets coming from the cc_library graph (in other words targets
reachable from cc_shared_library.deps, formerly cc_shared_library.roots).
However, these were not being linked silently without giving an error. It
also turns out that it's a valid use case not to require the library in the
cc_library graph  when for example owners of the cc_shared_library target want
users to only depend on the dynamic library and make their cc_library private
so that it's never linked statically.

cc_binary already allowed this and linked the unused direct dynamic_deps.

RELNOTES:none
PiperOrigin-RevId: 544076791
Change-Id: I78668c6cc26676922cd1478e290019ca4fccd675
…pyOf` is already an `ImmutableMap`.

Also use `forEach` for iteration to avoid creating entry set views.

PiperOrigin-RevId: 544088895
Change-Id: Iac12fe7754b5c95f5939e4f2124935abc26d955d
…hange`.

This way we have post-facto debugging information for why we dropped the analysis cache. In particular, this is useful for when the cause is a different build configuration; Blaze already prints a helpful configuration diff to stderr but the user might not have saved that or noticed it (and even comparing the full set of options we already log to INFO is a bit tedious).

PiperOrigin-RevId: 544102574
Change-Id: Ia97ea099dea20ec97e38259db609867192fc65ff
… so that synthetic classes don't end up without their context classes in dex shards, which will causing merging to fail in DexFileMerger. Fixes bazelbuild#16368.

RELNOTES: None
PiperOrigin-RevId: 544134712
Change-Id: Ib29f6659f18dd71be96a7985bc25cfb44e719ae5
Add a new feature to bazel: use_lto_native_object_directory.
When this feature is enabled, the thinlto objects are stored in a different directory as the .imports and .thinlto.bc file outputs from the indexing action, and thinlto can be used in the presence of tree artifacts.

Create LtoBackendActionTemplate.java to generate lto backend actions to tree artifact.

Minor: move constants to rules/cpp/CppFileTypes.java and rules/cpp/CppHelper.java for consistency.

Limitations:
There is still no support for .dwo files for tree artifacts, i.e. compiling using fission. TreeArtifacts do not generate minimized bitcode files, only full bitcode files, because of lack of support at the indexing stage.
PiperOrigin-RevId: 544144581
Change-Id: Idc6a638a6fccef6912b79948ea167dd6e651f156
PiperOrigin-RevId: 544163119
Change-Id: I43daab85586586d5b0acfb9f349a7259e45ee6b6
ResourceProcessorBusyBox is being used in the Android platform build, which includes CTS tests that set `--package-id 0x80` when running aapt2 link.  This produces R.txt files with ids that are out of range of java integers, for example `int color blue 0x80020000`.  In an R.java file, javac interprets these as negative integers, but IntFieldInitializer throws a NumberFormatException.  Make IntFieldInitializer act like javac by parsing the number as a Long and then narrowing it to an int.

PiperOrigin-RevId: 544184014
Change-Id: Ida26d00a4c487830a8b35bbdf58dd9a5c5411adc
…classes when sharding dexes, like DexFileSplitter in bazelbuild@9092303. Fixes bazelbuild#16368

RELNOTES: None
PiperOrigin-RevId: 546041575
Change-Id: I12fa8cff15b13534ca9a5682b7e9b43e6c860ef8
@ahumesky ahumesky requested a review from iancha1992 as a code owner July 6, 2023 19:07
@ahumesky ahumesky closed this Jul 6, 2023
@ahumesky ahumesky deleted the bazel-6.3.0-synthetic-classes-dexmapper branch July 6, 2023 19:08
@iancha1992 iancha1992 removed their request for review July 6, 2023 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.