Dear Adam, Thank you always for rewriting in C++ way!
> If you are uncomfortable with the CMake- and preprocessor-based > solution, you can solve such issues using templates as shown in > > https://github.com/adamreichold/poppler/commit/68f46dce62ad97bdbb22bf79ac50c128d899a302 Waao, it looks very smart. I'm quite sure that such smart idea cannot come to my rusted head living in C89 world :-) I'm *not* uncomfortable with cmake + preprocessor solution, but yours is far better than mine. If somebody finds new variant using something different from st_mtim, st_mtimespec - in my patch, CMakeList.txt & gfiles.cc should be changed, and re-run cmake. But in your patch, only gfiles.cc is needed to be modified, and no need to re-run cmake. It would be easier for further tweaking (if somebody needs). I want to hear other reviewers comment. Regards, mpsuzuki Adam Reichold wrote: > Hello, > > If it works on POSIX and builds on Darwin, it looks good to me. What I > would like would be else clauses in the CMake and preprocessor > definitions that give proper error messages. (Or maybe use the POSIX > variant as the default and only use mtimespec if as an override.) > > If you are uncomfortable with the CMake- and preprocessor-based > solution, you can solve such issues using templates as shown in > > https://github.com/adamreichold/poppler/commit/68f46dce62ad97bdbb22bf79ac50c128d899a302 > > with the related Travis build being > > https://travis-ci.org/adamreichold/poppler/builds/348193138 > > Best regards, Adam. > > Am 02.03.2018 um 03:34 schrieb suzuki toshiya: >> Hi, >> >> It seems that the counterpart in macOS libc corresponding to >> stat.st_mtim is stat.st_mtimespec. >> https://opensource.apple.com/source/xnu/xnu-201.5/bsd/sys/stat.h.auto.html >> >> I wrote a patch testing st_mtim availability by CHECK_STRUCT_HAS_MEMBER() >> suggested by William, and also testing st_mtimespec too, and reflect >> the result to the macro GET_MTIM_FROM_STATBUF(). >> https://github.com/mpsuzuki/poppler/commit/79d00ac08d672d572a7ec310b5a27eb66c956e4c >> >> Building on travis-ci.org finishes successfully. Yet I'm >> unsure such macro is following to the coding style of poppler. >> Also if anybody has a testing code to evaluate the code works >> well (do you have to make 2 file with nsec difference of the >> timestamp?). Please give me comment... >> >> Regards, >> mpsuzuki >> >> On 2/19/2018 1:42 PM, William Bader wrote: >>> Can you test for it in cmake? >>> https://cmake.org/cmake/help/v3.0/module/CheckStructHasMember.html >>> >>> ________________________________ >>> From: poppler <[email protected]> on behalf of Jeroen >>> Ooms <[email protected]> >>> Sent: Sunday, February 18, 2018 6:29 PM >>> To: Ihar Filipau >>> Cc: [email protected] >>> Subject: Re: [poppler] gfile.cc fails to build on macos due to >>> statbuf.st_mtim >>> >>> On Mon, Feb 12, 2018 at 3:04 PM, Ihar Filipau <[email protected]> wrote: >>>> On 2/12/18, Jeroen Ooms <[email protected]> wrote: >>>>> On Sun, Feb 11, 2018 at 12:11 PM, Albert Astals Cid <[email protected]> wrote: >>>>>> You're never assigning to tv_nsec in there but still use it in a >>>>>> comparison, >>>>>> that needs fixing. >>>>> You are right. I think we should compare modification time only by >>>>> seconds. The standard definition of 'struct stat' only specifies >>>>> st_ctime, so I don't think there is a portable way to get nanoseconds: >>>>> http://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/stat.h.html >>>> That's an old version of POSIX. Check the newer version: >>>> >>>> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_stat.h.html >>>> IOW, there is a standard portable way - since 2008, 10 years ago. It's >>>> just Mac OS X hasn't updated its POSIX support after v6, from >>>> 2004. >>> OK so how do you suggest this should be fixed? It would be great if >>> things would keep working on Mac OS. >>> >>> >> _______________________________________________ >> poppler mailing list >> [email protected] >> https://lists.freedesktop.org/mailman/listinfo/poppler >> > _______________________________________________ poppler mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/poppler
