mayah / tinytoml

A header only C++11 library for parsing TOML
BSD 2-Clause "Simplified" License
165 stars 31 forks source link

gmtime_r fails with MinGW GCC version 4.8.0 #25

Closed RobertBColton closed 7 years ago

RobertBColton commented 7 years ago

With a simple int main() and g++ -std=c++11 main.cpp -o tinytomltest.exe that only includes the tinytoml header, I get the following:

toml.h: In member function 'void toml::Value::write(std::ostream*, const string&, int) const':
toml.h:1298:25: error: 'gmtime_r' was not declared in this scope
         gmtime_r(&tt, &t);
                         ^

This was tested with an older version of MinGW that is shipped with the ENIGMA game engine. gcc -v gives:

gcc version 4.8.2 (GCC)

I tried with Visual Studio 2015 and created an empty Console Application, which worked just fine using the MSVC 2015 compiler.

So it loos like the entire issue is with MinGW. I guess tinytoml wants to use gmtime_r because this represents universal time. https://linux.die.net/man/3/gmtime_r

I know with this version of MinGW that I tested, localtime does in fact work. I know because it's used in ENIGMA's engine to implement GameMaker's date and time functions. https://github.com/enigma-dev/enigma-dev/blob/master/ENIGMAsystem/SHELL/Universal_System/Extensions/DateTime/date_time.cpp#L46

But that won't work with tinytoml because localtime would format the dates and times in the users local timezone making them be incorrect when read back from the toml file if read on a computer with a different timezone.

I read on another GitHub post that some things changed with POSIX in newer MinGW releases. I still could not solve the tinytoml issue by defining _POSIX_C_SOURCE 200809L even though I know it's irrelevant for my MinGW version.

The full source of what won't compile with MinGW gcc version 4.8.0:

#define _POSIX_C_SOURCE 200809L
#include <time.h>

#include "tinytoml.h"

int main() {

    return 0;
}

One work around I have found that makes this build with this MinGW is adding the following to tinytoml.h:

#if defined(_WIN32)
// gmtime_r can be defined by mingw
#ifndef gmtime_r
static struct tm* gmtime_r(const time_t* t, struct tm* r)
{
  // gmtime is threadsafe in windows because it uses TLS
  struct tm *theTm = gmtime(t);
  if (theTm) {
    *r = *theTm;
    return r;
  } else {
    return 0;
  }
}
#endif // gmtime_r
#else
extern struct tm* gmtime_r(const time_t* t, struct tm* r);
#endif

This patch was taken from here: http://redmine.webtoolkit.eu/boards/2/topics/7316

With this patch I can build the following with the MinGW GCC version 4.8.0, though it was hard to figure out how to use time types in tinytoml.h:

#include "tinytoml.h"

#include <fstream>

int main() {
    std::ofstream ofs("C:/Users/Owner/Desktop/tinytomltest.toml");
    toml::Table root;

    toml::Table sceneValue = toml::Table();
    sceneValue["name"] = "Scene0";
    time_t rawtime;
    localtime(&rawtime);
    sceneValue["created"] = std::chrono::system_clock::from_time_t(rawtime);

    root["scene"] = sceneValue;

    ofs << root;
    ofs.close();
    return 0;
}

And this is the output:


[scene]
created = 1970-03-16T07:53:44Z
name = "Scene0"

Perhaps this could be added to the examples in the README.md since it just prints the current date and time to make it clearer how to use dates and times?

mayah commented 7 years ago

Thanks. Let me consider a workaround... I didn't know gmtime is thread safe on Windows. Maybe it does not hold on cygwin? Supporting multi platform is always hard...

And... tinytoml's time type is std::chrono::system_clock::timepoint, so I believe http://en.cppreference.com/w/cpp/chrono/system_clock and http://en.cppreference.com/w/cpp/chrono/time_point will help.

mayah commented 7 years ago

@RobertBColton I made a patch to include the workaround you pointed out. Since I'm not using Windows (I'm basically living on Linux), I have not confirmed this on mingw. However I have Visual Studio 2017 installation, so I've confirmed it builds on VS2017. I hope it works for you.

https://github.com/mayah/tinytoml/pull/29

RobertBColton commented 7 years ago

I just applied your patched header and gave it a test:

time_t rawtime = time(0);
sceneValue["created"] = std::chrono::system_clock::from_time_t(rawtime);
[scene]
created = 2017-07-18T20:34:01Z
name = "Scene0"

Seems to work great! Feel free to close this. I am glad I helped as I may likely need your header for future projects. I really like TOML and I like your single-header approach.

mayah commented 7 years ago

OK. Then I'm merging the patch.

mayah commented 7 years ago

The patch has been merged. Closing.