Build a self-contained Win32 Python wheel for Concordance by Ben-Meister · Pull Request #57 · jaymzh/concordance

Ben-Meister · 2024-04-17T00:44:35Z

This pull request builds a 32-bit Win32 Python wheel for libconcord, the bindings and all of its dependencies. This wheel simplifies the installation (and removal) process on Windows. It also fixes the DLL load path issue, including the changes to DLL load path made in Python 3.8+.

The wheel may be used individually, or as part of an installation process. In the companion pull request to Congruity, we consume this wheel to make an all-in-one installer for Congruity. It installs a self-contained 32-bit version of Python, Congruity, this wheel, and all other package and binary dependencies required.

Because the compiled DLLs are 32-bit, a 32-bit Python version is required to use them. However, the 32-bit Python (and our installer) will happily run on a 64-bit Windows system. The wheel is compatible with and tested on Windows 11, 10, and 8.1, both the 32- and 64-bit versions.

The wheel is built as part of the ci-windows.yml GitHub workflow via the added make_wheel.py, which dynamically assembles the package using the same files that are built for the NSIS installer. A test has been added for the wheel to this workflow as well, based on the similar test present in ci-linux.yml.

The wheel and bindings are compatible with Python versions ~~both pre- and~~ post-3.8. In Python 3.8 (released in 2019), the DLL search path was narrowed, and os.add_dll_directory was introduced (link). We use with os.add_dll_directory to add the package directory to Python’s DLL search path when loading the libconcord DLLs (after which it’s immediately removed). If an app wants the DLLs in a different directory, the app can call os.add_dll_directory before importing the libconcord bindings. Fallback logic has been added to perform a similar method using PATH (temporary for the calling process only) when the bindings are used on Python 3.7 or earlier and os.add_dll_directory is not available.

The binding behavior, Python package format, and version compatibility are unchanged for Linux/Mac. On these platforms, the changes to libconcord.py are silently ignored.

swt2c · 2024-04-17T03:07:39Z

Thank you for your contribution. My initial quick thoughts: First of all, my opinion is that we don't need to support Python 3.7 as it has been EOL for a year or so, so you could probably drop that commit.

Second - why do we need separate pyproject.toml and setup.py files for Windows?

Ben-Meister · 2024-04-18T21:58:19Z

Both changes have been made as requested.

jaymzh

@swt2c - thoughts here?

jaymzh · 2025-03-05T23:01:21Z

+      run: |
+        pip install build
+        MINGW_SYSROOT_BIN="/usr/i686-w64-mingw32/sys-root/mingw/bin" concordance/win/make_wheel.py
+    - uses: actions/upload-artifact@v3


we'll need to move this to v4

jaymzh · 2025-03-05T23:01:30Z

        Start-Sleep -Seconds 5
        cd "C:\Program Files (x86)\Concordance"
        .\concordance.exe --version
+    - uses: actions/download-artifact@v3


Ben-Meister · 2025-03-06T22:04:47Z

Thank you both for reviewing this. Should I rebase the PR to the current changes?

I’m glad that this project is still being kept up to date.

FYI: I also have changes planned for submission as a subsequent PR, that build upon this PR to produce a version with curl supporting schannel, to fix the issue uncovered during testing on Windows wherein the required root certificates were not being downloaded. Scott @swt2c and I discussed this in my pull request to congruity congruity/congruity#12 (comment) and they are in my https://github.com/Ben-Meister/concordance/tree/buildcurl branch (from before the v4 artifact change so I’ll have to update those too). Let me know how to proceed.

Unfortunately I was absent from this project for far too long; my apologies. I’ll need to untangle what I have in order to finish this up.

swt2c · 2025-03-06T23:16:38Z

+        dll = line.split('=>')[1].strip()
+        if dll != 'not found':
+            shutil.copy2(dll, subdir)
+    shutil.copy2(os.path.dirname(os.path.abspath(__file__)) + '/../.libs/concordance.exe', subdir)


Why are you including concordance.exe in the wheel?

Also are the setup.py and pyproject.toml needed either?

Scott I appreciate your feedback.

Re: the EXE. I’ll make the change to remove it, but I see pros and cons.

Reasons to not include the EXE: it’s not required, we can reduce our surface/size etc. And I did produce a wheel without it, as I was getting false positives from VirusTotal when I included it.

Reasons to include the EXE: it only adds 160KB. Then you have concordance available. A nice option if you’d like to isolate problems to congruity or concordance, or simply want an alternative. Otherwise you must download and install concordance separately.

Re: setup.py and pyproject.toml: These are not included in the produced wheel. They are used in the staging process, since the make_wheel.py script puts together a temporary tree for the build (based off what you did for make_installer.py, thank you!) and then calls python3 -m build -w from that location.

swt2c · 2025-03-06T23:28:49Z

I'm wondering more generally if we should just start building wheels (maybe Windows-only for now) and actually publish them to PyPI? We'd have to build for all Pythons, but the contents would actually be the same for all, probably.

I've got some recent experience in this area and can help, if this is too much scope creep for your original PR. :-)

swt2c · 2025-03-07T00:41:08Z

I'm wondering more generally if we should just start building wheels (maybe Windows-only for now) and actually publish them to PyPI? We'd have to build for all Pythons, but the contents would actually be the same for all, probably.

I've got some recent experience in this area and can help, if this is too much scope creep for your original PR. :-)

Actually, strike that. Can we simplify this approach quite a bit by doing the following:
python3 -m build -w -C="--build-option=--plat-name" -C="--build-option=win32"
to build the wheel and then use delvewheel to "repair" the wheel (ie, include the .dlls it needs).

Then I think we don't need to the changes to setup.py and we don't need that custom wheel build script?

Ben-Meister · 2025-03-08T15:52:38Z

Publishing a Concordance wheel has value, as it would make it much easier to install and use from Python. And Scott, I like your PyPI idea for the future. I understand certainly that you have a lot of projects that you are responsible for. Is this something you would like to maintain? Unfortunately I will have to contribute on an as-available basis at this time.

My thought is that we can start with the minimum viable product that we have already been able to produce using this code right here: a Win32 wheel, published on GitHub. Then we can worry about other architectures, OS’s, and PyPI.

The delvewheel tool is a cool find (thanks!) but won’t work for our use case without nasty kludges. The reason is that we load the libconcord DLL dynamically using ~~cffi~~ cdll. delvewheel isn’t designed for that. It’s designed for a wheel that has PYDs already in it and needs their deps resolved. Unfortunately, telling delvewheel to add the DLL won’t "just work" for our use case. By the time we hack around everything, it will be more complex than this PR that we already have. I can go into more details if desired.

…LL path search list

…oncordance

swt2c · 2025-03-10T23:13:40Z

The delvewheel tool is a cool find (thanks!) but won’t work for our use case without nasty kludges. The reason is that we load the libconcord DLL dynamically using ~~cffi~~ cdll. delvewheel isn’t designed for that. It’s designed for a wheel that has PYDs already in it and needs their deps resolved. Unfortunately, telling delvewheel to add the DLL won’t "just work" for our use case. By the time we hack around everything, it will be more complex than this PR that we already have. I can go into more details if desired.

No, you are right, delvewheel is not a good fit.

jaymzh · 2026-04-21T23:27:16Z

as a non-windows user, I have almost no opinion on this, but happy to merge it if we think it's worthwhile. @swt2c and @Ben-Meister ?

Ben-Meister · 2026-04-27T16:38:05Z

I still think it's worthwhile. In lieu of PyPI, the wheel's the next best thing, and the work here would be considered a prereq for if we were to do PyPI. Makes usage a lot easier on Windows. And my all-in-one installer uses it too.

I'll ask the obvious question. Currently Windows still has support for 32-bit applications, and Python still has 32-bit builds for Windows, so this is fine for now. At some point in the future we might wind up doing 64-bit Windows builds. Are there any known code paths we would have to specifically watch for issues? Or is it a matter of try it, thoroughly test, and see what breaks?

Ben-Meister · 2026-04-28T04:33:24Z

Marking this one as draft for now since it's hardcoded Win32

Ben-Meister force-pushed the win32wheel branch 2 times, most recently from a8ac713 to 0ed3274 Compare April 18, 2024 20:46

jaymzh self-requested a review March 5, 2025 23:02

jaymzh reviewed Mar 5, 2025

View reviewed changes

swt2c reviewed Mar 6, 2025

View reviewed changes

Ben-Meister added 9 commits March 8, 2025 12:33

Enable option to run workflows manually

0916a61

Fix libconcord DLL name on Windows, and add the package path to the D…

7b95a8d

…LL path search list

Add script and configs to build Win32 wheel

35fa58f

Build Python wheel in CI

4c0f254

Test Python wheel on Windows

3bbf5dd

Use one set of config files to produce all Python packages/wheels

918d764

On Windows, wait for installer to finish before attempting to start C…

4a086f3

…oncordance

Update Windows CI to use up/download-artifact v4

606f7b3

Do not include EXE in produced wheel. Base deps on DLL instead of EXE.

f0084df

Ben-Meister force-pushed the win32wheel branch from a4ae76c to f0084df Compare March 8, 2025 17:42

Ben-Meister mentioned this pull request May 8, 2025

Windows: 64-Bit, use schannel for curl #65

Merged

Ben-Meister marked this pull request as draft April 28, 2026 04:32

Conversation

Ben-Meister commented Apr 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

swt2c commented Apr 17, 2024

Uh oh!

Ben-Meister commented Apr 18, 2024

Uh oh!

jaymzh left a comment

Choose a reason for hiding this comment

Uh oh!

jaymzh Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

jaymzh Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

Ben-Meister commented Mar 6, 2025

Uh oh!

swt2c Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

Ben-Meister Mar 8, 2025

Choose a reason for hiding this comment

Uh oh!

swt2c commented Mar 6, 2025

Uh oh!

swt2c commented Mar 7, 2025

Uh oh!

Ben-Meister commented Mar 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

swt2c commented Mar 10, 2025

Uh oh!

jaymzh commented Apr 21, 2026

Uh oh!

Ben-Meister commented Apr 27, 2026

Uh oh!

Ben-Meister commented Apr 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Ben-Meister commented Apr 17, 2024 •

edited

Loading

Ben-Meister commented Mar 8, 2025 •

edited

Loading