https://pantsbuild.org/ logo
#development
Title
# development
a

ancient-vegetable-10556

08/11/2022, 9:34 PM
As part of the pyoxidizer work, it looks like we need an
__init__.py
file in the
pants
namespace package to make resource loading work (per a deficiency in early versions of
importlib.resources
. I’ve successfully made
pants
an explicit namespace package, and removed the code from
conftest
that checks that there is no
__init__.py
file; I didn’t quite grok the consequences of there being an
__init__.py
file present in both of our wheel distributions, but it looks like it’s fine if they’re identical. Are we OK to ship this particular package layout change? See conftest.py (cc @enough-analyst-54434)
h

happy-kitchen-89482

08/11/2022, 9:36 PM
AFAIK the important thing is that
pants
remain a namespace package, which it was before thanks to PEP 420. So I think that as long as there is a pkgutil namespace package incantation in the new
__init__.py
then everything should still work.
Or so I believe
a

ancient-vegetable-10556

08/11/2022, 9:37 PM
Right, and that incantation is present in the attached PR (and the loading stuff does work), the question is whether it’s likely there’ll be an issue with both wheels shipping an identical init file
h

happy-kitchen-89482

08/11/2022, 9:38 PM
I believe that this should work
But is important to test by installing the
pantsbuild.testutils
and
pantsbuild.pants
wheels in a clean venv and making sure that you can import from both
a

ancient-vegetable-10556

08/11/2022, 9:40 PM
👍
b

bitter-ability-32190

08/11/2022, 9:42 PM
Plus one from me
a

ancient-vegetable-10556

08/11/2022, 9:45 PM
Copy code
Last login: Tue Aug  9 12:54:03 on ttys013
chrisjrn@chrisjrns-MacBook-Pro ~ % python3.9 -m venv ./venvs/pants-namespace-pac
kage-flobble
chrisjrn@chrisjrns-MacBook-Pro ~ % source ./venvs/pants-namespace-package-flobble/bin/activate
(pants-namespace-package-flobble) chrisjrn@chrisjrns-MacBook-Pro ~ % pip install
 src/pants/dist/pantsbuild.pants-2.14.0.dev5-cp39-cp39-macosx_12_0_arm64.whl
Processing ./src/pants/dist/pantsbuild.pants-2.14.0.dev5-cp39-cp39-macosx_12_0_arm64.whl
Collecting setuptools<64.0,>=63.1.0

... [SNIP]

Using cached urllib3-1.26.11-py2.py3-none-any.whl (139 kB)
Installing collected packages: types-toml, types-setuptools, types-PyYAML, ijson, ansicolors, urllib3, ujson, typing-extensions, toml, six, setuptools, setproctitle, PyYAML, pyparsing, psutil, pex, idna, charset-normalizer, certifi, requests, python-lsp-jsonrpc, packaging, fasteners, humbug, pantsbuild.pants
  Attempting uninstall: setuptools
    Found existing installation: setuptools 62.3.2
    Uninstalling setuptools-62.3.2:
      Successfully uninstalled setuptools-62.3.2
Successfully installed PyYAML-6.0 ansicolors-1.1.8 certifi-2022.6.15 charset-normalizer-2.1.0 fasteners-0.16.3 humbug-0.2.7 idna-3.3 ijson-3.1.4 packaging-21.3 pantsbuild.pants-2.14.0.dev5 pex-2.1.103 psutil-5.9.0 pyparsing-3.0.9 python-lsp-jsonrpc-1.0.0 requests-2.28.1 setproctitle-1.2.2 setuptools-63.4.3 six-1.16.0 toml-0.10.2 types-PyYAML-6.0.3 types-setuptools-62.6.1 types-toml-0.10.8 typing-extensions-4.3.0 ujson-5.4.0 urllib3-1.26.11
WARNING: There was an error checking the latest version of pip.
(pants-namespace-package-flobble) chrisjrn@chrisjrns-MacBook-Pro ~ % pip install dist/pantsbuild.pants.testutil-2.14.0.dev5.tar.gz
WARNING: Requirement 'dist/pantsbuild.pants.testutil-2.14.0.dev5.tar.gz' looks like a filename, but the file does not exist
Processing ./dist/pantsbuild.pants.testutil-2.14.0.dev5.tar.gz
ERROR: Could not install packages due to an OSError: [Errno 2] No such file or directory: '/Users/chrisjrn/dist/pantsbuild.pants.testutil-2.14.0.dev5.tar.gz'

WARNING: There was an error checking the latest version of pip.
(pants-namespace-package-flobble) chrisjrn@chrisjrns-MacBook-Pro ~ % pip install src/pants/dist/pantsbuild.pants.testutil-2.14.0.dev5.tar.gz
Processing ./src/pants/dist/pantsbuild.pants.testutil-2.14.0.dev5.tar.gz
  Preparing metadata (setup.py) ... done
Requirement already satisfied: pantsbuild.pants==2.14.0.dev5 in ./venvs/pants-namespace-package-flobble/lib/python3.9/site-packages (from pantsbuild.pants.testutil==2.14.0.dev5) (2.14.0.dev5)

... [SNIP]

Requirement already satisfied: charset-normalizer<3,>=2 in ./venvs/pants-namespace-package-flobble/lib/python3.9/site-packages (from requests->humbug==0.2.7->pantsbuild.pants==2.14.0.dev5->pantsbuild.pants.testutil==2.14.0.dev5) (2.1.0)
Using legacy 'setup.py install' for pantsbuild.pants.testutil, since package 'wheel' is not installed.
Installing collected packages: iniconfig, tomli, py, pluggy, attrs, pytest, pantsbuild.pants.testutil
  Running setup.py install for pantsbuild.pants.testutil ... done
Successfully installed attrs-22.1.0 iniconfig-1.1.1 pantsbuild.pants.testutil-2.14.0.dev5 pluggy-1.0.0 py-1.11.0 pytest-7.0.1 tomli-2.0.1
WARNING: There was an error checking the latest version of pip.
(pants-namespace-package-flobble) chrisjrn@chrisjrns-MacBook-Pro ~ % python
Python 3.9.13 (main, May 24 2022, 21:13:54)
[Clang 13.0.0 (clang-1300.0.29.30)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from pants.testutil import pytest_util
>>> from pants import version
>>> from pants.bin import pants_loader
>>>
@happy-kitchen-89482 ^---
e

enough-analyst-54434

08/11/2022, 10:57 PM
Installing both in 1 venv is not in fact how to do this. You want to install each n different paths, then combine the paths with, say, PYTHONPATH and confirm you can import from both.
Installing in 1 path just works around the problem.
A cheap way to do this is
pex ./one.whell ./other.wheel
- that will drop you in a repl with each installed indivudally and added to sys,path individually.
You can thro in
-v
to get the synthesized sys.path printed out before you drop in the repl to have confidence it's a good test.
Basically, the whole point of ns packages is handling seperate sys.path entries. Without that, you do not actually engage ns packages, you just mush them all together in 1 tree and things look like a normal single root package tree.
1
a

ancient-vegetable-10556

08/12/2022, 5:27 PM
@enough-analyst-54434 Fun story, looks like the Python import machinery is broken for namespace packages that are distributed across multiple
sys.path
entries. Subpackages work fine, but modules and resources do not. I’ll be working on a minimal reproduction and filing a bug.
e

enough-analyst-54434

08/12/2022, 5:28 PM
That means it's always broken since the only point of ns packages is, in fact, handling presence on multiple sys.path entries. Super odd that would be missed.
a

ancient-vegetable-10556

08/12/2022, 5:28 PM
(pip expands all of the wheels into the
site-packages
directory, and things look like a normal single root package tree.
e

enough-analyst-54434

08/12/2022, 5:28 PM
Yeah, that was the point I was trying to get across.
a

ancient-vegetable-10556

08/12/2022, 5:29 PM
It working for the subpackages case makes sense, that’s the 99% use case.
I wouldn’t have taken a second look at it if there weren’t a case where putting a module in the namespace package weren’t a documented case at https://packaging.python.org/en/latest/guides/packaging-namespace-packages/
h

happy-kitchen-89482

08/12/2022, 5:30 PM
I’m missing something here. If the only issue with ns packages is splitting them across sys.path entries, then why were they ever a thing? Wouldn’t what pip does mean that things would work in practice even with non-namespace package splits?
e

enough-analyst-54434

08/12/2022, 5:30 PM
Ok, I'm not sure about that! But sure.
@happy-kitchen-89482 your machine uses 1 sys.path, but Python defines purelib and platlib entries and those can be different and are on Fedora.
lib64/site-packages vs lib/site-packages
a

ancient-vegetable-10556

08/12/2022, 5:31 PM
To be clear — ns packages means that you can split across multiple sys.path entries, not that you have to split across sys.path entries
h

happy-kitchen-89482

08/12/2022, 5:31 PM
I see, but things might still have worked for the most part, assuming both sides of the split got installed into the same
site-packages
a

ancient-vegetable-10556

08/12/2022, 5:31 PM
pip puts everything in the one place, pex does not
Right.
e

enough-analyst-54434

08/12/2022, 5:32 PM
And Pip, despite being a thing, is not a thing to Python, since deb, yum, etc don't use it at all.
a

ancient-vegetable-10556

08/12/2022, 5:32 PM
I am going to put together a reproduction case and submit my first ever CPython bug
so exciting
h

happy-kitchen-89482

08/12/2022, 5:33 PM
So it’s come to this…
a

ancient-vegetable-10556

08/12/2022, 5:36 PM
hrrrm, I might actually be confused here, and it might just be resource files that are the problem
which is good and bad
e

enough-analyst-54434

08/12/2022, 5:38 PM
I mean, moving pants/VERSION is a self-contained thing IIUC if that's all this is about. Move it and update maybe 2-3 other files?
a

ancient-vegetable-10556

08/12/2022, 5:38 PM
I’ve fixed it by including
pants/VERSION
into both wheels
e

enough-analyst-54434

08/12/2022, 5:38 PM
That's not good!
a

ancient-vegetable-10556

08/12/2022, 5:38 PM
the problem is that
pants/VERSION
is hardcoded in the runner script, so moving it would inconvenience a lot of people
(the location of
pants/VERSION
is hardcoded, that is)
@enough-analyst-54434 Yeah, it means the file needs to be updated in lockstep. Not ideal 😕
OK. Monkey patch time
h

happy-kitchen-89482

08/12/2022, 6:51 PM
Hang on, the
pants
script only cares about the VERSION location when you use set PANTS_SHA, I think?
That is advanced usage
b

bitter-ability-32190

08/12/2022, 6:51 PM
+1 to benjys, personally
h

happy-kitchen-89482

08/12/2022, 6:54 PM
So here’s what will avoid all problems, I think: • Move VERSION to a subpackage • Switch our release scripts to use that new location for the rewriting hack • symlink the current location to the subpackage location Thus, only the new location will show up in the wheel, and will be the canonical version. The symlink will be present at the github link used by the
pants
script for as long as we need to roll out an update to that. And in any case, even if it breaks a few people, it’s only (AFAICT) if they set PANTS_SHA, which is rare, and we can tell them to upgrade the
pants
script. Which we will be pushing people to do at some point anyway in order to consume the new pyoxidized pants binary…
To clarify, the
pants
script appears to only care about the location of VERSION at a public github URL and even that only when using PANTS_SHA. So the blast radius of changing the canonical location of VERSION (so it can be loaded by importlib without resorting to funkiness) is way smaller than I thought.
a

ancient-vegetable-10556

08/12/2022, 7:54 PM
everything works without moving the file around now. I think there may still be a bug in importlib-resources, but it doesn’t seem to affect us for the moment.
we may want to consider moving the VERSION file sometime, but we do not need to do that for the time being, from what I can tell
🎉 1