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/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 > > > > >
From 32d8a5a43acb9c655215a1fcd10a09c4c619264d Mon Sep 17 00:00:00 2001
From: Adam Reichold <adam.reich...@t-online.de>
Date: Mon, 12 Feb 2018 08:09:00 +0100
Subject: [PATCH 1/2] Explicitly anchor destructor of text_box_data to avoid
linker errors using Clang on Mac OS X.
---
cpp/poppler-page.cpp | 2 ++
cpp/poppler-private.h | 2 ++
2 files changed, 4 insertions(+)
diff --git a/cpp/poppler-page.cpp b/cpp/poppler-page.cpp
index 83d48f07..9ae569b0 100644
--- a/cpp/poppler-page.cpp
+++ b/cpp/poppler-page.cpp
@@ -290,6 +290,8 @@ ustring page::text(const rectf &r, text_layout_enum layout_mode) const
/*
* text_box object for page::text_list()
*/
+text_box_data::~text_box_data() = default;
+
text_box::~text_box() = default;
text_box::text_box(text_box_data *data) : m_data{data}
diff --git a/cpp/poppler-private.h b/cpp/poppler-private.h
index 3753567f..c4ca6e57 100644
--- a/cpp/poppler-private.h
+++ b/cpp/poppler-private.h
@@ -70,6 +70,8 @@ void delete_all(const Collection &c)
struct text_box_data
{
+ ~text_box_data();
+
ustring text;
rectf bbox;
std::vector<rectf> char_bboxes;
--
2.16.2
From cd18920922be95656a5e28a51850e40de56e15d7 Mon Sep 17 00:00:00 2001
From: Adam Reichold <adam.reich...@t-online.de>
Date: Sat, 3 Mar 2018 20:43:55 +0100
Subject: [PATCH 2/2] Use the detection idiom to use the proper struct stat
file for high-resolution mtime on Mac OS X.
---
goo/gfile.cc | 29 +++++++++++++++++++++++++++--
goo/gtypes.h | 9 +++++++++
utils/parseargs.h | 4 ++--
3 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/goo/gfile.cc b/goo/gfile.cc
index e4c9b9fb..6844ba81 100644
--- a/goo/gfile.cc
+++ b/goo/gfile.cc
@@ -65,6 +65,31 @@
#define PATH_MAX 1024
#endif
+namespace {
+
+template< typename Stat, typename = void_t<> >
+struct StatMtim
+{
+ static const struct timespec& value(const Stat& stbuf) {
+ return stbuf.st_mtim;
+ }
+};
+
+// Mac OS X uses a different field name than POSIX 7 this detects it.
+template< typename Stat >
+struct StatMtim< Stat, void_t< decltype ( Stat::st_mtimespec ) > >
+{
+ static const struct timespec& value(const Stat& stbuf) {
+ return stbuf.st_mtimespec;
+ }
+};
+
+inline const struct timespec& mtim(const struct stat& stbuf) {
+ return StatMtim< struct stat >::value(stbuf);
+}
+
+}
+
//------------------------------------------------------------------------
GooString *getCurrentDir() {
@@ -687,7 +712,7 @@ GooFile::GooFile(int fdA)
{
struct stat statbuf;
fstat(fd, &statbuf);
- modifiedTimeOnOpen = statbuf.st_mtim;
+ modifiedTimeOnOpen = mtim(statbuf);
}
bool GooFile::modificationTimeChangedSinceOpen() const
@@ -695,7 +720,7 @@ bool GooFile::modificationTimeChangedSinceOpen() const
struct stat statbuf;
fstat(fd, &statbuf);
- return modifiedTimeOnOpen.tv_sec != statbuf.st_mtim.tv_sec || modifiedTimeOnOpen.tv_nsec != statbuf.st_mtim.tv_nsec;
+ return modifiedTimeOnOpen.tv_sec != mtim(statbuf).tv_sec || modifiedTimeOnOpen.tv_nsec != mtim(statbuf).tv_nsec;
}
#endif // _WIN32
diff --git a/goo/gtypes.h b/goo/gtypes.h
index a8d45194..80d16066 100644
--- a/goo/gtypes.h
+++ b/goo/gtypes.h
@@ -49,4 +49,13 @@ typedef unsigned int Guint;
typedef unsigned long Gulong;
typedef long long Goffset;
+template< typename... >
+struct void_type
+{
+ using type = void;
+};
+
+template< typename... Args >
+using void_t = typename void_type< Args... >::type;
+
#endif
diff --git a/utils/parseargs.h b/utils/parseargs.h
index f035fa14..46a45f62 100644
--- a/utils/parseargs.h
+++ b/utils/parseargs.h
@@ -24,12 +24,12 @@
#ifndef PARSEARGS_H
#define PARSEARGS_H
+#include "goo/gtypes.h"
+
#ifdef __cplusplus
extern "C" {
#endif
-#include "goo/gtypes.h"
-
/*
* Argument kinds.
*/
--
2.16.2
signature.asc
Description: OpenPGP digital signature
_______________________________________________ poppler mailing list poppler@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/poppler
