search

JeffersonLab / analyzer

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

Experimental CMAKE disaster. #164

Closed whit2333 closed 6 years ago

whit2333 commented 6 years ago

The current cmake build is bad for a few reasons. Why not just the base I provided and fix with subsequent issues?

  1. All the dependencies should be in the main CMakeLists.txt
  2. You should not install the cmake/Modules (that is just for the current projects build
  3. You don't have a PoddConfigTargets.cmake auto generated.
  4. Installing the headers into include/podd makes for a cleaner interface (and doesn't pollute the PREFIX/include
  5. If you just merged the hallac_evio cmake build you wouldn't need all the junk added in the cmake directory
  6. EVIO isn't found even though it is installed.
  7. I have an example where EVIO is built automatically : https://github.com/whit2333/analyzer/tree/testing
hansenjo commented 6 years ago

I appreciate that you took the time to test this contribution so quickly. Unfortunately, use of flamebait language will not help your reputation with us; in fact, I'd like to ask you expressly to moderate your tone.

Your list of issues seems to address largely matters of style, which I find not productive. You asked us to implement a CMake system, we did it, and it demonstrably works. That's how it is going to stay. Our project, our style, our rules. And if you really would like to know, my private buglist for your original contribution exceeded yours here in length and substance by about a score, so it does seem we're making progress overall.

I don't understand 3. PoddTargets*.cmake files are installed. They are used by PoddConfig.cmake, which is also installed.

As for 6, I assure you that EVIO is found if you set the environment variable $CODA (or $EVIO) to the top of the installation as provided and recommended by the JLab DAQ group. For example, setenv CODA /site/coda/3.05 on ifarm will do it. As already explained to you before, one of our project's rules is to not require modifications of any dependencies that we do not maintain. Hence, we are not going to support a modified EVIO unless absolutely unavoidable.

Here's a suggestion: provide specific, reproducible examples of something that does not work for you, and we'll implement a fix if reasonable.

whit2333 commented 6 years ago

Here's a suggestion: provide specific, reproducible examples of something that does not work for you, and we'll implement a fix if reasonable.

I was hoping you would do this on top of my contributions (since a cmake build did not exist to begin with), but instead you felt the need to redo everything and breaking my code in the process.