tpaviot / pythonocc-contrib

Miscanellous pythonocc contributions and experiments. Anayone can request for an write access.
16 stars 11 forks source link

Non-standard installation path of OCE-Config.cmake files in conda packages (unix) #10

Open rainman110 opened 8 years ago

rainman110 commented 8 years ago

The current conda scripts install the OCE Config package files into $PREFIX/lib. This is non-standard behaviour. This way, find_package(OCE) does not find OCE without an explicit path.

It would be better to install the OCE config files into the standard cmake paths, i.e.:

These paths are automatically selected by the OCE build system, if we do not set the variables

I suggest to simply remove these statements from build.sh. Then everything should be standard conform.

tpaviot commented 8 years ago

Ok Le 9 déc. 2015 22:16, "Martin Siggel" notifications@github.com a écrit :

The current conda scripts install the OCE Config package files into $PREFIX/lib. This is non-standard behaviour. This way, find_package(OCE) does not find OCE without an explicit path.

It would be better to install the OCE config files into the standard cmake paths, i.e.:

  • $PREFIX/lib/oce-0.16 (linux), or
  • $PREFIX/lib/cmake/oce (linux)
  • $PREFIX/OCE.Framework (mac)

These paths are automatically selected by the OCE build system, if we do not set the variables

  • OCE_INSTALL_CMAKE_DATA_DIR
  • OCE_INSTALL_PACKAGE_LIB_DIR

I suggest to simply remove these statements from build.sh. Then everything should be standard conform.

— Reply to this email directly or view it on GitHub https://github.com/tpaviot/pythonocc-contrib/issues/10.

jf--- commented 8 years ago

hi @rainman110 ,

Good point. I'm not sure whether the OCE.Framework is such a good idea on osx, I'd rather use the approach suggested for linux. Note that most large packages ( vtk, qt, boost ), dump their libraries in $PREFIX/lib.

When packaging your application, this does make life a little easier: an application is a specific conda environment, and all linked libs can be found in $PREFIX/lib. This makes packaging a lot easier. So, I'm a fan of the "flat directory" approach ;)

So, in conda speak, lets think of osx & linux -> unix for the moment, and not differentiate between the platforms. Is there a reason we shouldnt package windows libs in the suggested $PREFIX/lib/oce-0.16 ? This way we have a consistent location for all platforms :+1: . Lets not have platform specific locations if this can be reasonable avoided.

So, in my early conda recipe versions, indeed, I've been setting way too many variables! So thanks so much for correcting those in recent PR's and all for getting rid of

Cool, good catch @rainman110 , thanks again!

rainman110 commented 8 years ago

@jf--- Yes, you are right. We should handle all systems equally. Also, I did not mean to place the libraries outside $PREFIX/lib, I am just talkting about the OCEConfig.cmake scripts. Off course, all libraries (qt, boost, oce) should be placed in lib.

I figured out, that OCE_INSTALL_PACKAGE_LIB_DIR has nothing to do with the location of the cmake config files. Though, I am not sure, what it actually does (something with the RPATH handling)

I agree, that we should set OCE_INSTALL_CMAKE_DATA_DIR on ALL systems to $PREFIX/lib/oce. Adding also the version number to the path (i.e. oce-0.16) could be a bit complicated, as we don't have the version number available in the script (off course we could either set it in build.sh or meta.yml). Conda sets the environment variable PKG_VERSION, but this includes the whole version (e.g. 0.16.3dev).

jf--- commented 8 years ago

Cool, I think :-1: for version nr on $PREFIX/lib/oce(-0.16) too... Its clumsy indeed wrt packaging and version is dealt with meta.yaml / conda anyhow. Also, there is no need for it since conda envs are there such that we wont need to mix different versions of OCE anyway ( which is something you should never do )...

Handling RPATH can be tricky indeed, but lets rely on conda's "binary_relocation: true" in meta.yaml, which creates relative paths to your linked lib ( this is really a blessing on osx which can create hardcoded paths to your linked libs... )

Oh, btw, Thomas just merged my PR such that the "install with conda" badge links to your DLR-SC conda channel...

rainman110 commented 8 years ago

Fully agree...

Cool, now DLR is the official distributor :stuck_out_tongue_winking_eye: :airplane: :rocket:

tpaviot commented 8 years ago

The OCE-config.cmake issue should be discussed on the oce project page, there may be some other points of view or suggestions.

Thank you @rainman110 and DLR-SC for your packaging work with conda. Everything that makes pythonocc/oce easier to deploy is welcome in both projects.

jf--- commented 8 years ago

The least we can do for your hard work on making conda happen!

@rainman110 , @tpaviot , would this be the right moment to announce this on the pythonocc mailinglist, such to ask our community for feedback?

I've started to work on getting the ball rolling on TravisCI builds ( TravisCI showing I'm not much of a sys dev... OMG... :persevere: )

I think for conda to become to endorsed way of distributing pythonocc, its important that we know we have valid conda packages. I like the idea that the distributing binaries is a side-effect of a succesfull CI run on /master.

So thinking out loud on how this might pan out in the near future:

Curious to hear what you guys think...

jf--- commented 8 years ago

@tpaviot , the conda builds have not been advertised on the OCE mailing list. Is now a good moment?

tpaviot commented 8 years ago

The pythonocc-core repository is in a "0.16.3-dev" state for a while. It's worth switching up to the 0.16.3 release and then announce, at the same time, the conda feature.

For OCE, yes @jf--- it's a good moment, since it works ;)

jf--- commented 8 years ago

Alright, sounds like a plan. I'll get moving on an smesh conda recipe as well...

rainman110 commented 8 years ago

@tpaviot The default placement of the OCEConfig.cmake files is fine. I don't see any reason to change this. I think it is more or less conforming with the cmake standards. If you ask me, I would not change anything in OCE. This discussion here is more about were we put these files in the conda builds, as they weren't placed at the default position.

@jf--- + @tpaviot :+1: for CI builds. I also would add the git-sha to the version. Conda sets the environment variable GIT_DESCRIBE_HASH to query this. There are still the problems with conda+git on windows though.

One thought to the versioning. I am not sure, how conda detects new packages. If the version number is evaluated, the git sha might be problematic as it is not generally an increasing number. We might need an automatically increasing build counter.

jf--- commented 8 years ago

Yep, the hash is a much more authorative reference than some version nr.

@rainman110 , so the this is what python-versioneer handles well. If we'll start using CI builds for CD ( cont. delivery ) will need this for sure :sunglasses: