http://bugs.winehq.org/show_bug.cgi?id=22132
Summary: update configure to work with mpg123 1.11.0 Product: Wine Version: 1.1.41 Platform: All OS/Version: Linux Status: UNCONFIRMED Severity: enhancement Priority: P2 Component: -unknown AssignedTo: wine-bugs@winehq.org ReportedBy: thomas-forum@orgis.org
Created an attachment (id=26989) --> (http://bugs.winehq.org/attachment.cgi?id=26989) patch for configure of 1.1.41
mpg123 1.11.0 introduces a scheme to have more reliable ABI in the future: libmpg123.so will use default off_t, on 32 bit systems there will be libmpg123_64.so for 64 bit off_t.
So, a program should check for both libraries... I am attaching patches for configure.ac in 1.1.41 and git from today (hm, or yesterday).
http://bugs.winehq.org/show_bug.cgi?id=22132
--- Comment #1 from Thomas Orgis thomas-forum@orgis.org 2010-03-22 15:58:23 --- Created an attachment (id=26990) --> (http://bugs.winehq.org/attachment.cgi?id=26990) patch for configure of git version
http://bugs.winehq.org/show_bug.cgi?id=22132
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |download, patch
--- Comment #2 from Austin English austinenglish@gmail.com 2010-03-22 19:37:38 --- Please submit patches to wine-patches@winehq.org
http://bugs.winehq.org/show_bug.cgi?id=22132
--- Comment #3 from Thomas Orgis thomas-forum@orgis.org 2010-03-22 19:44:56 --- Oh, sorry, I missed that. Do you mean I should resend the patches or is this report now on the radar anyway?
http://bugs.winehq.org/show_bug.cgi?id=22132
--- Comment #4 from Vitaliy Margolen vitaliy@kievinfo.com 2010-03-22 20:35:08 --- Please send the patch, not a report to wine-patches mailing list to receive proper contribution.
http://bugs.winehq.org/show_bug.cgi?id=22132
Dmitry Timoshkov dmitry@codeweavers.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Platform|All |Other
http://bugs.winehq.org/show_bug.cgi?id=22132
--- Comment #5 from Alexandre Julliard julliard@winehq.org 2010-03-23 03:54:17 --- So we now have to change the name of the library and break backwards compatibility? This is getting more broken every day... I hope you'll reconsider, changing the name is a bad idea, especially since there's no real reason to break compatibility.
http://bugs.winehq.org/show_bug.cgi?id=22132
--- Comment #6 from Thomas Orgis thomas-forum@orgis.org 2010-03-23 04:32:31 --- Ok, so let me explain. First, I am talking about 32 bit Linux/Solaris systems only here. Nothing changes for systems that do not have ambiguous off_t size.
Binary compatibility has been broken at the time I introduced large file support in mpg123 1.5 . Before that, libmpg123.so always exposed an ABI with 32 bit off_t. After the change, the default install switched to 64 bit off_t. In any case, users were able to specify --enable-largefile or --disable-largefile to yield libmpg123.so with different ABI. That is troublesome.
Back then, I was not really aware of the mishap I am causing, also thinking that I am rather late switching on large file support and that the world is supposed to build for large files, at large. That is wrong. Many apps won't bother with this, ever, and not everyone aggrees that one should be able to play mp3s > 2G (actually, the big off_t is also needed to represent byte offsets in the _decoded_ stream).
The recent trouble with wine failing to build with libmpg123 on multilib systems made me more aware of this. And yes, I decided to break a bit of binary compatibility for the sake of ending the current state of latent binary incompatibility with itself.
On a system where you have optional large file support, the large-file-libmpg123 will be called libmpg123_<width_of_off_t>.so (so, currently that's libmpg123_64.so on 32 bit systems). The default size of off_t will be offered by libmpg123.so, as it was the case before mpg123 1.5 and as it is still the case for any install that did not enable large file support (for example to keep backwards binary compatibility!). I hope that distributions will adopt the practice to install both libmpg123.so and libmpg123_64.so to make apps happy.
I have to hurt somebody at this point. I decided to do a cut now to prevent the current situation to extend to the future and really start to hurt users. Wine is about the biggest project starting to use libmpg123 and I think it is a good idea to make sure that it works with least surprise once there is a stable release. I really did that rename for the sake of ABI stability: Once you built wine on a 32 bit box with large file support, it will depend on libmpg123_64.so and my change guarantees that this library will always provide an ABI with 64 bit off_t.
The old way of shape-shifting libmpg123.so would only cause trouble when installing a wine binary on a system where mpg123 was built using different large file setting. Doing that change now could come in time to keep a sane upgrade path on one major Linux distro: stable debian is on mpg123 1.4, pre-largefile. I'd like the next debian release to include 1.11 instead of one of the versions that _will_ break binary compatibility for anything built on stable debian.
Was I able to convince you that this decision is necessary to put an end to the legacy of bad decisions about the mpg123 API/ABI?
http://bugs.winehq.org/show_bug.cgi?id=22132
--- Comment #7 from Alexandre Julliard julliard@winehq.org 2010-03-23 05:32:25 --- Breaking compatibility once more is only making things worse. Now we not only have the problem with different function names (which we'll still have to deal with for years to come because of backwards compatibility), but you are adding a new library name mess on top of it, which we'll have to deal with too.
Most likely it means we'll have to switch to dynamic loading, and do some ugly magic to figure out the correct library/symbol names to load depending on which versions are available at run time. It also means that all distros will have to bump the library soname and ship two versions (which probably many won't do correctly and make it an even bigger mess).
The right solution is to export both function types from a single library and convert parameters, like glibc does. It's a little extra work for you, but it makes the life of every library user much easier. It's also nicely backwards compatible.
http://bugs.winehq.org/show_bug.cgi?id=22132
--- Comment #8 from Thomas Orgis thomas-forum@orgis.org 2010-03-23 10:03:50 --- I am sorry that you don't see this as improvement. I actually wanted to make things better. Also, I figured that it would be early enough to catch this now, before any stable release of wine using libmpg213 is out.
Last time I checked, wine version 1.0.x does not use libmpg123 yet. So, I actually don't see the big issue with backwards compatibility. Yes, there is the case of someone having a recent build of wine-1.1.x installed breaking it by upgrading mpg123 to 1.11. Then that someone needs to rebuild wine -- I consider that far less severe for a development version than for a stable end-user release. Isn't the development release series there to iron out issues before stable?
Isn't it perhaps a possibility to make wine require a minimum mpg123 version of 1.11.0? That's easily done using pkg-config macros ... then, one only needs to check for libmpg123_64 if large file support is enabled, otherwise libmpg123. Why do you really want to add ugly magic to make wine work with problematic incarnations of the mpg123 library?
I don't say that the separate library approach is the best one, but it is at least one that avoids incompatible libraries under the same name. Sure, it's the least-effort approach. But it is that for a reason: I need my life time for different things. This may be harsh, but I don't have an army of hackers behind be, begging to get some work to do for mpg123:-/ Also, I am trying to avoid code (conversion and overflow checking of exernal/internal off_t) that is specific to a platform (Linux/x86) that I don't use for daily work. I'm on x86-64 since quite some time now (that may be a reason for my earlier blindness on the large/smal file support issue).
I see now that the "right" solution would be to rewrite the API to avoid exposing off_t ... and avoid exchanging file descriptors. I don't have time for that, and I admit that I am really annoyed by this all (not you -- the situation).
I actually discussed with the debian mpg123 maintainer about this and in the end followed his recommendation of an soname change for the large file version. I tried to get input from people about this... nobody actually suggested that what I am doing is A Very Bad Thing. There are other libraries that have a 32 bit and a 64 bit offset variant -- but granted, most either ignore the issue or hide off_t. I fail to see why you really have to invest more work on this from your side than looking for both libs and using the one that matches your large file setup -- what my configure patch does.
The most important thing is to prevent screwing users by broken defaults. Now, when wine uses large file support by default (and possibly requires mpg123>=1.11), it will link with libmpg123_64 and it is clear that it requires a default install of mpg123 which gives you libmpg123_64. I really don't want you to regret using libmpg123. I do regret the mess I caused with the off_t in the API. I now see that you also are not happy with my solution, but it is not exactly impossible to adapt to it (and be safe from future troubles with this -- promised!). Can wine and libmpg123 still be friends?
http://bugs.winehq.org/show_bug.cgi?id=22132
--- Comment #9 from Dmitry Timoshkov dmitry@codeweavers.com 2010-03-23 10:31:24 ---
Last time I checked, wine version 1.0.x does not use libmpg123 yet. So, I actually don't see the big issue with backwards compatibility. Yes, there is the case of someone having a recent build of wine-1.1.x installed breaking it by upgrading mpg123 to 1.11. Then that someone needs to rebuild wine -- I consider that far less severe for a development version than for a stable end-user release. Isn't the development release series there to iron out issues before stable?
You are forgetting about Crossover and other binary Wine packages. They still need to work if a user upgrades his/her system.
http://bugs.winehq.org/show_bug.cgi?id=22132
--- Comment #10 from Dmitry Timoshkov dmitry@codeweavers.com 2010-03-23 10:34:19 --- And as Alexandre sugegsted please have a look how glibc copes with that, it certainly doesn't require different libraries for 32 and 64 bit off_t.
http://bugs.winehq.org/show_bug.cgi?id=22132
--- Comment #11 from Thomas Orgis thomas-forum@orgis.org 2010-03-23 11:05:59 --- While I am not eager to change the large file handling _again_ (now needing to provide libmpg123_64 for the future as symlink to the new dualmode library?), what I can offer is this: If someone comes up with a patch to mpg123 that makes the largefile build include the ABI for default off_t and which is not specific to one certain platform (meaning: it should not just work on GNU/Linux, but also on Solaris), without complicating things for systems with defined one off_t size, I will consider including it. But by myself, I do not see me coding this, sorry.
Oh, and it would need to safely handle passing of file descriptors from the client app to mpg123_open_fd(). I do not know exactly how exchangeable, if at all, descriptors are between small and large file code -- I just expect trouble.
But yes, I forgot about crossover and friends. Such packages face troubles with GNU/Linux distributions anyway -- you do a package for a certain distro, for the certain libs it installs. Else, an option for such packages is to include libraries that are tricky to rely on (so, everything except libc and libx11, perhaps).
But, this discussion is rather academic considering that mpg123 1.11 _is_ released now and the two sonames are fact now. When one has to deal with the legacy anyway, it wouldn't really help the wine project if mpg123 1.12 would include a dual-mode library, would it? It would still need to deal with possibilities of small-file libmpg123.so, large-file libmpg123.so, large-file libmpg123_64.so, and now possibly dual-mode libmpg123.so, which wouldn't add work, but also it wouldn't save any work:-/
http://bugs.winehq.org/show_bug.cgi?id=22132
--- Comment #12 from Dmitry Timoshkov dmitry@codeweavers.com 2010-03-23 11:18:03 --- (In reply to comment #11)
Oh, and it would need to safely handle passing of file descriptors from the client app to mpg123_open_fd(). I do not know exactly how exchangeable, if at all, descriptors are between small and large file code -- I just expect trouble.
File descriptors are just handles, they have nothing to do with file access.
But yes, I forgot about crossover and friends. Such packages face troubles with GNU/Linux distributions anyway -- you do a package for a certain distro, for the certain libs it installs. Else, an option for such packages is to include libraries that are tricky to rely on (so, everything except libc and libx11, perhaps).
Crossover single build works just fine on a wide range of different distros.
But, this discussion is rather academic considering that mpg123 1.11 _is_ released now and the two sonames are fact now. When one has to deal with the legacy anyway, it wouldn't really help the wine project if mpg123 1.12 would include a dual-mode library, would it? It would still need to deal with possibilities of small-file libmpg123.so, large-file libmpg123.so, large-file libmpg123_64.so, and now possibly dual-mode libmpg123.so, which wouldn't add work, but also it wouldn't save any work:-/
That's unfortunate. You have been already told before to do things the way glibc does them. In the worst case Wine should find another library which takes binary compatibility into account.
http://bugs.winehq.org/show_bug.cgi?id=22132
--- Comment #13 from Alexandre Julliard julliard@winehq.org 2010-03-23 13:34:05 --- (In reply to comment #11)
But, this discussion is rather academic considering that mpg123 1.11 _is_ released now and the two sonames are fact now. When one has to deal with the legacy anyway, it wouldn't really help the wine project if mpg123 1.12 would include a dual-mode library, would it? It would still need to deal with possibilities of small-file libmpg123.so, large-file libmpg123.so, large-file libmpg123_64.so, and now possibly dual-mode libmpg123.so, which wouldn't add work, but also it wouldn't save any work:-/
It would help if it were fixed quickly enough, before it makes it into too many distros. We don't have to address the problem unless it becomes widespread. Of course once the current version will have been around for a while it will be too late.
Oh well, I guess libmpg123 will have to join the (sadly) long list of broken Linux libraries that need special hacks and workarounds. It's not the end of the world, it's just disappointing because we had discussed this before, and you seemed truly interested in fixing this properly, so I had hoped we would be able to get it right.
http://bugs.winehq.org/show_bug.cgi?id=22132
--- Comment #14 from Thomas Orgis thomas-forum@orgis.org 2010-03-23 14:13:39 --- Dimitri: AFAIK, file descriptors are not just handles when it comes to large file mode. There are flags attached depending on small large file mode... and they differ in behaviour when hitting the 2G boundary, I'm quite sure. Unless I know enough to be sure that there is no problem, I have to assume that there is one.
And yes, you suggested that I should do the things like glibc does them, that is the most easy solution for users of the library. And heck, I _care_ for binary compatibility (you can judge by the amount of comment text I produce), but my resources are limited. I've read through http://ac-archive.sourceforge.net/largefile/ and pondered about what I can do to fix the situation. I took the advice of going for the low-maintenance twinlibs approach (http://ac-archive.sourceforge.net/largefile/use_twinlibs.html) which I can manage easily just by ensuring that my code is off_t clean and changing soname for the largefile variant. I took the warnings there seriously -- the wrapper code for dualmode needs time an care to be written and needs to be maintained. It is a possibility for me to introduce subtle bugs, that again cost time.
At least I am more friendly than GnuPG: http://www.gnupg.org/documentation/manuals/gpgme/Largefile-Support-_0028LFS_... ... I added the possiblity to have both versions installed, catering small file apps and large file apps. That is real value, the dualmode library would be sugar on top, and I drew the line there for me: I cannot invest _all_ my time in the mpg123 API (time I should spend on my "real" work anyway, totally unrelated to mpg123). Unless someone steps up to invest his time to introduce dual mode in libmpg123, I am sorry that it won't come.
So of course I hope this doesn't cause wine to drop libmpg123, and I am torn by trying to help and apparently making things worse... would it be better if I'd had changed nothing? I still think no... even if it's complicated, the twinlibs offer a stable ABI that one can rely on in future. First reliability, then convenience. I do care. But I also try to take care of myself and that includes limiting the nights I spend on hacking on such things.
Considering the long list of broken Linux libraries: I consider the idea of this optional large file support that defaults to small off_t being broken (especially in combination with autoconf macros that usually turn it on). But that is my opinion and that helps nobody. And, well, even if one considers the library to be broken, it is a brokeness rather specific to Linux/x86. It might be a brokenness on Solaris/x86, too, but only after the Sun Compiler stopped miscompiling parts of mpg123:-/
Considering the configure patch: Can I assume that, while you condemn the idea and really, really, do not like it, it will still be included in wine? Or did I really start a discussion now about wine changing to another library?
http://bugs.winehq.org/show_bug.cgi?id=22132
--- Comment #15 from Alexandre Julliard julliard@winehq.org 2010-03-23 14:47:08 --- (In reply to comment #14)
Considering the configure patch: Can I assume that, while you condemn the idea and really, really, do not like it, it will still be included in wine? Or did I really start a discussion now about wine changing to another library?
If you are not going to change it then we'll clearly have to support the current state, but it's not clear that that configure check is the right solution. We may have to resort to dynamic loading instead. Switching to a different library will certainly be considered if someone finds a viable alternative.
http://bugs.winehq.org/show_bug.cgi?id=22132
--- Comment #16 from Thomas Orgis thomas-forum@orgis.org 2010-03-26 08:09:15 --- Please have a read of the current news on http://mpg123.org ... there is a chance that I'll do a dualmode lib after all, and possibly scrap libmpg123_64 (or reduce it to an alias to libmpg123). But I am severely annoyed by this "hobby" taking more and more of my lifetime. More and more often I wish I wouldn't care so much on how useful my work is to others.
http://bugs.winehq.org/show_bug.cgi?id=22132
--- Comment #17 from Alexandre Julliard julliard@winehq.org 2010-03-26 08:56:45 --- Thanks for considering this, much appreciated. I can certainly understand your frustration with large file support, it's a mess. Backwards compatibility isn't fun... Feel free to drop me a line if you have something you'd like feedback on before it's made final.
http://bugs.winehq.org/show_bug.cgi?id=22132
--- Comment #18 from Thomas Orgis thomas-forum@orgis.org 2010-03-28 21:02:25 --- Please test the current http://mpg123.org/snapshot and give feedback about its dual-mode implementation. You find some accompanying words for that in the news section of http://mpg123.org .
http://bugs.winehq.org/show_bug.cgi?id=22132
--- Comment #19 from Alexandre Julliard julliard@winehq.org 2010-03-29 10:00:38 --- Looks good to me.
http://bugs.winehq.org/show_bug.cgi?id=22132
--- Comment #20 from Thomas Orgis thomas-forum@orgis.org 2010-03-31 04:43:53 --- Alrighty then... release is out. This bug may be closed.
http://bugs.winehq.org/show_bug.cgi?id=22132
Vijay Kamuju infyquest@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |RESOLVED CC| |infyquest@gmail.com Resolution| |FIXED
--- Comment #21 from Vijay Kamuju infyquest@gmail.com 2010-04-20 23:14:29 --- closing as per submitters request.
http://bugs.winehq.org/show_bug.cgi?id=22132
Dmitry Timoshkov dmitry@codeweavers.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |UNCONFIRMED Resolution|FIXED |
--- Comment #22 from Dmitry Timoshkov dmitry@codeweavers.com 2010-04-21 01:35:10 --- Nothing has been fixed in Wine. Probably Wine configure/code needs to be adapted for the new library, and still be compatible with an old one.
http://bugs.winehq.org/show_bug.cgi?id=22132
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |RESOLVED Resolution| |INVALID
--- Comment #23 from Alexandre Julliard julliard@winehq.org 2010-04-21 02:42:14 --- It doesn't need to be adapted, that's the point. libmpg123 does the right thing now.
http://bugs.winehq.org/show_bug.cgi?id=22132
Dmitry Timoshkov dmitry@codeweavers.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #24 from Dmitry Timoshkov dmitry@codeweavers.com 2010-04-21 02:51:51 --- Good to know, thanks. Closing.
-
wine-bugs＠winehq.org