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. > 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-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 >> >> >> >> >>
signature.asc
Description: OpenPGP digital signature
_______________________________________________ poppler mailing list poppler@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/poppler