Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes Windows and macOS wheel builds by addressing multiple build system and dependency issues. The changes enable successful wheel compilation across different platforms by properly configuring compilers, dependencies, and include paths.
Key changes:
- Platform-specific build configurations with proper ODBC library detection and compiler flags for Windows/Unix systems
- Reordered C++ includes to ensure SQL headers come after
pyodbc.hfor Windows compatibility - Fixed numpy include path handling to avoid cross-drive path resolution issues on Windows
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/artifacts_build.yml |
Adds dependency installations (sqlite, icu, meson), configures Windows builds with vcpkg, removes macOS x86 builds, and sets platform-specific environment variables |
meson.build |
Implements platform-specific compiler flags and ODBC detection logic, fixes numpy include path handling, and adds C++17 standard requirement |
subprojects/packagefiles/pyodbc/meson.build |
Mirrors platform-specific build configuration from main meson.build with Windows and Unix-specific compiler flags and ODBC detection |
src/npyodbcmodule.cpp |
Reorders includes to place SQL headers after pyodbc.h for Windows compatibility |
src/npcontainer.cpp |
Moves sqltypes.h include after other project headers for consistent include ordering |
subprojects/pyodbc.wrap |
Removes reference to textenc.patch which is no longer needed |
subprojects/packagefiles/patches/textenc.patch |
Deletes the patch file that is no longer used after include fixes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #53 by
pyodbc.h, since that header includeswindows.h, which is needed for types used by the SQL headers on Windows (why don't they just include it themselves?)np.get_include()directly instead of passing it toos.path.relpath, which fails if for some reason numpy is installed on a separate drive (with no common relative path), as it is on Windows github runnersI've also removed the
textenc.hpatch, we are not using it after having fixed some includes in #52.