Numbertext / libnumbertext

Number to number name and money text conversion libraries in C++, Java, JavaScript and Python & LibreOffice Calc Extension
BSD 3-Clause "New" or "Revised" License
68 stars 46 forks source link

#include "config.h" in public header harmful #3

Closed reneengelhard closed 6 years ago

reneengelhard commented 6 years ago

Soros.hxx:

/* Soros interpreter (see numbertext.org)
 * 2018 (c) László Németh
 * License: LGPL/BSD dual license */

#ifndef SOROS_HXX_
#define SOROS_HXX_

#include <iostream>
#include <iterator>
#include <string>

#ifndef _MSC_VER
#include "config.h"
#endif

#ifdef HAVE_BOOST_REGEX_HPP
  #include <boost/regex.hpp>
#else
  #include <regex>
#endif

is bad. That of course fails when building LibreOffice against a system-libnumbertext because the following happens (Soros.hxx is included by Numbertext.hxx):

In file included from /usr/include/libnumbertext/Numbertext.hxx:8:0,
                 from /data/rene/git/LibreOffice/master/lingucomponent/source/nu
mbertext/numbertext.cxx:41:
/usr/include/libnumbertext/Soros.hxx:13:10: fatal error: config.h: Datei oder Ve
rzeichnis nicht gefunden
 #include "config.h"
          ^~~~~~~~~~
compilation terminated.

(No such file or directory)

Regards,

Rene

reneengelhard commented 6 years ago

config.h isn't installed by make install.

And also (IMHO) shouldn't be. If at all a libnumbertext_config.h could be acceptable, but I mean just removing it and requiring would be better, imho. (Did https://salsa.debian.org/libreoffice-team/libnumbertext/commit/2419793f8ecfd47bf1ab5706fe775be88416154a for now.)

laszlonemeth commented 6 years ago

Rene, many thanks for the feedback and your quick fix. I will fix it in the upstream code, too.

(Moreover, Unfortunately, I haven't solved the system installation of the language data, yet, as you likely noticed.)

reneengelhard commented 6 years ago

(Moreover, Unfortunately, I haven't solved the system installation of the language data, yet, as you likely noticed.)

no, that actually works for me. At least they end up properly in /usr/share/libnumbertext :)

laszlonemeth commented 6 years ago

Fixed in commit 44dcd84fe31f6aa003a005cc3684d5cc036ae7c4

Rene, I've checked the system installation of the used numbertext-version.h successfully, so I hope, you can remove the Debian patch for config.h removing. Thanks for your bug report!