OpenWaterAnalytics / EPANET

The Water Distribution System Hydraulic and Water Quality Analysis Toolkit
MIT License
282 stars 206 forks source link

Implement an "Install" target for the CMake build scripts. #638

Open boinst opened 3 years ago

boinst commented 3 years ago

Hello. I am hoping to add the EPANET library to VCPKG build system. One issue that I have come up against is that the EPANET CMakes scripts to not provide an "install" target. If I were to make a pull request implementing such a target, would that be a welcome contribution?

I have a relevant open ticket in the VCPKG project.

LRossman commented 3 years ago

@boinst could you be more specific on how you want to modify the CMake scripts and if that would change the existing build process described in BUILDING.md. Are you sure that the highly domain-specific EPANET fits in with the other more general purpose programming libraries contained in VCPKG? Seems like it would be a fish out of water. Can you cite any other domain-specific libraries included in VCPKG -- I didn't see any from the list provided on their web page.

boinst commented 3 years ago

Thank you @LRossman for your reply. Long term fan by the way, I've been working with EPANET and EPASWMM for most of my career.

It is true that EPANET is domain specific. That doesn't seem to be a barrier for vcpkg support, however. There are 1600 packages, and just looking through the list, there are heaps of projects with little activity, and many that have been explicitly abandoned by the maintainers. The first one on the list, 3fd, has only a handful of users. antlr4, bcg729, basisu seem pretty niche, and I stopped reading when I got to "boost".

In answer to the "be more specific", I think all I have to do is add a call to the cmake function install.

The code from the 7-zip library looks like the below, I expect it would be something vaguely similar. It's just a matter of telling CMake which files constitute the "output" from the build.

install(
    TARGETS 7zip
    EXPORT 7zip
    ARCHIVE DESTINATION lib
    LIBRARY DESTINATION lib
    RUNTIME DESTINATION bin
)

# Headers makes relative includes so directory structure must be kept
foreach(HEADER ${PUBLIC_HEADERS})
    get_filename_component(HEADER_DIR ${HEADER} DIRECTORY)
    install(
        FILES ${HEADER}
        CONFIGURATIONS Release
        DESTINATION include/7zip/${HEADER_DIR}
    )
endforeach()

install(
    EXPORT 7zip
    DESTINATION share/7zip
    FILE 7zipConfig.cmake
    NAMESPACE 7zip::
    CONFIGURATIONS Release
)

You would not need to change readme.md. The existing build command cmake --build . --config Release would still work as described. The addition of an install target only gives the user the option of using it, they don't necessarily need to.

CMAKE_INSTALL_PREFIX=/usr/local/epanet
cmake --build . --config Release --target install

If you think the change would be welcome, I will take a stab at the implementation, and make a draft pull request for you to consider.

LRossman commented 3 years ago

@boinst thanks for your response. I poked around the list of VCPKG packages some more and found that NGSPICE (an open source version of SPICE) was included. SPICE is to electronic circuit analysis as EPANET is to pipe network hydraulics. So that answers my question concerning domain-specific libraries. Therefore I have no issues with you modifying the CMake scripts to make OWA EPANET work with VCPKG.

P.S. I installed VCPKG on my machine to see how it works. The result is over 26,500 files installed, including separate folders for each of the 1,675 libraries it currently ports. Seems like a lot of overhead if you just want to install one specific library.

boinst commented 3 years ago

Thank you @LRossman I will work on a pull request.

I agree, it's a lot of overhead. But, if you use a lot of third party libraries (I happen to use a whole bunch), it can save juggling a significant volume of build orchestration logic.