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

--loop --changed-since does not behave entirely as expected. #13757

Open
xlevus opened this issue Dec 1, 2021 · 3 comments
Open

--loop --changed-since does not behave entirely as expected. #13757

xlevus opened this issue Dec 1, 2021 · 3 comments
Assignees
Labels

Comments

@xlevus
Copy link
Contributor

xlevus commented Dec 1, 2021

Describe the bug
When running --loop with --changed-since, pants does not re-compute changed targets on each iteration.

How to reproduce:

  1. run ./pants --loop --changed-since=HEAD
  2. Modify a file
  3. nothing happens.
  4. restart ./pants --loop --changed-since=HEAD
  5. delete a file
  6. Pants goes into an error-loop.

Pants version
2.8.0

OS
Linux

@xlevus xlevus added the bug label Dec 1, 2021
@Eric-Arellano

This comment was marked as outdated.

@stuhood
Copy link
Member

stuhood commented Jan 6, 2022

Yea, this would be good to fix. We also shouldn't eagerly re-run in case of exceptions.

@stuhood
Copy link
Member

stuhood commented Apr 1, 2022

There has been some interest in tackling this one recently, so here are some thoughts on how to do so (roughly organized as things which could be separate commits, some of which could be separate PRs):

  1. Expose get_git as a @rule, and extend the Git return type of the @rule (or a wrapper around it) from EngineAwareReturnType, with an implementation of EngineAwareReturnType.cacheable which always returns False.
  2. Convert calculate_specs into a @rule, and convert its call to get_git into a Git (or wrapper type) argument to the calculate_specs @rule. Continue to call it as if it wasn't a @rule, but now by passing the Git argument explicitly.
    • BUT: Don't register it as a @rule yet in def rules in its file: doing so will cause graph ambiguity.
  3. Register calculate_specs as a @rule in def rules, and adjust the signature that @goal_rules are invoked with to not take the Specs as an explicit argument. They'll be calculated implicitly from the OptionsBootstrapper instead.

stuhood pushed a commit that referenced this issue Apr 12, 2022
First part of the suggested changes in [#13757](#13757 (comment)). This should leave `calculate_specs()` working "as is" for now while hopefully allowing something like `await Get(GitResult, GitRequest)` to get `Git` in other rules.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants