Build on Windows

[adrido]

The first steps to make a working windows build. I will add detailed Instructions how to build on windows later.

[Rogier-5]

General remarks:

• I'd like the patch to be split, so that changes that are logically different, are in different commits, even though they all work towards the same goal (compiling using MSVC on windows)
• In principle, it is not recommended practise to include foreign code in a project... After some thinking about it, I concluded that in the case of getopt and dirent, the benefits probably outweigh the cost. Please add them in 2 separate commits, in a subdirectory (e.g. 'windows'), and add a README file to that directory that describes where they were obtained from, so that people can easily go and fetch updated versions.
• In the MSVC build file / build configuration, specify that subdirectory as an additional standard / system include path to the compiler, so that #include <dirent.h> etc. just works.
• There seems to be a newer version of dirent: https://github.com/tronkko/dirent
• I suppose there is a MSVC project file, or build-file, or so, that you use, and that somebody compiling with MSVC would need ? Could you include that as well ? Preferably a version in text format, if that's practical and sensible ?

[adrido]

Ok, thanks for feedback, I will try to do my best. You can believe me

Do you want the VS project files in the root directory or in a subdirectory? All (required) VS files are (more or less) human readable in a text, script and xml file format. There exists much database and compiled files, but they are created automatically. (I hope I can filter all that files out via .gitignore). I also does not like that dirent and getopt are included directly. But that is the simplest ( and in my opinion best) way. I made only bad experience with "Copy file x from http://... paste it there and file y there and ...". Most time I got a only a 404 page (even on github). Just about interest: the empty line at file end: is this just a design issue or is it important?

[Rogier-5]

Ok, thanks for feedback, I will try to do my best.

Your effort is much appreciated ! Thanks.

Do you want the VS project files in the root directory or in a subdirectory?

Does it matter to VS where they are ?

For Linux, the CMake files are in the root directory of the build. I see no reason why the MSVC files can't be there either. From what I understand, there are just a few (.sln and .vcxproj ?). That's manageable.

All (required) VS files are (more or less) human readable in a text, script and xml file format.

BTW: I assume you didn't use CMake in the end ? If you did manage to get it working, then I suppose no additional VS files are needed at all ? as they will be generated by CMake.

There exists much database and compiled files, but they are created automatically. (I hope I can filter all that files out via .gitignore).

Maybe the following helps: https://github.com/github/gitignore, and in particular: https://github.com/github/gitignore/blob/master/VisualStudio.gitignore

I also does not like that dirent and getopt are included directly. But that is the simplest ( and in my opinion best) way. I made only bad experience with "Copy file x from http://... paste it there and file y there and ...". Most time I got a only a 404 page (even on github).

That's what I figured too. No problem.

Just about interest: the empty line at file end: is this just a design issue or is it important?

Actually, it's not meant to be an empty line (but that may be how it looks on windows?), but just a 'newline' character (i.e. LF, which is converted to two characters (CR LF) on windows) at the end of the last line (just like at the end of all other lines).

Example: If you open a text file:

Hello world!
Bye, bye.

in a hex editor, you should get something like this (on windows):

00000000  48 65 6c 6c 6f 20 77 6f  72 6c 64 21 0d 0a 42 79  |Hello world!..By|
00000010  65 2c 20 62 79 65 2e 0d  0a                       |e, bye...|

(assuming the file is not in windows unicode format (UTF16)). Note the '0d 0a' (CR LF) at the end of every line, including on the last line (on linux that would just be '0a' (LF)). My comments were about files that don't have the CRLF at the very end. Like this:

00000000  48 65 6c 6c 6f 20 77 6f  72 6c 64 21 0d 0a 42 79  |Hello world!..By|
00000010  65 2c 20 62 79 65 2e                              |e, bye.|

I don't know whether Windows uses CRLF as a line separator (none is needed at the end of the last line), or as a line terminator (one is needed at the end of the last line). As many Linux tools expect a line terminator (not a separator), on every line including the last, it's best to have one. If that means there's an empty last line in your editor, then I'd appreciate if there is one...

[adrido]

Does it matter to VS where they are ?

There are a few more required. I moved all files in a subdirectory "MSVC" except the minetestmapper.sln. In order to work with git the .sln file have to be in the root directory.

I assume you didn't use CMake in the end ? If you did manage to get it working, then I suppose no additional VS files are needed at all ? as they will be generated by CMake.

I decided to not use CMake. CMake is a great tool if it searches for all dependencies automatically and you get a working project file within some seconds. But CMake on Windows requires much more "handwork" and a lot of time. If the CMakesList is configured correctly you have to search all dependencies by hand in the internet. configure again and agian. If you have luck you get some working project files. (Worked fine with lib gd). With your CMakesList I had no luck. (Could not detect compiler, could not detect c++ version, problems with docs, ...). A build system is made to make life easier, not more complicated. In MSVC directory I added a packages.config file where VS fetches and installs all required dependencies automatically (except lib gd. This have to be added manually).

I split it up into more commits, I hope its more clearly now what each commit is doing.

But not using CMake also makes some problems: VERSION_MAJOR and VERSION_MINOR are not defined. USE_LEVELDB is 0 although all dependencies of leveldb are aviable by packages.config. INSTALL_PREFIX is also not defined. I would not recommend to use a constant INSTALL_PREFIX since Windows programs can be installed everywhere even on a USB-stick or on a network drive.

Best would be a version.h file (instead of the version file) somewhere, where VERSION_MAJOR and VERSION_MINOR are defined.This could be included into rc file compiled by the resource compiler and linked into the final .exe.

I hope I can solve this issues somehow, than Ill add build instructions and create a new pull request.

[Rogier-5]

Does it matter to VS where they are ?

There are a few more required. I moved all files in a subdirectory "MSVC" except the minetestmapper.sln. In order to work with git the .sln file have to be in the root directory.

OK. Fine with me.

I assume you didn't use CMake in the end ? If you did manage to get it working, then I suppose no additional VS files are needed at all ? as they will be generated by CMake.

I decided to not use CMake. CMake is a great tool if it searches for all dependencies automatically and you get a working project file within some seconds. But CMake on Windows requires much more "handwork" and a lot of time. If the CMakesList is configured correctly you have to search all dependencies by hand in the internet. configure again and agian. If you have luck you get some working project files. (Worked fine with lib gd). With your CMakesList I had no luck. (Could not detect compiler, could not detect c++ version, problems with docs, ...). A build system is made to make life easier, not more complicated.

Agree. And especially for the causal user who wants to compile minetestmapper himself.

In MSVC directory I added a packages.config file where VS fetches and installs all required dependencies automatically (except lib gd. This have to be added manually).

That is much easier indeed ! I might start to like Windows :-)

I split it up into more commits, I hope its more clearly now what each commit is doing.

But not using CMake also makes some problems: VERSION_MAJOR and VERSION_MINOR are not defined.

USE_LEVELDB is 0 although all dependencies of leveldb are aviable by packages.config.

But it does set USE_SQLITE to non-zero if (and only if) sqlite3 is compiled in (and sqlite3 dependencies are available) ? That sounds like a bug in VS ?

INSTALL_PREFIX is also not defined. I would not recommend to use a constant INSTALL_PREFIX since Windows programs can be installed everywhere even on a USB-stick or on a network drive.

INSTALL_PREFIX is intended to be configured by the person compiling minetestmapper, although there should be a reasonable default. You might opt for 'c:\program files\minetestmapper\' or so as a default, assuming that most people compiling and installing minetest would install it there. That would allow minetestmapper to search for colors files etc. in a predefined location. However, given the fact that minetestmapper is more likely to be anywhere else, I suppose an empty default would be better. That shouldn't harm functionality:

The advantage on windows (or at least, using MinGW. I don't know about MSVC...) is that minetestmapper is able to easily determine the location of its executable. Is that the same using MSVC ? If it can find itself, then there is not really a need for a non-empty INSTALL_PREFIX, as it will be able to start the colors.txt search from the location of the executable.

So, I'd say: leave it empty, but if possible, allow the user compiling minetest to change it if desired.

Best would be a version.h file (instead of the version file) somewhere, where VERSION_MAJOR and VERSION_MINOR are defined.This could be included into rc file compiled by the resource compiler and linked into the final .exe.

Even if version were renamed to version.h (which is an option), you'll need a minor version as well. My goal adding the minor version was to (try to :-) uniquely identify the version (i.e. git commit) of minetestmapper the user is using, including whether the user modified his version before compiling. That goal is not achievable using a fixed value in a file, in whatever format it could be. Would it be possible for VS to generate a version.h (or whatever format(s) you need: version.rc, version.xyz, ...), or run a script or program that does so ?

If possible, I'd very much prefer if the full version were computed. If necessary, I could write a batch file or a small c++ program that computes it (assuming I can communicate with git). VS could then compile the c++ file and use it (or the batch file) to generate version.h. Maybe it's also doable (and easier) using powershell ? Currenty, CMake computes the version as:

MAJOR = <latest git tag of the form YYYYMMDD>
MINOR = <commit id> (for unmodified tree)
MINOR = <commit id>-WIP (for modified tree)

i.e. the output of git describe --long "--match=[0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9]" --dirty=-WIP --abbrev=8, but you may have figured that out already. If there is no git tree available (e.g. the user downloaded a ZIP file), it does use version, and a checksum of all .cpp and .h files (which, BTW, is not adequate, so I'll need to adapt it anyway...)).

Note that I don't particularly care whether the minor version on Windows is textually exactly identical to the Linux one, although it should be predictable, and I would like to be able to compute what the Windows version should be in a particular case (so that I know & can verify if necessary what version / what commit of minetestmapper a user reporting a bug used when compiling minetestmapper).

I hope I can solve this issues somehow, than Ill add build instructions and create a new pull request.

Don't hesitate to ask if you have questions or run into problems.

[Rogier-5]

Hi adrido,

Just wondering about the status ? Are you still making changes / improvements, or are you waiting for a (re)action from me ?

I just pushed some new commits to minetestmapper, and there is one change that will affect the windows build: libgd only supports the ISO8859-2 character set, so I made some changes to better render non-ASCII characters. I don't exactly know how this affects windows, but the implemenation has a posix-specific part. I tried to make a windows implementation as well (see the branch char-encoding-win32), but couldn't really test it :-) It does compile.

Of course, the entire conversion can just remain disabled on windows, as it is currently in the master branch, but removing the compiler #warning messages should give a better compilation experience :-)

[adrido]

Oh sorry, I wrote an comment some days ago, but somehow I forgot to hit the "Comment" button :-/

Ill first finish the MSVC branch, than ill take a look into char-encoding-win32.

The old message: Now I generate a versioninfo.h file with a modified version of https://github.com/Thell/git-vs-versioninfo-gen . I had to change two lines to fit to your version tags. The Windows Decimal version looks like 2015,03,10,0 The Windows version string will look now look like 2015.03.10.26.g0e8f9.dirty 26 is the commits behind the last tag g0e8f9 is the last commit dirty means there are changes on files, which has not been committed.

Would this be OK?

[adrido]

Of course, the entire conversion can just remain disabled on windows, as it is currently in the master branch, but removing the compiler #warning messages should give a better compilation experience :-)

:laughing: Yes removing this would give a better experience, but not because its a warning, more because #warning is not known to the preprocessor, so its a error at this line :-)

Maybe you want to use something like this (if this works with your compiler)

#define STRING2(x) #x
#define STRING(x) STRING2(x)
#pragma message (__FILE__ "(" STRING(__LINE__) "): test")

about libiconv: since 15th December 2015 this is also available as a NuGet package so it would pretty easy to implement it in the Windows build. On this way you may have not to differ between _win32 or not, just do it like you would do on Linux. :-) This would keep the code more clean and simple

There exists also an project that provides a iconv compatible interface for the Windows-API. Currently I don't know if this is better or not. https://github.com/win-iconv/win-iconv

While trying to compile some other errors occurred: CharEncodingConverterIconf.cpp Line 8+15 include does not exist and so nl_langinfo() does also not work.

ERROR C2664 "size_t libiconv(libiconvt,const char *,sizet ,char ,sizet )" : Convert argument 2 from "char *_" in "const char **" not possible. \CharEncodingConverterIConv.cpp 73

CharEncodingConverter.cpp line 3: Since the Windows build process does not use c-make there is no cmake-config.h. something like

ifdef USE_CMAKE_CONFIG_H

include "cmake_config.h"

else

include "config.h"

endif

would work

mapper.cpp line 24: You included db-postgeresql.h wich would fail to build because because db-pgsql tries to include dependencies that are not installed. (even if USE_PGSQL = 0) so you might want to remove this line, or only include it if USE_PGSQL is 1.

I hope I did not forgett something.

[Rogier-5]

Of course, the entire conversion can just remain disabled on windows, as it is currently in the master branch, but removing the compiler #warning messages should give a better compilation experience :-):

:laughing: Yes removing this would give a better experience, but not because its a warning, more because #warning is not known to the preprocessor, so its a error at this line :-)

Well:-) ...at least it servers its main purpose: to make sure it does not remain unnoticed :-)

Maybe you want to use something like this (if this works with your compiler)

#define STRING2(x) #x
#define STRING(x) STRING2(x)
#pragma message (__FILE__ "(" STRING(__LINE__) "): test")

I'll (try and :-) remember. Doesn't the compiler already print location and line number by itself when it prints a #pragma message message ?

about libiconv: since 15th December 2015 this is also available as a NuGet package so it would pretty easy to implement it in the Windows build. On this way you may have not to differ between _win32 or not, just do it like you would do on Linux. :-) This would keep the code more clean and simple

Indeed it would. I could have gotten away with writing less code :-( So much for trying to be portable :-) I'll simplify it again (with your blessing...) (replacing the CharEncoding* classes with an iconv call...)

There exists also an project that provides a iconv compatible interface for the Windows-API. Currently I don't know if this is better or not. https://github.com/win-iconv/win-iconv

Whatever works, produces good results, and is convenient, I'd say. If it's the same API, it can always be switched later if necessary.

While trying to compile some other errors occurred: CharEncodingConverterIconf.cpp Line 8+15 include does not exist and so nl_langinfo() does also not work.

So another way is still needed to obtain the current character encoding used... I made a guess at some suitable functions in the code targeted at windows, but I don't know if they are OK, and if they produce the information iconv() can use, and if they work both from the gui and from the command-line. From what I understand, windows GUI and windows command-line use different character encodings :-(

ERROR C2664 "size_t libiconv(libiconvt,const char *,sizet ,char ,sizet )" : Convert argument 2 from "char *_" in "const char **" not possible. \CharEncodingConverterIConv.cpp 73

I specifically casted the 'const'-ness away a few lines earlier because else my compiler complains... I haven't got a solution yet. Maybe a macro ? (#define iconv(...) iconv(...))

CharEncodingConverter.cpp line 3: Since the Windows build process does not use c-make there is no cmake-config.h. something like

ifdef USE_CMAKE_CONFIG_H

include "cmake_config.h"

else

include "config.h"

endif

would work

It should have been #include "config.h" anyway. config.h includes cmake_config.h (if it can).

mapper.cpp line 24: You included db-postgeresql.h wich would fail to build because because db-pgsql tries to include dependencies that are not installed. (even if USE_PGSQL = 0) so you might want to remove this line, or only include it if USE_PGSQL is 1.

I really should be doing some compilation testing on a machine where database libraries are not all installed... It is on my todo-list...

Anyway: that include should not be there at all. It's not needed. I wonder how it ended up there :-) I pushed an emergency fix...

[adrido]

Maybe you want to use something like this (if this works with your compiler)

   #define STRING2(x) #x
   #define STRING(x) STRING2(x)
   #pragma message (__FILE__ "(" STRING(__LINE__) "): test")

I'll (try and :-) remember. Doesn't the compiler already print location and line number by itself when it prints a #pragma message message ?

The odd thing is: no, it doesn't. :confused:

While trying to compile some other errors occurred: CharEncodingConverterIconf.cpp Line 8+15 include does not exist and so nl_langinfo() does also not work.

So another way is still needed to obtain the current character encoding used... I made a guess at some suitable functions in the code targeted at windows, but I don't know if they are OK, and if they produce the information iconv() can use, and if they work both from the gui and from the command-line. From what I understand, windows GUI and windows command-line use different character encodings :-(

as you maybe know, you can get the codepage from calling GetConsoleOutputCP() in the Windows API The following snippet prints the CP as a readable (but localized) string, so its not usable by iconv

    CPINFOEX CPInfoEx;
    UINT cp = GetConsoleOutputCP();
    GetCPInfoEx(cp, 0, &CPInfoEx);
    cout << CPInfoEx.CodePageName;

Afaik Minetst does also use iconv but I don't know how minetest solved this problem. the win-iconv provided a conversation table to convert the codepage id to a string that can used, and there is a not that some code where taken from libiconv. Maybe iconv provides already a conversation table? I could not find the sourcecode of iconv, so I could not check it. https://github.com/win-iconv/win-iconv/blob/master/win_iconv.c#L213

ERROR C2664 "size_t libiconv(libiconvt,const char ,size_t ,char ,sizet )" : Convert argument 2 from "char " in "const char " not possible. \CharEncodingConverterIConv.cpp 73

I specifically casted the 'const'-ness away a few lines earlier because else my compiler complains... I haven't got a solution yet. Maybe a macro ? (#define iconv(...) iconv(...))

I think I found where the problem was. You casted a const char * into a const char * but than you wrote it into a char * . I fixed that, and now it compiles without problems:

std::string CharEncodingConverterIConv::convert(const std::string &src)
{
    std::string dst;

    char toBuffer[ICONV_BUFSIZE + 1];
    const char *fromBufP;
    char *toBufP;
    size_t fromBufLen, toBufLen;

    fromBufLen = src.length();
    // Assume that iconv() does not write to the source array...
    fromBufP = src.c_str(); //afaik string.c_str() returns always a const char pointer
    toBufLen = ICONV_BUFSIZE;
    toBufP = toBuffer;

    size_t rv;
    do {
        rv = libiconv(m_iconv, &fromBufP, &fromBufLen, &toBufP, &toBufLen);
//...

[Rogier-5]

I think I found where the problem was. You casted a const char * into a const char * but than you wrote it into a char * . I fixed that, and now it compiles without problems:

I had to cast it because my iconv prototype looks like:

       size_t iconv(iconv_t cd,
                    char **inbuf, size_t *inbytesleft,
                    char **outbuf, size_t *outbytesleft);

Which does not seem logical :-( - although maybe it is for historical reasons (compatibility with older code) ? Anyway, my compiler will complain about the const char *...

I'd suggest to use (in porting.h):

#ifndef _WIN32
#define iconv(cd, inbuf, inbytes, outbuf, outbytes) iconv((cd), (char **)(inbuf), (inbytes), (outbuf), (outbytes))
#endif 

Unless you can think of a better alternative ?

A test program that uses this macro compiles and works using both gcc and clang, while it doesn't without the macro, so it should be OK on linux.

[adrido]

hmm no I does not have a better idea.

I know, this is not the official source, but afaik the *in files get preprocessed by Cmake? https://github.com/madewokherd/libiconv/blob/7748bec1c92f4f4b7e35d5cf1e9ef86f95bb9154/include/iconv.h.in#L83

extern size_t iconv (iconv_t cd, @ICONV_CONST@ char* * inbuf, size_t *inbytesleft, char* * outbuf, size_t *outbytesleft);

The placeholder @ICONV_CONST@ gets replaced by cmake? May it be that on some systems its not const and on some its const? Depending on Cmake configuration?

On this page I stumbled over another line:

sprintf (buf, "CP%u", GetACP ());

Is it as easy as it looks like? Just prepend CP before the codepage. Than it would be easy to implement a getLocalCharset function in porting.h

[proller]

sleep ms can be replaced with portable std::this_thread::sleep_for(std::chrono::milliseconds(123))

[adrido]

works fine for me :+1: