CPython: Conan 2.0 #21387

Ahajha · 2023-11-26T15:55:16Z

Specify library name and version: cpython/all

I will be leaving the versions as-is in this PR, as changing them tends to break things, and I want to keep the scope small. A separate PR for that is here: #22599 (depends on this PR)

I believe there is some value in having a 2.7 recipe available, but probably not worth it for the CCI maintainers to deal with. If specifically needed, there is a mostly working 2.7 (and 3.7) version available a few commits back from the end of this branch.

Previously, the packages were not 100% relocatable on *nix. For now I always set PYTHONHOME in the test packages, not sure if this is a perfect idea but for now it seems to be a reasonably not-hacky solution.

Short paths are enabled. Lots of files in pycache directories are packaged - this is intentional, as they are compiled (bytecode/JIT) versions of all the standard library modules.

Known intermittent issues:

Sometimes, when packaging on the Mac CI, there will be an error relating to a file already existing. If I had to guess the frequency, probably around 25% of the time this happens. I haven't been able to reliably reproduce it.

There are many CPython issues/PRs out right now:

Resolves #21357
Resolves #20695
Resolves #19374
Resolves #10808
Resolves #12373 (no project upgrade occurs)
Resolves #8198
Resolves #9890 (was only broken on Linux/Mac. Some other with_xxx options don't work, but that's a separate issue)

Unsure if it fixes this, I test locally on an M2 with apple-clang 15 so similar but not exact: #13900
This should be able to be closed after this, since this is the last recipe using the old libuuid recipe: #19084
This is a problem with CPython being static by default. In future versions, CPython doesn't even properly support static mode, but it's still annoying that the default configuration is invalid: #9333
Unsure, seems like a CPython problem itself: #8608

This PR is based on the following, and should no longer be needed:
Closes #18064

There was also an attempt to fix the libuuid issue:
Closes #19547

I've read the contributing guidelines.
I've used a recent Conan client version close to the currently deployed.
I've tried at least one configuration locally with the conan-center hook activated.

# Conflicts: # recipes/cpython/all/conanfile.py

ghost · 2023-11-26T16:14:42Z

I detected other pull requests that are modifying cpython/all recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

Ahajha · 2024-03-18T20:32:21Z

Should be good to go now, I think I got all of the pre-3.8 logic.

uilianries

I reviewed the PR now, it's looking good, only minor details to be checked.

uilianries · 2024-03-19T09:42:42Z

recipes/cpython/all/conanfile.py

-    @property
-    def _version_tuple(self):
-        return tuple(self._version_number_only.split("."))
+    short_paths = True


Why short_paths is needed now?

Python packages compiled versions of all of the modules shipped in the stdlib in a __pycache__ folder. These paths end up being very long, you can look at some of the hooks in the earlier runs of the PR to see such file names (./lib/python3.10/site-packages/setuptools/_distutils/__pycache__/versionpredicate.cpython-310.pyc, for example).

It would be a bad idea to remove these, as I believe they would just get regenerated when they are used, breaking cache immutability.

uilianries · 2024-03-19T10:01:13Z

recipes/cpython/all/conanfile.py

+            ))
+        if self.settings.compiler == "intel-cc":
+            tc.configure_args.append("--with-icc")
+        if os.environ.get("CC") or self.settings.compiler != "gcc":


Suggested change

if os.environ.get("CC") or self.settings.compiler != "gcc":

if self.settings.compiler != "gcc":

Avoid using external environment variables, it will result in not reproducible scenarios.

This was here prior, I'll double check that it doesn't break anything if removed.

I think I can actually remove this condition entirely, seems that the flag was removed in 3.8: https://github.com/python/cpython/blob/main/Misc%2FNEWS.d%2F3.8.0a1.rst#L7485

Much better, please, remove then. Thank you for checking it!

Done. The condition above it was also able to be removed.

recipes/cpython/all/conanfile.py

uilianries · 2024-03-19T10:13:44Z

recipes/cpython/all/conanfile.py

+            # For debugging configure errors
+            try:
+                autotools.configure()
+            except ConanException:
+                with open(os.path.join(self.build_folder, "config.log"), 'r') as f:
+                    self.output.info(f.read())
+                raise


Suggested change

# For debugging configure errors

try:

autotools.configure()

except ConanException:

with open(os.path.join(self.build_folder, "config.log"), 'r') as f:

self.output.info(f.read())

raise

autotools.configure()

Let's keep it simple, please. Who is working on the recipe is able to access the build folder and read any file.

Fair enough, this was mainly to debug the xvmc issue in CCI.

uilianries · 2024-03-19T10:27:53Z

recipes/cpython/all/conanfile.py

+            if (os.path.isfile(os.path.join(self.package_folder, "lib"))):
+                # FIXME not sure where this file comes from
+                self.output.info(f"{os.path.join(self.package_folder, 'lib')} exists, but it shouldn't.")
+                rm(self, "lib", self.package_folder)


Suggested change

if (os.path.isfile(os.path.join(self.package_folder, "lib"))):

# FIXME not sure where this file comes from

self.output.info(f"{os.path.join(self.package_folder, 'lib')} exists, but it shouldn't.")

rm(self, "lib", self.package_folder)

# FIXME not sure where this file comes from

rm(self, "lib", self.package_folder)

Let's simplify: The rm only removes files, and does not raise error for file not found.

I might remove this hack altogether, I don't know if it ever actually worked. I'll check through some logs to see.

It seems intermittent, and actually hasn't happened recently (only checked some recent logs, not going through all of them). I think it's best to just remove it, if it comes up we can just rerun the job.

Done, removed the whole condition.

recipes/cpython/all/conanfile.py

uilianries · 2024-03-19T10:42:25Z

recipes/cpython/all/conanfile.py


        if self.options.env_vars:
            bindir = os.path.join(self.package_folder, "bin")
-            self.output.info("Appending PATH environment variable: {}".format(bindir))
+            self.output.info(f"Appending PATH environment variable: {bindir}")
            self.env_info.PATH.append(bindir)


Suggested change

self.env_info.PATH.append(bindir)

self.env_info.PATH.append(bindir)

self.buildenv_info.prepend_path("PATH", bindir)

self.runenv_info.prepend_path("PATH", bindir)

Env info is Conan 1.x and will be deprecated in the future. In Conan 2.x you should use runenv or buildenv.For Python I would say both cases are valid because I see python scripts during build time, only to generate C/C++ code, but also I saw some packaged tools that are python scripts too.

I added these for all the existing variables, as well as a note regarding the old version's removal at some point.

uilianries · 2024-03-19T10:44:00Z

recipes/cpython/all/conanfile.py

+        python_root = self.package_folder
+        if self.options.env_vars:
+            self.output.info(f"Setting PYTHON_ROOT environment variable: {python_root}")
+            self.env_info.PYTHON_ROOT = python_root


Suggested change

self.env_info.PYTHON_ROOT = python_root

self.env_info.PYTHON_ROOT = python_root

self.buildenv_info.define_path("PYTHON_ROOT", python_root)

Ahajha · 2024-03-19T13:09:58Z

@uilianries Thanks for the review, I'll do some testing and add the suggestions later today or tomorrow.

Co-authored-by: Uilian Ries <uilianries@gmail.com>

Ahajha · 2024-03-20T01:44:13Z

@uilianries I believe I addressed all the review points.

My only (very minor) concern left is regarding the environment variables, I think we may be able to simplify one or more of:

The variables themselves
The conf info
The conditions behind them and their related conf info being defined (should we use an env_vars option, or have them be unconditionally defined? I don't know if other packages do this) (others are conditional on Windows or Apple, maybe just have them be unconditional since I don't think the conditions are actually correct)
The usage of both the env vars and the conf info (for example, we no longer need to specify the full path to the python executable, we can just run python and it will work)

~~It works for now, so it can stay as-is, but maybe could use some more thought.~~
There's an odd issue with PYTHONHOME on Windows, defining it fixes one thing but breaks another, not defining breaks the first thing but fixes the other. The previous behavior was not defining it, so I left it that way with a FIXME (also, it's easier to work around, whereas the other way it breaks a lot more). I added a note in the code if you want to do some reading.

I think it might need more thought, but now it works (for now). I can add it to my TODO issue (that I believe you saw) (rather than try to fix it now) if you agree.

For context, currently we have:
Env: PATH, PYTHON, PYTHONHOME, PYTHON_ROOT
Conf: user.cpython:python, user.cpython:pythonhome, user.cpython:module_requires_pythonhome, user.cpython:python_root

I also did a bit of research on these variables:

PYTHON I can't find existence of anywhere, we certainly don't use it and it isn't an official variable. I would prefer to omit it if it isn't obvious why it's needed. The conf info variable might continue to be useful, currently we use it to tell CMake where the executable is.
PYTHONHOME is fine to include, but has a weird issue on Windows that I mentioned above. Also not quite sure what would happen on other systems (it's currently not set in the env info for those platforms, not quite sure why). We currently use the conf variable to work around this issue.
PYTHON_ROOT is not an official Python environment variable, I think it's specifically needed by Boost.Python (and even then, I can't find any official documentation, only random users, and nothing of substance in the last decade), my guess is the original PR included it for that specific case. Going forward, I would probably prefer to omit it entirely.

conan-center-bot · 2024-03-20T07:54:16Z

Conan v1 pipeline ✔️

All green in build 12 (c5fa0c5e567a1dd8e032f82376b95c057a40d1bb):

cpython/3.10.0:
All packages built successfully! (All logs)
cpython/3.9.7:
All packages built successfully! (All logs)
cpython/3.8.12:
All packages built successfully! (All logs)

Conan v2 pipeline ✔️

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

All green in build 12 (c5fa0c5e567a1dd8e032f82376b95c057a40d1bb):

cpython/3.10.0:
All packages built successfully! (All logs)
cpython/3.9.7:
All packages built successfully! (All logs)
cpython/3.8.12:
All packages built successfully! (All logs)

uilianries · 2024-03-20T08:00:03Z

@Ahajha Thank you so much for your detailed information! It helps a lot to understand the current state of this PR. At this point I'm comfortable to accept it, I'll only talk to @RubenRBS first, to have a final review, but if is all okay, it should be merged by today.

Thank you again for this huge effort, this is one of most tricky recipes in CCI.

jwillikers · 2024-03-21T12:20:02Z

Ahajha · 2024-03-21T13:47:18Z

Thanks to everyone involved for helping this along! I learned a lot from working on this PR for sure :)

ilatypov · 2024-06-14T16:49:08Z

python -m pip fails missing pyexpat, socket on Windows. Sigh. The original submission took pains to use conan's dependencies for Python 2. I don't know if that worked (I cannot even download cpython 2.7.18 because it is not compatible with the new Conan API). Am I doing the right thing? If I do, could that be added as a smoke test (currently failing)?

I guess the use of cpython from conan may have a limited user base because using conan implies using Python. But it's needed for bootstrapping in the corporate requirements of using approved binary repositories (such as Conan Center). And of course the conan way of building C Python could eventually substitute the less-straightforward build systems, enabling C Python development bootstrapping.

CMD> bin\python -m pip
Traceback (most recent call last):
[...]
  File "C:\Users\XXX\cpython\bin\lib\site-packages\pip\_vendor\pkg_resources\__init__.py", line 35, in <module>
    import plistlib
  File "C:\Users\XXX\cpython\bin\Lib\plistlib.py", line 65, in <module>
    from xml.parsers.expat import ParserCreate
  File "C:\Users\XXX\cpython\bin\Lib\xml\parsers\expat.py", line 4, in <module>
    from pyexpat import *
ModuleNotFoundError: No module named 'pyexpat'

For visual studio, the patches are needed to use conan packages instead of python binaries.
See this repo and this repo.

For python 2.7, I had to apply some fixes to make it build with more recent visual studios.
These patches will never hit upstream, as 2.7 is dead.

Originally posted by @madebr in #1510 (comment)

Ahajha · 2024-06-14T17:50:08Z

@ilatypov There was discussion about leaving Python 2 support, but we ended up dropping it. That said, I'm more than willing to re-add it if you think there's a use case for it in Conan Center.

Regarding the failing test, do you know if this ever worked?

Also regarding "The original submission took pains to use conan's dependencies for Python 2" - Is this just saying that Python 2 was originally supported, or that it used Conan dependencies? If the latter, I don't think anything has changed - the recipe still goes to great lengths to use Conan's dependencies.

I've seen slow but steady adoption of the cpython package within ConanCenter, mainly in places where Conan's minimum Python version isn't enough. When I started this PR (branching off of a few others), I had a use case of easily switching Python versions within a multi-repo application as an alternative to essentially requiring some containerization.

All that said, I'm happy to look into the failing test and re-add Python 2.

ilatypov · 2024-06-14T18:56:03Z

I hijacked this thread that showed amazing collaboration. I did not have concerns about resurrecting conan's Python 2 and, unfortunately, I don't know if the smoke test would work with conan's Python 2. It was my attempt to rely on the previous work that seemed successful in packaging conan's Python with its dependencies. (The _supports_modules attribute seems to be related to a capability of building additional modules but not to conan's own packaging of cpython?) I am concerned that Python 3 was failing where it could be used for setting up access to any other package through the trust to the binaries published in Conan Center.

Perhaps one day I could learn from this group and submit a useful pull request.

Ahajha · 2024-06-14T19:08:28Z

I'm a little confused as to your exact problem, but I have a guess as to the solution:

The _supports_modules attribute is representative of a Python limitation, not a Conan one. If statically linking Python on Windows, no extension modules can be built, so perhaps that's why you're seeing this issue? You could set cpython/*:shared=True if you haven't already, perhaps that's what you're looking for?

ilatypov · 2024-06-14T19:20:37Z

I am not linking, just using Python from Conan Center. Its smoke test above (-m pip) is failing.

Ahajha · 2024-06-14T19:22:31Z

Let me clarify - Regardless of what you're doing with the package, on Windows, if cpython/*:shared=False, which is the default, then the extension modules (such as expat) will not be available. So setting cpython/*:shared=True I think will fix your issue.

I think this is a poor default regardless - I have a PR out to change it.

ilatypov · 2024-06-14T19:46:36Z

It worked for 3.8.12 here, thank you. I am new to conan, so I forgot that Conan Center's binary artifacts are not the ultimate goal of this GitHub project, but only its optimization. The Conan Center's recipes are the goals (important deliverables) of this GitHub project. My building C Python for Windows from source using the available recipe took only minutes and worked great.

PS> conan install --requires=cpython/3.8.12 -o shared=True -u --build=missing -vvv --deployer=full_deploy
[...]
PS> .\full_deploy\host\cpython\3.8.12\Release\x86_64\bin\python.exe -m pip
Usage:
  C:\Users\XXX\full_deploy\host\cpython\3.8.12\Release\x86_64\bin\python.exe -m pip <command> [options]

Commands:
[...]

P.S. I wish Conan Center ~~hosted simpler proto-conan binaries sufficient to build C Python, bootstrapping a full conan. Or it could have~~ hosted binary proto-cpython packages downloading and untarring which would allow running python -m pip and installing conan.

memsharded and others added 23 commits October 5, 2023 11:30

wip

a3c8eb5

cpython: migrate to Conan v2

342f926

Merge branch 'migrate/cpython' into cpython

2f838f9

# Conflicts: # recipes/cpython/all/conanfile.py

cpython: fix autotools install step

8c8671a

cpython: fix libuuid requirement

c1c9f2e

cpython: update test_package

960811e

cpython: temporarily disable FindPythonX tests

522d5d9

cpython: package_type should be "library"

9c83e18

cpython: tidy

6c496e6

cpython: ncurses is required transitively

35fac91

cpython: fix shared test

ec99e2e

cpython: use cpp_info.aggregated_components() for deps

220c472

cpython: bsddb has been remove in Python 3

2e54e11

cpython: fix openssl support

51a3e11

Fix package layout

3cc7f61

Fix new generators in test package

1840f62

Misc fixes

d0bcea7

Remove redundant test

f0797ef

Add FIXMEs

c766334

Fix running test package multiple times in a row

6a1ebd9

Handle conan 1/2 differences in test_package

1539332

Fix MSBuild

1c2d2d1

Misc Conan 2.0 fixes

a665c98

ghost mentioned this pull request Nov 26, 2023

[service] Potentially conflicting recipes #3571

Closed

ghost mentioned this pull request Nov 26, 2023

cpython: migrate to Conan v2 #20960

Closed

conan-center-bot added Failed Version conflict There is a version conflict when solving the dependencies graph labels Nov 26, 2023

This comment has been minimized.

Sign in to view

cpython: minor tweaks, bump deps

b8a0b24

conan-center-bot removed the Failed label Mar 18, 2024

This comment has been minimized.

Sign in to view

Remove calculated Python major version

ab8ca5f

This comment has been minimized.

Sign in to view

jwillikers approved these changes Mar 18, 2024

View reviewed changes

uilianries reviewed Mar 19, 2024

View reviewed changes

Ahajha and others added 3 commits March 19, 2024 20:54

Use autotools.install() directly

9466a2b

Co-authored-by: Uilian Ries <uilianries@gmail.com>

Remove outdated configure flags and some hacks

bc6d232

Add 2.0 versions of env variables

3988348

conan-center-bot added the Failed label Mar 20, 2024

This comment has been minimized.

Sign in to view

Remove PYTHONHOME from env info for now

c5fa0c5

conan-center-bot removed the Failed label Mar 20, 2024

jwillikers approved these changes Mar 20, 2024

View reviewed changes

AbrilRBS approved these changes Mar 21, 2024

View reviewed changes

conan-center-bot merged commit e78f2d5 into conan-io:master Mar 21, 2024
23 checks passed

	if os.environ.get("CC") or self.settings.compiler != "gcc":
	if self.settings.compiler != "gcc":

	self.env_info.PYTHON_ROOT = python_root
	self.env_info.PYTHON_ROOT = python_root
	self.buildenv_info.define_path("PYTHON_ROOT", python_root)

CPython: Conan 2.0 #21387

CPython: Conan 2.0 #21387

Conversation

Ahajha commented Nov 26, 2023 • edited Loading

ghost commented Nov 26, 2023 • edited by ghost Loading

This comment has been minimized.

This comment has been minimized.

Ahajha commented Mar 18, 2024

This comment has been minimized.

uilianries left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ahajha Mar 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ahajha commented Mar 19, 2024

Ahajha commented Mar 20, 2024 • edited Loading

This comment has been minimized.

conan-center-bot commented Mar 20, 2024

Conan v1 pipeline ✔️

Conan v2 pipeline ✔️

uilianries commented Mar 20, 2024

jwillikers commented Mar 21, 2024

Ahajha commented Mar 21, 2024

ilatypov commented Jun 14, 2024 • edited Loading

Ahajha commented Jun 14, 2024

ilatypov commented Jun 14, 2024 • edited Loading

Ahajha commented Jun 14, 2024 • edited Loading

ilatypov commented Jun 14, 2024 • edited Loading

Ahajha commented Jun 14, 2024 • edited Loading

ilatypov commented Jun 14, 2024 • edited Loading

Ahajha commented Nov 26, 2023 •

edited

Loading

ghost commented Nov 26, 2023 •

edited by ghost

Loading

Ahajha Mar 19, 2024 •

edited

Loading

Ahajha commented Mar 20, 2024 •

edited

Loading

ilatypov commented Jun 14, 2024 •

edited

Loading

ilatypov commented Jun 14, 2024 •

edited

Loading

Ahajha commented Jun 14, 2024 •

edited

Loading

ilatypov commented Jun 14, 2024 •

edited

Loading

Ahajha commented Jun 14, 2024 •

edited

Loading

ilatypov commented Jun 14, 2024 •

edited

Loading