Fixes to build MATLAB toolboxes on CI #3521
Conversation
| @@ -25,7 +25,9 @@ jobs: | |||
| use_matlab: true | |||
|
|
|||
| - name: Build MATLAB ToolBox | |||
| run: make -C matlab toolbox | |||
| run: | | |||
| make -C cpp srcs | |||
There was a problem hiding this comment.
We were missing the C++ dependencies here
| @@ -130,8 +130,6 @@ jobs: | |||
| with: | |||
| working_directory: ${{ matrix.working_directory || '.' }} | |||
| flags: ${{ matrix.test_flags }} | |||
| # Don't test matlab on Windows (see setup-dependencies/action.yml) | |||
| if: matrix.config != 'matlab' || runner.os != 'Windows' | |||
There was a problem hiding this comment.
MATLAB For Windows should work with the fixes to Windows builds.
| @@ -115,9 +115,15 @@ $(icetoolbox_file):: $(icethunk_target) $(slice2matlab_path) $(lang_srcdir)/lib/ | |||
| cp -rf $(cpp_bindir)/slice2matlab $(lang_srcdir)/toolbox/build/ | |||
| # Doc files | |||
| cp -rf $(lang_srcdir)/toolbox/doc $(lang_srcdir)/toolbox/build | |||
| ifneq (,$(MATLAB_COMMAND)) | |||
There was a problem hiding this comment.
MATLAB_COMMAND is set by CI to the executable that we need to run. We were already using this in the tests, but there were a few additional places where we still used matlab directly.
| @@ -6,36 +6,30 @@ | |||
| <Configuration Condition="'$(Configuration)' == ''">Release</Configuration> | |||
| <RootDir>$([System.IO.Path]::GetFullPath( $(MSBuildThisFileDirectory)..\msbuild ))</RootDir> | |||
| <ToolboxDir>$(MSBuildThisFileDirectory)..\toolbox\build</ToolboxDir> | |||
There was a problem hiding this comment.
This MSBuild project was trying to guess the version and the matlab home using the matlab executable from path. It was far too complicated.
I simplified to use a default and allow override with env variables. It was the usage of matlab here what was causing the CI issues.
| <MatlabVersion Condition="'$(MatlabVersion)' == ''">R2024a</MatlabVersion> | ||
| <MatlabHome Condition="'$(MatlabHome)' == ''">C:\Program Files\MATLAB\$(MatlabVersion)</MatlabHome> | ||
| <!-- Override with the env settings --> | ||
| <MatlabVersion Condition="'$(MATLAB_VERSION)' != ''">$(MATLAB_VERSION)</MatlabVersion> |
There was a problem hiding this comment.
Should this go up on line 10?
Presumably if you set MATLAB_VERSION but not MATLAB_HOME, you want MatlabHome to be set correctly.
This PR include fixes to build MATLAB toolboxes for Windows and Linux on CI.