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-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

Reply via email to