Closed mcaceresb closed 3 years ago
Thanks @mcaceresb!
These issues may possibly be a legacy of the fact that I wrote the first version of the matlab builder myself. :-)
It may also be related to the bug flagged in gslab_python/#166
From your description it sounds like a good idea to fix this. I suggest that @veli-m-andirin reviews the snippet and plays around with the functionality, and after @veli-m-andirin reviews I can incorporate it into my broader review of the Template.
Thanks for checking.
Thanks @mcaceresb! Happy to review the snippet and check the functionality.
Continued in #21.
@jmshapir I had a look here and that is exactly the second issue I highlighted, so it would be fixed by #21.
For this issue, we fixed the problems with the Matlab builder outlined here. The second seemed the most pressing, and is the same issue that had been pointed out gslab_python/#166.
Handling the log was more nuanced than expected. On *nix
, the old method did save a log file because Matlab was run from the console, so everything was printed to stdout
, but not so on Windows. We now save to the log using Matlab's diary
function and we redirect stdout
to a dummy log that is deleted after the program runs (if we don't redirect stdout
then the log would be printed twice on *nix
).
The Matlab program is run inside a try, catch block that exits with status 1 on error and 0 on no error. Hence errors do not cause Matlab to hang.
Before the program runs, the program's directory gets added to Matlab's path via the addpath
function.
During the PR we made some additional changes not specific to Matlab:
When there is an error, the exception is raised outside the try, except block. It used to be raised inside, causing Python to complain about encountering an exception while handling an exception.
The log file prints the start and end timestamps even when there is an error.
A cleanup
method was implemented to run code even if there is an error. This is how Matlab deletes the intermediate source and log files it creates at the root of the directory, but it can be implemented by any builder.
@jmshapir Now that #12 is merged we have a working repo. While I wait for @veli-m-andirin's feedback on #10, I wanted to highlight some problems with the
matlab
builder.First, it doesn't seem to save anything to the log file.
Second, matlab doesn't have a "batch mode" so if there is an error (or if a user forgets to put an
exit
in their.m
file) matlab will not exit.Third, we copied the gslab builder, which makes a copy of the source file into the repository root and runs the copy. This means that matlab auxiliary programs, typically saved in the same directory as the source, are no longer available (since matlab is running from the repo's root).
I had already coded a fix for all these issues (it's a 15-line snippet) but I didn't want to delay #12. It's a matter of whether you think it's a good idea (the only drawback is it adds complexity to the matlab call; the pro is that it would solve the issues above). If you think it makes sense, I can ask @veli-m-andirin to review the snippet.