#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 Carsten 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-nogas (Scott C. Kennedy), 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)
Change History (64)
comment:1 Changed 15 years ago by gellule.xg@…
Changed 15 years ago by gellule.xg@…
| Attachment: | patch-largefile64.diff added |
|---|
comment:6 Changed 15 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 15 years ago by ryandesign (Ryan Carsten Schmidt)
| Owner: | changed from macports-tickets@… to ryandesign@… |
|---|---|
| Status: | new → assigned |
| Summary: | off64_t is not defined on OS X → zlib 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 15 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 15 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 15 years ago by ryandesign (Ryan Carsten Schmidt)
Sure, open a ticket for VLC, and for mplayer-devel and any other ports where we find this problem.
comment:13 Changed 15 years ago by ryandesign (Ryan Carsten Schmidt)
comment:16 Changed 15 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 15 years ago by ryandesign (Ryan Carsten 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:21 Changed 15 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 15 years ago by jmroot (Joshua Root)
| Attachment: | Portfile.diff added |
|---|
Changed 15 years ago by jmroot (Joshua Root)
| Attachment: | zlib.h.diff added |
|---|
comment:22 Changed 15 years ago by ryandesign (Ryan Carsten 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 follow-up: 24 Changed 15 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 Changed 15 years ago by pacobell@…
Replying to msm@…: Yes, this seems to be the most correct and elegant approach. Kudos!
comment:26 Changed 15 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:28 follow-ups: 29 35 Changed 15 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 Changed 15 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:35 Changed 15 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.
comment:36 Changed 15 years ago by diegotheblind+macports@…
| Cc: | diegotheblind+macports@… added |
|---|
Cc Me!
comment:37 follow-up: 53 Changed 15 years ago by ryandesign (Ryan Carsten 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:51 Changed 15 years ago by Russell-Jones-OxPhys (Russell Jones)
| Cc: | russell.jones@… added |
|---|
Cc Me!
comment:53 Changed 15 years ago by ryandesign (Ryan Carsten 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 15 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 15 years ago by gellule.xg@…
Portfile with patch-zconf.h.in.diff patch and patchzlib variant set as default
comment:56 Changed 15 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 15 years ago by ryandesign (Ryan Carsten Schmidt)
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
Committed the patch that can't possibly break anything in r65036. Thanks, Joshua.
comment:58 Changed 15 years ago by ryandesign (Ryan Carsten 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.
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.