Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support abi3 packages with a python version independent site-packages directory #669

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

isuruf
Copy link
Member

@isuruf isuruf commented Jan 15, 2024

cc @conda-forge/core

When building abi3 packages, we just need to do export PIP_TARGET=$PREFIX/lib/site-packages when building downstreams and then everything else should work properly. (We also have to fix the metadata in meta.yaml in downstream recipes.

@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

recipe/sitecustomize.py Outdated Show resolved Hide resolved
Copy link
Member

@xhochy xhochy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea!

recipe/sitecustomize.py Outdated Show resolved Hide resolved
@@ -0,0 +1,2 @@
import site, sys, os
site.addsitedir(os.path.join(sys.prefix, 'lib', 'python', 'site-packages'))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One advantage of $PREFIX/lib/site-packages is that it matches windows layout.

@mbargull
Copy link
Member

I'm +1 on using lib/python/site-packages.
lib/site-packages would be a thing that'd "just work" on Windows but is awkward on non-Windows.

This touches on the larger installation scheme/directory layout and Conda-(noarch:)python-special handling topics.
I'd like for us to discuss this in a conda-community meeting (what are the chances -- today we have once :D ).
(I do believe it is worthwhile for us to assess the -- admittedly higher -- costs of changing the installation scheme on Windows vs. our everyday costs the current scheme incurs.)

@wolfv
Copy link
Member

wolfv commented Jan 17, 2024

It just occurred to me that there is one issue with this and that is the pyc compiled files.

Noarch packages compile the pyc files on the fly, while the arch' packages should ship the pyc files.

There are two reasons for pre-compilation:

  • reduces startup time when first invoking any python command
  • makes it easier to remove the entirety of the package

Do we have a plan for that?

@mbargull
Copy link
Member

mbargull commented Jan 17, 2024

Noarch packages compile the pyc files on the fly, while the arch' packages should ship the pyc files.

Yes, that's one of the details I was going to address if we had more time.
So, conda(/mamba) explicitly invoke compileall for noarch: python somewhere deep down in its code (aka hard-coded special handling that we can not easily reuse for non-noarch subdirs).

If conda hadn't had that functionality but we as packagers wanted such noarch: python-like packages, we'd need to run some post-link action for this.
This could just be some script that invokes compileall with the package's list of .py files.

Now, I'm not suggesting we do this via post-link scripts (many pitfalls: shell compat, registering of .pyc files so they can be removed, .exe wrapper on Windows, etc.; one would rather want a helper package for this stuff), but conceptually some post-link action would be needed to makes this generally usable without special-handling-code inside the package manager itself.

This would be something we would have to figure out and carefully implement (via helper package(s) or some plugin nowadays, if really needed).
(Oh yeah, I actually half-ass planned out that stuff 6 or so years ago, but didn't implement anything...)


Well, and then there is the mbargull-does-not-approve-but-working solution:
We could actually reuse the noarch: python code paths for this.
I.e., mark all these packages as noarch: python but add some system-specific dependency to make them only installable on a system the extensions are compiled for.
Additionally, since that would invoke the site-packages->lib/pythonX.Y/site-packages conversions for noarch: python packages, we might need a site-packages->lib/pythonX.Y/site-packages symlink on the Unixes (to ensure a non-version-specific path can be resolved in case it's recorded somewhere (e.g., rpath)).
This should work, but is in my eyes rather ugly since we continue to rely on the internal special-handling code etc.

([EDIT]: There are, of course, other more intrusive or elaborate ways than the two given above to go about this, e.g., hack on pycache_prefix to this (please don't!), or have actual additional packages for the .pyc files alone (which would be the "cleanest" solution since no additional link-time-handling would be needed).)

@JeanChristopheMorinPerso

Hello from Anaconda, it was called out today that it would be desirable to also build abi3 packages (when possible) on defaults. I like the idea since it would allow us to avoid compiling uslessly for all python versions supported. But I'm wondering how it will all work in the conda ecosystem. How will conda know that it can or can't install an abi3 package? If abi4 becomes a thing, how will conda know what to install or not to install?

@wolfv
Copy link
Member

wolfv commented Jan 17, 2024

@mbargull in pixi / rattler we actually do not do the precompilation (yet) but we are starting to remove empty directories that only contain a __pycache__ folder. That would work for these packages and is something we could also implement in mamba / conda.

@mbargull
Copy link
Member

Hello from Anaconda, it was called out today that it would be desirable to also build abi3 packages (when possible) on defaults.

Happy to see the Anacondian packager in the loop here! :)

How will conda know that it can or can't install an abi3 package? If abi4 becomes a thing, how will conda know what to install or not to install?

On conda-forge we have these python_abi packages to distinguish implementations (PyPy support):

We can do similar things for abi3.

@mbargull
Copy link
Member

@mbargull in pixi / rattler we actually do not do the precompilation (yet) but we are starting to remove empty directories that only contain a __pycache__ folder. That would work for these packages and is something we could also implement in mamba / conda.

It makes sense to have such functionality for missing path metadata.
It is, however yet again an implicit specialized thing to do.
I'd prefer if we'd go for more general concepts to do these things.
I.e., preferably avoid untracked files (IDK if conda would/could handle "dummy" path data entries (i.e., no size/checksum) for .pyc to just be able to register these files as "potentially existing and to be removed when package is unlinked").

(IDK how it's implemented, but if you do this "remove empty directories that only contain a __pycache__ folder", I'd suggest to have this conceptually in a separate "clean up environment" functionality that can be invoked separately and optionally in a "post-unlink if transactions removed packages" phase. That way you could have these Python-specific stuff added in a well-defined place at least.)

@wshanks
Copy link

wshanks commented Feb 20, 2024

In my testing, the python dependency version of the output package was still getting pinned to a single minor version even when I tried to override the bounds and ignore run exports. Has anyone gotten that not to be the case? It might be another item before abi3 support (if I wasn't missing something).

@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

  •         'Test On Native Only' is deprecated.
    

This was used for disabling testing for cross-compiling.

This has been deprecated in favor of the top-level `test` field.
It is now mapped to `test: native_and_emulated`.
        Failed validating 'deprecated' in schema['properties']['test_on_native_only']:
            {'anyOf': [{'type': 'boolean'}, {'type': 'null'}],
             'default': False,
             'deprecated': True,
             'description': 'This was used for disabling testing for '
                            'cross-compiling.\n'
                            '\n'
                            '```warning\n'
                            'This has been deprecated in favor of the top-level '
                            '`test` field.\n'
                            'It is now mapped to `test: native_and_emulated`.\n'
                            '```',
             'title': 'Test On Native Only'}

        On instance['test_on_native_only']:
            True

@isuruf
Copy link
Member Author

isuruf commented Mar 15, 2024

@mbargull, would it be okay to merge here as we figure out pyc compilation? It'll be easier for me to play with it.

@isuruf
Copy link
Member Author

isuruf commented Mar 25, 2024

Ping on this

@mbargull
Copy link
Member

@mbargull, would it be okay to merge here as we figure out pyc compilation? It'll be easier for me to play with it.

Sorry, missed this previously.

If you want to test it out for some niche packages or builds you'd not put on conda-forge/label/main, we could merge.
IMO we should not use it (and discourage the use) for general usage until this is conceptually more fleshed out.


I haven't worked out a concept for this yet, but I could imagine us having a python-noarch-support helper package that does the post-link/pre-unlink etc. for us. Roughly:

  1. package python-noarch-support has:
    1. the sitecustomize.py that you proposed to add to python here,
    2. a build helper script/program to collect paths of .py files for which on install time the .pyc will be built,
    3. a install helper script/program to
      1. call python -m compileall for the list collected in 2. (i.e., equivalent to conda.gateways.disk.create.compile_multiple_pyc), and
      2. possibly an equivalent to conda.gateways.disk.create.create_python_entry_point if we want to phase out the noarch: python handling from conda's internals for good at some point,
      3. possibly register the new paths in corresponding conda-meta/*.json file.
    4. if 1.3.3. is not done, an uninstall helper script/program to remove the created .pyc(/entry point) files.
  2. Have noarch packages depend on python-noarch-support on
    1. build time for sitecustomize.py and 1.2. (e.g., collect in share/python-noarch-support/<package-build>.json)
    2. run time for sitecustomize.py and the post-link/pre-unlink helpers.
      (This would entail such packages have simple post-link/pre-unlink scripts calling <helper> <package-build>.)

Do we have tracking issue for this yet? I'd copy the above into that then.

My suggestion for now would be to put sitecustomize.py in a new python-noarch-helper package instead of python.

@baszalmstra
Copy link
Member

Thanks @isuruf for pointing out this PR to me.

Reading between the lines I notice there seems to be a general consensus in here that it would be good to remove specialized behavior from installers (noarch: python in this case). Although I generally agree with this I just want to point out that it can also offers some benefits to have these specializations because it allows the installer to optimize better for these special cases.

Pyc compilation especially is an interesting optimization. Rattler does parallel installation of packages which means it can already start pyc compilation of files even when certain packages are still being downloaded (as long as the interpreter package and its dependencies are installed first). Since compilation can be slow this can make a big impact on the total installation time.

Not directly related to this PR (I completely support this change) but perhaps something to keep in mind.

@isuruf
Copy link
Member Author

isuruf commented Jul 1, 2024

I opened a CEP at conda/ceps#86

@wolfv
Copy link
Member

wolfv commented Jul 2, 2024

Exciting! :)

@minrk
Copy link
Member

minrk commented Aug 19, 2024

This is super exciting! Does python_abi need an update to enable this as well? I'm not sure how a package can properly depend on python_abi and accept cp312 and cp313 (and 314, etc.), but not cp313t.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants