search

kleinerm / Psychtoolbox-3

This is kleinerm's git repository for development of Psychtoolbox-3. Regular end users should stay away from it, unless instructed by him otherwise, and use the official Psychtoolbox-3 GitHub page or distribution system for production releases.
104 stars 304 forks source link

using $HOME causes errors in MATLAB compiler #219

Closed iandol closed 2 years ago

iandol commented 2 years ago

For some reason unix('echo $HOME') causes compilation errors when code is compiled with MATLAB compiler (at least on my macOS laptop). getenv('HOME') does work and so using the command isdeployed can switch between the previous code and getenv. I'm not sure why this code captures a shell command as getenv has been around for ages and is also supported by Octave: https://octave.org/doc/v6.4.0/Environment-Variables.html#Environment-Variables — another option would be to just use getenv whether deployed or not...

kleinerm commented 2 years ago

Weird, but maybe some meaningful restriction behind that one.

Unconditional use of getenv() should be fine, simplifying your commit further. Question though if the HOME environment variable is defined in deployed applications?

Your commit messages also need to be more descriptive, e.g., starting with the name of the function you fixed/modified if this is a single function change. A la: "PsychtoolboxConfigDir(): Use getenv() for find the home instead of ..." or something like that. Your current commit message gives no idea what your commit does or what it changes if one just looks at the one-line summary of each commit in the commit history.

iandol commented 2 years ago

Yes $HOME works as expected via getenv from a deployed app without issue, I'm not entirely sure why using the unx/system command and echo fails but it may be the sh environment is somehow not set up properly in this case. I'm happy to simplify this to use getenv (which should be more efficient anyway). Regarding the commit message, I'll create a new pull with a more descriptive name, so I'll close this and open a new one...