search

JeffersonLab / analyzer

HallA C++ Analyzer
BSD 3-Clause "New" or "Revised" License
7 stars 54 forks source link

Added CMake build #159

Closed whit2333 closed 6 years ago

whit2333 commented 6 years ago

Added a cmake build next to other builds. Hopefully this catches on with the new students. I think we should move to using cmake because it is most common and will teach students how to do things professionally from the start.

hansenjo commented 6 years ago

Hi Whit, thanks for the contribution. It doesn't work right out of the box though - the configuration stops immediately with

Could not find a package configuration file provided by "EVIO" with any of the following names: EVIOConfig.cmake evio-config.cmake

EVIO does not come with a CMake configuration file. Could you add your CMake module for finding it in this pull request? Our SCons build includes a FindEVIO function, so does the Make system. It expects either CODA or some EVIO_* environment variables to be set.

Ole

whit2333 commented 6 years ago

I have added already add cmake for evio here https://github.com/JeffersonLab/hallac_evio/pull/2. Does evio have a github repo?

If you build evio via https://github.com/whit2333/hallac_evio you shouldn't have a problem.

Also if you want to see how to use the analyzer (eg libHallA) in another project, checkout https://github.com/whit2333/hcana

sawjlab commented 6 years ago

I added the Cmake changes to hallac_evio so that CMake build systems for analyzer and hcana can be tested.

hansenjo commented 6 years ago

Actually, I'm not sure that us patching EVIO is the best idea. Additionally I see a few more issues:

My rules for Podd:

So out of the box, I'm inclined to defer this contribution because it does not respect the project rules. I'm sure this can be remedied easily.

Ole

whit2333 commented 6 years ago

Hi Ole,

Actually, I'm not sure that us patching EVIO is the best idea.

Why not? It changes nothing if you don't use cmake.

  • SCons finds EVIO via $CODA, but CMake ignores $CODA and expects a patched EVIO. The build systems should behave consistently.

Environment variables are bad. This is an old-school way to find libraries. That said, it could be trivially added.

  • Whit's CMake expects EVIO to be installed in Whit's way under a specific prefix, which also needs to match the Podd/hcana install prefix. This will not work with our current EVIO installs.

I want to emphasize the separation of the source/build/install. Where you install is up to you but the default should be /usr/local like it is globally.

  • Whit's CMake requires a bunch of dependencies that Podd/hcana don't have. Also it requires C++17. Seriously?

That was a test to see what compiler you are using! :) This should be c++11. (ideally c++14).

  • Must work on RHEL7 without non-base repo installs except ROOT and EVIO. Additional dependencies (e.g boost) would have to be agreed on in advance.

  • Must not require modifications of any dependencies. For instance, we don't patch ROOT. Vanilla installs of deps must work.

So out of the box, I'm inclined to defer this contribution because it does not respect the project rules. I'm sure this can be remedied easily.

It doesn't break any of these (except c++17). In fact, if you don't use cmake you shouldn't notice a thing.

BTW ROOT 5 is no longer supported and shouldn't be used.

-- Whitney R. Armstrong

hansenjo commented 6 years ago

Well, look - if you make a contribution somewhere, you don't want to end up telling the project's developers how to do things. What would be the point for us of having a CMake build system in our official distribution that doesn't work smoothly in our environment. Such a contribution is better kept in your own personal repository then.

Ole

whit2333 commented 6 years ago

It breaks nothing and adds extra functionality. I don't see why you should object.

hansenjo commented 6 years ago

Closed with commit aa4c6a8