Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 486782 - =app-cdr/cdrtools-3.01_alpha17: fails to build on a uclibc system
Summary: =app-cdr/cdrtools-3.01_alpha17: fails to build on a uclibc system
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Daniel Pielmeier
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: uclibc-porting
  Show dependency tree
 
Reported: 2013-10-02 11:41 UTC by Anthony Basile
Modified: 2016-04-09 16:33 UTC (History)
1 user (show)

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


Attachments
Renamed clone to clonetoc (cdrtools-3.01-rename-clone.patch,2.49 KB, patch)
2013-10-03 11:51 UTC, Anthony Basile
Details | Diff
Build failure on uclibc (build.log,201.52 KB, text/plain)
2013-10-04 12:07 UTC, Anthony Basile
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Anthony Basile gentoo-dev 2013-10-02 11:41:14 UTC
The recent bump of cdrtools introduced a new variable clone in readcd/readcd.c.  This is an unfortunate name because it collides with the definition of clone in <bits/sched.h>.  Because of the different header stacking on a uclibc system, we hit this bug whereas it is missed on a glibc system.  Nonetheless, its probably not a good idea to collide on this name.

This is easily fixed by renaming clone to clonet (or something equally appropriate) in readcd/readcd.c, but upstream should probably be alerted too.

Reproducible: Always
Comment 1 Anthony Basile gentoo-dev 2013-10-03 11:51:58 UTC
Created attachment 360046 [details, diff]
Renamed clone to clonetoc

I'm not sure how upstream will feel about renaming this option from -clone to -clonetoc, but the approach is correct.
Comment 2 Samuli Suominen gentoo-dev 2013-10-03 14:50:24 UTC
well, the patch looks good to me. 

only thing that's missing is post to upstream mailing list and a link to it from this bug -- (and perhaps billie's ack, but i'd say it is a go after the upstream ml post)
Comment 3 Daniel Pielmeier gentoo-dev 2013-10-03 14:59:26 UTC
I already contacted upstream (per mail) about the issue. Normally Jörg has a short response time but until now I did not hear from him. He also has a bugzilla account so he can respond here directly.
Comment 4 Daniel Pielmeier gentoo-dev 2013-10-04 10:38:09 UTC
Jörg responded to me per mail and told me the clone variable exists in readcd since 12 years. He has a point there as I found the clone variable in cdrtools-2.00 from 12/25/2002. So this is not a recent change in cdrtools.

Can you provide more information, build failure, etc..
Comment 5 Anthony Basile gentoo-dev 2013-10-04 12:07:42 UTC
Created attachment 360114 [details]
Build failure on uclibc

It may be that the inclusion of a new header brought in <bits/sched.h> and thus the definition of clone() which wasn't exposed before.
Comment 6 joerg.schilling 2013-10-04 12:50:23 UTC
(In reply to Anthony Basile from comment #5)
> Created attachment 360114 [details]
> Build failure on uclibc
> 
> It may be that the inclusion of a new header brought in <bits/sched.h> and
> thus the definition of clone() which wasn't exposed before.

The background for this problem seems to be <stdio.h> which clearly is a part of libc and that is not permitted to do this kind of name space pollution.

System include files on Linux in general are a big problem as some of the files are part of the kernel and others are part of libc and similar. One problem is that (in contrary to other OS) the kernel and libc do not come from the same hand on Linux. The other problem is that the Linux kernel folks refuse to do their homework and only deliver broken kernel include files for kernel/user interfaces that do not work with user space programs - mainly because they are based on on standard type names.

So in general it is hard to tell who should be blamed for include file based problems... for your problem, I still like to blame the creator of stdio.h
Comment 7 Anthony Basile gentoo-dev 2013-10-04 17:36:51 UTC
(In reply to joerg.schilling from comment #6)
> So in general it is hard to tell who should be blamed for include file based
> problems... for your problem, I still like to blame the creator of stdio.h

The following header stacking on a uclibc system is not there on glibc:

stdio.h
     bits/uClibc_stdio.h
          bits/uClibc_mutex.h
               pthread.h
                    sched.h
                         bits/sched.h

The inclusion of <bits/uClibc_mutex.h> in <bits/uClibc_stdio.h> starts the problem.  So the "correct" solution is to fix this in uclibc's stdio.h but this is harder than it seems.

A hacky solution we could adopt in Gentoo is to conditionally apply the above patch for only uclibc systems:

    use uclibc && epatch "${FILESDIR}"/fix-uclibc.patch
Comment 8 joerg.schilling 2013-10-06 11:01:56 UTC
If a specific implementation has more than one problem, a fix is usually hard.
An implementation that did never get a code review by skilled people tends to
accumulate problems.

The related include files (starting from what stdio.h includes) most likely 
expose the problem because they seem to expose implementation details that
do not belong to the scope of visibility of the using code.

A multi-thread aware stdio uses flockfile()/ftrylockfile()/funlockfile().
More must not be visible to the calling code and this includes in special
how these functions are implemented internally.
Comment 9 Anthony Basile gentoo-dev 2016-01-02 08:21:00 UTC
I'd like to add that patch conditionally using something like

    use elibc_uclibc && epatch "${FILESDIR}"/uclibc-specific.patch


since upstream isn't going to include it and the fix to the uclibc headers isn't going to happen anytime soon.

1) we need cdrtools to make release media

2) we're down to just this one pkg as far as the overlay goes.  we want to move 100% in tree, see bug #570552
Comment 10 Daniel Pielmeier gentoo-dev 2016-01-15 19:55:47 UTC
Feel free to add this patch if holds you back.
Comment 11 Anthony Basile gentoo-dev 2016-04-09 16:33:24 UTC
(In reply to Daniel Pielmeier from comment #10)
> Feel free to add this patch if holds you back.

I added this to cdrtools-3.02_alpha06.ebuild and it is conditional on ELIBC=uclibc.  Don't worry I won't let it be a burden to you.  If it needs updating, I'll take care of it.