https://pantsbuild.org/ logo
Join Slack
Powered by
As part of the pyoxidizer work, it looks like we n...
# development
a
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
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
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
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
👍
b
Plus one from me
a
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
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
@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
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
(pip expands all of the wheels into the 
site-packages
directory, and things look like a normal single root package tree.
e
Yeah, that was the point I was trying to get across.
a
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
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
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
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
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
pip puts everything in the one place, pex does not
Right.
e
And Pip, despite being a thing, is not a thing to Python, since deb, yum, etc don't use it at all.
a
I am going to put together a reproduction case and submit my first ever CPython bug
so exciting
h
So it’s come to this…
a
hrrrm, I might actually be confused here, and it might just be resource files that are the problem
which is good and bad
e
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
I’ve fixed it by including 
pants/VERSION
into both wheels
e
That's not good!
a
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
Hang on, the 
pants
script only cares about the VERSION location when you use set PANTS_SHA, I think?
That is advanced usage
b
+1 to benjys, personally
h
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
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
Open in Slack
PreviousNext
Powered by