-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows #39595
base: main
Are you sure you want to change the base?
Conversation
…ace to Arrow in Debug mode on Windows
…ace to Arrow in Debug mode on Windows
|
@kou Could you please tell me how to clear |
|
Thanks for the PR @ShaiviAgarwal2! I agree with @kou's comment that this should only require checking the |
…ace to Arrow in Debug mode on Windows
…ace to Arrow in Debug mode on Windows
@ShaiviAgarwal2 it looks like the MATLAB Windows CI workflow is failing: https://github.com/apache/arrow/actions/runs/7627307276/job/20775886597?pr=39595#step:9:35. The error message you added is being triggered right now in CI which may suggest that the logic for checking if the build mode is Debug may need to be modified. |
I tried solving the error but wasn't able to do clear completely |
@ShaiviAgarwal2 - I believe what is happening here is a result of the fact that Visual Studio is a Multi Configuration Generator (as opposed to a Single Configuration Generator like Make). Multi Configuration Generators ignore the value of The following section from the CMake documentation on Build Configurations describes what seems to be going on here in more detail: https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html#build-configurations To make this work as desired, I believe we will need to use a generator expression to check if a |
…ace to Arrow in Debug mode on Windows
@ShaiviAgarwal2 - are you still planning on working on this? If there aren't any additional updates in the next few days, then I think it is probably reasonable to assume this PR has gone stale and to close it for now. We can always follow up with another PR if anyone (including you) wants to revisit this later. |
@kevingurney I am working on it |
@ShaiviAgarwal2 - please refer to my previous comment about using generator expressions: |
Rationale for this change
The changes aim to identify the build mode (Debug or Release) of MATLAB by checking the MSVC Runtime library it is linked against. If MATLAB is built with a non-debug version, building in Debug mode is disallowed, and an error message is displayed, guiding the user to switch to Release mode.
What changes are included in this PR?
Added a block of code to check the dependencies of the MATLAB executable, specifically looking for references to MSVCP\d+.dll. If such references are found, it indicates a non-debug version of the MSVC Runtime, and an error message is displayed.
Are these changes tested?
Yes, the changes have been manually tested
Are there any user-facing changes?
No, these changes focus on improving the build process for compatibility with MATLAB. Users will not observe any differences in the functionality or usage of the MATLAB Interface to Arrow.