libvips / build-win64-mxe

76 stars 15 forks source link

v8.11.2: possible lcms2 problems with 32-bit static build #31

Closed lovell closed 3 years ago

lovell commented 3 years ago

I'm seeing failures using the v8.11.2 binaries with sharp on 32-bit Windows for all tests relating to colour profiles:

icc_transform: unable to load or find any compatible input profile

https://ci.appveyor.com/project/lovell/sharp/builds/39989275/job/ynmhkpqnmfx4nv7g

64-bit Windows works as-expected:

https://github.com/lovell/sharp/runs/3066815431?check_suite_focus=true

I'm still investigating this, but opening an early issue here in case anyone else is experiencing similar problems.

Could the vips-dev-w32-web-8.11.2-static.zip binaries have missed or skipped the lcms dependency, perhaps as a result of the thread-safety patches?

https://github.com/libvips/build-win64-mxe/releases/tag/v8.11.2

kleisauke commented 3 years ago

I could reproduce this, will investigate further. libvips was successfully built against lcms in the 32-bit binaires.

PS> vips.exe --vips-config | Select-String "lcms"
ICC profile support with lcms: yes (lcms2)

Given that the 32-bit binaries from the v8.11.0-rc1 release were fine, I think this could indeed have been caused by the thread-safety patches introduced in PR #28. I noticed this within the build log:

checking for gmtime_r... no
checking for _gmtime64_s... yes
<snip>
/data/mxe/tmp-lcms-i686-w64-mingw32.static.posix.web/lcms2-2.12/src/cmsplugin.c:1005:26: warning: incompatible pointer types passing 'time_t *' (aka 'long *') to parameter of type 'const __time64_t *' (aka 'const long long *') [-Wincompatible-pointer-types]
    t = _gmtime64_s(&tm, &now) == 0 ? &tm : NULL;
                         ^~~~
/data/mxe/usr/i686-w64-mingw32.static.posix.web/mingw/include/time.h:166:73: note: passing argument to parameter '_Time' here
  _SECIMP errno_t __cdecl _gmtime64_s (struct tm *_Tm,const __time64_t *_Time);
                                                                        ^

(it will probably have to use the _gmtime32_s function on 32-bit Windows)

kleisauke commented 3 years ago

I'm testing this patch now:

Details ```diff From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Kleis Auke Wolthuizen Date: Wed, 14 Jul 2021 16:20:00 +0200 Subject: [PATCH 3/3] Prefer to use gmtime_s instead diff --git a/configure.ac b/configure.ac index 1111111..2222222 100644 --- a/configure.ac +++ b/configure.ac @@ -81,8 +81,21 @@ AX_APPEND_COMPILE_FLAGS(["-fvisibility=hidden"]) # Motorola and SPARC CPUs), define `WORDS_BIGENDIAN'. AC_C_BIGENDIAN -# Check for functions that some compilers lack (or name differently) -AC_CHECK_FUNCS([gmtime_r _gmtime64_s]) +# Check for threadsafe variants of gmtime +# Note: check for gmtime_s is a bit more complex as it is implemented as a macro +AC_CHECK_FUNCS(gmtime_r, [], [ + AC_MSG_CHECKING([for gmtime_s]) + AC_LINK_IFELSE( + [AC_LANG_PROGRAM([[#include ]], [[ + time_t t; + struct tm m; + gmtime_s(&m, &t); + return 0; + ]])], + [AC_MSG_RESULT([yes]) + AC_DEFINE([HAVE_GMTIME_S], [1], [gmtime_s can be used])], + [AC_MSG_RESULT([no])] + )]) # Point to JPEG installed in DIR or disable JPEG with --without-jpeg. AC_ARG_WITH(jpeg, diff --git a/src/cmsplugin.c b/src/cmsplugin.c index 1111111..2222222 100644 --- a/src/cmsplugin.c +++ b/src/cmsplugin.c @@ -993,7 +993,7 @@ void* CMSEXPORT cmsGetContextUserData(cmsContext ContextID) cmsBool _cmsGetTime(struct tm* ptr_time) { struct tm* t; -#if defined(HAVE_GMTIME_R) || defined(HAVE__GMTIME64_S) +#if defined(HAVE_GMTIME_R) || defined(HAVE_GMTIME_S) struct tm tm; #endif @@ -1001,8 +1001,8 @@ cmsBool _cmsGetTime(struct tm* ptr_time) #ifdef HAVE_GMTIME_R t = gmtime_r(&now, &tm); -#elif defined(HAVE__GMTIME64_S) - t = _gmtime64_s(&tm, &now) == 0 ? &tm : NULL; +#elif defined(HAVE_GMTIME_S) + t = gmtime_s(&tm, &now) == 0 ? &tm : NULL; #else _cmsEnterCriticalSectionPrimitive(&_cmsContextPoolHeadMutex); t = gmtime(&now); ```
kleisauke commented 3 years ago

Seems to work! Upstream PR: https://github.com/mm2/Little-CMS/pull/270.

kleisauke commented 3 years ago

Commit 9ea5c2a7d17ed58be193e23f88d3562a60e49793 resolves this, this will be in the forthcoming v8.11.3 release (or if preferred, I could make a v8.11.2-build2 release).

lovell commented 3 years ago

Thanks Kleis, let's wait for v8.11.3.

kleisauke commented 3 years ago

The libvips v8.11.3 Windows binaries are now available with this fix included.