search

pantsbuild / pants

The Pants Build System
https://www.pantsbuild.org
Apache License 2.0
3.27k stars 626 forks source link

Sharing mypy cache for the same project with different buildroots #18830

Open csqzhang opened 1 year ago

csqzhang commented 1 year ago

Is your feature request related to a problem? Please describe. I'd would like to report an issue when we migrate to pants 2.15 from 2.14. We use Pants in our Jenkins CI/CD pipeline. In our setup, Jenkins will work on each pipeline in different folders. For example, a branch name AB would be under /tmp/workspace/AB, and branch name CD would be under/tmp/workspace/CD. We notice that Pants will not be able to share mypy cache across branch after 2.15. I believe the issue comes from this pull request https://github.com/pantsbuild/pants/pull/18061. It seems that Pants now replies on buildroot (e.g. a path) to differentiate repos.

ref: https://pantsbuild.slack.com/archives/C046T6T9U/p1682039225688279

Describe the solution you'd like

  1. Provide an option to disable the option of creating mypy cache per buildroot? or 2. use some pre-defined env variable to decide the repo?

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

Additional context Add any other context or screenshots about the feature request here.

huonw commented 1 year ago

Sorry for the impact (I think I noticed/filed the original issue!).

Just confirming: is the only impact of this pants check ... in the different build roots is slower? Or, are there other non-performance impacts too?

paiforsyth commented 1 year ago

I am on Qiang's team, and so far we have only noticed performance impacts.

csqzhang commented 1 year ago

Hi team, I am wondering if there is a plan on this issue?

thejcannon commented 1 year ago

Since this has a potential impact for soundness of the operations, it'd have to be approached very carefully.

I don't think it's on anyone's radar that is a maintainer, so you'd likely have to be doing the work yourselves. Feel free to get a discussion going in #development on Slack if that's something you'd wanna pick up

huonw commented 2 weeks ago

Relevant code: https://github.com/pantsbuild/pants/blob/0392d378d6b9c6c8cecd17ac5bffa0698b5eb454/src/python/pants/backend/python/typecheck/mypy/rules.py#L76-L105 https://github.com/pantsbuild/pants/blob/0392d378d6b9c6c8cecd17ac5bffa0698b5eb454/src/python/pants/backend/python/typecheck/mypy/rules.py#L224

Maybe a path forward here would be to make the {sha256(build_root.path.encode()).hexdigest()}part of that second link customisable, for instance, a new cache_path_disambiguator option with default value default=lambda _: pants.base.build_environment.get_buildroot()

https://github.com/pantsbuild/pants/blob/0392d378d6b9c6c8cecd17ac5bffa0698b5eb454/src/python/pants/backend/python/typecheck/mypy/subsystem.py#L66-L120

This becomes a sharp "you better know what you're doing"/"if there's problem, it's your own fault" configuration option, but opens the door for more control.

(E.g. a user had a different-but-related request around customising the cache based on different mypy args https://pantsbuild.slack.com/archives/C046T6T9U/p1724987069966649?thread_ts=1724161347.765349&cid=C046T6T9U)