[openib-general] OpenSM/osm_log API: Use symbol versionsrather than polluting namespace

[Michael S. Tsirkin]

Quoting r. Doug Ledford <dledford at redhat.com>:
> Subject: Re: [openib-general] OpenSM/osm_log API: Use symbol versionsrather than polluting namespace
> 
> On Wed, 2006-09-06 at 18:16 +0300, Michael S. Tsirkin wrote:
> > Quoting r. Hal Rosenstock <halr at voltaire.com>:
> > > Subject: Re: OpenSM/osm_log API: Use symbol versions rather than polluting namespace
> > > 
> > > On Wed, 2006-09-06 at 09:42, Michael S. Tsirkin wrote:
> > > > Quoting r. Hal Rosenstock <halr at voltaire.com>:
> > > > > Subject: OpenSM/osm_log API: Use symbol versions rather than polluting namespace
> > > > > 
> > > > > OpenSM/osm_log API: Rather than polluting the namespace with needless
> > > > > symbols, use symbol versions and have a versioned osm_log_init rather
> > > > > than adding osm_log_init_v2 as an additional API
> > > > > 
> > > > > This patch is intended to be applied to both trunk and 1.1 versions.
> > > > > 
> > > > > Signed-off-by: Doug Ledford <dledford at redhat.com>
> > > > > Signed-off-by: Hal Rosenstock <halr at voltaire.com>
> > > > 
> > > > This preserves the ABI, but would this not break the API?
> > > 
> > > Yes, this patch changes the API (in a most trivial way).
> > 
> > So all users need to change code or they won't compile against the new
> > library?
> 
> Yes, and that is the correct way to handle this change.

I disagree.

In my opinion, asking all users to add a parameter they don't care about is
worse than having multiple functions with a convenient set of options.  And if
there is a low cost way to help apps compile without code change, I don't see
why it makes sense to create work.

Even if this were a good idea, I don't think introducing a flag day for all
users without warning is as good way to extend library APIs. I would expect at
least one release where both new and old functions are available.

> I could see
> leaving the whole log init change out entirely, but if it's going to go
> in, this is the right way to do it.

Maybe it should be left out. Whether the issue this addresses is critical
for release is Hal's call. But if the change affects other modules
I think it's clear we won't be able to take the fix.

> > Not sure what do you mean by upward compatible. This API change does not seem to
> > be backward compatible - won't it break building dependent applications?
> > If so is not something you should do after code freeze.
> 
> APIs change.

APIs should not change with every release.

> Any app you can build can compensate.

Sure it seems simple if you are RedHat and rebuild the whole OS.
However, let us look at an application vendor that cares about portability.
What this "trivial" change involves is:

1. add a configure hook to check library version installed
2. define an approprite macro
3. add a wrapper in header file to call the appropriate function
4. update the application to use the wrapper instead of the function
   directly

All this after a supposed code freeze.

> The goal is to keep
> apps that aren't recompiled working, and to make apps that are
> recompiled compliant with the latest version of the function.

We are past code freeze. I agree with Hal that it might be hard to draw a line
between a critical and a non-critical bugfix. However, an API change
that
1. is purely cosmetical
2. requires code changes in dependent applications
3. is not uncontroversial
is, for me, obviously beyond that line.

-- 
MST