On Sat, Aug 28, 2021 at 12:08:05AM +0200, Pavel Cahyna wrote: > On Thu, Aug 26, 2021 at 07:18:24PM -0500, Corey Minyard wrote: > > On Fri, Aug 27, 2021 at 01:27:17AM +0200, Pavel Cahyna wrote: > > > On Thu, Aug 26, 2021 at 05:05:27PM -0500, Corey Minyard wrote: > > > > On Thu, Aug 26, 2021 at 10:26:34PM +0200, Pavel Cahyna wrote: > > > > > Hello Corey, > > > > > > > > > > On Wed, Aug 25, 2021 at 08:31:13PM -0500, Corey Minyard wrote: > > > > > > On Wed, Aug 25, 2021 at 08:24:05PM +0200, Pavel Cahyna wrote: > > > > > > > at a quick glance at header file diffs, it seems to me that the > > > > > > > Windows > > > > > > > DLL changes are introducing library API changes even for > > > > > > > non-Windows > > > > > > > builds. Specifically, commit: > > > > > > > > > > > > > > 26e0921e77b6db359e7b018e8d439fcd1222d891 seems to affect the API > > > > > > > of libIPMIlanserv.so.0.0.1 > > > > > > > cb416caa52dd73e53ada88ccda4aa496154519e8 seems to affect the API > > > > > > > of libOpenIPMIcmdlang.so.0.0.5 > > > > > > > > > > > > > > Is that correct? If so, it seems that those libraries should have > > > > > > > their > > > > > > > major version numbers bumped. > > > > > > > > > > > > Yeah, I suppose. lanserv is not that critical for library > > > > > > compatibility, and I seriously doubt anyone is using cmdlang. So I > > > > > > didn't work about it. > > > > > > > > > > > > If you like I can update these and do a new release. > > > > > > > > > > Thank you for the quick reply. Updating the major version is > > > > > certainly a > > > > > valid approach, but I would actually prefer to preserve compatibility. > > > > > This way maintainers of distribution packages would not need to worry > > > > > whether those libraries are important enough or not to introduce a > > > > > compatibility package (which would be the correct approach, but quite > > > > > an > > > > > inconvenience). Actually, preserving compatibility does not seem that > > > > > difficult. It seems to be enough to declare those symbols as weak > > > > > (conditionally - not on Windows) and call them only if the new > > > > > function > > > > > pointers are NULL. Attached is a patch which does just that (for > > > > > lanserv). Let me know what you think and I can do similar approach for > > > > > cmdlang. > > > > > > > > This will work fine, I think. A few things I would like to change... > > After some more investigation, I think that this will not work fine, > actually. sys_data_t size seems to be part of the library ABI (it is > allocated by the user, not by the library) and it has changed, so the > new version is still ABI incompatible (my proposed change restores API > compatibility though). (That is, libIPMIlanserv.so.0.0.1 - > libOpenIPMIcmdlang.so.0.0.5 seems to be an easier case.)
Yeah. Really, nobody is using the cmdlang library besides internal users, and nobody using lanserv is going to care. Just updating the versions should be enough. > > > > > Can you not modify dllvisibility.h and add all the include file stuff to > > > > a separate include file? It doesn't have anything to do with dll > > > > visibility, so it really doesn't belong there. And it make it easier to > > > > yank out later. Plus you can comment why the code is there. > > > > > > > > Can you deprecate the weak symbols? That way people will know they > > > > shouldn't use them any more. > > > > > > I don't think this is feasible, as the only code that uses the symbols > > > is actually the library itself, so the deprecation warning will be > > > printed only when compiling the library, not when compiling the user > > > code. Or do you have some tip on how to do that? > > > > There were prototypes for all these functions, you would need to re-add > > the prototypes and then deprecate them. > > > > I tried that, but that does not seem to work, because it causes the > compiler to emit a warning when the functions are called. But the user > code is not calling the functions, it defines them. It is the library who > uses the functions. Am I missing something? Oh, duh, yeah. That's probably why it wasn't done on posix_vlog(). -corey > > > > The posix_vlog symbol > > > also does not seem to have been visibly deprecatred before being > > > removed. > > > > Yeah, that wasn't a good idea. It's better if people are alerted. > > Probably, but my point was rather that you would not have been able to > make the declaration emit a warning when the deprecated interface gets > used, even if you had wanted to, due to what I wrote above - user code > does not actually use the fnction, it is a callback, so it defines the > function and the library uses it. > > P. > _______________________________________________ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
