[poppler] gfile.cc fails to build on macos due to statbuf.st_mtim

[suzuki toshiya]

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 <poppler-bounces at lists.freedesktop.org> on behalf of Jeroen Ooms <jeroen at berkeley.edu>
>>> Sent: Sunday, February 18, 2018 6:29 PM
>>> To: Ihar Filipau
>>> Cc: poppler at lists.freedesktop.org
>>> 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 <thephilips at gmail.com> wrote:
>>>> On 2/12/18, Jeroen Ooms <jeroen at berkeley.edu> wrote:
>>>>> On Sun, Feb 11, 2018 at 12:11 PM, Albert Astals Cid <aacid at kde.org> 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
>> poppler at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/poppler
>>
>