libcdio-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Libcdio-devel] Fwd: autoheader in the global namespace


From: Rocky Bernstein
Subject: Re: [Libcdio-devel] Fwd: autoheader in the global namespace
Date: Mon, 15 Oct 2012 20:24:31 -0400

Sorry for the delay. I've been out hiking.

On Fri, Oct 12, 2012 at 7:41 AM, Robert William Fuller <
address@hidden> wrote:

> Well, you are kind of picking on me, but I think I can take it :-) Let's
> see....
>

That's good. No offence was intended.


> I took a few years off from development and am only now getting back into
> it.  In the past, I worked out of the CVS repository for libcdio, so I was
> doing the right thing.
>
> I was trying to get back into development gently with what I thought was
> the simple task of making my application build with autotools.  (Of course
> there is nothing simple about autotools...)  The most expedient way for me
> to accommodate autotools was to use the pre-compiled packages for the
> various platforms that I was targeting.  Also, this would have the side
> benefit of giving me the chance to track the changes that happened to
> libcdio--while I was out of it--incrementally.  Meaning it makes sense for
> me to account for changes released with 0.83 before I move up to git.  I
> did not know that I would stumble across an autotools problem that had
> already been fixed.  At this point, it makes sense for me to move forward
> to git.
>
> In fairness, you only announced the impending release 3 days ago, on the
> 9th.  I was trying to finish my autotools task before starting the next
> task.  I think I am there.  So, point well taken, and I will move on to
> git.  If you give me a little time, you will get feedback from me on git.
>
> As for the packagers, would it help to make a release candidate?  Would
> the packagers try those out?


I'm not against this if there is overwhelming consensus that it would be
used.  But there'd have to be a show of more interest.


>  For example, maybe a release candidate could be put in to Debian
> unstable/testing?  Maybe fixes could be integrated back into libcdio and
> the release package pushed into testing/unstable?  I am not really sure how
> these things work and the ins/outs.  I suppose I will find out soon enough
> as I try to package my own application for Debian.
>
> Rob
>
>
> On 10/12/2012 04:48 AM, Rocky Bernstein wrote:
>
>> Folks -
>>
>> I had been debating to let this drop with that comment I originally made
>> below, but it has been nagging me.
>>
>> The reason I announce releases in advance is so that people can try them
>> out in advance. And invariably people start to try them out afterwards.
>> (I'm looking at you Fedora and other packagers). Sure, I get the fact that
>> you don't want to be the guinea pigs when others can handily do the job.
>>
>> And not to pick on Robert, but notice of this year's release is not the
>> time to start trying out last year's release. If last year's release had
>> been tried out in advance of last year's release, maybe we wouldn't have
>> made the mistakes made last year.
>>
>>
>> ---------- Forwarded message ----------
>> From: Rocky Bernstein<address@hidden>
>> Date: Thu, Oct 11, 2012 at 11:14 PM
>> Subject: Re: [Libcdio-devel] autoheader in the global namespace
>> To: libcdio developer's mailing list<address@hidden>
>>
>>
>>
>>
>> On Thu, Oct 11, 2012 at 9:15 PM, Robert William Fuller<
>> address@hidden>  wrote:
>>
>>  Indeed, that is the case.  The CDIO_ prefix is a post version 0.83 change
>>> to libcdio.  I found the commit here:
>>>
>>> http://git.savannah.gnu.org/****gitweb/?p=libcdio.git;a=****commit;h=**<http://git.savannah.gnu.org/**gitweb/?p=libcdio.git;a=**commit;h=**>
>>> 39bd8104b8576c71c2c170f59c39ed****c2823b6bf5<http://git.**
>>> savannah.gnu.org/gitweb/?p=**libcdio.git;a=commit;h=**
>>> 39bd8104b8576c71c2c170f59c39ed**c2823b6bf5<http://git.savannah.gnu.org/gitweb/?p=libcdio.git;a=commit;h=39bd8104b8576c71c2c170f59c39edc2823b6bf5>
>>> >
>>>
>>>
>>> I suppose I could develop only for versions>= 0.90 which is not yet
>>> released.  I guess I need to build libcdio from git and develop against
>>> that.  Of course that means all sorts of ugliness, like overriding the
>>> include and library paths for libcdio to be /usr/local and things like
>>> that....
>>>
>>>
>> You read the post where I said I thought there would be a release towards
>> the end of the month, right?
>>
>>
>>
>>> Rob
>>>
>>>
>>> On 10/11/2012 09:01 PM, Robert William Fuller wrote:
>>>
>>>  It's not limited to MacOS. My 64-bit Wheezy (Debian testing) distro does
>>>> not have the CDIO_ prefix on things. Do I need a later version of
>>>> libcdio than what will ship with Wheezy? I have libcdio 0.83 on Wheezy
>>>> and it lacks the CDIO_ prefix. From cdio_config.h on Wheezy:
>>>>
>>>> /* Define to the full name and version of this package. */
>>>> #define PACKAGE_STRING "libcdio 0.83"
>>>>
>>>> /* Define to the one symbol short name of this package. */
>>>> #define PACKAGE_TARNAME "libcdio"
>>>>
>>>> /* Define to the home page for this package. */
>>>> #define PACKAGE_URL ""
>>>>
>>>> /* Define to the version of this package. */
>>>> #define PACKAGE_VERSION "0.83"
>>>>
>>>> Rob
>>>>
>>>> On 10/11/2012 08:11 PM, Rocky Bernstein wrote:
>>>>
>>>>  All #define's in cdio_config.h should start CDIO_. For example:
>>>>>
>>>>>
>>>>> #define CDIO_PACKAGE "libcdio"
>>>>>
>>>>> The transformation is done inside include/cdio/Makefile.am. For
>>>>> GNU/Linux,
>>>>> the relevant portion expands in Makefile to:
>>>>>
>>>>> @/bin/sed -r -e 's/^(#[ \t]*define) /\1 CDIO_/'
>>>>> $(top_builddir)/config.h>>****cdio_config.h
>>>>>
>>>>>
>>>>> There is some adjustment for other kinds of "sed" options.
>>>>>
>>>>> So most likely your sed isn't sed'ing on OSX.
>>>>>
>>>>>
>>>>> On Thu, Oct 11, 2012 at 7:29 PM, Robert William Fuller<
>>>>> address@hidden>  wrote:
>>>>>
>>>>>   Recently, I re-tooled my project (cued) to use autotools. I started
>>>>> out
>>>>>
>>>>>> by doing the naive thing when it came to including my project's
>>>>>> config.h.
>>>>>> Here is an excerpt from rip.c:
>>>>>>
>>>>>> #include "config.h" // HAVE_CDIO_MMC_LL_CMDS_H
>>>>>> #include "unix.h"
>>>>>> #include "util.h"
>>>>>>
>>>>>> #define DO_NOT_WANT_PARANOIA_******COMPATIBILITY
>>>>>>
>>>>>> #include<cdio/cdio.h>
>>>>>> #include<cdio/mmc.h>  // CDIO_MMC_READ_TYPE_ANY
>>>>>> #ifdef HAVE_CDIO_MMC_LL_CMDS_H
>>>>>> #include<cdio/mmc_ll_cmds.h>
>>>>>> #endif
>>>>>>
>>>>>> This seemed to work fine. I proceeded to make my project compile under
>>>>>> Linux, Open Indiana (Solaris), and FreeBSD. So far so good. Then, I
>>>>>> tried
>>>>>> to port to MacOS, not because I am particularly concerned about
>>>>>> supporting
>>>>>> it, but because I figured it was different enough from the other 3
>>>>>> platforms that something might break.
>>>>>>
>>>>>> Consequently, when trying to build under MacOS, I was rewarded with
>>>>>> this
>>>>>> nastiness:
>>>>>>
>>>>>> gcc -DHAVE_CONFIG_H -I. -I../.. -I/opt/local/include
>>>>>> -I/opt/local/include -I/opt/local/include -I/opt/local/include
>>>>>> -I../../lib/cued -std=gnu99 -Wall -Wstrict-aliasing=3 -Wformat=2 -g
>>>>>> -O2
>>>>>> -MT rip.o -MD -MP -MF .deps/rip.Tpo -c -o rip.o rip.c
>>>>>> In file included from /opt/local/include/cdio/types.******h:34,
>>>>>> from /opt/local/include/cdio/cdio.******h:35,
>>>>>> from rip.c:25:
>>>>>> /opt/local/include/cdio/cdio_******config.h:306:1: warning: "PACKAGE"
>>>>>>
>>>>>> redefined
>>>>>> In file included from rip.c:20:
>>>>>> ../../config.h:91:1: warning: this is the location of the previous
>>>>>> definition
>>>>>> In file included from /opt/local/include/cdio/types.******h:34,
>>>>>> from /opt/local/include/cdio/cdio.******h:35,
>>>>>> from rip.c:25:
>>>>>> /opt/local/include/cdio/cdio_******config.h:309:1: warning:
>>>>>>
>>>>>> "PACKAGE_BUGREPORT" redefined
>>>>>>
>>>>>> Add a half dozen more of these warnings, and you get the gist.
>>>>>> Apparently,
>>>>>> there was a conflict between my project's config.h and libcdio's
>>>>>> config.h.
>>>>>> Yet, the warning only showed up on MacOS, not any of the other
>>>>>> platforms.
>>>>>>
>>>>>> The reason it showed up on MacOS is that MacPorts installs header
>>>>>> files in
>>>>>> /opt/local rather than /usr/include. GCC treats system headers
>>>>>> differently
>>>>>> than other headers. It ignores redefined macros if they are redefined
>>>>>> by
>>>>>> system headers (i.e. in /usr/include). This is documented under the
>>>>>> GCC
>>>>>> option "-isystem".
>>>>>>
>>>>>> So PACKAGE_NAME was defined to be "libcdio" rather than "cued", the
>>>>>> name
>>>>>> of my package. VERSION was defined to be "0.83" instead of "1.20".
>>>>>> This is
>>>>>> a somewhat insidious problem because I got NO warning on ANY platform
>>>>>> other
>>>>>> than MacOS.
>>>>>>
>>>>>> Next, I searched this mailing list to see what I could learn. I think
>>>>>> I
>>>>>> fixed my problem by switching to the following code (but maybe not,
>>>>>> read
>>>>>> on:)
>>>>>>
>>>>>> #ifdef HAVE_CONFIG_H
>>>>>> #include "config.h" // HAVE_CDIO_MMC_LL_CMDS_H
>>>>>> #define __CDIO_CONFIG_H__ // avoid conflicts with libcdio
>>>>>> #endif
>>>>>> #include "unix.h"
>>>>>> #include "util.h"
>>>>>>
>>>>>> #define DO_NOT_WANT_PARANOIA_******COMPATIBILITY
>>>>>>
>>>>>> #include<cdio/cdio.h>
>>>>>> #include<cdio/mmc.h>  // CDIO_MMC_READ_TYPE_ANY
>>>>>> #ifdef HAVE_CDIO_MMC_LL_CMDS_H
>>>>>> #include<cdio/mmc_ll_cmds.h>
>>>>>> #endif
>>>>>>
>>>>>> I am not totally satisfied with this solution. If every library that I
>>>>>> used included its config.h in the global namespace, I might end up
>>>>>> with
>>>>>> something like this (assuming that other projects create config.h
>>>>>> include
>>>>>> guards which they generally do not:)
>>>>>>
>>>>>> #ifdef HAVE_CONFIG_H
>>>>>> #include "config.h"
>>>>>> #define __CDIO_CONFIG_H__
>>>>>> #define __SNDFILE_CONFIG_H__
>>>>>> #define __CDDB_CONFIG_H__
>>>>>> etc.
>>>>>>
>>>>>> MOREOVER, I am not sure that<cdio/types.h>  will always do the right
>>>>>> thing
>>>>>> when I define __CDIO_CONFIG_H__ before including it. You might argue I
>>>>>> should have used the other proposed solution for these sorts of
>>>>>> conflicts
>>>>>> which would look something like this:
>>>>>>
>>>>>> #include<cdio/cdio.h>
>>>>>> #include<cdio/cdio_unconfig.h>  # remove *all* symbols libcdio defines
>>>>>>
>>>>>> #include "config.h"
>>>>>> #include "unix.h"
>>>>>> #include "util.h"
>>>>>>
>>>>>> #define DO_NOT_WANT_PARANOIA_******COMPATIBILITY
>>>>>>
>>>>>> #include<cdio/mmc.h>  // CDIO_MMC_READ_TYPE_ANY
>>>>>> #ifdef HAVE_CDIO_MMC_LL_CMDS_H
>>>>>> #include<cdio/mmc_ll_cmds.h>
>>>>>> #endif
>>>>>>
>>>>>> Note that I need to include my "config.h" before<cdio/mmc_ll_cmds.h>,
>>>>>> BUT
>>>>>> I am supposed to include the cdio headers before my "config.h". Also
>>>>>> I am
>>>>>> not convinced that cdio_unconfig.h won't remove some definitions that
>>>>>> are
>>>>>> included in the GCC specs for some platform or in the system header
>>>>>> files
>>>>>> for that platform. For example, cdio_unconfig.h undefines "const".
>>>>>>
>>>>>> Now the general autotools lore seems to be that you should never
>>>>>> include
>>>>>> config.h in a header file, but only in a .c file. This is generally
>>>>>> attributed to the lack of include guard. I think that is missing the
>>>>>> point. Here, I will argue that you should never include config.h in a
>>>>>> header file because of the global namespace pollution and the
>>>>>> potential for
>>>>>> silent conflicts that show up as bugs.
>>>>>>
>>>>>> One path out of this conflict is to remove code such as this from
>>>>>> <cdio/read.h>:
>>>>>>
>>>>>> #ifndef EXTERNAL_LIBCDIO_CONFIG_H
>>>>>> #define EXTERNAL_LIBCDIO_CONFIG_H
>>>>>> /* Need for HAVE_SYS_TYPES_H */
>>>>>> #include<cdio/cdio_config.h>
>>>>>> #endif
>>>>>>
>>>>>> #ifdef HAVE_SYS_TYPES_H
>>>>>> /* Some systems need this for off_t and ssize. */
>>>>>> #include<sys/types.h>
>>>>>> #endif
>>>>>>
>>>>>> However, things aren't so simple when it comes to<cdio/types.h>, which
>>>>>> seems to have a real need to include its config.h.
>>>>>>
>>>>>> Another path out of this conflict would be to create yet another
>>>>>> config.h
>>>>>> file, perhaps named cdio_header_config.h that contains ONLY the macros
>>>>>> needed by the cdio headers, such as HAVE_SYS_TYPES_H and
>>>>>> HAVE_STDINT_H.
>>>>>> Yet again, /usr/include/cdio/types.h complicates things because there
>>>>>> is
>>>>>> so much that it needs.
>>>>>>
>>>>>> So, I am stuck, but still thinking about the problem. Thoughts?
>>>>>>
>>>>>> Rob
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>


reply via email to

[Prev in Thread] Current Thread [Next in Thread]