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

Albert Astals Cid aacid at kde.org
Sun Mar 4 21:15:40 UTC 2018 

El diumenge, 4 de març de 2018, a les 9:21:49 CET, Adam Reichold va escriure:
> Hello Albert,
> 
> Am 03.03.2018 um 23:26 schrieb Albert Astals Cid:
> > El dissabte, 3 de març de 2018, a les 21:15:48 CET, Adam Reichold va 
escriure:
> >> And this time around, without a bunch of typos... Sorry for the noise.
> > 
> > Wouldn't it make more sense to have the void_t definition in the anonymous
> > namespace in gfile.cc for now until we really need to use it somewhere
> > else?
> I am unsure if the next user will find it if it is tucked away there,
> but attached is the simplified patch the does this and hence only
> touches gfile.cc.

Pushed.

Cheers,
  Albert

> 
> Best regards, Adam.
> 
> > Cheers,
> > 
> >   Albert
> >> 
> >> Am 03.03.2018 um 21:03 schrieb Adam Reichold:
> >>> 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/68f46dce62ad97bdbb22bf
> >>>>>> 79
> >>>>>> ac5
> >>>>>> 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/68f46dce62ad97bdbb22bf
> >>>>>> 79
> >>>>>> ac5
> >>>>>> 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.aut
> >>>>>>> o.
> >>>>>>> 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/79d00ac08d672d572a7ec310b
> >>>>>>> 5a
> >>>>>>> 27e
> >>>>>>> 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
> >>>> 
> >>>> _______________________________________________
> >>>> poppler mailing list
> >>>> poppler at lists.freedesktop.org
> >>>> https://lists.freedesktop.org/mailman/listinfo/poppler

More information about the poppler mailing list