-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Add cpython 3.13.0 #25536
base: master
Are you sure you want to change the base?
Add cpython 3.13.0 #25536
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@ErniGH not sure why there's a missing binary error. I expect this should be working and ready for review. |
There was a problem hiding this 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;", "")
@grossag Thanks! I'll apply the patch later today. |
This comment has been minimized.
This comment has been minimized.
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. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
|
@grossag Very interesting! I wonder if |
This comment has been minimized.
This comment has been minimized.
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). |
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 |
There was a problem hiding this 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.
@grossag I added a couple of small changes to the test package, mainly just moving things to and from scopes. |
Conan v1 pipeline ✔️Warning Conan Center will stop receiving updates for Conan 1.x packages soon - please see announcement. All green in build 8 (
Conan v2 pipeline ✔️
All green in build 8 (
|
@ErniGH Can you take a look at this when you get the chance? Thanks! |
There was a problem hiding this 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.
cpython/3.13.0
Key changes:
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.
TODO: Either in this PR or in a followup, update mpdecimal to 4.0.0 in 3.13. See python/cpython#115119. PR to add that version: #25896