And this time around, without a bunch of typos... Sorry for the noise. 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/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 <[email protected]> on behalf of >>>>>> Jeroen Ooms <[email protected]> Sent: Sunday, February 18, 2018 6:29 >>>>>> PM >>>>>> To: Ihar Filipau >>>>>> Cc: [email protected] >>>>>> 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 <[email protected]> >> wrote: >>>>>>> On 2/12/18, Jeroen Ooms <[email protected]> wrote: >>>>>>>> On Sun, Feb 11, 2018 at 12:11 PM, Albert Astals Cid <[email protected]> >> 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 >>>>> [email protected] >>>>> https://lists.freedesktop.org/mailman/listinfo/poppler >>> >>> _______________________________________________ >>> poppler mailing list >>> [email protected] >>> https://lists.freedesktop.org/mailman/listinfo/poppler >> >> >> >> >> >> >> >> _______________________________________________ >> poppler mailing list >> [email protected] >> https://lists.freedesktop.org/mailman/listinfo/poppler
From 32d8a5a43acb9c655215a1fcd10a09c4c619264d Mon Sep 17 00:00:00 2001 From: Adam Reichold <[email protected]> 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 0d370fb543182639b1d6a75c445894c6899ca5b5 Mon Sep 17 00:00:00 2001
From: Adam Reichold <[email protected]>
Date: Sat, 3 Mar 2018 20:43:55 +0100
Subject: [PATCH 2/2] Use the detection idiom to use the proper struct stat
field 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..8b780e11 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 and 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 [email protected] https://lists.freedesktop.org/mailman/listinfo/poppler
