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-boun...@lists.freedesktop.org> on behalf of > >>>>>>>> Jeroen Ooms <jer...@berkeley.edu> Sent: Sunday, February 18, 2018 > >>>>>>>> 6:29 > >>>>>>>> PM > >>>>>>>> To: Ihar Filipau > >>>>>>>> Cc: poppler@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 > >>>>>>>> <thephil...@gmail.com> > >>>> > >>>> wrote: > >>>>>>>>> On 2/12/18, Jeroen Ooms <jer...@berkeley.edu> wrote: > >>>>>>>>>> On Sun, Feb 11, 2018 at 12:11 PM, Albert Astals Cid > >>>>>>>>>> <aa...@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@lists.freedesktop.org > >>>>>>> https://lists.freedesktop.org/mailman/listinfo/poppler > >>>>> > >>>>> _______________________________________________ > >>>>> poppler mailing list > >>>>> poppler@lists.freedesktop.org > >>>>> https://lists.freedesktop.org/mailman/listinfo/poppler > >>>> > >>>> _______________________________________________ > >>>> poppler mailing list > >>>> poppler@lists.freedesktop.org > >>>> https://lists.freedesktop.org/mailman/listinfo/poppler _______________________________________________ poppler mailing list poppler@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/poppler