Menu
▾
▴
1 Attachments
#354 build fixes for MinGW
This patch includes fixes to be able to build static and shared libraries using MinGW compiler.
configure.ac and Makefile.am are patched to add "-DDJVUAPI= -DDDJVUAPI= -DMINILISPAPI=" to the compiler flags if you're building a static library for MinGW (otherwise static builds aren't working).
JPEGDecoder.cpp has some strange redefinitions of basic Windows types that make the build fail on some compiler versions. I've put some preprocessor checks for MinGW target around it.
I've removed "-Wl,--export-all-symbols" from linker flags which requires the export of the classes GSafeFlags and GMonitor.
Here's a much better patch. It will use »__attribute__((visibility("default")))« if a GNU compiler is used which works well on MinGW and Linux targets.
Why the changes in JPEGDecoder.cpp?
The following change:
+libdjvulibre_la_CXXFLAGS += -fvisibility=hidden
expects option -fvisibility to work with all compilers.
This will break on any platform that uses makefiles but with
a compiler that either is not gcc or is an older version of gcc.
Think windows with an intel compiler for instance.
Annoying.
Does clang understand visibility as well?
The changes in JPEGDecoder are needed because the build will otherwise break for some people on MinGW32: https://github.com/mxe/mxe/issues/3012
Besides I really wonder what these strange redefines are even meant for. They're not needed for GCC Linux and MinGW so I guess they're some workaround for MSVC?
-fvisibility is known by clang but I guess you can make it optional. The user can always add it to the build flags themself.
INT32: see the comment on INT32 in
https://github.com/libjpeg-turbo/libjpeg-turbo/blob/ec32420f6b5dfa4e86883d42b209e8371e55aeb5/jmorecfg.h#L128C1-L134C1
Earlier versions of jpeglib did not guard this definition for windows.
But now we seem to have the opposite problem. I just pushed a change for
this
where I first include enough generic djvu files to make sure that
<windows.h>
is included and defines INT32, then include the JPEG file with INT32
defined away.</windows.h>
-fvisibility: This is a useful change for all gcc platforms, but one
that
will need to be tested on all platforms (of which the full list is
unknown...)
pthreads: What is the benefit of using MINGW's pthread emulation on
Windows?
Same thing again: it can make sense to give priority to pthread if
detected,
but will this work on all platforms?
On 2023-11-06 08:56, Marshall wrote:
My understanding is that many of these changes are motivated by the
desire to allow the compilation of a static libdjvu library, something
which was not supported on windows so far. However it is much more
important for me to make sure that the compilation of a dll version of
libdjvu works perfectly.
You write: "I've removed "-Wl,--export-all-symbols" from linker flags
which requires the export of the classes GSafeFlags and GMonitor." Just
to make sure, do you confirm that adding DJVUAPI on Monitor and
GSafeFlags is enough to make --export-all-symbols useless when compiling
libdjvu as a dll?
On 2023-11-06 14:21, Leon Bottou wrote:
You should also enable pthread checks for Windows because implementations are avaiable for MinGW compiler. And if pthread wasn't found then HAVE_PTHREAD won't be set.
Regarding -fvisibility: you could add compiler flag checks to configure.ac for these and other flags.
Last edit: Marshall 2023-11-06
This was at least the case during my tests, yes. But I will do some more tests.
Update: this isn't needed anymore it seems.
I've tested enabling pthreads because of a warning I got during compilation, coming from a line in miniexp.cpp:
The Windows code in there just seems very MSVC specific. Enabling the pthreads test in configure.ac should be okay as it will test if it's present and usable on that platform. Or maybe make the user specify to use pthreads with an configure option and otherwise fall back to default Win32 API (--enable-threads=posix|win32|auto|none). Currently the pthreads test is only disabled in configure.ac for Windows platform so nothing will change for other platforms at all.
Update:
You should consider replacing the AC_PATH_PTHREAD macro in configure.ac with AX_PTHREAD from autoconf-archive which seems to do a better job at detecting the correct flags.
Last edit: Marshall 2023-11-07
Now I remember this horror. The handling of thread local storage under windows is a catastrophe. The pthread code uses the same hack in fact. This used to work only for dlls and msvc, but they now have comments saying that it also works for static libs using recent versions of msvc, and they have other hacks for other compilers. They seem to have done a lot of back and forth between different solutions.
The memory leak only occurs when a program calls the libdjvu miniexp functions from threads different from the main thread. Since nothing guarantees that programs doing so will create these threads using the same pthreads library, having the problem solved for pthreads does not mean much. In fact the worst cases happen when the calling program and libdjvu use different versions of pthreads, which may manage this problem in different ways.
Related
Bugs: #354
Is it better to use the MSCV code for MinGW? Or does it really make no difference?
The primary use case for the static library is to build and link everything to a single exe binary using the same MinGW compiler/toolchain, so the chance that a different pthread library is used should be low.
08.11.2023 16:28:51 Leon Bottou leonb@users.sourceforge.net:
Related
Bugs: #354