Opened 3 months ago

Closed 2 months ago

#69455 closed defect (fixed)

poppler @24.03.0: build failure on macOS 10.14 and earlier due to use of std::filesystem

Reported by: lukaso (Lukas Oberhuber) Owned by: dbevans (David B. Evans)
Priority: Normal Milestone:
Component: ports Version: 2.8.1
Keywords: tiger leopard snowleopard lion mountainlion mavericks yosemite elcapitan sierra highsierra mojave Cc: mascguy (Christopher Nielsen), tehcog (tehcog), thetrial (alabay)
Port: poppler

Description (last modified by lukaso (Lukas Oberhuber))

:info:build cd /Users/distiller/macports-gimp2-x86_64/var/macports/build/_Users_distiller_macports-gimp2-x86_64_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_graphics_poppler/poppler/work/build/utils && /Users/distiller/macports-gimp2-x86_64/bin/cmake -E cmake_link_script CMakeFiles/pdfdetach.dir/link.txt --verbose=ON
:info:build /usr/bin/clang++ -Wall -Wextra -Wpedantic -Wno-unused-parameter -Wcast-align -Wformat-security -Wframe-larger-than=65536 -Wmissing-format-attribute -Wnon-virtual-dtor -Woverloaded-virtual -Wmissing-declarations -Wundef -Wzero-as-null-pointer-constant -Wshadow -Wweak-vtables -fno-exceptions -fno-check-new -fno-common -D_DEFAULT_SOURCE -O2 -g -pipe -Os -DNDEBUG -stdlib=libc++ -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX10.13.sdk -arch x86_64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.13.sdk -mmacosx-version-min=10.13 -Wl,-search_paths_first -Wl,-headerpad_max_install_names -Wl,-headerpad_max_install_names -liconv -Wl,-syslibroot,/Library/Developer/CommandLineTools/SDKs/MacOSX10.13.sdk CMakeFiles/pdfdetach.dir/parseargs.cc.o CMakeFiles/pdfdetach.dir/Win32Console.cc.o CMakeFiles/pdfdetach.dir/pdfdetach.cc.o -o pdfdetach  -Wl,-rpath,/Users/distiller/macports-gimp2-x86_64/lib ../libpoppler.135.0.0.dylib 
:info:build Undefined symbols for architecture x86_64:
:info:build   "std::__1::__fs::filesystem::path::__filename() const", referenced from:
:info:build       std::__1::enable_if<__is_pathable<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >::value, std::__1::__fs::filesystem::path&>::type std::__1::__fs::filesystem::path::append<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) in pdfdetach.cc.o
:info:build   "std::__1::__fs::filesystem::path::lexically_normal() const", referenced from:
:info:build       _main in pdfdetach.cc.o
:info:build   "std::__1::__fs::filesystem::__current_path(std::__1::error_code*)", referenced from:
:info:build       _main in pdfdetach.cc.o
:info:build ld: symbol(s) not found for architecture x86_64
:info:build clang: error: linker command failed with exit code 1 (use -v to see invocation)

Attachments (2)

poppler.log (685.3 KB) - added by lukaso (Lukas Oberhuber) 3 months ago.
build log
HighSierrra-GCC13.log (643.3 KB) - added by rmottola (Riccardo) 2 months ago.
gcc 13 on 10.13

Download all attachments as: .zip

Change History (30)

Changed 3 months ago by lukaso (Lukas Oberhuber)

Attachment: poppler.log added

build log

comment:1 Changed 3 months ago by lukaso (Lukas Oberhuber)

Description: modified (diff)

comment:2 Changed 3 months ago by lukaso (Lukas Oberhuber)

Last edited 3 months ago by ryandesign (Ryan Carsten Schmidt) (previous) (diff)

comment:3 Changed 3 months ago by ryandesign (Ryan Carsten Schmidt)

Cc: mascguy added
Keywords: tiger leopard snowleopard lion mountainlion mavericks yosemite elcapitan sierra highsierra mojave added
Owner: set to dbevans
Status: newassigned
Summary: poppler fails to build on 10.13poppler @24.03.0: build failure on macOS 10.14 and earlier due to use of std::filesystem
Version: 2.8.1

I would guess those missing symbols are part of std::filesystem which is only available in the version of libc++ that comes with macOS 10.15 and later.

A workaround might be to have poppler use the macports-libcxx port.

comment:4 Changed 3 months ago by lukaso (Lukas Oberhuber)

Would it be possible for the change be reverted until a better solution is found?

comment:5 in reply to:  4 Changed 3 months ago by barracuda156

Replying to lukaso:

Would it be possible for the change be reverted until a better solution is found?

Does legacysupport work though?

  1. S. It will probably build fine with GCC, but then you need to use libstdc++.

comment:6 Changed 2 months ago by rmottola (Riccardo)

on 10.13 High Sierra I tried building with GCC 13. I attach the build log.

It appears to fail here, on a convertible thing, different from filesystem

:info:build In file included from /opt/local/libexec/gcc13/libc++/include/c++/v1/__type_traits/common_reference.h:16,
:info:build                  from /opt/local/libexec/gcc13/libc++/include/c++/v1/type_traits:434,
:info:build                  from /opt/local/libexec/gcc13/libc++/include/c++/v1/algorithm:1710,
:info:build                  from /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_graphics_poppler/poppler/work/poppler-24.03.0/goo/GooString.cc:42:
:info:build /opt/local/libexec/gcc13/libc++/include/c++/v1/__type_traits/is_convertible.h:71:8: error: expected identifier before '__is_convertible'
:info:build    71 | struct __is_convertible
:info:build       |        ^~~~~~~~~~~~~~~~
:info:build /opt/local/libexec/gcc13/libc++/include/c++/v1/__type_traits/is_convertible.h:71:8: error: expected unqualified-id before '__is_convertible'
:info:build /opt/local/libexec/gcc13/libc++/include/c++/v1/__type_traits/is_convertible.h:77:40: error: expected identifier before '__is_convertible'
:info:build    77 | template <class _T1, class _T2> struct __is_convertible<_T1, _T2, 0, 1> : public false_type {};
:info:build       |                                        ^~~~~~~~~~~~~~~~
:

Changed 2 months ago by rmottola (Riccardo)

Attachment: HighSierrra-GCC13.log added

gcc 13 on 10.13

comment:7 Changed 2 months ago by kencu (Ken)

using gcc to build against libc++ is highly experimental.

Iain has reported the libc++ headers do not work correctly at present for this purpose, and a set of modified libc++ headers will be needed. These don’t currently exist.

not to mention that the system libc++ won’t have filesystem support in it anyway, so you’re no further ahead.

building against macports-libcxx, if it works properly, is likely the only hope for libc++ systems.

comment:8 Changed 2 months ago by lukaso (Lukas Oberhuber)

Does legacysupport work though?

I'm not able to use legacy support because I cross compile on a recent version of the OS against the older SDK. Legacy support doesn't work in this scenario.

comment:9 in reply to:  8 Changed 2 months ago by kencu (Ken)

Replying to lukaso:

Does legacysupport work though?

I'm not able to use legacy support because I cross compile on a recent version of the OS against the older SDK. Legacy support doesn't work in this scenario.

make an overlay repo rolling poppler back until this gets sorted out?

comment:10 Changed 2 months ago by lukaso (Lukas Oberhuber)

comment:11 Changed 2 months ago by lukaso (Lukas Oberhuber)

make an overlay repo rolling poppler back until this gets sorted out?

Yes. Unfortunately, because GIMP has *so many* dependencies, if I want to keep the build running, I'm constantly doing overlays. That ends up being most of the maintenance work.

Philosophically, I'm also of the opinion (though I'm not a macports maintainer) that broken code shouldn't be left on the master branch. So I'm not sure why that's considered the best solution. Also, isn't that what the -devel ports are for?

But I understand how hard it is to maintain all this stuff, so I get it. Not trying to dunk on anyone.

comment:12 Changed 2 months ago by kencu (Ken)

macports officially supports the last three systems, but tries to support older darwin systems. It’s harder when software evolves past what those systems can provide.

poppler code isn’t broken…it wants to use filesystem. Current systems provide that. Users of current systems want a current poppler.

Either macports-libcxx will work out, or there will need to be fallbacks placed in the port to use older popplers on older systems.

There are also some other replacements for filesystem that might be interesting to look into, eg

https://github.com/gulrak/filesystem

comment:13 in reply to:  10 Changed 2 months ago by tomio-arisaka (Tomio Arisaka)

Replying to lukaso:

Here's the code that causes the problem: https://gitlab.freedesktop.org/poppler/poppler/-/commit/d169ac132d36ba1459000a27cada084e1633859a

Yes, and the commit is not a bug-fix.

So I made a patch which reverts the commit in order to build poppler-24.03.0 on macOS 10.13. It works well for me.

$ git clone https://gitlab.freedesktop.org/poppler/poppler.git
$ 
$ cd ./poppler/
$ 
$ git diff -p --raw d169ac132d36ba1459000a27cada084e1633859a d8e7a8a9dd4ddd642c35f5fd1538396be2a68aa9 utils/pdfdetach.cc

comment:14 in reply to:  12 Changed 2 months ago by tomio-arisaka (Tomio Arisaka)

Replying to kencu:

There are also some other replacements for filesystem that might be interesting to look into, eg

https://github.com/gulrak/filesystem

MacPorts includes ghc-filesystem:

$ port info ghc-filesystem
ghc-filesystem @1.5.14 (devel)
Variants:             debug, tests

Description:          An implementation of C++17 std::filesystem for
                      C++11/C++14/C++17/C++20 on Windows, macOS, Linux and
                      FreeBSD.
Homepage:             https://github.com/gulrak/filesystem

Build Dependencies:   cmake
Platforms:            any
License:              MIT
Maintainers:          none

So I tried to build Poppler-24.03.0 with ghc-filesystem. My example is as follows:

$ diff -u Portfile.orig Portfile
--- Portfile.orig	2024-03-06 04:56:12.000000000 +0900
+++ Portfile	2024-03-10 05:30:00.000000000 +0900
@@ -9,7 +9,7 @@
 conflicts           poppler-devel xpdf-tools
 set my_name         poppler
 version             24.03.0
-revision            0
+revision            2
 
 categories          graphics
 license             GPL-2+
@@ -69,6 +69,17 @@
 compiler.c_standard   2011
 compiler.thread_local_storage yes
 
+if {${os.major} < 19} {
+    # change std::filesystem to ghc::filesystem in order to avoid an error
+    depends_build-append \
+        port:ghc-filesystem
+    patchfiles-append \
+        patch-utils_pdfdetach.cc.diff
+    post-extract {
+        system -W ${worksrcpath} "cp -fRp ${prefix}/include/ghc ./utils/"
+    }
+}
+
 # match clang, not strequal
 patchfiles-append   patch-cmake_modules_PopplerMacros.cmake.diff
 
$ cat ./files/patch-utils_pdfdetach.cc.diff 
--- utils/pdfdetach.cc.orig	2024-03-04 02:30:22.000000000 +0900
+++ utils/pdfdetach.cc	2024-03-10 05:20:20.000000000 +0900
@@ -45,7 +45,17 @@
 #include "Error.h"
 #include "Win32Console.h"
 
+#if __MAC_OS_X_VERSION_MIN_REQUIRED && __MAC_OS_X_VERSION_MIN_REQUIRED < 101500
+    #include "ghc/filesystem.hpp"
+    namespace fs {
+        using namespace ghc::filesystem;
+        using ifstream = ghc::filesystem::ifstream;
+        using ofstream = ghc::filesystem::ofstream;
+        using fstream = ghc::filesystem::fstream;
+    }
+#else
 #include <filesystem>
+#endif
 
 static bool doList = false;
 static int saveNum = 0;
@@ -89,6 +99,7 @@
     const GooString *s1;
     Unicode u;
     bool isUnicode;
+    std::error_code ec;
 
     Win32Console win32Console(&argc, &argv);
 
@@ -194,9 +205,9 @@
 
         // save all embedded files
     } else if (saveAll) {
-        std::filesystem::path basePath = savePath;
+        ghc::filesystem::path basePath = savePath;
         if (basePath.empty()) {
-            basePath = std::filesystem::current_path();
+            basePath = ghc::filesystem::current_path(ec);
         }
         basePath = basePath.lexically_normal();
 
@@ -230,7 +241,7 @@
             if (filename.empty()) {
                 return 3;
             }
-            std::filesystem::path filePath = basePath;
+            ghc::filesystem::path filePath = basePath;
             filePath = filePath.append(filename).lexically_normal();
 
             if (filePath.generic_string().find(basePath.generic_string()) != 0) {
@@ -292,8 +303,8 @@
                 targetPath.append(uBuf, n);
             }
 
-            const std::filesystem::path basePath = std::filesystem::current_path().lexically_normal();
-            std::filesystem::path filePath = basePath;
+            const ghc::filesystem::path basePath = ghc::filesystem::current_path(ec).lexically_normal();
+            ghc::filesystem::path filePath = basePath;
             filePath = filePath.append(targetPath).lexically_normal();
 
             if (filePath.generic_string().find(basePath.generic_string()) != 0) {

It works well for me.

comment:15 Changed 2 months ago by kencu (Ken)

using ghc filesystem looks pretty tempting…

I am no c++ namespace expert, but is there some way that the build can substitute ghc::filesystem calls for std::filesystem calls transparently, ie without patching every call?

ghc filesystem upstream I guess would be the place I might better ask this…

comment:16 Changed 2 months ago by jrandall814

OS 10.13.6, MacPorts 2.9.1

poppler 24.3.0_0 fails to build for me as well, but I am unsure that the cause is the same one that's being treated in this ticket. The key passage in my log seems to be this one:

:info:build /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_release_tarballs_ports_graphics_poppler/poppler/work/poppler-24.03.0/fofi/FoFiType1.cc:31:10: fatal error: 'charconv' file not found
:info:build #include <charconv>
Last edited 2 months ago by ryandesign (Ryan Carsten Schmidt) (previous) (diff)

comment:17 Changed 2 months ago by kencu (Ken)

I think the idea with the namespace is to replace all the std::filesystem:: parts in the code with fs::

comment:18 in reply to:  16 Changed 2 months ago by kencu (Ken)

Replying to jrandall814:

OS 10.13.6, MacPorts 2.9.1

fatal error: 'charconv' file not found

Yes, that’s a separate issue from this ticket, being tracked here #69200.

Last edited 2 months ago by ryandesign (Ryan Carsten Schmidt) (previous) (diff)

comment:19 in reply to:  15 Changed 2 months ago by kencu (Ken)

Replying to kencu:

ghc filesystem upstream I guess would be the place I might better ask this…

https://github.com/gulrak/filesystem/issues/180

comment:20 in reply to:  17 Changed 2 months ago by tomio-arisaka (Tomio Arisaka)

Replying to kencu:

I think the idea with the namespace is to replace all the std::filesystem:: parts in the code with fs::

Yes, I think so.

My example needs to be cleaned up :) But I am not going to make a pull-request.

By the way, ghc::filesystem::current_path() is not provided. So I have to substitute ghc::filesystem::current_path(std::error_code& ec) for std::filesystem::current_path().

comment:21 Changed 2 months ago by tehcog (tehcog)

Cc: tehcog added

comment:22 in reply to:  12 ; Changed 2 months ago by erikbs

My fix proposal: https://github.com/macports/macports-ports/pull/23035

Poppler builds just fine (and works) when I compile with Clang 16 (chosen automatically because Clang 3.5 is blacklisted) and link against macports-libcxx on OS X 10.9.

I do not see any reason to turn to third-party implementations of standard library functionality when macports-libcxx seems to fix the problem? The port already links the legacy-support library, so I assume that one extra line in the Portfile (legacysupport.use_mp_libcxx yes) is better than patching the source code to use another filesystem library?

comment:23 Changed 2 months ago by kencu (Ken)

using macports-libcxx has pluses and minuses to it, but certainly is the easiest fix for Intel systems that can use clang and only want to run on the OS they are building on.

the potential advantage of ghc-filesystem is that Lukas could build that on a newer system and run it on an older system, as he wants to do... however, that approach to building software with Macports is pretty much doomed without a lot more effort to make it work right anyway, so no great loss if he can't do that.

comment:24 Changed 2 months ago by thetrial (alabay)

Cc: thetrial added

comment:25 Changed 2 months ago by thetrial (alabay)

I tried to build poppler with clang-16 and clang-17 under macOS 10.12, both failed. But I did not understand the macports-libcxx linking thing?

comment:26 in reply to:  25 Changed 2 months ago by erikbs

Replying to thetrial:

But I did not understand the macports-libcxx linking thing?

Putting that statement in the Portfile makes MacPorts insert a few compile/link options so that Clang links against /opt/local/lib/libcxx/libc++.dylib (updated libc++ provided through MacPorts) instead of /usr/lib/libc++.dylib (provided by the OS, will be whatever version came with the OS). Compiling with Clang 16/17 is in itself not enough, because, like older Clang versions, these newer versions will also link against the libc++ version in /usr/lib by default, and it lacks std::filesystem support on 10.14 and older.

comment:27 in reply to:  22 Changed 2 months ago by ewenmcneill (Ewen McNeill)

Replying to erikbs:

My fix proposal: https://github.com/macports/macports-ports/pull/23035 [...] The port already links the legacy-support library, so I assume that one extra line in the Portfile (legacysupport.use_mp_libcxx yes) is better than patching the source code to use another filesystem library?

Thanks for this PR proposal. I manually applied that change to my local port file on macOS 10.14 (sudo port edit poppler), and poppler built successfully from source. Which is handy because a bunch of other ports depend on poppler (and want to pull in a later poppler :-/ )

To me too this seems like the cleanest work around for poppler wanting newer C++ features than macOS 10.14 shipped with.

Ewen

comment:28 Changed 2 months ago by erikbs

Resolution: fixed
Status: assignedclosed

In b6d032d7beea0f04f754042ac59e5a9f8a375915/macports-ports (master):

poppler,poppler-devel: use MP libc++ on legacy

Fixes: #69455

Note: See TracTickets for help on using tickets.