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

Min/MaxSupportedVersion and IsSupportedVersion on PythonEngine #1798

Merged
merged 1 commit into from May 26, 2022

Conversation

lostmsu
Copy link
Member

@lostmsu lostmsu commented May 25, 2022

Does this close any currently open issues?

fixes #1724

Checklist

Check all those that are applicable and complete.

  • Make sure to include one or more tests for your change
  • Updated the CHANGELOG

@lostmsu
Copy link
Member Author

lostmsu commented May 25, 2022

:/ I am puzzled. What does this change have to do with segfaults? Only on *nix? Ugh...

src/runtime/PythonEngine.cs Show resolved Hide resolved
@@ -127,6 +127,10 @@ public static string PythonPath
}
}

public static Version MinSupportedVersion => new(3, 7);
public static Version MaxSupportedVersion => new(3, 10, int.MaxValue, int.MaxValue);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe something more "readable", like 999 instead of maxint?

Copy link
Member Author

@lostmsu lostmsu May 26, 2022

Choose a reason for hiding this comment

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

I thought of that, but it won't work if there are builds of Python that use build number not as patch (e.g. patch .4), but in a Windows manner like .19041.365, or maybe do MaxSupportedVersionExclusive. Alternatively, we can only leave the IsSupportedVersion check and provide an error string in a separate property.

What do you think?

@filmor
Copy link
Member

filmor commented May 26, 2022

:/ I am puzzled. What does this change have to do with segfaults? Only on *nix? Ugh...

Just tested it as well, it only happens on Mono...

@filmor
Copy link
Member

filmor commented May 26, 2022

Further observations:

  • The culprit is the call, the comparisons in the tests seem fine
  • Only the enum conversions with max int/long/ulong fail, if I remove these, the tests run though
  • Only happens when all tests are run in order

@filmor filmor merged commit 1492d7d into pythonnet:master May 26, 2022
@lostmsu lostmsu deleted the SupportedVersions branch May 26, 2022 19:22
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.

None yet

2 participants