Fix header installation path

[danyspin97]

Do not install the headers in /usr/include/dragonbox-1.1.3/dragonbox but install them in /usr/include/dragonbox.

[jk-jeon]

Thanks for the PR and sorry for the delay.

I am currently not sure what is the common best practice, to include the version, or to not include the version. Do you have some other reference projects that I can look at?

[jk-jeon]

Sorry @danyspin97, I concluded that having an extra directory is a good thing, because it prevents people from accidently including the library without being explicit about using it. Adding the right include directory should not be a big issue for any build system you use I guess. So I close this PR.

[ecorm]

The problem is not the depth of the directory structure, but that the install location changes depending on the version used.

Having different directories for different major versions (e.g. version 1, version 2) is common practice, but not for minor versions when one uses semantic versioning.

[jk-jeon]

@ecorm Thanks for the input. Do you know of some references that I can have a look at?

[ecorm]

@jk-jeon I don't have a reference off the top of my head to give you. Certainly not Ryu, as they messed things up like having really short function names in the global namespace, and also perhaps macros without a prefix to help avoiding clashes with other library/application macros.

I'll see if I can find a reference/example and will get back to you.

[jk-jeon]

@ecorm Thanks a lot. I think this provides the rationale behind the practice I got to follow. Note that the one who opened that issue is the one who wrote the CMake script of this project.

[ecorm]

I think Dobiasd/FunctionalPlus#222 provides the rationale behind the practice I got to follow.

I don't agree with their approach, and it goes against the convention in Linux. Let's say I have an open-source library that depends on dragonbox. I don't want to impose which version of the dragonbox my users must have on their system, except for the major version (that is 1.x.x or 2.x.x). Let's say in my hypothetical library, I only impose major version 1 of dragonbox. If you follow the semantic versioning principle, then my users can supply any version 1.x.x of dragonbox, and my library should be able to compile with it, assuming it only uses features from the 1.0.0 release.

For example:

#ifndef MYLIBRARY_HPP
#define MYLIBRARY_HPP

#include <string>
#include <dragonbox/dragonbox.h>

// The following would be annoying to users who don't have the exact same version of dragonbox
// on their system. They would need to patch this source file!
// #include <dragonbox-1.1.3/dragonbox/dragonbox.h>

namespace mylib
{

inline std::string doubleToString(double x)
{
    constexpr int buffer_length = 1 + // for '\0'
        jkj::dragonbox::max_output_string_length<jkj::dragonbox::ieee754_binary64>;
    char buffer[buffer_length];
    return std::string(jkj::dragonbox::to_chars(x, buffer));
}

} // namespace mylib

#endif // MYLIBRARY_HPP

If my library depends on dragonbox 1.1 or later because of new features or bugfixes, that could be checked with version macros:

#ifndef MYLIBRARY_HPP
#define MYLIBRARY_HPP

#include <string>
#include <dragonbox/dragonbox.h>

#if JKJ_DRAGONBOX_VERSION_MAJOR != 1 || JKJ_DRAGONBOX_VERSION_MINOR < 1
    #error MyLibrary requires dragonbox 1.1 or later
#endif

Having dragonbox 1.1 or later installed could also be checked with my library's CMake script if dragonbox exports things correctly in its CMake scripts.

If you follow semantic versioning, and you release a version 2 of dragonbox where you allow yourself to break its API, then you should install it under a dragonbox-2 subdirectory by default so that it can co-exist with dragonbox version 1:

#ifndef MYLIBRARY_HPP
#define MYLIBRARY_HPP

#include <string>

#ifdef MYLIBRARY_USE_LEGACY_DRAGONBOX
#include <dragonbox/dragonbox.h>
#else
#include <dragonbox-2/dragonbox.h>
#endif

If a developer needs to maintain multiple versions of dragonbox for different projects, they can use CMAKE_INSTALL_PREFIX to specify where dragonbox is installed:

cmake --install . --prefix /home/me/code/dragonbox-1.1.3

In the developer's CMakeLists.txt for their app, they can add /home/me/code/dragonbox-1.1.3 as an include search directory, and in their code they just need to do this:

#include <dragonbox/dragonbox.h>

[ecorm]

Whenever you see

• usr/include/somelib-1.1
• usr/include/somelib-1.2

It's because the library didn't strictly follow the semantic versioning convention. Some apps/libraries in the system depend on the behavior of somelib v1.1 whereas others depend on the behavior of v1.2. If one is careful not to alter behavior within a major version, then any app should be able to use the latest version of somelib-1.x. Note that it's acceptable to add new features within a major version (this is where you'd increment the minor version), as long as it doesn't break code that was using the older minor versions. I don't normally consider fixing bugs to be "altering behavior". In those rare instances where an app depends on buggy behavior, you could allow the user to define a macro that re-enables the buggy behavior in later versions.

If the behavior of dragonbox can alter between minor versions (say, the decimal string output like removing trailing zeroes), and there's a chance that apps depend on the old behavior, then there may be an argument for installing under usr/include/dragonbox-1.1. usr/include/dragonbox-1.2, etc as shown above. Having separate directories for patch versions (e.g. 1.1.2, 1.1.3, etc) is going too far, IMO.

Whenever you see

• usr/include/somelib
• usr/include/somelib-2

It means the library properly followed the semantic versioning convention where they only broke the API in major versions. Some system apps are relying on the old somelib, and others rely on somelib-2, the latter not having a backwards-compatible API with somelib version 1.

[jk-jeon]

@ecorm I see what you mean.

For the first point about the Linux convention, I don't have much to say. There was a related comment on that in the issue thread I mentioned. At this point I'm not conclusive on this matter. I need to have some more research on this topic.

For the second point about the include directory, I think I'm not agreeing with you on it. I think deciding which version of library to include is basically the job of build system, not of source code. If the user correctly set up the include directory (which I believe should be automatic if the CMake script of this repo is written correctly), then the source code just need to have #include <dragonbox/dragonbox.h> regardless of the version it's willing to use.

Your suggestion about the possibility of coexistence of two different versions behind a macro switch seems tempting, but my gut feeling is that it is a brittle hack basically. Because, (1) we anyway need to have a single version of dragonbox across the whole project because otherwise it's an ODR time bomb waiting to explode, and (2) the way to have a single version across the whole project is to define that macro switch through the build system. In that case, it would be just better to write the build script correctly to find the right include directory, rather than to do some macro hack.

To be fair, we could also have an inline namespace to prevent the ODR issue, but anyway I feel uneasy about having multiple versions of the same library in a single project.

Thanks a lot for all the input anyway. I have to think about this more.

[danyspin97]

A note before continuing the discussion: dragonbox is now shipped in Linux distributions as it is a Libreoffice dependency.

For the first point about the Linux convention, I don't have much to say. There was a related comment on that in the issue thread I mentioned. At this point I'm not conclusive on this matter. I need to have some more research on this topic.

I think a quick inspection of /usr/include/ in a Linux distribution could be a good starting point. There is no package that is shipped with a bugfix version in the directory; the build system would have to change the version included every time there is a bugfix version update. This is not correct because the bugfix version update does not break the API/ABI, there is no need to depend on the bugfix version. Let's also consider that libreoffice has like 30 or more dependencies, if every library did the same as dragonbox it would not scale.

Having a major version like dragonbox-1/dragonbox.h would also be fine, but that's different than dragonbox-${version}/dragonbox/dragonbox.h. We can discuss more this option if you prefer, and I would also be open to help.

[jk-jeon]

@danyspin97 Thanks for the reply. Would you mind opening an issue about this matter? I think it would be better to discuss further in an issue thread instead of a PR.

Anyway, here is what I think right now:

1. If it is currently not possible to set the install directory into /usr/include/dragonbox from the user's side (without a terrible hack and/or something very unconventional) , this should be considered a bug. I have to fix it.

2. If it is possible to do so, still we need to decide which one should be the default. This I think is something I need to have more discussion.