Bug 64360 - pa_format_info_free2 has disappeared
Summary: pa_format_info_free2 has disappeared
Status: RESOLVED NOTABUG
Alias: None
Product: PulseAudio
Classification: Unclassified
Component: core (show other bugs)
Version: unspecified
Hardware: Other All
: medium critical
Assignee: pulseaudio-bugs
QA Contact: pulseaudio-bugs
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 63665
  Show dependency treegraph
 
Reported: 2013-05-08 14:36 UTC by David Henningsson
Modified: 2013-06-04 13:02 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Description David Henningsson 2013-05-08 14:36:46 UTC
In 3.99.1 the symbol pa_format_info_free2 is no longer exported. This breaks the ABI.
Comment 1 Arun Raghavan 2013-05-08 14:40:08 UTC
Even if pa_format_info_free2() was never part of public API as exposed in headers?
Comment 2 Tanu Kaskinen 2013-05-08 14:44:15 UTC
pa_format_info_free2() was defined in internal.h, i.e. it was not part of the public API. It seems that pa_format_info_free2() was (and still is) listed in map-file, though, so it has been possible to use the function from applications, but I still don't think there has been any real ABI break.
Comment 3 David Henningsson 2013-05-08 14:59:19 UTC
(In reply to comment #1)
> Even if pa_format_info_free2() was never part of public API as exposed in
> headers?

Yes.

That's the rules of libtool. And Debian/Ubuntu nowadays automatically checks this and complains loudly (i e build failure) if a symbol is removed.

We need to put it back.

Btw, I did the same thing for FluidSynth once (removed a symbol not exposed in the headers), and got loud complaints from more than one distro maintainer. I ended up putting it back.
Comment 4 Tanu Kaskinen 2013-05-08 15:22:51 UTC
(In reply to comment #3)
> (In reply to comment #1)
> > Even if pa_format_info_free2() was never part of public API as exposed in
> > headers?
> 
> Yes.
> 
> That's the rules of libtool.

I don't think so. pa_format_info_free2() was never part of the ABI, except in a very technical sense that isn't relevant here, in my opinion.

> And Debian/Ubuntu nowadays automatically checks
> this and complains loudly (i e build failure) if a symbol is removed.

Is there no way to skip the check?

> We need to put it back.
> 
> Btw, I did the same thing for FluidSynth once (removed a symbol not exposed
> in the headers), and got loud complaints from more than one distro
> maintainer. I ended up putting it back.

Why do the distro maintainers demand adding cruft back? I understand if they complain and call us stupid and whatever, because we cause extra work for them, but there's absolutely no long term need to add pa_format_info_free2() back. It's only the distro tools that will cause trouble, no application code will ever break because of this.
Comment 5 David Henningsson 2013-05-09 03:26:06 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #1)
> > > Even if pa_format_info_free2() was never part of public API as exposed in
> > > headers?
> > 
> > Yes.
> > 
> > That's the rules of libtool.
> 
> I don't think so. pa_format_info_free2() was never part of the ABI, except
> in a very technical sense that isn't relevant here, in my opinion.
> 
> > And Debian/Ubuntu nowadays automatically checks
> > this and complains loudly (i e build failure) if a symbol is removed.
> 
> Is there no way to skip the check?

Having read up a little, it seems there are a few different ways of working around this check, yes.

> > We need to put it back.
> > 
> > Btw, I did the same thing for FluidSynth once (removed a symbol not exposed
> > in the headers), and got loud complaints from more than one distro
> > maintainer. I ended up putting it back.

Looking back, I misremembered, it was only one distro maintainer. The other complained about something else. (Reference - http://sourceforge.net/apps/trac/fluidsynth/ticket/98 )

> Why do the distro maintainers demand adding cruft back? I understand if they
> complain and call us stupid and whatever, because we cause extra work for
> them, but there's absolutely no long term need to add pa_format_info_free2()
> back. It's only the distro tools that will cause trouble, no application
> code will ever break because of this.

It would be good to have more opinions on this. Sjoerd, what do you think?
Comment 6 Sjoerd Simons 2013-05-09 09:44:26 UTC
> > Why do the distro maintainers demand adding cruft back? I understand if they
> > complain and call us stupid and whatever, because we cause extra work for
> > them, but there's absolutely no long term need to add pa_format_info_free2()
> > back. It's only the distro tools that will cause trouble, no application
> > code will ever break because of this.
> 
> It would be good to have more opinions on this. Sjoerd, what do you think?

The distro tools don't cause "troubles" they do their job and red flag whenever symbols disappear and the ABI is striclty speaking broken, which can cause issues for applications.. What the tools can't do is work out that this bit of ABI was never used by applications ever, hence maintainers coming to complain because also they can't know if this is the case :)

In practise I'm ok with dropping a symbol from a library (in debian dropping it from the symbols file which the tools use to check), but only if people can guarantee that this was never ever meant to be external ABI in the first place and really nobody uses it (which might be the case here already, didn't read the full thread).

From a distribution perspective, what i do tend to ask upstreams is to take their ABI and soname versioning seriously (you may be surprised, but some folks just don't care because they're not directly impacted by getting it wrong).. Part of that is making sure you only expose the symbols that one actually cares about and is happy to maintain over time (until the soname version is bumped essentially). Note that this isn't a complaint about pulseaudio, just a general statement :)


So i guess, again with my distro hat on, If the pulseaudio maintainers are happy to say: This symbol won't be used by any applications and you won't get a massive fallout of applications that stop working in your distro if pulse is updated to this version.. Then i would be okish with it being dropped :) (Oh and if it causes a massive fallout, i'll come back and shout at you, be warned :p)
Comment 7 Arun Raghavan 2013-05-13 05:09:44 UTC
(In reply to comment #6)
[...]
> From a distribution perspective, what i do tend to ask upstreams is to take
> their ABI and soname versioning seriously (you may be surprised, but some
> folks just don't care because they're not directly impacted by getting it
> wrong).. Part of that is making sure you only expose the symbols that one
> actually cares about and is happy to maintain over time (until the soname
> version is bumped essentially). Note that this isn't a complaint about
> pulseaudio, just a general statement :)

I try to do this for every release, making sure changes to API are correctly reflected in version-info changes.
 
> So i guess, again with my distro hat on, If the pulseaudio maintainers are
> happy to say: This symbol won't be used by any applications and you won't
> get a massive fallout of applications that stop working in your distro if
> pulse is updated to this version.. Then i would be okish with it being
> dropped :) (Oh and if it causes a massive fallout, i'll come back and shout
> at you, be warned :p)

This was never exposed in headers, so if someone's using it, they've brought it upon themselves.
Comment 8 David Henningsson 2013-05-13 05:33:34 UTC
Fair enough. But I think we should at least write something in the release notes, with a reference to this bug. E g:

 pa_format_info_free2 symbol dropped

The pa_format_info_free2 symbol was never part of the public API, but was still exported in the ABI in the previous version of PulseAudio. It is now completely dropped. Since it was never exposed in any header files, the only programs affected should be test tools specifically looking for missing symbols.
Comment 9 Tanu Kaskinen 2013-05-13 09:58:56 UTC
Note added to http://www.freedesktop.org/wiki/Software/PulseAudio/Notes/4.0


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.