-
Notifications
You must be signed in to change notification settings - Fork 1
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
[#699] Python 3.12 update. #700
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #700 +/- ##
==========================================
- Coverage 79.05% 79.05% -0.01%
==========================================
Files 47 47
Lines 7739 7738 -1
Branches 1172 1151 -21
==========================================
- Hits 6118 6117 -1
Misses 1355 1355
Partials 266 266 see 1 file with indirect coverage changes
|
df9c125
to
46b3f64
Compare
pavement.py
Outdated
@@ -120,7 +120,7 @@ def compile_file(fullname, ddir=None, force=0, rx=None, quiet=0): | |||
if not force: | |||
try: | |||
mtime = int(os.stat(fullname).st_mtime) | |||
expect = struct.pack('<4sl', imp.get_magic(), mtime) | |||
expect = struct.pack('<4sl', MAGIC_NUMBER, mtime) |
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.
thanks for the update... the compile_file code shoudl be removed ... and just use upstream code
We had this "fork" here to support py2 migration... and I forgot to clean the code
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.
Changes looks good. Great work with py3.12 and compat updates.
I did more py3 only cleanup, while doing the review.
I also updated the GHA YAML to run the tests on latest Ubuntu
I tried to simplify the docker YAML code as without older centos, we should be able to use standard GitHub Actions rules.
I have also fixed a bug in the testing code, so we can have a new release
git log -1 --format='%H' | ||
|
||
# This action also fails on CentOS 5/6. | ||
- uses: actions/checkout@v4 |
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 went with default git chechout code ... to simplify our GHA yaml
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.
The downside is it only supports distributions on which GitHub Actions Runner actually runs.
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.
Well... it works on any distribution on which nodejs 16 or 20 can be executed.
I have changed the code and it looks like it also works for Alpine ...for example
GitHub Actions has a separate node_js version compiled for Alpine.
GitHub Actions are doing with nodejs, something similar to what we do with pythia... they distribute various nodejs versions with each GitHub Action runner.
you can mark this as resolved and merge
There's one more thing needed before a new |
Yes... for chevah/compat is not that critical.. I am a bit worried about the new libnsl dependency ... but I see that the code works on OracleLinux 8 I think that we should update the GitHub Actions on Pythia to run some minimal tests via Docker on multiple centos versions ... just to make sure we got our dynamic libraries right |
Tracked at chevah/pythia#68. |
Scope
Fixes #699
Updates for Python 3.12.
Changes
Use
pynose
andimportlib.utils
instead ofnose
andimp
.Drive-by changes:
pythia.*
to latest versions from pythia repo.How to try and test the changes
reviewers: @adiroiban
Have only tried to have things green and to follow #693.
Do check if the diff looks alright.