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

Sat Mar 3 20:03:09 UTC 2018

Hello again,

attached is a patch against master that fixes the Clang/libc++ linking
issue with the cpp frontend's text_box_data destructor and the
non-standard high-resolution mtime field name on Mac OS X.

Best regards, Adam.

Am 03.03.2018 um 20:18 schrieb Albert Astals Cid:
> El divendres, 2 de març de 2018, a les 13:24:37 CET, suzuki toshiya va 
> escriure:
>> 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/68f46dce62ad97bdbb22bf79ac5
>>> 0c128d899a302
>> 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.
> 
> This kind of template substitution by "failure" are a bit weird to get around, 
> but on the other hand it's quite self contained and i guess you could add some 
> text explaining "this substitution is used on Darwin" and "this substitution 
> is used on Linux".
> 
> I'm fine with that, but you're going to need to send a patch against poppler 
> git not against mpsuzuki's repo if you want inclusion upstream :)
> 
> Cheers,
>   Albert
> 
>>
>> 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/68f46dce62ad97bdbb22bf79ac5
>>> 0c128d899a302
>>>
>>> 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.htm
>>>> l
>>>>
>>>> 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/79d00ac08d672d572a7ec310b5a27e
>>>> b66c956e4c
>>>>
>>>> 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.htm
>>>>>>> l
>>>>>>
>>>>>> That's an old version of POSIX. Check the newer version:
>>>>>>
>>>>>> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_stat.h.htm
>>>>>> l
>>>>>> 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
>>
>> _______________________________________________
>> poppler mailing list
>> poppler at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/poppler
> 
> 
> 
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-macosx-build.patch
Type: text/x-patch
Size: 3791 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/poppler/attachments/20180303/fdce44e8/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 525 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/poppler/attachments/20180303/fdce44e8/attachment.sig>