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

Sun Mar 4 08:09:54 UTC 2018

Adam Reichold wrote:
> Hello mpsuzuki,
> 
> Am 04.03.2018 um 06:11 schrieb suzuki toshiya:
>> Dear Albert & Adam,
>>
>>> 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 :)
>> Indeed, and sorry, I should add another point.
> 
> The patch I posted contains both the struct stat and the ~text_box_data
> fixes.

Oh! I slipped to check that, thank you for correction and
posting 2 issues at once. Sorry to all who were confused
by my post.

Regards,
mpsuzuki

>> Jeroen found the textlist patch for cpp frontend causes linker
>> error on Mac OS X, Adam wrote a patch to fix it.
>>
>> https://github.com/mpsuzuki/poppler/commit/9282c93399ca213a071d61f3a5faca4dbc6dc669
>>
>> (I attached this patch for the official main trunk)
>>
>> I wish if this patch is applied. Sorry for making you bothered
>> about this.
>>
>> Should we need some comments about the explicit anchoring of
>> the default destructor?
> 
> Personally, I do not think this is necessary since the commit message
> already explains the rationale.
> 
> Best regards, Adam.
> 
>> Regards,
>> mpsuzuki
>>
>> Albert Astals Cid wrote:
>>> 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
>>>
>>>
>>>
>>>
> 