commit c56497707f283f3aa1ab34abbcb88470fb5f3207
author gregce <gregce@google.com> Thu Jul 08 13:10:19 2021 -0700
committer Copybara-Service <copybara-worker@google.com> Thu Jul 08 13:11:57 2021 -0700
tree c98f1b7b99aac4250ba6183b694ffcad6462813b
parent 60417909eab070373d821106329b363d1c414b18

Tidy ConfiguredTargetFunction exception handling.

tl;dr: refactor messy code for clearer error messages

 - Clarify the difference between ConfiguredValueCreationException and DependencyEvaluationException:
 - ConfiguredValueCreationException: a failure of the target being evaluated. The current target is responsible for reporting the error to the user.
 - DependencyEvaluationException: a failure of one of its deps. The dep will report its own error so the current target only needs to report extra contextualizing information. Sometimes (unfortunately) this isn't possible, so this is togglable.
 - Include locations more consistently
 - Consolidate compute() catch logic
 - Consolidate error message reporting into a single place (in the compute() method) as much as possible
 - Clear error messages before sending them to ConfiguredTargetFunction. This
   prevents BuildTool from redundantly reporting those same messages.

This is only a partially successful attempt to clean up CTF's general error handling and reporting. The affected code has a lot of mutual dependencies that are hard to factor out. There's plenty more room for improvement. My hope is this at least improves on the status quo.

Followup ideas:

 - Try to reduce the number of Exception types. It's hard to keep track of them, adds a lot of code, and developers will inevitably use the wrong exception the wrong way
 - Better consolidate DependencyEvaluationException: it either represents a dep configured target that reported its own errors or it doesn't
 - Consolidate AspectFunction similarly to ConfiguredTargetFunction

Examples:

####################################
BEFORE (long error repeats twice):
####################################

ERROR: /myroot/errortest/BUILD:6:10: configurable attribute "srcs" in //errortest:cc doesn't match this configuration. Would a default condition help?

Conditions checked:
 //errortest:foo

To see a condition's definition, run: bazel query --output=build <condition label>.

This instance of //errortest:cc has configuration identifier 1fae62e. To inspect its configuration, run: bazel config 1fae62e.

For more help, see https://docs.bazel.build/configurable-attributes.html#why-doesnt-my-select-choose-what-i-expect.

ERROR: Analysis of target '//errortest:cc' failed; build aborted: configurable attribute "srcs" in //errortest:cc doesn't match this configuration. Would a default condition help?

Conditions checked:
 //errortest:foo

To see a condition's definition, run: bazel query --output=build <condition label>.

This instance of //errortest:cc has configuration identifier 1fae62e. To inspect its configuration, run: bazel config 1fae62e.

For more help, see https://docs.bazel.build/configurable-attributes.html#why-doesnt-my-select-choose-what-i-expect.

FAILED: Build did NOT complete successfully (41 packages loaded, 154 targets configured)

####################
AFTER:
####################

ERROR: /myroot/errortest/BUILD:6:10: configurable attribute "srcs" in //errortest:cc doesn't match this configuration. Would a default condition help?

Conditions checked:
 //errortest:foo

To see a condition's definition, run: bazel query --output=build <condition label>.

This instance of //errortest:cc has configuration identifier 5ece2f8. To inspect its configuration, run: bazel config 5ece2f8.

For more help, see https://docs.bazel.build/configurable-attributes.html#why-doesnt-my-select-choose-what-i-expect.

ERROR: Analysis of target '//errortest:cc' failed; build aborted:
FAILED: Build did NOT complete successfully (41 packages loaded, 154 targets configured)

####################################
BEFORE (redundant messaging):
####################################

ERROR: While resolving configuration keys for //testapp:g: no such target '//barr:c1': target 'c1' not declared in package 'barr' defined by /myroot/barr/BUILD
ERROR: Analysis of target '//testapp:g' failed; build aborted: no such target '//barr:c1': target 'c1' not declared in package 'barr' defined by /myroot/barr/BUILD

####################
AFTER:
####################
ERROR: /myroot/barr/BUILD: no such target '//barr:c1': target 'c1' not declared in package 'barr' defined by /myroot/barr/BUILD
ERROR: /myroot/testapp/BUILD:19:8: errors encountered resolving select() keys for //testapp:g
ERROR: Analysis of target '//testapp:g' failed; build aborted:
PiperOrigin-RevId: 383699655