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

cpython: add 3.13.1 #25536

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

cpython: add 3.13.1 #25536

wants to merge 17 commits into from

Conversation

Ahajha
Copy link
Contributor

@Ahajha Ahajha commented Oct 7, 2024

cpython/3.13.0

Key changes:

See this issue for a list of all removed dependencies.

Also worth noting, the other minor versions have some patch bumps (3.8.20, 3.9.20, 3.10.15, 3.11.10). I can add those in this PR, or perhaps it can wait.

3.8 is also officially EOL as of October 7th, 2024. We can leave it in the recipe for now but it should probably be removed at some point.

@conan-center-bot

This comment has been minimized.

@ErniGH ErniGH self-assigned this Oct 7, 2024
@conan-center-bot conan-center-bot added the Missing dependencies Build failed due missing dependencies in Conan Center label Oct 8, 2024
@conan-center-bot

This comment has been minimized.

@Ahajha
Copy link
Contributor Author

Ahajha commented Oct 8, 2024

@ErniGH not sure why there's a missing binary error. I expect this should be working and ready for review.

Copy link

@grossag grossag left a comment

Choose a reason for hiding this comment

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

I gave this a try on my company's conan builds and found that Windows MSVC doesn't build Python 3.13.0 successfully.

The presenting error is that there is a line that no longer patches correctly:

replace_in_file(self, self._msvc_project_path("_decimal"), r"..\Modules\_decimal\libmpdec;", "")

But the overall issue is that cpython changed the formatting of that file in python/cpython@849e071

I was able to fix it using this diff:

diff --git a/recipes/cpython/all/conanfile.py b/recipes/cpython/all/conanfile.py
index d54593a..c5bd6f9 100644
--- a/recipes/cpython/all/conanfile.py
+++ b/recipes/cpython/all/conanfile.py
@@ -378,16 +378,19 @@ class CPythonConan(ConanFile):
         replace_in_file(self, self._msvc_project_path("_ssl"), '<Import Project="openssl.props" />', "")

         # For mpdecimal, we need to remove all headers and all c files *except* the main module file, _decimal.c
-        self._regex_replace_in_file(self._msvc_project_path("_decimal"), r'.*Include=\"\.\.\\Modules\\_decimal\\.*\.h.*', "")
-        self._regex_replace_in_file(self._msvc_project_path("_decimal"), r'.*Include=\"\.\.\\Modules\\_decimal\\libmpdec\\.*\.c.*', "")
+        if Version(self.version) < "3.13":
+            self._regex_replace_in_file(self._msvc_project_path("_decimal"), r'.*Include=\"\.\.\\Modules\\_decimal\\.*\.h.*', "")
+            self._regex_replace_in_file(self._msvc_project_path("_decimal"), r'.*Include=\"\.\.\\Modules\\_decimal\\libmpdec\\.*\.c.*', "")
+            replace_in_file(self, self._msvc_project_path("_decimal"), r"..\Modules\_decimal\libmpdec;", "")
+        else:
+            # https://github.com/python/cpython/commit/849e0716d378d6f9f724d1b3c386f6613d52a49d
+            # changed _decimal.vcxproj enough that we need different patching code.
+            self._regex_replace_in_file(self._msvc_project_path("_decimal"), r'.*Include=\"\.\.\\Modules\\_decimal\\windows\\.*\.h.*', "")
+            self._regex_replace_in_file(self._msvc_project_path("_decimal"), r'.*Include=\"\$\(mpdecimalDir\)\\libmpdec\\.*\.h.*', "")
+            self._regex_replace_in_file(self._msvc_project_path("_decimal"), r'.*Include=\"\$\(mpdecimalDir\)\\libmpdec\\.*\.c.*', "")
+            replace_in_file(self, self._msvc_project_path("_decimal"), r"..\Modules\_decimal\windows;$(mpdecimalDir)\libmpdec;", "")
         # There is also an assembly file with a complicated build step as part of the mpdecimal build
         replace_in_file(self, self._msvc_project_path("_decimal"), "<CustomBuild", "<!--<CustomBuild")
         replace_in_file(self, self._msvc_project_path("_decimal"), "</CustomBuild>", "</CustomBuild>-->")
-        # Remove extra include directory
-        replace_in_file(self, self._msvc_project_path("_decimal"), r"..\Modules\_decimal\libmpdec;", "")

@Ahajha
Copy link
Contributor Author

Ahajha commented Oct 9, 2024

@grossag Thanks! I'll apply the patch later today.

@conan-center-bot

This comment has been minimized.

@grossag
Copy link

grossag commented Oct 11, 2024

It looks good to me, but I wonder if the Conan team knows why CI isn't running. It would be good to get successful builds. I tried looking through the GitHub workflows to understand the CI system but it appears to be a separate system.

@conan-center-bot

This comment has been minimized.

@conan-center-bot conan-center-bot removed the Missing dependencies Build failed due missing dependencies in Conan Center label Oct 11, 2024
@conan-center-bot

This comment has been minimized.

@Ahajha
Copy link
Contributor Author

Ahajha commented Oct 12, 2024

I've seen this error before. It seems that the Windows build silently tries to grab another version of Python through Nuget, which is obviously concerning but I haven't been able to track down why or where it does this. It seems to be sporadic, a rerun may fix it.

On an somewhat related note, I'm curious if Conan is looking into some sort of sandboxing approach for builds that prevent these silent network calls altogether. I know Bazel does something like that, and networking is opt-in.

@grossag
Copy link

grossag commented Oct 12, 2024

I've seen this error before. It seems that the Windows build silently tries to grab another version of Python through Nuget, which is obviously concerning but I haven't been able to track down why or where it does this. It seems to be sporadic, a rerun may fix it.

On an somewhat related note, I'm curious if Conan is looking into some sort of sandboxing approach for builds that prevent these silent network calls altogether. I know Bazel does something like that, and networking is opt-in.

We actually ran into this same issue in our CI. It depends upon whether the CI is using the Conan APIs to build.

  • If CI uses one subprocess per build package, things are fine because naturally the environment of the subprocess is isolated and won't leak back into the parent or other subprocesses.
  • If CI uses conan_api, packages are built in-proc and the CPython tests leak PYTHONHOME into each other unless the CI specifically resets the env after the package is built. Specifically, https://github.com/conan-io/conan-center-index/blob/master/recipes/cpython/all/test_package/conanfile.py makes all sorts of modifications to the environment.

@Ahajha
Copy link
Contributor Author

Ahajha commented Oct 13, 2024

@grossag Very interesting! I wonder if self.env_info also affects this. Regardless, I can update the tests to use scoped env vars and hopefully that fixes it. Worst case scenario, if it only affects Conan 1.x then we can wait a few weeks for that pipeline to be dropped.

@conan-center-bot

This comment has been minimized.

@grossag
Copy link

grossag commented Oct 13, 2024

Any guess as to why Windows MSVC and MacOS Intel didn't build? MSVC with shared=True should work (shared=False is an invalid configuration).

@Ahajha
Copy link
Contributor Author

Ahajha commented Oct 13, 2024

Known issue. There are a lot of invalid configurations, mostly coming from dependencies. There's a PR out to reduce the number of these here: #25500
See specifically this comment about the MSVC shared=True build: #25500 (comment)

Copy link

@grossag grossag left a comment

Choose a reason for hiding this comment

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

I integrated your change into my company's Conan build and it works well, including the test changes.

@Ahajha
Copy link
Contributor Author

Ahajha commented Oct 26, 2024

@grossag I added a couple of small changes to the test package, mainly just moving things to and from scopes.

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

Warning

Conan Center will stop receiving updates for Conan 1.x packages soon - please see announcement.

All green in build 8 (c55cb3ded02bf4afe87c86275d2f645e2ad4dff8):

  • cpython/3.13.0:
    Built 16 packages out of 22 (All logs)

  • cpython/3.12.2:
    Built 16 packages out of 22 (All logs)

  • cpython/3.10.14:
    Built 18 packages out of 22 (All logs)

  • cpython/3.12.7:
    Built 16 packages out of 22 (All logs)

  • cpython/3.8.19:
    Built 20 packages out of 22 (All logs)

  • cpython/3.9.19:
    Built 20 packages out of 22 (All logs)

  • cpython/3.11.9:
    Built 18 packages out of 22 (All logs)


Conan v2 pipeline ✔️

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

All green in build 8 (c55cb3ded02bf4afe87c86275d2f645e2ad4dff8):

  • cpython/3.13.0:
    Built 4 packages out of 10 (All logs)

  • cpython/3.12.7:
    Built 4 packages out of 10 (All logs)

  • cpython/3.11.9:
    Built 4 packages out of 10 (All logs)

  • cpython/3.12.2:
    Built 4 packages out of 10 (All logs)

  • cpython/3.10.14:
    Built 4 packages out of 10 (All logs)

  • cpython/3.8.19:
    Built 6 packages out of 10 (All logs)

  • cpython/3.9.19:
    Built 6 packages out of 10 (All logs)

@Ahajha
Copy link
Contributor Author

Ahajha commented Oct 28, 2024

@ErniGH Can you take a look at this when you get the chance? Thanks!

Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

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

Hello @Ahajha ! Thank you for your PR and trying to update CPython.

As you should noted, this recipe is one of most fragile in CCI and probably in the top 3 of recipes with more issues. Said that, patching the recipe is extremely avoided, unless is something official and well known.

About those removed libraries, could you please point exactly where it's changed (e.g. link to git tree at the changed line).

Also worth noting, the other minor versions have some patch bumps (3.8.20, 3.9.20, 3.10.15, 3.11.10). I can add those in this PR, or perhaps it can wait.

I would prefer not. CPython is hard to review and be validated. Changing more versions will delay even more this PR.

recipes/cpython/all/conandata.yml Outdated Show resolved Hide resolved
@Ahajha
Copy link
Contributor Author

Ahajha commented Nov 29, 2024

@uilianries I made some updates:

  • I added links to the relevant changes in the PR description
  • I removed the 3.13 libffi patch, as I discovered in CPython: Fix MSVC/all shared #25890 the libffi patches are no longer necessary since 3.11 (the other versions' patches are removed in that PR)
  • Since I made some changes to the mpdecimal recipe, the Windows/all shared builds are running again, but they don't work. So I've added a temporary invalid configuration until we merge CPython: Fix MSVC/all shared #25890 so we can keep the status quo.
  • I updated mpdecimal to 4.0.0 in 3.13, with a link to the relevant change.

@Ahajha Ahajha changed the title Add cpython 3.13.0 cpython: add 3.13.1 Dec 10, 2024
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.

6 participants