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

Include dependency edge info in transition errors #19251

Closed
wants to merge 2 commits into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Aug 15, 2023

When a Starlark transition implementation function fails, it now prints an error such as:

ERROR: /home/user/example/BUILD.bazel:3:11: On dependency edge //pkg:wrapper (ed19e2b) -|exports|-> //pkg:real_target: Errors encountered while applying Starlark transition

Previously, the generated error messages would only point to the BUILD file location of the target with the outgoing edge as well as a stack trace of the transition function (the latter only for Starlark errors, not those encountered during option conversion).

@fmeum fmeum marked this pull request as ready for review August 15, 2023 11:51
@fmeum fmeum requested a review from a team as a code owner August 15, 2023 11:51
@fmeum fmeum requested review from gregestren and removed request for a team August 15, 2023 11:51
@github-actions github-actions bot added awaiting-review PR is awaiting review from an assigned reviewer team-Configurability Issues for Configurability team labels Aug 15, 2023
@fmeum
Copy link
Collaborator Author

fmeum commented Aug 15, 2023

@gregestren This was born out of the need to debug a failing transition created by a macro. Happy to change essentially anything about the error message, I just found that having "from target" and attribute in it. "to target" isn't really necessary information as the transition logic can't depend on it, so I'm happy to drop this if you find it confusing.

When a Starlark transition implementation function fails, it now prints
an error such as:

```
ERROR: /home/user/example/BUILD.bazel:3:11: On dependency edge //pkg:wrapper -|exports|-> //pkg:real_target: Errors encountered while applying Starlark transition
```

Previously, the generated error messages would only point to the BUILD
file location of the target with the outgoing edge as well as a stack
trace of the transition function (the latter only for Starlark errors,
not those encountered during option conversion).
@gregestren
Copy link
Contributor

I honestly find having toTarget less confusing. While you're right that the transition logic doesn't use it, it it's natural to me to read a dependency edge as a from -[attribute]> to trio.

@gregestren gregestren added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Aug 15, 2023
@fmeum
Copy link
Collaborator Author

fmeum commented Aug 15, 2023

@gregestren I just noticed that this could be even more useful if it also included the configuration hash. Sorry for the approval invalidation, could you take another look?

@fmeum fmeum requested a review from gregestren August 15, 2023 13:54
@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Aug 15, 2023
@fmeum fmeum deleted the dependency-edge branch August 16, 2023 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Configurability Issues for Configurability team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants