Opened 9 years ago

Closed 9 years ago

Last modified 3 years ago

#24067 closed defect (fixed)

zlib 1.2.4 causes build failures because off64_t is not defined on Mac OS X

Reported by: gellule.xg@… Owned by: ryandesign (Ryan Schmidt)
Priority: Normal Milestone:
Component: ports Version:
Keywords: Cc: jknockaert@…, silver_samurai@…, jschwab@…, dershow, easye, bgschaid@…, mf2k (Frank Schima), nox@…, rcw3@…, macports@…, jo.net@…, mkae (Marko Käning), cjones051073 (Chris Jones), king.tobo@…, sck@…, jsg72@…, dsturnbull@…, diegotheblind+macports@…, lang@…, mike.whitlaw@…, ben-macports@…, mydogis@…, lars@…, clusty1@…, csumma@…, dmz@…, Russell-Jones-OxPhys (Russell Jones), pixilla (Bradley Giesbrecht), social.thiago@…
Port: zlib

Description

The newest version of zlib expect off64_t to de available when _LARGEFILE64_SOURCE is defined. This is not the case on OS X.

Attachments (6)

patch-largefile64.diff (850 bytes) - added by gellule.xg@… 9 years ago.
Portfile.diff (631 bytes) - added by jmroot (Joshua Root) 9 years ago.
zlib.h.diff (946 bytes) - added by jmroot (Joshua Root) 9 years ago.
zconf.h.diff (264 bytes) - added by msm@… 9 years ago.
add include for dtrace.h to zconf.h
Portfile (1.9 KB) - added by gellule.xg@… 9 years ago.
Portfile with patch-zconf.h.in.diff patch and patchzlib variant set as default
patch-zconf.h.in.diff (268 bytes) - added by gellule.xg@… 9 years ago.
The patch itself…

Download all attachments as: .zip

Change History (64)

comment:1 Changed 9 years ago by gellule.xg@…

Ports that use zlib 1.2.4 will fail through the inclusion of zlib.h. I'm attaching a patch that adds a check for __APPLE__ and changes the zlib API to use off_t instead of off64_t.

Changed 9 years ago by gellule.xg@…

Attachment: patch-largefile64.diff added

comment:2 Changed 9 years ago by jknockaert@…

Cc: jknockaert@… added

Cc Me!

comment:3 Changed 9 years ago by silver_samurai@…

Cc: silver_samurai@… added

Cc Me!

comment:4 Changed 9 years ago by jschwab@…

Cc: jschwab@… added

Cc Me!

comment:5 Changed 9 years ago by dershow

Cc: dersh@… added

Cc Me!

comment:6 Changed 9 years ago by gellule.xg@…

Actually I was wrong. A message from Mark Adler (from zlib) suggests another direction related to ghostscript (#24061). A quote from him: "I replicated exactly that error if I add a -D_LARGEFILE64_SOURCE to the compile options when making zlib. All you need to do is find where the ghostscript source does that and kill it."

This bug report might need to be closed as "not a defect".

comment:7 Changed 9 years ago by ryandesign (Ryan Schmidt)

Owner: changed from macports-tickets@… to ryandesign@…
Status: newassigned
Summary: off64_t is not defined on OS Xzlib 1.2.4 causes build failures because off64_t is not defined on Mac OS X

But we are seeing the same failure with ghostscript, mplayer-devel, and VLC so far.

comment:8 Changed 9 years ago by gellule.xg@…

I read a little about _LARGEFILE64_SOURCE there: http://www.gnu.org/software/libc/manual/html_mono/libc.html#index-g_t_005fLARGEFILE64_005fSOURCE-48 It looks like setting _LARGEFILE64_SOURCE should make off64_t available. Both mplayer and VLC turn it on but never use off64_t. They actually also use _FILE_OFFSET_BITS=64 which makes off_t (and related) 64 bits directly. To me it looks like both project moved on to a 64bits off_t and should not set _LARGEFILE64_SOURCE anymore.

comment:9 Changed 9 years ago by gellule.xg@…

Removing _LARGEFILE64_SOURCE from the VLC build fixes it. I could attach a patch, but here is not the right location. Should I open a VLC ticket? I'll look into mplayer, but its seems that getting rid of _LARGEFILE64_SOURCE would fix the issue as well.

comment:10 Changed 9 years ago by ryandesign (Ryan Schmidt)

Sure, open a ticket for VLC, and for mplayer-devel and any other ports where we find this problem.

comment:11 Changed 9 years ago by ryandesign (Ryan Schmidt)

qt4-mac: #24072

comment:12 Changed 9 years ago by easye

Cc: easieste@… added

Cc Me!

comment:13 Changed 9 years ago by ryandesign (Ryan Schmidt)

ghostscript: #24061
MPlayer, mplayer-devel: #24079
VLC, VLC09: #24082

comment:14 Changed 9 years ago by bgschaid@…

Cc: bgschaid@… added

Cc Me!

comment:15 Changed 9 years ago by mf2k (Frank Schima)

Cc: macsforever2000@… added

Cc Me!

comment:16 Changed 9 years ago by nox@…

Cc: nox@… added
Version: 1.8.2

We should do either what has been said in the first comment, or patch zlib to use uint64_t instead of off64_t

comment:17 Changed 9 years ago by ryandesign (Ryan Schmidt)

But the statement in comment 6 from Mark Adler, co-author of zlib, seems to suggest this is a bug in other software, not a bug in zlib. If you believe a change needs to be made in zlib, I'd like to hear confirmation from the developers of zlib that the change is the correct solution to the problem.

comment:18 Changed 9 years ago by ryandesign (Ryan Schmidt)

gnome-vfs: #24083

comment:19 Changed 9 years ago by rcw3@…

Cc: rcw3@… added

Cc Me!

comment:20 Changed 9 years ago by macports@…

Cc: macports@… added

Cc Me!

comment:21 Changed 9 years ago by jmroot (Joshua Root)

The other software may technically be doing the wrong thing, but these functions (and off64_t) are never defined on OS X, so we really should just add a check for !defined(__APPLE__) or something to zlib.h.

Changed 9 years ago by jmroot (Joshua Root)

Attachment: Portfile.diff added

Changed 9 years ago by jmroot (Joshua Root)

Attachment: zlib.h.diff added

comment:22 Changed 9 years ago by ryandesign (Ryan Schmidt)

Thanks Joshua. As the number of ports discovered that have problems with zlib 1.2.4 continues to grow, I'd be happier with a solution like this that can be implemented at the source of the problem. I'll test and commit this shortly, and consider all the other tickets duplicates of this one at this point.

comment:23 Changed 9 years ago by msm@…

i experienced this problem as well, but i found that off64_t is defined on osx in sys/dtrace.h. here is the solution i arrived at:

--- zconf.h.orig	2010-03-16 23:57:38.000000000 -0400
+++ zconf.h	2010-03-17 08:49:15.000000000 -0400
@@ -377,6 +377,9 @@
 
 #ifdef _LARGEFILE64_SOURCE
 #  include <sys/types.h>
+#if defined(__APPLE__)
+#  include <sys/dtrace.h>
+#endif
 #endif
 
 #ifndef SEEK_SET

comment:24 in reply to:  23 Changed 9 years ago by pacobell@…

Replying to msm@…: Yes, this seems to be the most correct and elegant approach. Kudos!

Changed 9 years ago by msm@…

Attachment: zconf.h.diff added

add include for dtrace.h to zconf.h

comment:25 Changed 9 years ago by jo.net@…

Cc: jo.net@… added

Cc Me!

comment:26 Changed 9 years ago by gellule.xg@…

I would not include sys/dtrace.h, it's not really related to the off64_t problem. I agree that zconf.h could be a better location for a patch and I would add something like:

+#  if defined(__APPLE__)
+#    define off64_t off_t
+#  endif

since off_t is already 64 bits.

comment:27 Changed 9 years ago by mkae (Marko Käning)

Cc: MK-MacPorts@… added

Cc Me!

comment:28 Changed 9 years ago by gellule.xg@…

Also, a "zconf.h" patch should actually be applied to zconf.h.in (processed by configure into zconf.h) as follows:

--- zconf.h.in.orig     2010-03-17 10:32:37.000000000 -1000
+++ zconf.h.in  2010-03-17 10:33:13.000000000 -1000
@@ -377,6 +377,9 @@
 
 #ifdef _LARGEFILE64_SOURCE
 #  include <sys/types.h>
+#  ifdef __APPLE__
+#    define off64_t off_t
+#  endif
 #endif
 
 #ifndef SEEK_SET

comment:29 in reply to:  28 Changed 9 years ago by msm@…

would it be worth it to replicate the original typedef to ensure the size of the container?

e.g. the define line would become

 +#  ifdef __APPLE__
 +typedef int64_t off64_t
 +#  endif

comment:30 Changed 9 years ago by cjones051073 (Chris Jones)

Cc: jonesc@… added

Cc Me!

comment:31 Changed 9 years ago by king.tobo@…

Cc: king.tobo@… added

Cc Me!

comment:32 Changed 9 years ago by sck@…

Cc: sck@… added

Cc Me!

comment:33 Changed 9 years ago by jsg72@…

Cc: jsg72@… added

Cc Me!

comment:34 Changed 9 years ago by dsturnbull@…

Cc: dsturnbull@… added

Cc Me!

comment:35 in reply to:  28 Changed 9 years ago by andre.david@…

Replying to gellule.xg@…:

Also, a "zconf.h" patch should actually be applied to zconf.h.in (processed by configure into zconf.h) as follows:

--- zconf.h.in.orig     2010-03-17 10:32:37.000000000 -1000 
+++ zconf.h.in  2010-03-17 10:33:13.000000000 -1000 
@@ -377,6 +377,9 @@ 
 
 #ifdef _LARGEFILE64_SOURCE 
 #  include <sys/types.h> 
+#  ifdef !__APPLE!__ 
+#    define off64_t off_t 
+#  endif 
 #endif 
 
 #ifndef SEEK_SET

This allows me to continue building qt4-mac.

ps - I noticed that there is a zconf.h.in and a zconf.h.cmakein. In my case patching the former was enough.

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

comment:36 Changed 9 years ago by diegotheblind+macports@…

Cc: diegotheblind+macports@… added

Cc Me!

comment:37 Changed 9 years ago by ryandesign (Ryan Schmidt)

There are too many suggestions in this bug report for me to know what to do. I have asked the developers of zlib for assistance.

comment:38 Changed 9 years ago by lang@…

Cc: lang@… added

Cc Me!

comment:39 Changed 9 years ago by mike.whitlaw@…

Cc: mike.whitlaw@… added

Cc Me!

comment:40 Changed 9 years ago by ben-macports@…

Cc: ben-macports@… added

Cc Me!

comment:41 Changed 9 years ago by mydogis@…

Cc: mydogis@… added

Cc Me!

comment:42 Changed 9 years ago by lars@…

Cc: lars@… added

Cc Me!

comment:43 Changed 9 years ago by ryandesign (Ryan Schmidt)

kdelibs3: #24064

comment:44 Changed 9 years ago by ryandesign (Ryan Schmidt)

openmpi: #24126

comment:45 Changed 9 years ago by ryandesign (Ryan Schmidt)

kdelibs4: #24128

comment:46 Changed 9 years ago by clusty1@…

Cc: clusty1@… added

Cc Me!

comment:47 Changed 9 years ago by clusty1@…

Cc: clusty1@… removed

Cc Me!

comment:48 Changed 9 years ago by clusty1@…

Cc: clusty1@… added

Cc Me!

comment:49 Changed 9 years ago by csumma@…

Cc: csumma@… added

Cc Me!

comment:50 Changed 9 years ago by dmz@…

Cc: dmz@… added

Cc Me!

comment:51 Changed 9 years ago by Russell-Jones-OxPhys (Russell Jones)

Cc: russell.jones@… added

Cc Me!

comment:52 Changed 9 years ago by pixilla (Bradley Giesbrecht)

Cc: brad@… added

Cc Me!

comment:53 in reply to:  37 Changed 9 years ago by ryandesign (Ryan Schmidt)

Replying to ryandesign@…:

There are too many suggestions in this bug report for me to know what to do. I have asked the developers of zlib for assistance.

Mark Adler continues to state that we should not patch zlib, but that the error should be corrected in each program experiencing this error.

Mark Adler wrote:

I recommend letting those package maintainers know about the error and the correct solution, which is to use _FILE_OFFSET_BITS=64 instead of _LARGEFILE64_SOURCE. This error may cause other problems later with those packages. In general it is far better to correct errors at the source than to workaround errors elsewhere. And the fix is simple.

comment:54 Changed 9 years ago by gellule.xg@…

Then why don't we patch zlib to get people going before fixing all the other ports? Meanwhile we could use a variant to zlib to have the option of not applying the patch and helping with the fixing in others.

I'm attaching two things: patch-zconf.h.in.diff (The actual patch that defined off64_t as off_t in zconf.h) Portfile (Wich include the variant +patchzlib, set as the default variants)

I would have loved to use patch-delete, but there seems to be something wrong deleting all the patches from the patchfiles definition. (Will submit a bug report for that one...)

Changed 9 years ago by gellule.xg@…

Attachment: Portfile added

Portfile with patch-zconf.h.in.diff patch and patchzlib variant set as default

Changed 9 years ago by gellule.xg@…

Attachment: patch-zconf.h.in.diff added

The patch itself...

comment:55 Changed 9 years ago by social.thiago@…

Cc: social.thiago@… added

Cc Me!

comment:56 Changed 9 years ago by jmroot (Joshua Root)

My patch makes __APPLE__ being defined equivalent to _LARGEFILE64_SOURCE not being defined. It can't possibly break anything. Commit it now and fix the rest of the world later.

comment:57 Changed 9 years ago by ryandesign (Ryan Schmidt)

Resolution: fixed
Status: assignedclosed

Committed the patch that can't possibly break anything in r65036. Thanks, Joshua.

comment:58 Changed 9 years ago by ryandesign (Ryan Schmidt)

When updating zlib to 1.2.5, I removed the patch; see #24568. The zlib developer told me it should no longer be necessary, and I confirmed that gnome-vfs and VLC, two ports which would not build with zlib @1.2.4_0, build just fine with zlib @1.2.5_0.

Note: See TracTickets for help on using tickets.