Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 383179 - sys-libs/zlib header pollutes the macro namespace, leading to build failures
Summary: sys-libs/zlib header pollutes the macro namespace, leading to build failures
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: [OLD] Core system (show other bugs)
Hardware: All All
: Normal major (vote)
Assignee: Gentoo's Team for Core System packages
URL: http://mail.madler.net/pipermail/zlib...
Whiteboard:
Keywords:
: 384303 (view as bug list)
Depends on: 384303
Blocks: 383113 383309 383349 383351 383371 383431 383569 383585 383627 383697 383783 383805 383813 383833 383845 384473 384483 384547 384999 385477 385569 389863 390093 401471 403387
  Show dependency tree
 
Reported: 2011-09-16 05:31 UTC by Ben Longbons
Modified: 2023-08-19 04:38 UTC (History)
11 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ben Longbons 2011-09-16 05:31:06 UTC
sys-libs/zlib is one of those packages that still takes steps to work with pre-standard C compilers. In doing so, it declares a couple of macros, to differently handle function prototypes.

The macros with bad names are: ON(args) and OF(args). ON(args) is new in zlib 1.2.5.1, which triggered a build failure, but both of these are names that any sane user would expect to be able to use in their own program.

Possible solutions:
1. Replace the single use of ON (for function gzprintf) with OF, which has the same definition (except when !defined(STDC) && defined(Z_HAVE_STDARG_H) )
2. Change the macros to start with Z_
3. Apply a sed transformation instead of depending on macros. Tricky, since some of the argument lists are multiple lines, and wouldn't be applicable to whatever ancient systems.


Reproducible: Always

Steps to Reproduce:
emerge www-client/lynx with =sys-libs/zlib-1.2.5.1
Comment 1 SpanKY gentoo-dev 2011-09-16 21:40:55 UTC
should be all set now in the tree; thanks for the report!

Commit message: Rename OF/ON defines in exported header to avoid collisions with other packages
http://sources.gentoo.org/sys-libs/zlib/zlib-1.2.5.1-r1.ebuild?rev=1.1
Comment 2 Nikos Chantziaras 2011-09-16 23:14:56 UTC
Aren't you changing zlib's API here? If I write an app using those macros, it won't compile on Gentoo anymore.
Comment 3 Ben Longbons 2011-09-17 03:05:11 UTC
The point of this bug is that they were internal macros, not public ones.

Also, you should *probably* not be writing new code for pre-ANSI compilers.
Comment 4 SpanKY gentoo-dev 2011-09-17 03:08:34 UTC
using the macros "OF" and "IF" in your own code is dumb
Comment 5 Sergey Gridnev 2011-09-17 20:16:14 UTC
(In reply to comment #4)
> using the macros "OF" and "IF" in your own code is dumb

OK, it is stupid but this change has broke at least 2 packages:
chromium and vlc.

I know that it is "untested" and for you my own risk but I`ve personally masked this file (>zlib-1.2.5.1) because this change did not allow my system to update.

With best regards
Comment 6 Ben Longbons 2011-09-17 21:17:57 UTC
(In reply to comment #5)
> OK, it is stupid but this change has broke at least 2 packages:
> chromium and vlc.

The problem with chromium is that it's bundling a copy of zlib, which is apparently being linked against the system headers.

Here is a list of the files in www-client/chromium-14.0.835.163 that use the macro:


./third_party/zlib/zconf.h
./third_party/zlib/zutil.c
./third_party/zlib/crc32.c
./third_party/zlib/gzio.c
./third_party/zlib/deflate.c
./third_party/zlib/inflate.c
./third_party/zlib/infback.c
./third_party/zlib/deflate.h
./third_party/zlib/inffast.h
./third_party/zlib/zutil.h
./third_party/zlib/zlib.h
./third_party/zlib/trees.c
./third_party/zlib/inftrees.h
./third_party/zlib/contrib/minizip/zip.h
./third_party/zlib/contrib/minizip/iowin32.h
./third_party/zlib/contrib/minizip/zip.c
./third_party/zlib/contrib/minizip/unzip.h
./third_party/zlib/contrib/minizip/ioapi.c
./third_party/zlib/contrib/minizip/unzip.c
./third_party/zlib/contrib/minizip/iowin32.c
./third_party/zlib/contrib/minizip/ioapi.h
./third_party/libpng/pngconf.h

./sdch/open-vcdiff/src/zconf.h
./sdch/open-vcdiff/src/zlib.h

The copy in pngconf.h is also present in system libpng, but is properly guarded in both the system version and the bundled version.
Comment 7 SpanKY gentoo-dev 2011-09-17 22:55:51 UTC
if you have problems building random packages, then file bugs to get them fixed.  this is not a tracker bug for those issues.
Comment 8 Sergey Gridnev 2011-09-18 10:29:56 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > OK, it is stupid but this change has broke at least 2 packages:
> > chromium and vlc.
> 
> The problem with chromium is that it's bundling a copy of zlib, which is
> apparently being linked against the system headers.
> 
> Here is a list of the files in www-client/chromium-14.0.835.163 that use the
> macro:
> 
> 
> ./third_party/zlib/zconf.h
> ./third_party/zlib/zutil.c
> ./third_party/zlib/crc32.c
> ./third_party/zlib/gzio.c
> ./third_party/zlib/deflate.c
> ./third_party/zlib/inflate.c
> ./third_party/zlib/infback.c
> ./third_party/zlib/deflate.h
> ./third_party/zlib/inffast.h
> ./third_party/zlib/zutil.h
> ./third_party/zlib/zlib.h
> ./third_party/zlib/trees.c
> ./third_party/zlib/inftrees.h
> ./third_party/zlib/contrib/minizip/zip.h
> ./third_party/zlib/contrib/minizip/iowin32.h
> ./third_party/zlib/contrib/minizip/zip.c
> ./third_party/zlib/contrib/minizip/unzip.h
> ./third_party/zlib/contrib/minizip/ioapi.c
> ./third_party/zlib/contrib/minizip/unzip.c
> ./third_party/zlib/contrib/minizip/iowin32.c
> ./third_party/zlib/contrib/minizip/ioapi.h
> ./third_party/libpng/pngconf.h
> 
> ./sdch/open-vcdiff/src/zconf.h
> ./sdch/open-vcdiff/src/zlib.h
> 
> The copy in pngconf.h is also present in system libpng, but is properly guarded
> in both the system version and the bundled version.

I`m not going to post bugs about each broken package in gentoo, or I`ll have to divorce first... I can cope with such issues manually without waiting for fixed ebuilds. But I still don`t understand why the header file of system package was changed just to make "lynx" works. I really don`t understand the motivation of such changes. Anyway, I`ll continue reading this bug all some related and wait for final decision. Till this moment I`ll have to stay with previous version of zlib.
Comment 9 Nikos Chantziaras 2011-09-18 10:42:22 UTC
(In reply to comment #8)
> But I still don`t understand why the header file of system package was
> changed just to make "lynx" works.

It was not.  Lynx was patched to build with vanilla zlib. See bug 383113.
Comment 10 SpanKY gentoo-dev 2011-09-18 19:11:04 UTC
(In reply to comment #8)

sorry, but this is crap.  file a new bug and stop spamming here.  and learn how to quote replies.
Comment 11 Martin von Gagern 2011-09-19 15:42:27 UTC
If there already is a bug report for some package using the OF macro, and you don't want to wait on that bug being resolved in portage, you can probably work around the issue by setting CPPFLAGS='-DOF=_Z_OF':

# CPPFLAGS='-DOF=_Z_OF' emerge -1a <name-of-broken-package>

Worked for me for media-libs/libextractor-0.5.20c. Note that the packages should really be fixed properly, though, so if there is no bug report, file one before using this work-around. If CPPFLAGS doesn't work, you might try adding it to CFLAGS instead.
Comment 12 SpanKY gentoo-dev 2011-09-19 16:06:49 UTC
i thought i tried that and it didn't work.  i had to do something like:
  CPPFLAGS='-DOF(x)=x' emerge -1a <name-of-broken-package>

but maybe i screwed something up along the way
Comment 13 Boney McCracker 2011-09-20 16:28:07 UTC
This works:
CPPFLAGS='-DOF=_Z_OF' emerge -1a <name-of-broken-package>

This does not:
CPPFLAGS='-DOF(x)=x' emerge -1a <name-of-broken-package>
Comment 14 Thomas J. Moore 2011-09-20 17:02:25 UTC
Renaming OF breaks numerous packages which depend on this.  I have 12 build failures due to this change (dev-lang/R, 8 different games, media-video/vlc, sci-libs/gdal).  If a package uses zlib, it should be aware of the definitions, so the package which depends on zlib and does not deal with this problem is the one that is broken (e.g. www-client/lynx).  I have temporarily worked around this by adding -DOF=_Z_OF to CFLAGS in /etc/portage/env/..., but this needs to be reverted and fixed differently.
Comment 15 SpanKY gentoo-dev 2011-09-20 17:10:26 UTC
(In reply to comment #14)

you need to read Comment 10
Comment 16 Denys Dmytriyenko 2011-09-20 18:11:22 UTC
I'm hoping this gets fixed soon.

Meanwhile I was able to temporarily "fix" virtuoso-server (#383349) with this CPPFLAGS hack:

CPPFLAGS='-DOF=_Z_OF' emerge -1a <name-of-broken-package>
Comment 17 Nikos Chantziaras 2011-09-21 01:05:11 UTC
(In reply to comment #3)
> The point of this bug is that they were internal macros, not public ones.

Now they are public.


> Also, you should *probably* not be writing new code for pre-ANSI compilers.

Yes, bad idea.  But it's in my liberty to write code however I see fit.

This is getting ugly.  I think upstream really needs to fix this, but they don't seem to have an opinion:

http://mail.madler.net/pipermail/zlib-devel_madler.net/2011-September/002621.html
Comment 18 SpanKY gentoo-dev 2011-09-21 04:40:46 UTC
(In reply to comment #17)

two things:
 - they're still not public.  every single failure has been due to people copying the minizip code out of the zlib tree and not fully localizing it.  it hasn't been random projects looking at the zlib headers and going "gee, OF looks nifty, let's use it".  i'm ignoring libpng because of its close roots to the zlib project.
 - no one said you couldn't write pre-ansi code.  go nuts.  however, if you use the internal OF/ON macros, your code is wrong.  no two ways about it.
Comment 19 Nikos Chantziaras 2011-09-21 04:56:15 UTC
(In reply to comment #18)
>  - no one said you couldn't write pre-ansi code.  go nuts.  however, if you use
> the internal OF/ON macros, your code is wrong.  no two ways about it.

Yeah, but it's still not your/our call to make.  If someone wrote bad code, fixing it like this is not a good idea because we deviate from vanilla zlib in a non-compatible way.

All I'm saying is that we can't pretend to be able to fix this. We really, really can't. We need to wait for the situation to clear up upstream before coming up with "fixes". When they accept a solution or come up with their own, then we can backport that solution or add a compatible one.

Right now, keeping this in the tree creates more problems than it's worth. There is nothing to gain from trying at this moment. We only end up wasting time (ours and yours.)
Comment 20 SpanKY gentoo-dev 2011-09-21 05:01:54 UTC
fortunately, as maintainer of zlib, it is my call.  and no, we don't need to wait on zlib for anything.  nothing is stopping maintainers from cleaning up their respective upstream projects.
Comment 21 Nikos Chantziaras 2011-09-21 05:11:18 UTC
(In reply to comment #20)
> fortunately, as maintainer of zlib, it is my call.

Oh, I didn't know that. I thought Mark Adler maintained it. So I guess this fix will be in the next version?
Comment 22 SpanKY gentoo-dev 2011-09-21 14:20:08 UTC
i was referring to maintaining zlib in Gentoo, not upstream
Comment 23 Nikos Chantziaras 2011-09-21 14:25:03 UTC
(In reply to comment #22)
> i was referring to maintaining zlib in Gentoo, not upstream

Then we're back to the fact that you can only pretend to be able to fix this. Whatever you come up with, if it's not what upstream will be doing, you didn't fix jack.
Comment 24 SpanKY gentoo-dev 2011-09-21 15:14:36 UTC
go ahead and apply that logic to all the patches Gentoo applies.  once you realize it's fundamentally broken, come back and talk some more.
Comment 25 Nikos Chantziaras 2011-09-21 15:24:18 UTC
The other patches don't break upstream compatibility. But do what you want. It's a bad call. You've made plenty of those lately. This is just another one. IMO, you don't have the skills and insight to mess with this stuff. So when you try, breakage happens. I hope you retire soon.
Comment 26 Alexis Ballier gentoo-dev 2011-09-21 15:36:00 UTC
FWIW: I'm going to make vlc depend on <zlib-1.2.5.1-r1 until something has landed in zlib upstream, so that something can land in vlc upstream. The dep will temporarily avoid current breakage. If you wish to convince vlc's upstream to drop those macros, be my guest, I'll happily backport any patch; but I won't try to convice anyone until the situation wrt zlib's upstream is clarified.
Comment 27 SpanKY gentoo-dev 2011-09-21 16:03:07 UTC
(In reply to comment #25)

thanks for your input.  very valuable.  now go bother someone else.

(In reply to comment #26)

there's no reason at all to do that.  i'll respond on the relevant vlc bug.
Comment 28 Andreas Proteus 2011-09-21 20:58:20 UTC
Bickering maintainer polution not pretty neither consrtuctive.
Comment 29 SpanKY gentoo-dev 2011-09-21 21:29:24 UTC
there hasn't been any Gentoo maintainer bickering.  please refer to the relevant bug reports before making incorrect statements.
Comment 30 Justin Lecher (RETIRED) gentoo-dev 2011-09-24 14:57:44 UTC
*** Bug 384303 has been marked as a duplicate of this bug. ***
Comment 31 Thomas J. Moore 2011-12-14 04:59:11 UTC
(In reply to comment #15)
> (In reply to comment #14)
> 
> you need to read Comment 10

I apologize for writing my comment in anger without reading the entire comment list first, but your response is "learn how to reply" (to what?) and "file bug reports against other packages that my API changes broke", neither of which is helpful.  You broke the zlib API without discussion for apparently no reason (from other comments, I gather lynx was fixed differently).  If ON() was really introduced in the latest version, you have plenty of reason to report it upstream, and temporarily rename it, but OF() has been around for years, and like it or not, a lot of code depends on it, even if it's due to mindless copy-and-paste from another project.  Unfortunately, I can't file bug reports against packages I use and maintain outside of portage, so this gentoo-only API breakage is still a problem.  I guess I should file a new bug, "gratuitous sys-libs/zlib gentoo-only API changes break non-gentoo code", but since you obviously don't intend to fix it anyway, what's the point?

Yes, I know this bug is 3 months old, and was "resolved/fixed" before I even posted my first comment.  I just masked this stupid "fix" out back then, hoping it would "unfix" itself.  I didn't care until today, when portage forced me to use a more recent zlib.  At least it's easy enough to edit the "fix" out of the ebuild (and other infected ebuilds) for my local copy.
Comment 32 SpanKY gentoo-dev 2011-12-20 23:52:57 UTC
you've already been shown many ways to fix broken code that has mindlessly copy and pasted stuff (if they can't use the proper common libminizip code which zlib provides).  i don't see any problems here.
Comment 33 Larry the Git Cow gentoo-dev 2023-08-19 04:37:41 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=491bf8bd3ef621da49b168736a04f23aa4c7719c

commit 491bf8bd3ef621da49b168736a04f23aa4c7719c
Author:     Sam James <sam@gentoo.org>
AuthorDate: 2023-08-19 04:31:19 +0000
Commit:     Sam James <sam@gentoo.org>
CommitDate: 2023-08-19 04:31:19 +0000

    sys-libs/zlib: drop macro renaming hacks
    
    Renaming zlib's `OF()` macro has caused problems for various packages; fortunately
    C23 kills K&R prototypes so its use should be dying out anyway. If anyone is still
    using it, we should just patch it out entirely rather than renaming it.
    
    Other distros have been coping fine without the rename anyway. The most important
    point being that this is against POLA and leads to problems when people try to build
    things using zlib that haven't been fixed yet.
    
    See also the legendary godot patch to workaround this:
    https://github.com/godotengine/godot/blob/93409b8e64a9bc3c271ab4a7489b59a43bc0d048/thirdparty/minizip/patches/unbreak-gentoo.patch.
    
    As of zlib-1.3, zlib doesn't even use it by itself.
    
    Bug: https://bugs.gentoo.org/383179
    Signed-off-by: Sam James <sam@gentoo.org>

 profiles/package.mask                                 | 4 ++--
 sys-libs/zlib/{zlib-1.3.ebuild => zlib-1.3-r1.ebuild} | 6 ------
 2 files changed, 2 insertions(+), 8 deletions(-)