XDI: C library autotools build system modernized #55
Conversation
* now uses libtool to generate the shared/static library * no need for separate platform-dependent build scripts * pkg-config file added * only the essential files for generating a development environment should be tracked by git. All others can be generated with autoreconf (which replaces the reconf script)
|
@tschoonj that's a lot of changes, and sort of assumes that using the full stack of autotools is desirable. To date we have preferred "simplest possible build scripts that can work". My own experience is that maintaining autotools scripts can be more work than maintaining the actual code. XDI isn't moving very fast (by design!) and is pretty basic code without a lot of dependencies. We could probably get away from using anything from autotools and just use Makefiles. I wouldn't necessarily advocate for that, but it might be the simplest and most stable approach. What advantage do these tools have for XDI now and for the next ten years? What additional maintenance do these add to this relatively simple code base? I think we want to be careful about adding unneeded dependencies. That's not to say that I couldn't be persuaded that making these changes is a good idea, but I'm not sure I see a problem that they solve. Deleting the very simple Windows and Mac build scripts seems unnecessary to me, unless they are broken. Basically: What is the motivation for this change? |
|
Hi Matt, The only new dependency (for developers of XDI, not for users of the package) is libtool, which is a very common one. Nowadays no one would install autoconf and automake without libtool too. The advantage of libtool is that it solves the mess that is building shared libraries on different platforms, without having to resort to build scripts. So running configure, make, make install will produce the shared and static libraries with correct extension and comes with library versioning support as a bonus. The reason I created this PR that I was interested in your work, cloned it, and tried to build it. However autoreconf complained because every macro except the AC_PROG_CC, AC_PROG_CPP and AC_PROG_RANLIB was used either incorrectly or was obsolete. I think one would need an autotools installation that's at least 10 years old to process the configure.ac file without warnings and errors. The pkg-config file is quite handy as it allows users to easily get the necessary compiler and linker flags for developing software against the library. In my experience maintaining buildscripts is always a pain: at some point you always end up making modifications to support features that come with new releases of operating systems and compilers, as well as to deal with problems related with the build tools themselves (CMake is quite famous for this). |
|
FWIW, the libtools chain worked fine on my Ubuntu 15.04 machine. @tschoonj, what kinds of systems have your used to successfully build libxdi using your tool chain? I am thrilled that you are looking into using XDI in DAWN. That's great! My preference would be to see languages wrap libxdifile rather than reimplement it. While XDI is not complicated and an XDI implementation would be easier (shorter, more robust, ...) in almost any language other than C, the advantage of wrapping a C implementation is that, then, everyone reads an XDI file in the same way. Hopefully, then, the implementation follows the spec rather than having the spec keep up with a proliferation of implementations. |
My point is not how common it is but that it is a dependency that does complicate the maintenance. We want XDI to work for twenty years. So adding any required tools needs to be done carefully. That's not to say that it should not be done.
How many different platforms have you used your new build tools on? Which compilers and OS versions have you used?
See, my view is that "autotools scripts from ten years ago are now out of date" suggests to me that a) the new versions probably won't work on older systems, and b) the current autotools scripts are likely to be out of date in another ten years. Both of those are problems we should avoid. I think my inclination would be to ask whether we actually need autotools at all. What does XDI need to have configured? I think it's just the location of the standard library include files. I think it doesn't really depend on anything else.
I agree completely. Do we need these? |
|
Hello again, I have tested the build on Linux (RHEL 6) and it works fine. This morning I gave it a try on Mac OS X and it compiled fine but the xdi_reader program segfaulted when I tried it with the cu_metal_rt.xdi file. It turns out that there is a bug in the library which I discovered using valgrind on Linux (where it doesn't lead to the segfault but it's there nevertheless). The reason why you didn't see it with your valgrind Perl script is that by default it only checks for memory violations caused by dynamic allocations, not using static arrays where your code depends on heavily. I ran it as: I wouldn't be surprised if it also caused a segfault on Windows. I will look into fixing this. Now back to the comments raised by @newville I apologize if I wasn't clear before about libtool. Libtool is a GNU package that is an integral part of the GNU autotools buildchain. It is highly unlikely that anyone would have a system with autoconf and automake but not libtool. Furthermore, it is only a dependency that matters for the developers of libxdi and does not impact the end-user at all. A release of a GNU autotools based software package is done using the following steps: In this case one would end up with a file called The good thing is that the tarball is backwards compatible and will work on older systems. It is true that in time there will be changes to the macros upstream that eventually will cause @bruceravel I agree that a core C library with language wrappers is the ideal way to go (I use this approach myself for xraylib). However my colleagues seem to prefer relying as much as possible on pure Java implementations as they found it hard dealing with the platform dependent shared libraries as well as the possibility of segfaults in libraries which would make the entire software crash. I will run it by them and see what they think. |
|
Bug is fixed in 972fc6b |
|
I can now confirm that the library works on Mac OS X (El Capitan with clang) and on Windows 7 (64-bit with gcc 5.1.0 64-bit). However there is a problem on Windows: xdi_reader only works if the processed file has Unix line endings. I did some digging in the code but I didn't find the underlying cause. Interestingly I can read the testfile with dos line endings on Linux without problems. |
|
@tschoonj: I did not know about the exp-sgcheck tool in Valgrind. Cool! Thanks for teaching me about that! As for the Java implementation, perhaps I could say a few more words. A few years back, when we first started thinking about a format for XAS data interchange, one of the proposals was based on CIF. It wasn't a very popular choice -- the most common complaint being that CIF is just kind of weird. Weirdness, for me, wasn't the real issue. The real issue is that the folks who developed the CIF standard never actually made an implementation of the standard. As a result, subsets of CIF have been implemented over and over again in various ways, in various languages, of various levels of completeness, and of various levels of quality. Even though XDI is not very complicated, its complicated enough that this same thing could happen. Enforcing a policy of "language wraps the C library" certainly does not prevent different language implementations from doing different things, but it does provide guidance to the implementer in a way that encourages consistency. That said, I doubt that I would advocate the rejection of a PR that includes a native implementation in Java or any other language, but it would make me 😦 As for the argument about segfaults in the underlying library, unit testing and knowledgeable contribution such as your comments above go a long way towards addressing that concern. |
|
@newville: As for a deciding about build systems, I am inclined to accept Tom's PR, although it would require some documentation, which he didn't provide. Should we choose to accept the PR, I'll take care of that. @tschoonj: Could you say a few more words about "Windows 7 (64-bit with gcc 5.1.0 64-bit)". Is that with cygwin, mingw, something else? As for the DOS line endings issue, I'll look into that. Thanks. |
|
I would gladly write any documentation you need. Perhaps more important is that pulling in this PR implies that the users will need access to source code tarballs instead of cloning the repository after you make a release. Github could host them for you or you could host them somewhere else. It is also possible, but this requires a bit more work, to bring the language bindings into the same package. What do you think? For the Windows build I used the GCC compiler offered by TDM-GCC (basically MinGW-w64) on an msys shell. I will discuss the library next week with my colleagues and explain your thoughts on this matter. I will get back to you on this. |
set libtool version to 0:0:0
|
Fixed the Windows parsing bug. Thank you Wikipedia! |
What I'm suggesting is that maybe it's better to not use configure at all. As an example, to build the XDI dlls for Larch (Windows, Mac, Linux), I just used scripts like the ones here: I'm not saying that this approach is necessarily better for the standard XDI build, but I certainly find short makefiles a whole lot simpler to maintain and tweak than using autoconf. And @tschoonj, yes we do understand autoconf, how to build libraries with configure, and the difference between developers and users. If the configure scripts that we started with (probably borrowed from ifeffit and so dating from fifteen years ago) are now broken, the current ones will probably need maintenance over that time scale too. My concern is not that the autoconf tools aren't readily available, but that they add a level of complication, and that this complication might not be needed. Indeed, XDI is a very simple C library with very little dependencies. It can be built for Windows, Mac, and GNU/Linux without autoconf at all. If autoconf has actual benefits, then it might be worth that complication. What are those benefits? If maintaining the XDI library did not require any knowledge of autoconf at all, that could be considered a benefit. If we're aiming for support for next fifteen years, and hope that others can take over maintenance, it might be that using autoconf is not worth the benefit. I'm a little surprised by the build and read errors. I build and use XDI all the time (collecting data at my beamline with it at the moment). I'll try to reproduce these. |
|
In my experience any buildsystem comes with maintenance: whether you use simple, platform dependent scripts or GNU autotools (or alternatives). But both approaches come with a fundamental difference: in the former case you shift the responsability of adapting the script to suit a particular environment onto the end-user who may or may not be very familiar with compiling software from source. The buildscripts you provide are designed to work well on the environments you work with personally or for the people that provided you with these scripts, and will require manual editing as soon as the end-user wants to use a differently named compiler, particular flags or installation directory. The buildsystem on the other hand shifts that responsibility onto the developer: GNU autotools is designed to automatically detect the environment it is running in and set all compilation and linking flags accordingly. The tarballs they produce can also be seamlessly integrated into Linux packages or added to Homebrew for Mac OS X users. I believe that you are also seriously overstating the work on maintaining the buildscripts. We are talking over about 30 lines in configure.ac and Makefile.am. I am confident that it will be trivial for both the current and future developers to bring them back in line with the latest changes in the GNU autotools should the need for that arrive. I stated before that buildscripts are a pain to maintain, but in addition to that they are a necessary evil to ensure that people find it easy to install and use software. |
|
The current state of the build scripts in the XAS-Data-Interchage repository is not so good. The repo would be improved by doing what either of you suggest. @tschoonj: Matt is right. @newville: Tom is right. Since libxdifile is simple,
While I agree that the Makefile in the second directory is easier to understand than the libtools starting files, the "solution" in the first directory should be a non-starter even for you. Requiring python is as onerous as requiring libtools and should be outside the bounds of this discussion. Neither of you are presenting a solution that works outside of a fairly narrow scope. Neither solution will work with the Intel compiler, for example. Neither is solution for what to do with, say, Visual Studio or the Android SDK. Generalizing build scripts is hard. All that said, Tom found two bugs and proposed solutions for both. I'd like to fold those two simple fixes into master. I already pushed some changes to my little Valgrind-based testing script to do the bound checking that Tom suggested. The debate over build systems shouldn't preclude applying those fixes. |
|
I completely agree with all of that, and with everything Tom said too. Of course, I am not always opposed to autoconf, and I definitely do not suggest a python-based solution for XDI. I was just trying to point out that the build scripts do not need to be much more than: gcc -c -o xdifile.o xdifile.c
gcc -c -o strutil.o strutil.c
gcc -c -o slre.o slre.c
gcc -c -o xdi_reader.o xdi_reader.c
gcc -shared -o libxdifile.so xdifile.o strutil.o slre.o
gcc -o xdi_reader xdi_reader.o xdifile.o strutil.o slre.o with changes to the name of the output dynamic library, and maybe a few extra linker flags for Windows. My main objection to the current PR is that it deletes working build scripts for OSX and Win32. I totally agree that the file-ending bug is important! I'm surprised we didn't see this, and we should definitely add a test for it. The Mac reading bug confuses me. I haven't investigated carefully, but I just verified that I read that file using Python and libxdfile no problem and do see an Abort using xdi_reader. Perhaps the problem is in the xdi_reader.c, not in the library? I'll try to investigate, but it might take a couple days. |
|
I am happy to read we are converging on this! A few more remarks though:
|
I have no objection at all to ditching SLRE. Its behavior and its documentation have never matched, which makes it hard and frustrating to use beyond the very simplest of expressions. |
|
I agree that slre has to go right away. Had no idea it was GPL... Yikes! Replace with what? Can XDI get by without a regex machine at all? Except for date validation, I think the use of slre is pretty simple, no? |
Simple yes. Get by without regex? Not so easily. slre is used a lot -- matching against edge and element symbols, matching against allowed characters in metadata families and names, checking whether extension metadata fileds are versioned, and so on. It won't be hard work to replace slre, but it's not 5 minutes of work either. Not using a regex engine at all sounds like much more work, and more fragile, to boot. |
|
Replace with what? I certainly agree that using regex has advantages. It also has some cost of "no standard C library". That's not to say it shouldn't be used. It looks like it would be a lot of work to have to replace the use of slre_match() with stuff from the string library. I don't know an obvious choice for a regex library. |
|
PCRE appears to be widely used for regex. And it has a more friendly license too. |
|
An advantage of SLRE was that it was small enough to be completely included as source code, and not add an external dependency. Having an external dependency would be a big step for XDI. Including PRCE as source code might be possible, but might also add a lot of work for build and maintenance. Again, I'm not opposed, just saying that this sort of thing should be done with careful consideration of what impact is on maintenance and portability. |
|
PCRE indeed appears to be too big and complex to copy into XDI. Alternatively, consider providing the user the option to choose between the bundled SLRE and the externally linked PCRE, while clearly stating what the influence of that decision on the license of XDI will be. It would require some work to set up conditional compilation but it's really not that complicated with autotools. |
Hi Bruce and Matt,
I updated the C buildscripts: they are now up to date with the latest developments in the GNU autotools.
A summary:
should be tracked by git. All others can be generated with
autoreconf(which replaces the
reconfscript)I will look into rewriting the library in Java soon as it may be useful for DAWN
Best regards,
Tom