-
Notifications
You must be signed in to change notification settings - Fork 97
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
Streamline build process to use a single make and config file #144
Conversation
LGTM. You should be able to delete |
|
||
if __name__ == '__main__': | ||
usage = 'Please specify a valid make target (Matlab or Octave)' | ||
if len(sys.argv) < 1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be 2, not 1
else: | ||
mex = matlab_bin + "/mex" | ||
paths = "-L%(zmq_lib)s -I%(zmq_inc)s" % cfg | ||
make_cmd = '"%s" -O %s -lzmq ./src/messenger.c' % (mex, paths) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the quotes around the executable I get an OSError: [Errno 2] No such file or directory
on my mac.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, looking at the original version it looks like those quotes were there. But yeah I got the same error on Linux with Matlab (must be a windows-only thing).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably just os.system
has different semantics than check_output
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, you reminded me to use shlex.split
, which will handle spaces in paths.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And yes, that does make sense, since we are passing a list of strings.
try: | ||
os.remove('messenger.o') | ||
except OSError: | ||
os.remove('src/messenger.o') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still get an error here when building for matlab. I ran the mex command manually and it looks like I don't get a .o
file anywhere, actually.
Streamline build process to use a single make and config file
No description provided.