You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "ShaiviAgarwal2 (via GitHub)" <gi...@apache.org> on 2024/01/14 06:57:55 UTC
[PR] GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows [arrow]
ShaiviAgarwal2 opened a new pull request, #39595:
URL: https://github.com/apache/arrow/pull/39595
### 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows [arrow]
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney closed pull request #39595: GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows
URL: https://github.com/apache/arrow/pull/39595
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows [arrow]
Posted by "ShaiviAgarwal2 (via GitHub)" <gi...@apache.org>.
ShaiviAgarwal2 commented on PR #39595:
URL: https://github.com/apache/arrow/pull/39595#issuecomment-2031488877
> @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
According to me, only one check was not successful everything else is perfect
So could you please tell me or guide me on what all things need to be done by me
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows [arrow]
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on PR #39595:
URL: https://github.com/apache/arrow/pull/39595#issuecomment-2030001106
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows [arrow]
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on code in PR #39595:
URL: https://github.com/apache/arrow/pull/39595#discussion_r1453680689
##########
matlab/CMakeLists.txt:
##########
@@ -131,6 +131,14 @@ if(NOT Arrow_FOUND)
build_arrow()
endif()
+# Identify MATLAB build mode (Debug or Release)
+if(WIN32)
+ execute_process(COMMAND dumpbin /dependents ${Matlab_MAIN_PROGRAM} OUTPUT_VARIABLE DEPENDENTS)
Review Comment:
@kou and @ShaiviAgarwal2 - sorry for delay!
To answer your question: all publicly released versions of MATLAB are built in release mode (i.e. linked against a release version of the MSVC Runtime Libraries).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows [arrow]
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on PR #39595:
URL: https://github.com/apache/arrow/pull/39595#issuecomment-1894484308
Thanks for the PR @ShaiviAgarwal2!
I agree with @kou's comment that this should only require checking the `CMAKE_BUILD_TYPE` and should not require looking at the dependencies of the `Matlab_MAIN_PROGRAM`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows [arrow]
Posted by "ShaiviAgarwal2 (via GitHub)" <gi...@apache.org>.
ShaiviAgarwal2 commented on code in PR #39595:
URL: https://github.com/apache/arrow/pull/39595#discussion_r1451718110
##########
matlab/CMakeLists.txt:
##########
@@ -131,6 +131,14 @@ if(NOT Arrow_FOUND)
build_arrow()
endif()
+# Identify MATLAB build mode (Debug or Release)
+if(WIN32)
+ execute_process(COMMAND dumpbin /dependents ${Matlab_MAIN_PROGRAM} OUTPUT_VARIABLE DEPENDENTS)
Review Comment:
Should I resolve the changes? or wait for @kevingurney confirmation?
##########
matlab/CMakeLists.txt:
##########
@@ -131,6 +131,14 @@ if(NOT Arrow_FOUND)
build_arrow()
endif()
+# Identify MATLAB build mode (Debug or Release)
+if(WIN32)
+ execute_process(COMMAND dumpbin /dependents ${Matlab_MAIN_PROGRAM} OUTPUT_VARIABLE DEPENDENTS)
Review Comment:
Should I resolve the changes?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows [arrow]
Posted by "ShaiviAgarwal2 (via GitHub)" <gi...@apache.org>.
ShaiviAgarwal2 commented on code in PR #39595:
URL: https://github.com/apache/arrow/pull/39595#discussion_r1452021890
##########
matlab/CMakeLists.txt:
##########
@@ -131,6 +131,14 @@ if(NOT Arrow_FOUND)
build_arrow()
endif()
+# Identify MATLAB build mode (Debug or Release)
+if(WIN32)
+ execute_process(COMMAND dumpbin /dependents ${Matlab_MAIN_PROGRAM} OUTPUT_VARIABLE DEPENDENTS)
Review Comment:
Yaa sure :))
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows [arrow]
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on code in PR #39595:
URL: https://github.com/apache/arrow/pull/39595#discussion_r1459581795
##########
matlab/CMakeLists.txt:
##########
@@ -131,6 +131,14 @@ if(NOT Arrow_FOUND)
build_arrow()
endif()
+# Check the Build mode on Windows
+if(WIN32)
+ string(TOUPPER "${CMAKE_BUILD_TYPE}" CMAKE_BUILD_TYPE_UPPER)
+ if(CMAKE_BUILD_TYPE_UPPER STREQUAL DEBUG)
+ message(FATAL_ERROR "You can't build this with Debug because MATLAB is built with Release. You must use Release")
Review Comment:
```suggestion
message(FATAL_ERROR "Building the MATLAB interface in Debug mode is not supported.")
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #39595:
URL: https://github.com/apache/arrow/pull/39595#discussion_r1451678854
##########
matlab/CMakeLists.txt:
##########
@@ -131,6 +131,14 @@ if(NOT Arrow_FOUND)
build_arrow()
endif()
+# Identify MATLAB build mode (Debug or Release)
+if(WIN32)
+ execute_process(COMMAND dumpbin /dependents ${Matlab_MAIN_PROGRAM} OUTPUT_VARIABLE DEPENDENTS)
Review Comment:
@kevingurney Is there any MATLAB main program that is built in Debug mode on Windows?
If all MATLAB main programs are built in Release mode on Windows, we don't need to check this.
We just need to check our build mode instead:
```cmake
if(WIN32)
string(TOUPPER "${CMAKE_BUILD_TYPE}" CMAKE_BUILD_TYPE_UPPER)
if(CMAKE_BUILD_TYPE_UPPER STREQUAL DEBUG)
message(FATAL_ERROR "...")
endif()
endif()
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows [arrow]
Posted by "ShaiviAgarwal2 (via GitHub)" <gi...@apache.org>.
ShaiviAgarwal2 commented on code in PR #39595:
URL: https://github.com/apache/arrow/pull/39595#discussion_r1451997620
##########
matlab/CMakeLists.txt:
##########
@@ -131,6 +131,14 @@ if(NOT Arrow_FOUND)
build_arrow()
endif()
+# Identify MATLAB build mode (Debug or Release)
+if(WIN32)
+ execute_process(COMMAND dumpbin /dependents ${Matlab_MAIN_PROGRAM} OUTPUT_VARIABLE DEPENDENTS)
Review Comment:
> @kevingurney Is there any MATLAB main program that is built in Debug mode on Windows?
>
> If all MATLAB main programs are built in Release mode on Windows, we don't need to check this.
>
> We just need to check our build mode instead:
>
> ```cmake
> if(WIN32)
> string(TOUPPER "${CMAKE_BUILD_TYPE}" CMAKE_BUILD_TYPE_UPPER)
> if(CMAKE_BUILD_TYPE_UPPER STREQUAL DEBUG)
> message(FATAL_ERROR "...")
> endif()
> endif()
> ```
@kou Should I start working upon this change?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows [arrow]
Posted by "ShaiviAgarwal2 (via GitHub)" <gi...@apache.org>.
ShaiviAgarwal2 commented on PR #39595:
URL: https://github.com/apache/arrow/pull/39595#issuecomment-1910582936
> @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
could you please guide me hoe to solve it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #39595:
URL: https://github.com/apache/arrow/pull/39595#discussion_r1452014300
##########
matlab/CMakeLists.txt:
##########
@@ -131,6 +131,14 @@ if(NOT Arrow_FOUND)
build_arrow()
endif()
+# Identify MATLAB build mode (Debug or Release)
+if(WIN32)
+ execute_process(COMMAND dumpbin /dependents ${Matlab_MAIN_PROGRAM} OUTPUT_VARIABLE DEPENDENTS)
Review Comment:
Can we wait for a response from @kevingurney (or @sgilmore10 / @leihou6116 may know)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #39595:
URL: https://github.com/apache/arrow/pull/39595#issuecomment-1890937560
`archery lint --cmake-format --fix` will fix it.
See also: https://arrow.apache.org/docs/developers/cpp/development.html#code-style-linting-and-ci
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #39595:
URL: https://github.com/apache/arrow/pull/39595#issuecomment-1890866226
:warning: GitHub issue #35239 **has been automatically assigned in GitHub** to PR creator.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows [arrow]
Posted by "ShaiviAgarwal2 (via GitHub)" <gi...@apache.org>.
ShaiviAgarwal2 commented on code in PR #39595:
URL: https://github.com/apache/arrow/pull/39595#discussion_r1451718110
##########
matlab/CMakeLists.txt:
##########
@@ -131,6 +131,14 @@ if(NOT Arrow_FOUND)
build_arrow()
endif()
+# Identify MATLAB build mode (Debug or Release)
+if(WIN32)
+ execute_process(COMMAND dumpbin /dependents ${Matlab_MAIN_PROGRAM} OUTPUT_VARIABLE DEPENDENTS)
Review Comment:
Should I resolve the changes?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows [arrow]
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on PR #39595:
URL: https://github.com/apache/arrow/pull/39595#issuecomment-2992515107
Closing this PR as it appears to have gone stale.
@ShaiviAgarwal2 if you would like to continue working on this, please feel free to open a follow up PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows [arrow]
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on code in PR #39595:
URL: https://github.com/apache/arrow/pull/39595#discussion_r1453680689
##########
matlab/CMakeLists.txt:
##########
@@ -131,6 +131,14 @@ if(NOT Arrow_FOUND)
build_arrow()
endif()
+# Identify MATLAB build mode (Debug or Release)
+if(WIN32)
+ execute_process(COMMAND dumpbin /dependents ${Matlab_MAIN_PROGRAM} OUTPUT_VARIABLE DEPENDENTS)
Review Comment:
@kou and @ShaiviAgarwal2 - sorry for delay!
To answer your question: all publicly released versions of MATLAB are built in release mode (i.e. linked against a release version of the MSVC Runtime Libraries).
I agree with @kou's suggestion that addressing this issue should not require looking at the dependencies of `Matlab_MAIN_PROGRAM`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows [arrow]
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on PR #39595:
URL: https://github.com/apache/arrow/pull/39595#issuecomment-2032266144
@ShaiviAgarwal2 - please refer to my previous comment about using generator expressions:
https://github.com/apache/arrow/pull/39595#issuecomment-1910891024
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows [arrow]
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on PR #39595:
URL: https://github.com/apache/arrow/pull/39595#issuecomment-1908434294
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows [arrow]
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on PR #39595:
URL: https://github.com/apache/arrow/pull/39595#issuecomment-1910891024
@ShaiviAgarwal2 - I believe what is happening here is a result of the fact that Visual Studio is a [Multi Configuration Generator](https://cmake.org/cmake/help/latest/prop_gbl/GENERATOR_IS_MULTI_CONFIG.html) (as opposed to a Single Configuration Generator like Make). Multi Configuration Generators ignore the value of [`CMAKE_BUILD_TYPE`](https://cmake.org/cmake/help/latest/variable/CMAKE_BUILD_TYPE.html#variable:CMAKE_BUILD_TYPE). As a side effect, it appears to be defaulting to the value `Debug`, despite the fact that we are [explicitly building in Release mode in CI via `matlab_build.sh` (`--config Release`)](https://github.com/apache/arrow/blob/9c0a7617d953a74abaec5c14318bc2f46d0d176a/ci/scripts/matlab_build.sh#L34).
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 `Debug` build is being performed with Visual Studio, rather than using a conditional, config-time check based on the value `CMAKE_BUILD_TYPE`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-35239: [MATLAB] Report error when building the MATLAB Interface to Arrow in Debug mode on Windows [arrow]
Posted by "ShaiviAgarwal2 (via GitHub)" <gi...@apache.org>.
ShaiviAgarwal2 commented on PR #39595:
URL: https://github.com/apache/arrow/pull/39595#issuecomment-1890931179
@kou Could you please tell me how to clear `Dev / Lint C++, Python, R, Docker, RAT (pull_request)` this check !!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org