-
Notifications
You must be signed in to change notification settings - Fork 517
[FIX] Fixed issue with cross compiling using MINGW-w64 #1731
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
Conversation
|
Tested cross compilation with Ubuntu 22.04 and MinGW-w64 x86_64 and MXE |
|
Hi, can you rebase, push the rebase, and run the command(this could be found in the format directory as well) in the root directory(./ccextractor). |
|
The pull request was done directly via github web, I've resynced the fork with the latest master and hopefully that should be enough |
What exactly is this format test? The formatting was for the changes matched the formatting for the existing file. |
Actually it just runs a better version of CLANG format. |
|
It's making a LOT of changes to the formatting of the existing code unrelated to this patch. Maybe that should be handled separately to avoid confusion. |
formatting changes a recommended by the clang test
I've made the one correction to the formatting of the changes as recommended by the script you provided above related to the changes made in this patch. The rest of the formatting changes recommended aren't related to this patch. |
|
Works now. 🎉 |
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.
Pull Request Overview
This PR fixes cross-compilation issues when using MINGW-w64 by adding proper conditional compilation directives to handle differences between MSVC and MINGW build environments.
- Adds conditional includes for
iconv.hto use native MINGW headers instead of bundled win_iconv - Prevents redefinition of
ssize_ttype that is natively provided by MINGW - Updates CMake configuration to exclude MSVC-specific include directories for MINGW builds
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/lib_ccx/ts_tables_epg.c | Adds MINGW-specific iconv header inclusion |
| src/lib_ccx/ccx_encoders_common.h | Adds MINGW-specific iconv header inclusion |
| src/lib_ccx/ccx_common_platform.h | Guards ssize_t typedef to prevent redefinition in MINGW |
| src/CMakeLists.txt | Excludes MSVC-specific include directory for MINGW builds |
| docs/CHANGES.TXT | Documents the MINGW-w64 cross-compilation fix |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
CCExtractor CI platform finished running the test files on windows. Below is a summary of the test results, when compared to test for commit a34ba0f...:
NOTE: The following tests have been failing on the master branch as well as the PR:
All tests passing on the master branch were passed completely. Check the result page for more info. |
|
@steel-bucket is something amiss here or is it good to commit? I see the test have passed but the Linux build was aborted. |
|
For the linux regression not running, it's most probably an issue with the sample platform and not your PR. |
CCExtractor CI platform finished running the test files on linux. Below is a summary of the test results, when compared to test for commit 4b5f68a...:
All tests passing on the master branch were passed completely. Check the result page for more info. |
|
Looks like it's good now |
|
@hrideshmg these are the changes needed to build using MinGW |
|
@prateekmedia is this good for a commit? It's the only way to build a static version of ccextractor for windows ATM |
|
@rboy1 can you confirm if the steps you used for verifying this is similar to our build windows workflow or you used different commands? Like did you used Visual Studio along with vcpkg to build on Windows? |
|
Yes it's a similar workflow, except using MinGW-w64 (vs MsBuild or VS) and MXE (vs vcpkg) to build it and the dependencies. Essentially cross compiling for windows in Ubuntu. |
|
@rboy1 This seems interesting workflow, could you also add this to compilation steps and in wiki? I will merge this after that. Seems like a good addition and something we don't do. |
prateekmedia
left a comment
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.
LGTM! We can later add more docs regarding this method.
Here's a script you can use to cross build a static windows release using MXE on Ubuntu 22.04 with the GUI. This doesn't use RUST, so it may need to tweaked to build with RUST |
…ractor#1731)" This reverts commit 1c7e2a0.
In raising this pull request, I confirm the following (please check boxes):
My familiarity with the project is as follows (check one):
Fixed build script for cross compiling using MinGW-w64
Some file path's and headers are specific to VS, MinGW provides them natively, updated CMakeFile and source code to differentiate between MS/VS environments and MinGW environments.