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

Backtrack through calls from @rules to synchronous engine methods #15979

Merged
merged 5 commits into from
Jun 29, 2022

Conversation

stuhood
Copy link
Member

@stuhood stuhood commented Jun 28, 2022

As reported in #15954, backtracking doesn't currently work when a synchronous method like Workspace.write_digests is the source of a MissingDigest error. This was due to a TODO left behind in #15761, where we did not propagate the Failure type "through" @rule bodies.

To fix this, we add a conversion from Failure to PrErr which wraps in a well known exception type, and then look for that type when converting back from PyErr to Failure.

Fixes #15954.

@stuhood stuhood added needs-cherrypick category:bugfix Bug fixes for released features labels Jun 28, 2022
@stuhood stuhood added this to the 2.13.x milestone Jun 28, 2022
stuhood added 4 commits June 28, 2022 15:34
…rrors out `@rule` bodies.

[ci skip-build-wheels]
…` method.

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
@stuhood stuhood force-pushed the stuhood/python-missing-digests branch from c267b73 to 9c418b0 Compare June 28, 2022 22:34
@stuhood stuhood marked this pull request as ready for review June 28, 2022 22:39
@stuhood
Copy link
Member Author

stuhood commented Jun 28, 2022

Commits are useful to review independently.

Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat test!

src/python/pants/base/exceptions.py Outdated Show resolved Hide resolved
src/rust/engine/src/externs/mod.rs Show resolved Hide resolved
# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
@stuhood stuhood enabled auto-merge (squash) June 28, 2022 23:03
@stuhood stuhood merged commit ba9f863 into pantsbuild:main Jun 29, 2022
@stuhood stuhood deleted the stuhood/python-missing-digests branch June 29, 2022 03:23
tdyas pushed a commit to tdyas/pants that referenced this pull request Jun 29, 2022
…antsbuild#15979)

As reported in pantsbuild#15954, backtracking doesn't currently work when a synchronous method like `Workspace.write_digests` is the source of a `MissingDigest` error. This was due to a TODO left behind in pantsbuild#15761, where we did not propagate the `Failure` type "through" `@rule` bodies.

To fix this, we add a conversion from `Failure` to `PrErr` which wraps in a well known exception type, and then look for that type when converting back from `PyErr` to `Failure`.

Fixes pantsbuild#15954.
# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
stuhood added a commit to stuhood/pants that referenced this pull request Jun 29, 2022
…antsbuild#15979)

As reported in pantsbuild#15954, backtracking doesn't currently work when a synchronous method like `Workspace.write_digests` is the source of a `MissingDigest` error. This was due to a TODO left behind in pantsbuild#15761, where we did not propagate the `Failure` type "through" `@rule` bodies.

To fix this, we add a conversion from `Failure` to `PrErr` which wraps in a well known exception type, and then look for that type when converting back from `PyErr` to `Failure`.

Fixes pantsbuild#15954.
# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
stuhood added a commit that referenced this pull request Jun 29, 2022
…Cherry-pick of #15979) (#16001)

As reported in #15954, backtracking doesn't currently work when a synchronous method like `Workspace.write_digests` is the source of a `MissingDigest` error. This was due to a TODO left behind in #15761, where we did not propagate the `Failure` type "through" `@rule` bodies.

To fix this, we add a conversion from `Failure` to `PrErr` which wraps in a well known exception type, and then look for that type when converting back from `PyErr` to `Failure`.

Fixes #15954.

[ci skip-build-wheels]

Co-authored-by: Stu Hood <stuhood@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:bugfix Bug fixes for released features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

./pants run crashes when remote_cache_eager_fetch = false
3 participants