Skip to content
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

Build should include install option #59

Closed
tranqui opened this issue Apr 6, 2018 · 6 comments
Closed

Build should include install option #59

tranqui opened this issue Apr 6, 2018 · 6 comments
Assignees
Labels
documentation feature Brand new functionality

Comments

@tranqui
Copy link

tranqui commented Apr 6, 2018

@FTurci comment in #50 thread:

Other remark: "make" should be followed by "make install" and we should not have plenty of executables copied everywhere. "cmake" has also a prefix option. This was a source of confusion when we tried to identify the issue.

@merrygoat
Copy link
Contributor

merrygoat commented Apr 8, 2018

My bad on copying the executable around, I will fix the integration tests to not do this.

@tranqui tranqui added documentation feature Brand new functionality labels Apr 17, 2018
@merrygoat merrygoat added this to the Release Version 1.0 milestone Apr 18, 2018
@merrygoat
Copy link
Contributor

There are 3 stages to the make process. "cmake" should prepare the makefile, "make .." should actaully compile the file and then "make install" should do post compilation things. There are a couple of requests here:

  • Provide option to "cmake" to set directory the binary is copied to by adding a parameter on compile
  • Make more clear the default directory if a custom option is not used
  • Add the lib directory to PYTHONPATH so the wrapper can be called directly
  • Document all of this in the readme file

@merrygoat merrygoat assigned merrygoat and unassigned tranqui Apr 26, 2018
@merrygoat
Copy link
Contributor

@tranqui Three out of the above 4 bullet points have been done in the latest build but I don't think I can edit PYTHONPATH from a cmake file in a platform independent way - correct me if I'm wrong.

merrygoat added a commit that referenced this issue Apr 28, 2018
@tranqui
Copy link
Author

tranqui commented Apr 28, 2018

That’s fantastic Peter :)

Is it possible to move the library into the system python path then, so PYTHONPATH remains the same but it can still find the library?

@merrygoat
Copy link
Contributor

No because python path is not just a directory, it is a series of directories that are searched - if you haven't used use python much it may well be that python path is not defined.

Can we just do it by adding current directory to sys.path in any Python scripts we provide? (as we have done in the unit tests). If users are writing their own scripts then they are probably sufficiently savvy to set PYTHONPATH. We can add a note to this effect in the readme too.

@tranqui
Copy link
Author

tranqui commented Apr 30, 2018

Okay then, that seems like a reasonable compromise. I can write something in the readme for *NIX users if you like

@tranqui tranqui closed this as completed May 2, 2018
tranqui pushed a commit that referenced this issue May 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation feature Brand new functionality
Projects
None yet
Development

No branches or pull requests

2 participants