Pants: mypy plugin fails; site dir is in PYTHONPATH

[RemiTorracinta]

I've been trying to update from pants 1.21.0 to 1.24.0 .

When I run this command (on both mac and linux)
./pants lint.mypy tool/::
I get an error just on the newer version of pants:

/Users/me/my_buildroot is in the PYTHONPATH. Please change directory so it is not.

This happens even if I pin mypy to 0.710, the version we were defaulting to with pants 1.21.0 .

You can seen here that mypy doesn't want a folder to be both in PYTHONPATH and site packages:
https://github.com/python/mypy/blob/v0.710/mypy/modulefinder.py#L488

One potentially unique feature of our repo is that we set pythonpath in pants.ini to the root of our repo.

pythonpath: +[
    "%(buildroot)s",
  ]
backend_packages: +[
    "package.plugin1",
    "package.plugin2",
  ]

However, I tried changing this global pythonpath config (which I think should only affect the python environment in which pants itself is running) and it didn't seem to help.

[Eric-Arellano]

Hey @RemiTorracinta, to follow up from Slack, are you able to pin down which Pants version this starting breaking with between 1.21.0 and 1.24.0? It would help us if we can figure out when this first broke.

[RemiTorracinta]

Hey @Eric-Arellano , I tested it with the commands you suggested:

./pants;   ./pants list :: > /dev/null; ./pants filedeps :: > /dev/null; ./pants clean-all; ./pants lint.mypy my/tool/:lib

The issue pops up specifically when I upgrade to 1.22.0. Let me know if you want me to try dev/RC releases.

[Eric-Arellano]

That is super helpful! Thank you for taking the time to test that!

Could you please confirm that 1.22.0.dev0 fails for you? I think we found the offending commit: https://github.com/pantsbuild/pants/pull/8328. If that indeed is the culprit, I can try to figure out what's going on with that PR that would cause your issue.

[RemiTorracinta]

Yep, 1.22.0.dev0 fails too.

[Eric-Arellano]

Hey @RemiTorracinta, sorry for the radio silence from me. I took a look and see why that PR causes your failure. If interested, read the comment on https://github.com/pantsbuild/pants/pull/8328/files#diff-d11ee5c423cbc4877d5b05d64e52f8d2R177.

I believe the solution is for Pants to get smarter with the monkeypatch site.getsitepackages = lambda: sys.path[:] by filtering out bad entries like your PYTHONPATH value. I reached out to the PR author John to confirm.

A possible alternative solution. I'm not sure how your repo is configured, specifically if you use a build root, but could you try the spiritual equivalent of this, please?

[GLOBAL]
pythonpath = ["%(buildroot)s/src/python"]

This is what Toolchain uses and it works for us.

[jsirois]

@RemiTorracinta why is /Users/me/my_buildroot in site packages in the 1st place?

[jsirois]

... because I put it there via PYTHONPATH, gotcha.

[jsirois]

@Eric-Arellano has the relevant piece of code pointed out. The trick will be solving this style issue while not breaking the issue that code was added to fix.

[jsirois]

@RemiTorracinta although the fix appears to be straightforward I can't reproduce the issue. Can you perhaps ammend this example repo such that it can be used to reproduce your issue?: https://github.com/pantsbuild/issues-9177

If I throw all the weirdness I can at it, mypy works just fine:

$ PEX_INHERIT_PATH=prefer PYTHONPATH=$PWD ./pants lint.mypy ::
...
14:01:46 00:02   [lint]
14:01:46 00:02     [scalafix]
14:01:46 00:02     [scalafmt]
14:01:46 00:02     [scalastyle]
14:01:46 00:02     [checkstyle]
14:01:46 00:02     [jvm-dep-check]
14:01:46 00:02     [mypy]
14:01:46 00:02       [create_mypy_pex]
14:01:46 00:02       [check]
src/python/test.py:2: error: No return value expected

FAILURE: mypy failed: code=1


               Waiting for background workers to finish.
14:01:47 00:03   [complete]
               FAILURE

[RemiTorracinta]

Hi @jsirois , here is the reproduction:
https://github.com/pantsbuild/issues-9177/pull/1

The base repo was a big help in isolating the problem, thanks for that!

[RemiTorracinta]

Hi all,
I can work around this issue by moving our code around so there's a single source root. But would love to have a word on this issue before undertaking that.

[Eric-Arellano]

Hey Remi, thanks for creating a reproduction! That is helpful and we appreciate you taking the time.

John is OOO most of April, but I believe will get to this when he's back. This is a use case we do want to support; not supporting your use case is a bug.

I'm glad you found a workaround in the meantime, although apologies that it's hacky.

[RemiTorracinta]

No problem, thanks Eric.

[jsirois]

Thanks for the repro @RemiTorracinta. taking a look...

[Eric-Arellano]

Thanks for fixing @jsirois! @RemiTorracinta, this fix will be included in the dev release on Friday.

[RemiTorracinta]

Thank you both!