Ensure outputs are downloaded before sending TargetComplete event via BEP

[coeuvre]

Description of the feature request:

Currently, when building with --remote_download_toplevel, outputs of toplevel targets / aspects are only guaranteed downloaded before the build command exits.

However, the consumers of BEP, e.g. IDE, may listen to TargetComplete event from BEP and expect outputs are ready to be processed when the event is fired.

What underlying problem are you trying to solve with this feature?

We should wait for the downloads before sending TargetComplete event to not break the expectations for consumers of BEP.

Which operating system are you running Bazel on?

No response

What is the output of bazel info release?

No response

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

#6862
b/273514526

[clint-stripe]

@coeuvre is there any work around besides disabling --remote_download_toplevel?

We have been using that flag for a while, and we're blocked by this issue as we upgrade from 5.4.0 to 6.1.1.

Our CI system uses BEP events to report test failures early and to locate artifacts -- we're seeing a last_message BEP event before some artifacts are written to disk, which is breaking assumptions our system made about locating build outputs on disk.

[coeuvre]

I am not aware of a workaround, but I am working on the fix and hopefully include it in 6.2.

[coeuvre]

@bazel-io flag

[keertk]

@bazel-io fork 6.2.0

[coeuvre]

Fixed by a5dde12.

[keertk]

@coeuvre should this be included in 6.3.0?

[coeuvre]

It's very hard to cherry-pick that commit into 6.x due to massive structure changes recently. But I understand that this is a blocking issue so I will figure out how to fix it for 6.x. Please mark it as blocker for 6.3 for now.

[keertk]

@bazel-io fork 6.3.0

[keertk]

Hi @coeuvre wanted to check if we should hold off on 6.3 for this? We tentatively scheduled RC1 for 7/5.

[coeuvre]

Yes, we should. I will work on the workaround for 6.3.0 now.

[hoj-stripe]

@coeuvre Hi! We're trying to use the --experimental_remote_cache_eviction_retries flag and start evicting remote cache, but we're getting errors since we're also using--experimental_remote_download_outputs=toplevel. After bisecting on commits, we found out that the exact issue we are facing is unblocked with this commit(a5dde12) mentioned in this issue.

I was wondering if you could help us unblock by backporting this commit onto 6.x. Have you had any luck on cherrypicking this commit to Bazel 6.x or finding the workaround for the evicted cache? Any help would be appreciated!

[coeuvre]

a5dde12 cannot be cherry-picked into 6.x because of missing many other commits. The dedicated fix I created for 6.3 is a subset of a5dde12.

Can you share more details about the error so that I can learn the underlying issue and maybe create another dedicated fix? (it's better if we can move the discussion into a new issue)