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

bump cibuildwheel action to v2.17.0 #7672

Merged
merged 4 commits into from
Jun 13, 2024
Merged

bump cibuildwheel action to v2.17.0 #7672

merged 4 commits into from
Jun 13, 2024

Conversation

arvidn
Copy link
Owner

@arvidn arvidn commented May 11, 2024

No description provided.

@xavier2k6
Copy link
Contributor

@arvidn try using Boost 1.82.0 instead.

@arvidn
Copy link
Owner Author

arvidn commented May 11, 2024

it looks like there's some progress. Now the failure is a rename that happens after the build, here.

rename build\lib.win-amd64-cpython-37\libtorrent.cp37-win_amd64.pyd -> build\lib.win-amd64-cpython-37\libtorrent\__init__.cp37-win_amd64.pyd
error: [WinError 2] The system cannot find the file specified: 'build\\lib.win-amd64-cpython-37\\libtorrent.cp37-win_amd64.pyd' -> 'build\\lib.win-amd64-cpython-37\\libtorrent\\__init__.cp37-win_amd64.pyd'

@xavier2k6
Copy link
Contributor

It should make no difference to error but just as an FYI v2.18.0 of cibuildwheel action has been released.

@qstokkink
Copy link

Thanks for picking this up! I'd like to see this fixed, so I'll support to the best of my ability.

[Click me for analysis]

If I'm interpreting the log correctly, this is the "rename" log entry just before failure:

log.info("rename %s -> %s", src, dst)

I assume the following os.rename(src, dst) is the cause of the crash.

On the other builds this succeeds. For example, Mac:

common.copy /Users/runner/work/libtorrent/libtorrent/bindings/python/build/lib.macosx-10.9-x86_64-cpython-37/libtorrent.cpython-37m-darwin.so
    ...updated 276 targets...
    rename build/lib.macosx-10.9-x86_64-cpython-37/libtorrent.cpython-37m-darwin.so -> build/lib.macosx-10.9-x86_64-cpython-37/libtorrent/__init__.cpython-37m-darwin.so

However, on Windows, build\lib.win-amd64-cpython-37\libtorrent.pyd is apparently created instead of the expected build\lib.win-amd64-cpython-37\libtorrent.cp37-win_amd64.pyd:

common.copy D:\a\libtorrent\libtorrent\bindings\python\build\lib.win-amd64-cpython-37\libtorrent.pyd
    bin\msvc-14.3\rls\adrs-mdl-64\cxstd-14-iso\pythn-3.7\libtorrent.pyd
            1 file(s) copied.
    ...updated 266 targets...
    rename build\lib.win-amd64-cpython-37\libtorrent.cp37-win_amd64.pyd -> build\lib.win-amd64-cpython-37\libtorrent\__init__.cp37-win_amd64.pyd

As I see it, you could remedy this by either changing the setup.py output-detection logic or the build logic that creates the output in the first place (probably in the jamfile).

@qstokkink
Copy link

@arvidn if you want some help and you don't mind me hammering your GitHub Actions builds for a bit, I can try getting the action (back) up and running. Just let me know and I'll pick this up.

@qstokkink
Copy link

qstokkink commented Jun 4, 2024

According to the following line, the EXT_SUFFIX is empty on Windows. I'm not 100% sure but that might be causing the failure.

2024-05-11T16:35:18.8519849Z   using python : 3.7 : "C:\\Users\\runneradmin\\AppData\\Local\\Temp\\cibw-run-ffbtcev5\\cp37-win_amd64\\build\\venv\\Scripts\\python.exe" : "C:\\Users\\runneradmin\\AppData\\Local\\Temp\\cibw-run-ffbtcev5\\cp37-win_amd64\\build\\venv\\include" "C:\\Users\\runneradmin\\AppData\\Local\\pypa\\cibuildwheel\\Cache\\nuget-cpython\\python.3.7.9\\tools\\include" "C:\\Users\\runneradmin\\AppData\\Local\\pypa\\cibuildwheel\\Cache\\nuget-cpython\\python.3.7.9\\tools\\Include" : "C:\\Users\\runneradmin\\AppData\\Local\\Temp\\cibw-run-ffbtcev5\\cp37-win_amd64\\build\\venv\\libs" : <libtorrent-python>on : "" ;

I think this may be due to #7649 switching from distutils.sysconfig.get_config_var("EXT_SUFFIX") to sysconfig.get_config_var("EXT_SUFFIX"). The former contains a workaround specific to Python 3.7 for Windows (see here). If this is the case, then you need to add in a workaround for sys.version_info < (3, 10) and platform.system() == 'Windows' in your setup.py to set the EXT_SUFFIX to _imp.extension_suffixes()[0] for that specific platform.

@arvidn arvidn force-pushed the bump-cibuildwheel branch 2 times, most recently from bd8e03d to e6557c6 Compare June 8, 2024 08:36
@arvidn
Copy link
Owner Author

arvidn commented Jun 8, 2024

the next issue:

+ bash -c "cd 'D:\a\libtorrent\libtorrent/bindings/python' && python test.py"
  Error: Command bash -c "cd 'D:\a\libtorrent\libtorrent/bindings/python' && python test.py" failed with code 2816. None

@arvidn
Copy link
Owner Author

arvidn commented Jun 8, 2024

exit code 2816, according to this means:

The EventMapping table refers to an invalid control [4] on dialog [2] for the event [3]

Possibly some issue related to the program running in a console, without a GUI.

the other python test fails with exit code 0xC0000005, which is ACCESS VIOLATION

@xavier2k6
Copy link
Contributor

exit code 2813

@arvidn Where did you see this?

#7672 (comment) shows 2816

The EventMapping table refers to an invalid control [4] on dialog [2] for the event [3]

@arvidn
Copy link
Owner Author

arvidn commented Jun 8, 2024

@qstokkink
Copy link

Almost there 🎉 I think 2816 is simply a segfault (11) in disguise due to bash running on Windows (for reasons explained here).

It would be easier to debug if we could check out the .whl that was produced. Could you skip the test command and share the wheel?

@qstokkink
Copy link

qstokkink commented Jun 10, 2024

Thanks for the wheel! I'll prod around and see what I can find.

Findings:

  • Executing test.py (using Python 3.7.9 and the generated wheel) passes the tests on my machine.
  • Invoking test.py both through bash -c and cmd.exe works on my machine.
  • b2 -l400 warnings=all warnings-as-errors=on release deterministic-tests works with Boost 1.78 on my machine.

Open question: why is Windows / Test (release) segfaulting? I pointed OPENSSL_LIB (in libtorrent's Jamfile) explicitly to [OSSL_INSTALLDIR]/lib/VC/x64/MD (instead of [OSSL_INSTALLDIR]/lib) when running locally. Perhaps that is the issue?

@qstokkink
Copy link

I don't think I can do anything more with the current log output. Replaying the logged commands locally with the same software versions works as expected. That implies that there is some path/configuration error specific to the GitHub Actions builder images.

My current theory is that the test code somehow pulls in the GitHub Action's default OpenSSL (OpenSSL 1.1.1w) instead of the version that the Action installs (3.3.1).

If fixing the path doesn't work, I can open a PR to dig through the environment variables but @arvidn I want your explicit permission/consent before I do that.

@qstokkink
Copy link

Hold on, there was a change since the last time the Windows / Test (release) passed. This command is now installing Win64OpenSSL-3_3_1.exe instead of Win64OpenSSL-3_3_0.exe:

command: choco install openssl --limitoutput --no-progress

There is a chance that adding --version=3.3.0 here will magically fix the Windows / Test (release) tests.

@qstokkink
Copy link

It seems like everything except Mac stuff is now operational (which I guess is getting fixed in #7606). 🥳

I consider my work done here and I'll retreat back to my usual repos until some other esoteric failure occurs again.

@arvidn
Copy link
Owner Author

arvidn commented Jun 13, 2024

@qstokkink thanks for the help! (and sorry for not having so much time to address this sooner)

@arvidn arvidn merged commit f53151b into RC_2_0 Jun 13, 2024
50 of 51 checks passed
@arvidn arvidn deleted the bump-cibuildwheel branch June 13, 2024 11:22
@qstokkink
Copy link

@arvidn no worries, thanks for getting everything up-and-running again. If there is still anything stopping you from uploading the wheels to PyPI for the next release, feel free to @ me.

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

3 participants