python312Packages.python-ldap: fix build#326321
Conversation
github-actions
bot
added
the
6.topic: python
ofborg
bot
added
8.has: package (new)
| build-system = [ | ||
| (setuptools.overrideAttrs { | ||
| postPatch = '' | ||
| substituteInPlace setuptools/_distutils/util.py \ | ||
| --replace-fail \ | ||
| "from distutils.util import byte_compile" \ | ||
| "from setuptools._distutils.util import byte_compile" | ||
| ''; | ||
| }) | ||
| ]; |
There was a problem hiding this comment.
I think we agreed on Matrix that this seems really wrong, but not on exactly what’s going on here or how it should be fixed. If this is required it should at least be in the main setuptools package, I guess.
There was a problem hiding this comment.
I believe cython/cython#5754 (comment) provides a clear picture of what happened and how the Python 3.12 distutils issue is specific to Nix. Though setuptools/_distutils is and should be setuptools-free, so I have no idea yet for an upstreamable fix.
There was a problem hiding this comment.
Can you elaborate why this is really wrong?
At first glance it looks like it would only affect the non-propagated build-system of this specific package. Is it about the import of an explicitly internal vendored distutils from setuptools? Or am I missing something obvious?
There was a problem hiding this comment.
Well, this is breaking on something setuptools is doing internally. It’s generating a script that references distutils.util, running it, and hitting an error. Therefore, either python-ldap is somehow holding setuptools in a way that is really wrong and triggering this error (and therefore we should patch python-ldap, not setuptools), or this would be broken in more packages than just python-ldap, so we should fix it in our setuptools package, or in our general Python environment handling if there’s something we’re doing to cause it. Patching setuptools only within python-ldap can’t be the right answer.
And indeed, @wegank’s link confirms to me that it’s an issue with our Python environment. Perhaps we should include this hack in our setuptools package for now until we figure out a better fix? Do we know the exact chain of features/code that cause setuptools to start executing this vendored distutils code path?
There was a problem hiding this comment.
There was a problem hiding this comment.
At first glance it looks like it would only affect the non-propagated build-system of this specific package. Is it about the import of an explicitly internal vendored distutils from setuptools? Or am I missing something obvious?
Setuptools provides a .pth file to make the vendored distutils importable via import distutils at every Python startup, except that the mechanism doesn't work for Nix. As a result, we're making PRs to solve missing distutils issues that don't exist on other distros.
There was a problem hiding this comment.
FWIW it’s probably not entirely a bad thing that it doesn’t work for us, because things really shouldn’t be using distutils anyway! But it is kind of annoying in the meantime.
If we don’t want to deal with it we could just make setuptools propagate a PYTHONPATH that lets distutils be found directly. I presume there’s a reason they did it the way they did, though.
There was a problem hiding this comment.
Thanks for the explaination. I miss-understood that the fix was wrong per se, my new understanding is that python3.pkgs.setuptools might be a better place to get it's behavior a bit closer to conventional venvs.
If we don’t want to deal with it we could just make setuptools propagate a PYTHONPATH > that lets distutils be found directly. I presume there’s a reason they did it the way > they did, though.
Thef first link @wegank posted seems to imply the reason could be to shadow an (older) distutils from the standard library where available:
This hack (ab)uses the
extra_pathbehavior in Setuptools to install a.pthfile with implicit behavior on startup to give higher precedence to the local version ofdistutilsover the version from the standard library.
This doesn't seem like a concern for us if we apply only for python >=3.12 (where there is no distutils in stdlib)
3 participants
Description of changes
As I can't afford a stdenv rebuild.
Closes #326296.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.