Skip to content

Rebase master#1

Closed
AlessandroPatti wants to merge 513 commits intouber-common:masterfrom
bazelbuild:master
Closed

Rebase master#1
AlessandroPatti wants to merge 513 commits intouber-common:masterfrom
bazelbuild:master

Conversation

@AlessandroPatti
Copy link

No description provided.

atetubou and others added 30 commits May 21, 2019 05:50
bazel supports application default credentials to access GCS bucket too.

Closes #8297.

PiperOrigin-RevId: 249233903
The jacoco runner was previously retrieved via an implicit attribute `$jacocorunner` on the `java_binary` and `java_test` rules. Retrieving the runner via the implicit attribute makes testing the tools in the `java_tools` release difficult and inconsistent (almost all other tools are defined via `java_toolchain`).

This PR makes `jacocorunner` part of `java_toolchain`. If `jacocorunner` is not found in the toolchain it falls back to the `$jacocorunner` attribute. The fallback behavior can be removed after a bazel release with this change.

Closes #8378.

PiperOrigin-RevId: 249241175
This is in preparation of flipping --incompatible_disable_deprecated_attr_params

#5818

RELNOTES: None.
PiperOrigin-RevId: 249241398
This will enable content-based output paths, as described at
https://docs.google.com/document/d/17snvmic26-QdGuwVw55Gl0oOufw9sCVuOAvHqGZJFr4/edit#heading=h.5mcn15i0e1ch.

See #8339.

Notes:

* I originally tried to put this in BuildRequestOptions since this goes
  beyond configuration (it also affects execution-phase logic). But
  piping that to the right consumers was a lot more awkward, so here
  it is.
* One nice thing about keeping this in BuildConfiguration is switching it
  automatically invalidates configured targets without having to pass
  external Skyframe context.
* This should arguably be a startup option since it's effect on action
  caches, the general build graph, action execution logic, and output
  metadata services could be potentially far-reaching. I wouldn't
  particularly call this a flag where it's safe to keep all Bazel's
  incremental state when switching. But I'm trying to avoid use of
  startup options and we might be able to get away just fine with
  with judicious use of reset() calls to things like SkyframeBuildView.
  For the current super-experimental alpha phase I think these risks
  are worth the ease-of-use putting this in BuildConfiguration offers.

PiperOrigin-RevId: 249241985
Change TargetPatternResolver.getTargetsInPackage and the
TargetPatternPreloader to return Collections, rather than
ResolvedTargets instances.

No implementation currently sets the error bit, and no caller requires
it, and the set of filtered targets is always empty.

Make sure that getTargetsMatchingPatternImpl uses a Set to avoid
quadratic runtime. While this may cancel out the improvements from this
change, I have some follow-up changes which use the result and don't
require a Set implementation. I missed this regression because I was
testing it with the additional changes, not independently.

PiperOrigin-RevId: 249243165
Decompressing large archives can take a while. Therefore, properly
honor interruption requests.

Improves on #8346.

Change-Id: I648bd1dc4b9e1f4b03065ffff3d9d66cd9cc92a1
PiperOrigin-RevId: 249246136
RELNOTES: None.
PiperOrigin-RevId: 249248191
RELNOTES: None.
PiperOrigin-RevId: 249252357
Certain operations in the repository cache take some time, e.g.,
verifying the checksum of a file. Allow interrupts while doing so.

While touching SkylarkRepositoryContext.java anyway, remove dead code there.

Fixes #8346

Change-Id: I26bbdb314554271949a34555600df1c35cc1544d
PiperOrigin-RevId: 249253196
As of 4a5e1b7, it was using getErrorPath twice, which could cause it to
loop indefinitely, trying to append the error file to itself. This
happened rarely, as the test runner script redirects stderr to stdout.
However, it could happen if the SpawnRunner wrote any extra output to
stderr, which the RemoteSpawnRunner does in some cases.

I have manually checked that this fixes the issue, and also added a
regression test.

Fixes #8320.

PiperOrigin-RevId: 249258656
See bazelbuild/continuous-integration#578 for context. The gist of it is that when PATH is not set (as happens when a binary using this toolchain is used as a tool from Starlark), the shell comes up with its own PATH to run the pywrapper script. However, it does not necessarily export this PATH, causing "which" to misbehave and fail to locate a Python interpreter.

The fix is to add "export PATH" and a regression test.

Fixes bazelbuild/continuous-integration#578, work toward #7899.

RELNOTES: None
PiperOrigin-RevId: 249263849
…ad network connection.

PiperOrigin-RevId: 249268815
PiperOrigin-RevId: 249279580
This can be used to quickly determine version information without needing to
start a server or unpack multiple levels of zip files.

PiperOrigin-RevId: 249288992
This is a modification of PR #8415, which changed `which` to `command -v` so it works when PATH isn't exported. `command` is more standard for this kind of use case (https://github.com/koalaman/shellcheck/wiki/SC2230). Unfortunately, `command -v` doesn't check the executable bit, so it's not as useful for us.

The previous fix for this issue (7f49531) was based on exporting PATH, but this changes the environment seen by the exec'd payload Python code.

The solution in this commit is to not export PATH but rather explicitly pass it to each invocation of `which`.

Fixes #8414. See also bazelbuild/continuous-integration#578. Closes #8415.

PiperOrigin-RevId: 249334274
Closes #8405.

PiperOrigin-RevId: 249336291
Renames `FileCount` to `FileCountInfo`.

https://github.com/bazelbuild/buildtools/blob/master/WARNINGS.md#name-conventions suggests providers should be UpperCamelCase ending with `Info`

Closes #8408.

PiperOrigin-RevId: 249338887
Progress towards #5816

RELNOTES: None.
PiperOrigin-RevId: 249341642
RELNOTES: Introduce --incompatible_disable_native_android_rules flag
PiperOrigin-RevId: 249348930
BranchDetailAnalyzer should have also been updated as part of ff1f745 when the jacoco version in bazel was updated.

Changes like this will be avoided after #8378 is merged, because it enables testing with the java coverage tools at head.

Closes #8417.

PiperOrigin-RevId: 249400130
set --incompatible_list_based_execution_strategy_selection=true

Fixing #8372

Closes #8434.

PiperOrigin-RevId: 249417095
See #3889

Change-Id: Id768d8bd0cb996af566cdda518772ea329e5c6b2

Closes #8420.

Change-Id: I59c7952582decee585bbb23f7e8dc6d88e21aa73
PiperOrigin-RevId: 249431252
Also enable jacocorunner in BUILD.java_tools since #8378 is now merged.

RELNOTES: None.
PiperOrigin-RevId: 249442764
Partial commit for third_party/*, see #8431.

Signed-off-by: iirina <elenairina@google.com>
It might be nicer to tell the user that we're going to wait, but that is
not currently possible. Instead, tell the user that Bazel waited for
upload to complete for a certain amount of time.

Updated test to avoid being too specific.

PiperOrigin-RevId: 249458848
laurentlb and others added 28 commits June 5, 2019 08:31
RELNOTES: None.
PiperOrigin-RevId: 251647331
RELNOTES: None.
PiperOrigin-RevId: 251650433
…ception.

Also, move some code that doesn't need to be in the try block out of it.

Closes #8266.

PiperOrigin-RevId: 251673653
PiperOrigin-RevId: 251684426
PiperOrigin-RevId: 251684680
…parsing.

This is the same behavior as during transitions. Since we don't know all related starlark options at the top of the build (like we do with native options) we need to make set to default == unset == unstored.

PiperOrigin-RevId: 251695315
Demote from global-always-mutable state to sometimes-mutable state that's
passed around, making it clear what depends on it.

We should be able to mark StartupOptions const in many of the places we use it,
but the GetArgumentArray calls peppered throughout the code rely on non-const
GetServerJavabase. I'll figure out how to deal with that in another pass, either
by making GetServerJavabase const, lifting its use to a place that doesn't
matter, only computing args once, or some combination of such.

I stuck to passing a StartupOptions* in the code to keep the mass
"globals->options->" to "startup_options->" migration less interesting. By
reference would probably be better, but I think maybe for another go...

As usual, a downside to this is growing argument lists, but I think that's
better than guessing or having no enforcement of when a method might update
globals->options.

PiperOrigin-RevId: 251700379
…ion class

- Rationale: Keep annotation attributes copied.
- Addressing post-submit comments of c89c11c

PiperOrigin-RevId: 251703322
- On Bazel website, the single-page option is not used.
- Remove the useless "Content" header

RELNOTES: None.
PiperOrigin-RevId: 251703368
PiperOrigin-RevId: 251708686
This is a performance optimization following up on https://bazel-review.googlesource.com/c/bazel/+/101952.

That change reduced the number of BuildOptions hash key computations on a simple manually trimming android_binary build from 809 to 475, compared to 138 when manual trimming is disabled (--enforce_transitive_configs_for_config_feature_flag=false). But I wanted to dig further into the remaining (+337) difference.

Profiling showed most of these were deps on platform and toolchain rules. For example, transitions from @bazel_tools//platforms:[host|target]_platform to their constituent constraint_values accounted for 192 of the difference. Transitions from toolchain rules to their deps made up most of the rest.

This highlights the following inefficiency in the intersection of manual trimming and toolchain resolution:

Given "rule -> unloaded toolchain context -> all toolchains and platforms and their dependencies", the current logic uses the rule's configuration for the toolchain context and top-level platform/toolchain definitions. If this happens when "rule" is an android_binary setting transitive_configs, this means those definitions inherit its feature flags in *their* configs.

Then *their* deps, which don't use feature flags, go through the standard manual trimming transition, which removes those flags which requires new BuildOptions for each one.

In other words:

 <@bazel_tools//platforms:target_platform, featureFlags=[foo, bar]>
    --> <some_platform_constraint1, featureFlags=[]>   # Remove flags: creates a new BuildOptions
    --> <some_platform_constraint2, featureFlags=[]>   # Remove flags: creates a new BuildOptions
    --> <some_platform_constraint3, featureFlags=[]>   # Remove flags: creates a new BuildOptions
    ...

and so on.

There's no need to do that for all those dependencies. Instead, we can apply the transition directly on the "rule -> unloaded toolchain context" dependency, of which there's only one. As long as toolchains and platforms don't actually need feature flags (which they shouldn't), this is safe.

So this PR does that. With that change, we get:

Manual trimming disabled:
--------------------------------------------------------------------------
BuildOptions hashkey calls:                                                       137
ConfigurationResolver transition.apply() calls:                                   447
Manual trimming calls creating a new BuildOptions instance:                       0
ConfiguredTargetFunction total deps evaluated:                                    442,617
ConfiguredTargetFunction non-trivial (not NOOP, NULL, Transition) deps evaluated: 1,419
ConfiguredTargetFunction non-trivial transition.apply calls:                      445

Manual trimming enabled, before this change:
--------------------------------------------------------------------------
BuildOptions hashkey calls:                                                       494
ConfigurationResolver transition.apply() calls:                                   588
Manual trimming calls creating a new BuildOptions instance:                       227
ConfiguredTargetFunction total deps evaluated:                                    443,126
ConfiguredTargetFunction non-trivial (not NOOP, NULL, Transition) deps evaluated: 1,932
ConfiguredTargetFunction non-trivial transition.apply calls:                      586

Manual trimming enabled, after this change:
--------------------------------------------------------------------------
BuildOptions hashkey calls:                                                       294
ConfigurationResolver transition.apply() calls:                                   466
Manual trimming calls creating a new BuildOptions instance:                       78
ConfiguredTargetFunction total deps evaluated:                                    438,699
ConfiguredTargetFunction non-trivial (not NOOP, NULL, Transition) deps evaluated: 1,496
ConfiguredTargetFunction non-trivial transition.apply calls:                      464

PiperOrigin-RevId: 251708695
…is is a general clean-up, but also I'm getting a bizarre test failure from the @AutoCodec annotation and it goes away when I remove it.

PiperOrigin-RevId: 251715454
*** Reason for rollback ***

Rolling forward with fix: keep passing artifact owner into generated artifact, sacrificing a bit of type safety for actual safety. This effectively reverts a few files from the original CL.

Also fix memory regression: key map of output file artifacts by label, since we can reuse existing labels. Moreover, this allows us to have an empty map for many configured targets: a build of a large internal target had the following numbers of targets per # of output files (apologies for the only partially sorted map):

{0=63649, 1=67148, 2=89175, 3=49347, 4=12712, 5=3027, 261=1, 6=247, 7=9, 775=1, 8=10, 9=82, 10=6, 11=3, 12=54, 13=4, 14=2, 15=24, 655=1, 271=2, 144=1, 17=2, 18=24, 276=1, 21=9, 3990=2, 24=18, 27=11, 156=3, 30=6, 31=2, 33=12, 34=1, 163=1, 36=5, 37=1, 38=2, 294=1, 39=5, 41=1, 42=3, 43=2, 45=9, 302=1, 47=1, 48=4, 561=1, 306=2, 54=2, 183=1, 57=1, 573=2, 318=2, 63=2, 65=1, 66=1, 198=1, 71=1, 72=2, 330=2, 75=2, 204=1, 76=1, 77=2, 80=1, 82=1, 84=3, 87=2, 346=2, 90=4, 480=1, 99=5, 102=1, 231=2, 363=1, 624=2, 117=1, 120=2, 123=2, 252=2}

So those are the sizes of the maps that are actually needed. With the regression fixed, this change actually has a significant progression because we no longer create a duplicate derived artifact (with PathFragments, etc.) per OutputFileConfiguredTarget. Moreover, we don't need to keep a map with every artifact as keys inside ActionLookupValue and RuleConfiguredTarget. The only regression remaining is that we eagerly create ActionLookupData objects for every artifact, versus only on demand during execution. That's not a major issue, and it's ameliorated by no longer needing to intern ActionLookupData objects, which has memory and CPU benefits. Total memory diff for analysis of our large internal target looks very nice (net -125M out of 5.88G, or 2% gain):

 objsize  chg   instances       space KB    class name
------------------------------------------------------
      24   +0     1345958       31545 KB    com.google.devtools.build.lib.actions.ActionLookupData
      70   +0        -334        1992 KB    [Ljava.lang.Object;
      16   +0      -19073        -298 KB    java.lang.Integer
      40   +0      -12795        -499 KB    com.google.common.collect.SingletonImmutableBiMap
      40   +0      -45433       -1774 KB    com.google.common.collect.MapMakerInternalMap$WeakKeyDummyValueEntry
      32   -8           0       -1852 KB    com.google.devtools.build.lib.analysis.ConfiguredAspect
      24   +0      -79943       -1873 KB    com.google.common.collect.SingletonImmutableSet
      24   +0      -79943       -1873 KB    com.google.common.collect.ImmutableEntry
      40   +0      -61756       -2412 KB    com.google.common.collect.RegularImmutableMap
      16   +0     -216644       -3385 KB    com.google.common.collect.RegularImmutableList
      24   +0     -216650       -5077 KB    com.google.common.collect.ImmutableMapEntrySet$RegularEntrySet
      44   -4      -61755       -7376 KB    [Ljava.util.Map$Entry;
      44   -3      -61757       -7409 KB    [Lcom.google.common.collect.ImmutableMapEntry;
      32   +0     -259112       -8097 KB    com.google.devtools.build.lib.actions.Artifact$DerivedArtifact
      24   +0     -522911      -12255 KB    com.google.devtools.build.lib.vfs.PathFragment
      24   +0     -525254      -12310 KB    java.lang.String
      24   +0     -541285      -12686 KB    com.google.common.collect.ImmutableMapEntry$NonTerminalImmutableMapEntry
      24   +0     -999865      -23434 KB    com.google.common.collect.ImmutableMapEntry
      85   +0     -524326      -56276 KB    [B
------------------------------------------------------
 total change:                -125514 KB

PiperOrigin-RevId: 251755552
…ains where needed.

#8531

RELNOTES: None.
PiperOrigin-RevId: 251796866
RELNOTES: None.
PiperOrigin-RevId: 251800089
RELNOTES: None.
PiperOrigin-RevId: 251805344
Fixes #8388.

RELNOTES:none
PiperOrigin-RevId: 251812159
Previously, the ListenableFuture callback was running using a direct
executor, which means it gets run in the thread that completes the
remote spawn future. In some cases, this may be a much smaller thread
pool, which can have a significant (negative) impact on build
performance.

Note that we primarily need to decouple the callback attached to the
remote spawn future. Subsequent futures can use direct executor if we're
reasonably sure that the attached-to future is already using the
includePool and there's no additional performance win due to
parallelism possible.

There is one problem here, which is that we use a slack pool for include
scanning, which does not guarantee decoupling, as it can reuse the
caller thread if the pool is fully used; this does not seem to be a
performance problem for the benchmark I tested.

With this change, I see no significant performance difference between
the sync and async include scanners.

PiperOrigin-RevId: 251812988
…s only when necessary to avoid unnecessary memory consumption

RELNOTES: None.
PiperOrigin-RevId: 251819397
Baseline: c2001a4

Cherry picks:

   + e67c961:
     Fix a non-determinism in create_embedded_tools.py.
   + 81aefe7:
     Remove unsupported cpu attribute from cc_toolchains.
   + 597e289:
     remote: made CombinedCache a composition of Disk and Http Cache
   + 942f7cf:
     C++: Fixes bug in C++ API with external repo aspects
   + 85a5a2b:
     Configure @androidsdk//:emulator_x86 and :emulator_arm to point
     to the unified emulator binary
   + 9835cb4:
     Automated rollback of commit
     844e4e2.
   + c963ba2:
     Windows, Python: fix arg. esc. also in host config
   + a1ea487:
     Do not pre-cache changed files under managed directories
   + 7dc78cd:
     Add explicit execution and target constraints for autodiscovered
     cc t?
   + dd9ac13:
     Fix a bug when a relative path is used for the execution log
   + 0ff19c6:
     Fix StandaloneTestStrategy.appendStderr
   + 7f49531:
     Fix the autodetecting Python toolchain on Mac
   + ddce723:
     Avoid exporting PATH unnecessarily
   + 35dd05a:
     Allow Starlark rules to be able to use the `exec_compatible_with`
   + cb82ed8:
     Release 0.26.0 (2019-05-28)
   + d1c0d20:
     Allow WORKSPACE file to be a symlink if no managed directories
     is used.
   + c3d2aa7:
     Fix ios, tvos and watchos arm64 constraints
   + 55e4205:
     Bump java_tools_javac10 from 3.1 to 3.2

Patch release on top of 0.26.0, fixing
- #8475
- #8520
- bazelbuild/intellij#845
Ubuntu 14.04 is end-of-life and Bazel CI will stop supporting it.
Context: https://groups.google.com/d/msg/bazel-dev/_D6XzfNkQQE/8TNKiNmsCAAJ

We still have to build it on Ubuntu 14.04 for a time, because Bazelisk currently tries to explicitly download ubuntu1404 binaries on all Linux platforms.

PiperOrigin-RevId: 251824753
As baseline we took the release of 0.26.0, and the only interesting
thing to know is what has been cherry-picked since then.

Change-Id: I59a773d48ed9acc3aa66cafeee6367b4c6b12a8f
We already have an internal event reporting the failure of a
fetch of an external repository. Make this a BuildEvent so
that it also gets reported in the Build Event Protocol; also,
refer to this event as root cause in case of fetch failures
rather than to any needed file of the external repository
individually.

Fixes #6670.

Change-Id: I4f31940fe0c5c709673a99e5657274e5db922cff
PiperOrigin-RevId: 251846972
The previous javac version used by the default_java_toolchain in Bazel had a non-standard modification to default to the Java 8 language level, which wasn't carried forward to 11. Adding default values for {source,target}_version in the default_java_toolchain to keep Bazel backwards compatibility.

Fixes #8539.

Closes #8569.

PiperOrigin-RevId: 251848351
chiragramani pushed a commit that referenced this pull request Oct 11, 2023
… order.

1. `TestAttempt` events would wait for the `TargetCompleteEvent` to be posted before being posted.

2. There was an implicit requirement for the `TestAttempt` events to be posted in a specific order.

3. This didn't break in the noskymeld case because we fulfilled this ordering by using the order of performing the attempts themselves. The sequence would look like:
    + post `TargetCompleteEvent`
    -> perform attempt #1
    -> post `TestAttempt` #1
    -> perform attempt #2
    -> post `TestAttempt` #2

4. With skymeld, however, it could happen like this:

    + defer `TargetCompleteEvent` to wait for `CoverageActionFinishedEvent`
    + perform attempt #1 -> defer posting `TestAttempt` #1 & wait for `TargetCompleteEvent`
    + perform attempt #2 -> defer posting `TestAttempt` #2 & wait for `TargetCompleteEvent`
    + `CoverageActionFinishedEvent` -> release & post `TargetCompleteEvent`
    + `TargetCompleteEvent` -> release & post `TestAttempt` #2
    + `TargetCompleteEvent` -> release & post `TestAttempt` #1

Due to (2), the undefined ordering in (4) would cause an issue.

This CL fixes that by ensuring a FIFO ordering of the deferred events.

PiperOrigin-RevId: 572165337
Change-Id: Iac4d023d946865b8b81f15b119417192dc4b5c53
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.