nexusformat / code

NeXus API code and helper applications
GNU Lesser General Public License v2.1
12 stars 16 forks source link

C++ Bindings: NeXusException.hpp dependent on NeXusFile.hpp #457

Closed martyngigg closed 6 years ago

martyngigg commented 6 years ago

To be able to include NeXusException.hpp one must first include NeXusFile.hpp to ensure NX_DLL is defined. This has recently caused us an issue when moving our code formatting to use clang-format v >=3.7, which sorts the include files so that they are alphabetical leading to a broken build.

The current workaround is to turn of clang-format for this section of code:


// clang-format off
#include <nexus/NeXusFile.hpp>
#include <nexus/NeXusException.hpp>
// clang-format on
martyngigg commented 6 years ago

It would appear that this was fixed in #433 but only for the cmake build. The debian build is still using configure so I would assume that it is now broken? Is configure now unsupported?

martyngigg commented 6 years ago

I hadn't spotted that the latest HEAD makes no mention of autotools. I'll close this as it's not longer relevant.

zjttoefs commented 6 years ago

That leaves the Debian packages in some state. We had a volunteer that wanted to make changes to the packet structure and take over maintaining in winter 2015/2016, but he never took action. Instead it got adopted by the Debian science team. They have so far only been updating the build scripts, they underlying code is more than 4 years old. Now the package has been removed from Debian testing and will drop out of support in Debian (and I guess Ubuntu) with the next release.

martyngigg commented 6 years ago

I see. This will certainly cause Mantid some issues.

Is CMake now the only supported build method? As debhelper already has CMake support I would have thought that transitioning debian/rules would not be too difficult. Do you think opening a PR to https://salsa.debian.org/science-team/nexus would help move things along?

zjttoefs commented 6 years ago

Unfortunately the code has been refactored so that python-nxs needs to be build from a different source, which requires extra vetting in Debian. That's why we needed to volunteer for it. The debian science team probably has an easier way of doing that than an outsider. A PR there would certainly help.

And yes, CMake only now.