Re: [Quantlib-dev] cmake configured files

[Jonathan S. <sw...@gm...>]

Hi Peter,

This part of the CMake build settings was added by @pkovacs. I think the
intention may have been to expose QL_HAVE_CONFIG_H as a CMake option with a
default of ON instead of always hardcoding it to ON. But as I mentioned,
there’s no real compelling use case (in my opinion) for setting it to OFF
so I agree that it would be much cleaner to just define HAVE_CONFIG_H in
CMakeLists.txt and thereby eliminate the need for configuring qldefines.hpp.

I actually didn’t realize there were two versions of qldefines.hpp - the
configured one and the non-configured one. That does indeed seem like a
recipe for inconsistency and is reason enough to try and consolidate the
two.

Looks like there’s one other inconsistency between the two qldefines.hpp -
the define for QL_USE_STD_UNIQUE_PTR was only added to the non-configured
one. This should be added to the configured one as well for consistency
until a decision is made about whether to configure or not.

Jonathan


2022년 5월 2일 (월) 03:12, Peter Caspers <pca...@gm...>님이 작성:

> Hi,
>
> thanks. I get how it works. My question is if the code duplication in
> qldefines.hpp / qldefines.hpp.cfg can be avoided by defining the macro
> HAVE_CONFIG_H in the cmake build, just as we do in the autoconf build.
> As far as I can see this would have the same effect as hardcoding
> "set(QL_HAVE_CONFIG_H ON)" and configuring qldefines.hpp. Apologies if
> you are not the right person to ask, I somehow connect your name with
> the recent cmake changes, but maybe there are more contributors behind
> that.
>
> Apart from the code duplication in qldefines.hpp and
> qldefines.hpp.cfg, I think this approach would also be more
> transparent: We build QuantLib as a cmake subproject of another cmake
> project ("ORE"). I recently noticed that this build uses
> userconfig.hpp instead of config.hpp, because we missed adding
> "build/QuantLib" as the first include path. Therefore, the original
> qldefines.hpp instead of the configured one was used and subsequently
> userconfig.hpp was included since HAVE_CONFIG_H is not defined as a
> C++ macro by cmake. All this goes through without any error or warning
> message and led to some confusion on our side. Without configuring
> qldefines.hpp we would have just gotten an error message that
> config.hpp isn't found, which obviously is a much better behaviour.
>
> For the version file: we now seem to maintain a manual version.hpp
> (for autoconf or windows builds) and a configured version.hpp that is
> populated from cmake variables, so we might get different version
> infos in a windows / autoconf and cmake build.
>
> Thanks
> Peter
>
> On Sun, 1 May 2022 at 09:10, Jonathan Sweemer <sw...@gm...> wrote:
> >
> > Hi Peter,
> >
> > I can’t claim to 100% understand the way that the configuration works on
> all the various platforms, but as far as I can tell the main reason for
> still configuring qldefines.hpp in the CMake build is to let the user
> choose between config.hpp and userconfig.hpp. When the QL_HAVE_CONFIG_H
> CMake variable is set to true (which it is by default) then qldefines.hpp
> is configured to include config.hpp, but when it is set to false then
> qldefines.hpp is configured to include one of the platform-specific files
> like config.msvc.hpp.
> >
> > As far as I understand the history, the various config.*.hpp files along
> with userconfig.hpp were basically a mechanism for enabling and disabling
> different features before the CMake build came into existence. With CMake,
> those features can be turned on and off more easily through build options.
> On that basis, I’m not sure it makes much sense to expose QL_HAVE_CONFIG_H
> to “opt-in” to the old way of configuring the build, but again I might be
> missing some nuance.
> >
> > My own opinion is that the CMake build should not overlap with any of
> the older mechanisms like userconfig.hpp and should instead be implemented
> in clean, standard, idiomatic CMake as much as possible.
> >
> > Not sure if that answers the question but hope it helps.
> >
> > Regarding the version, where is the other place it is maintained? I see
> another version in configure.ac - is this the one you are referring to?
> Not sure whether autoconf allows for reading a version from a file, but if
> it does, then perhaps CMake could configure the version from the same file
> as well.
> >
> > Jonathan
> >
> >
> > 2022년 5월 1일 (일) 01:14, Peter Caspers <pca...@gm...>님이 작성:
> >>
> >> Hey Luigi, Jonathan,
> >>
> >> I didn't follow all the cmake updates in detail so apologies if that
> >> was discussed before: Is there a reason why cmake configures
> >> qldefines.hpp? Couldn't we just define HAVE_CONFIG_H instead of
> >> QL_CONFIG_H in the top CMakeLists.txt and it would "just work" with
> >> the standard qldefines.hpp?
> >>
> >> Not directly related: We also configure version.hpp now. I have the
> >> slight feeling this could do more harm than good, since the version
> >> info has to be maintained at two places now?
> >>
> >> Thanks
> >> Peter
> >>
> >>
> >> _______________________________________________
> >> QuantLib-dev mailing list
> >> Qua...@li...
> >> https://lists.sourceforge.net/lists/listinfo/quantlib-dev
>