lballabio / QuantLib

The QuantLib C++ library
http://quantlib.org
Other
5.23k stars 1.78k forks source link

Missing config.hpp after make install using CMake-based build #396

Closed mcraveiro closed 6 years ago

mcraveiro commented 6 years ago

Hi QuantLib-devs,

thanks for a very useful library. I'm having a slightly strange problem, no doubt related to how I am building the library - but I'm afraid I can't quite figure out what I am doing wrong. I am trying to build an alpine container with a set of C++ libraries including QuantLib and OpenSourceRiskEngine (OSRE) [1].

Possibly quite important: I am using CMake for building QuantLib.

At any rate, I get QuantLib to build and install just fine - can't spot anything untoward in the build output - but when I look at the install directory, it is missing ql/config.hpp:

$  ls -l /usr/include/ql/config*
-rw-r--r--    1 root     root           957 Jan 23 13:55 /usr/include/ql/config.ansi.hpp
-rw-r--r--    1 root     root           963 Jan 23 13:55 /usr/include/ql/config.mingw.hpp
-rw-r--r--    1 root     root          2664 Jan 23 13:55 /usr/include/ql/config.msvc.hpp
-rw-r--r--    1 root     root          1508 Jan 23 13:55 /usr/include/ql/config.sun.hpp

This then causes OSRE to fail:

libtool: compile:  g++ -DHAVE_CONFIG_H -I. -I../../qle -I../.. -I../.. -I/home/Engine-2a56688ff4d91e378bf88620a45864824150fbc5/QuantLib -g -O2 -Wall -MT averageonindexedcoupon.lo -MD -MP -MF .deps/averageonindexedcoupon.Tpo -c averageonindexedcoupon.cpp  -fPIC -DPIC -o .libs/averageonindexedcoupon.o
In file included from /usr/include/ql/time/frequency.hpp:30:0,
                 from /usr/include/ql/time/period.hpp:30,
                 from /usr/include/ql/time/date.hpp:32,
                 from /usr/include/ql/event.hpp:28,
                 from /usr/include/ql/cashflow.hpp:28,
                 from /usr/include/ql/cashflows/coupon.hpp:28,
                 from /usr/include/ql/cashflows/floatingratecoupon.hpp:32,
                 from ../../qle/cashflows/averageonindexedcoupon.hpp:28,
                 from averageonindexedcoupon.cpp:19:
/usr/include/ql/qldefines.hpp:91:28: fatal error: ql/config.hpp: No such file or directory
    #include <ql/config.hpp>

In case it helps, my Dockerfile for QuantLib is as follows:

ARG quantlib_hash=a00d43fabf30ab1e7fcaeaa9f497a551b0de528c
RUN cd /home && \
    wget https://github.com/lballabio/QuantLib/archive/${quantlib_hash}.zip && \
    unzip ${quantlib_hash}.zip && \
    cd QuantLib-${quantlib_hash} && \
    mkdir build && \
    cd build && \
    cmake -DCMAKE_BUILD_TYPE=MinSizeRel -DCMAKE_INSTALL_PREFIX=/usr .. && \
    make -j3 && \
    make install && \
    cd /home && \
    rm -rf QuantLib-${quantlib_hash} ${quantlib_hash}.zip

Where the hash is of the latest commit in master at present. Any ideas on what I am doing wrong are greatly appreciated.

Cheers

Marco

[1] https://github.com/OpenSourceRisk/Engine

lballabio commented 6 years ago

I don't think you're doing anything wrong. As far as I can see, there's a bit of processing missing in the cmake installation.

As a workaround, may you try making a copy of <ql/config.ansi.hpp> as <ql/config.hpp> in your installation directory and see if that works?

mcraveiro commented 6 years ago

Thanks @lballabio, that indeed allowed QuantExt to continue compiling. For now I am going to leave the copying as a hack in my Dockerfile, but I will have a look into the autotools build to see how the config is generated - maybe I can lit the logic into CMake.

You can collect your pint next time you come to town! :-)

lballabio commented 6 years ago

Ok, great. Please leave the issue open so it stays on the radar.

jgsogo commented 6 years ago

Hi, it should be easy enough.

Here we are installing "every" *.hpp file, where is config.hpp located or generated? It should be enough adding the following line to the proper CMakeLists.txt:

install(FILES <path/to/config.hpp> DESTINATION include/ql)
lballabio commented 6 years ago

config.hpp is generated by running configure, which we of course don't do for a cmake build.

One possibility to fix this is that, like we do in the install part of ql/Makefile.am, we use sed to modify the installed qldefines.hpp and prevent config.hpp to be included; replacing HAVE_CONFIG_H with something which won't be defined by another package should do the trick. This will cause the preprocessor to continue and ultimately include config.ansi.hpp.

Another possibility might be to simply install a copy of config.ansi.hpp as config.hpp, which would have the same effect.

jgsogo commented 6 years ago

Just for reference: CMake has the configure_file function (link) to create files from templates substituting some parameters according to the build.

mcraveiro commented 6 years ago

OK I've had a quick gander at the generated file, and the definitions can be split into buckets - let me know if you agree.

Possibly not required

These are things that are more or less autotools historical, but in a CMake/modern environment we can take for granted (at least as a first stab at the problem, at any rate). It may also be that CMake does some of these checks and we could piggy back on them, if required - I'm afraid I am not a CMake expert, so not too sure.

/* Define to 1 if you have the <dlfcn.h> header file. */
#define QL_HAVE_DLFCN_H 1

/* Define to 1 if you have the <inttypes.h> header file. */
#define QL_HAVE_INTTYPES_H 1

/* Define to 1 if you have the <memory.h> header file. */
#define QL_HAVE_MEMORY_H 1

/* Define to 1 if you have the <stdint.h> header file. */
#define QL_HAVE_STDINT_H 1

/* Define to 1 if you have the <stdlib.h> header file. */
#define QL_HAVE_STDLIB_H 1

/* Define to 1 if you have the <strings.h> header file. */
#define QL_HAVE_STRINGS_H 1

/* Define to 1 if you have the <string.h> header file. */
#define QL_HAVE_STRING_H 1

/* Define to 1 if you have the <sys/stat.h> header file. */
#define QL_HAVE_SYS_STAT_H 1

/* Define to 1 if you have the <sys/types.h> header file. */
#define QL_HAVE_SYS_TYPES_H 1

/* Define to 1 if you have the <unistd.h> header file. */
#define QL_HAVE_UNISTD_H 1

/* Define to 1 if you have the ANSI C header files. */
#define QL_STDC_HEADERS 1

/* Define to the sub-directory where libtool stores uninstalled libraries. */
#define LT_OBJDIR ".libs/"

Required for packaging

The following defines I think will be useful if QuantLib starts making use of CPack[1], but may not be needed otherwise. However, it would do not harm to have them there now - except for the release process overhead. See last section.

/* Name of package */
#define QL_PACKAGE "QuantLib"

/* Define to the address where bug reports for this package should be sent. */
#define QL_PACKAGE_BUGREPORT "quantlib-dev@lists.sourceforge.net"

/* Define to the full name of this package. */
#define QL_PACKAGE_NAME "QuantLib"

/* Define to the full name and version of this package. */
#define QL_PACKAGE_STRING "QuantLib 1.11"

/* Define to the one symbol short name of this package. */
#define QL_PACKAGE_TARNAME "QuantLib"

/* Define to the home page for this package. */
#define QL_PACKAGE_URL ""

/* Define to the version of this package. */
#define QL_PACKAGE_VERSION "1.11"

Required in order to configure QuantLib

These are toggles that users switch on or off, but presumably cannot make use of at present when using CMake.

/* Define this if you want to enable the parallel unit test runner. */
/* #undef QL_ENABLE_PARALLEL_UNIT_TEST_RUNNER */

/* Define this if you want to enable sessions. */
/* #undef QL_ENABLE_SESSIONS */

/* Define this if you want thread-safe singleton initialization. */
/* #undef QL_ENABLE_SINGLETON_THREAD_SAFE_INIT */

/* Define this if you want to enable thread-safe observer pattern. */
/* #undef QL_ENABLE_THREAD_SAFE_OBSERVER_PATTERN */

/* Define this if tracing messages should allowed (whether they are actually
   emitted will depend on run-time settings.) */
/* #undef QL_ENABLE_TRACING */

/* Define this if error messages should include current function information.
   */
/* #undef QL_ERROR_FUNCTIONS */

/* Define this if error messages should include file and line information. */
/* #undef QL_ERROR_LINES */

/* Define this if extra safety checks should be performed. This can degrade
   performance. */
/* #undef QL_EXTRA_SAFETY_CHECKS */
/* Define this if you want to enable high resolution date class. */
#define QL_HIGH_RESOLUTION_DATE 1

/* Define this if negative yield rates should be allowed. */
#define QL_NEGATIVE_RATES 1

/* Define this if your compiler does not support Boost::uBLAS. */
/* #undef QL_NO_UBLAS_SUPPORT */

/* Define if running on a Mac OS X machine. */
/* #undef QL_PATCH_DARWIN */

/* Define if running on a Sun Solaris machine. */
/* #undef QL_PATCH_SOLARIS */

/* Define this to use indexed coupons instead of par coupons in floating legs.
   */
/* #undef QL_USE_INDEXED_COUPON */

Approach

I think we could easily use @jgsogo's idea of configure_file and allow users to set the toggle configuration options. For the packaging, it would require some duplication as I think we'd have to have the QuantLib version hardcoded into the CMakeLists.txt - unless someone has a better idea. The problem with this is that it will be one more step required when releasing.

What do you guys think? I can whip up a configure.in template and process it with config_file. I can define all of the required "tags" as CMake options, and use them as the parameters for the template, like so:

option(WITH_NEGATIVE_RATES "Allow negative yield rates" off)

Which is then mapped to QL_NEGATIVE_RATES and expanded on the template to 1 if set to on.

[1] https://en.wikipedia.org/wiki/CPack

lballabio commented 6 years ago

For the configuration part: for non-autotools builds (that is, Windows) we already have ql/userconfig.hpp, which as far as I can see is already included during cmake builds from ql/config.ansi.hpp. I'm not sure I'd add yet another configuration mechanism (it means maintaining the set of options in three different places), but it might be ok if you feel that it's more natural for cmake users.

In any case: configure.in is already generated by autotools and included in the distributed tarballs, so the template would need to have some other name.

mcraveiro commented 6 years ago

Ah, well in that case I think we should simply generate a config.hpp which is a copy of config.ansi.hpp...

mcraveiro commented 6 years ago

I've done the changes, just running some tests and will send a PR. Cheers

mcraveiro commented 6 years ago

Ok it works fine, but the one slight snag is that the include guards are not right :-(

$ grep -He quantlib_ config.hpp
config.hpp:#ifndef quantlib_config_ansi_h
config.hpp:#define quantlib_config_ansi_h

Does this warrant creating a template etc or is it good enough?

mcraveiro commented 6 years ago

Actually we can continue the discussion on the PR.

jgsogo commented 6 years ago

What about using a #pragma once so there is no mismatch in names? Every modern compiler supports it.

mcraveiro commented 6 years ago

Ok, apologies for the messing around with the PRs, unfortunately I hadn't set myself up properly in this PC. Should be ok now, hopefully. @jgsogo I leave that to @lballabio, but presumably you'd have to change all headers to make them consistent. That smells like a separate PR.

jgsogo commented 6 years ago

Yes, it was an open question; maybe there is a strong reason to keep those guards.

lballabio commented 6 years ago

No, just old habits from when we started in 2000. 😄

As for this particular case, I don't think the guards will be a problem. ql/qldefines.hpp will include either config.hpp or config.ansi.hpp, but not both.

mcraveiro commented 6 years ago

eh eh :-) By the by, do I need to update the Contributors.txt file as well? Bit of a small contribution really :-)

lballabio commented 6 years ago

If you want. Usually, I update it when I write release notes for a new QuantLib version.

mcraveiro commented 6 years ago

Ah cool excellent, I'll leave you to it then :-) let me know if you need any changes to the PR. Chers

lballabio commented 6 years ago

@jgsogo , just to make sure, may you pull this branch and check that the Windows build works as well?

mcraveiro commented 6 years ago

Ah yes good point - its a shame we don't yet have https://github.com/lballabio/QuantLib/pull/146 or this would be checked as part of the PR checks.

jgsogo commented 6 years ago

You are right, it's time to catch up with that branch again :fearful: At least, this afternoon I will compile this PR in windows to check if it works as expected.

jgsogo commented 6 years ago

I've just compiled it in windows (visual studio 2015, release, x86) and I can see config.hpp in the install directory. The file does not affect compilation, so it must be ok for every toolchain.

This is the file I get:

/* -*- mode: c++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */

/*
 Copyright (C) 2004 Ferdinando Ametrano
 Copyright (C) 2000, 2001, 2002, 2003 RiskMap srl

 This file is part of QuantLib, a free-software/open-source library
 for financial quantitative analysts and developers - http://quantlib.org/

 QuantLib is free software: you can redistribute it and/or modify it
 under the terms of the QuantLib license.  You should have received a
 copy of the license along with this program; if not, please email
 <quantlib-dev@lists.sf.net>. The license is also available online at
 <http://quantlib.org/license.shtml>.

 This program is distributed in the hope that it will be useful, but WITHOUT
 ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
 FOR A PARTICULAR PURPOSE.  See the license for more details.
*/

#ifndef quantlib_config_ansi_h
#define quantlib_config_ansi_h

#include <ql/userconfig.hpp>

#endif