Re: [Development] QtMultimedia BIC / header cleanliness issue
On 2013-01-22, Thiago Macieira wrote: > Sune, especially for you: will you try and patch Qt to change the sonam= > e if we=20 > don't do it? Sorry for answering a bit late. Given that we haven't yet formally published Qt5, no. if we had, then it would be a definately maybe depending on a insane amount of factors. /Sune ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtMultimedia BIC / header cleanliness issue
On Wed, Jan 23, 2013 at 02:44:07PM +, Koehne Kai wrote: > It makes the file name out of line with the other .so file names. There's a > good chance that this will break some deployment scripts which do just cpy > "lib*.5.so" Then those scripts are really broken, and we should discover it before we need to bump the so version of a (widely) used library. > ... Question the other way round: What do we gain from bumping the version, > except for the feeling that we've 'done the right thing"? I understood that > the chances anyone really running into this bc are miniscule. We find the broken deployment scripts, and we make sure that noone accidentally runs into this bc, which is the whole point of bumping the so-version in the first place... -- Martin Sandsmark ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtMultimedia BIC / header cleanliness issue
> -Original Message- > From: development-bounces+kai.koehne=digia@qt-project.org > [mailto:development-bounces+kai.koehne=digia@qt-project.org] On > Behalf Of Martin Sandsmark > Sent: Wednesday, January 23, 2013 3:38 PM > To: development@qt-project.org > Subject: Re: [Development] QtMultimedia BIC / header cleanliness issue > > On Tue, Jan 22, 2013 at 02:56:27PM +0100, Oswald Buddenhagen wrote: > > this isn't about a cover-up, but about not making a fuss about a > > virtual non-event (no pun intended). changelog entries, etc. are ok > > (like for any bugfix), while changing the soversion seems just a bit over > > the > top. > > Why is bumping the so-version a lot of fuss? It makes the file name out of line with the other .so file names. There's a good chance that this will break some deployment scripts which do just cpy "lib*.5.so" ... Question the other way round: What do we gain from bumping the version, except for the feeling that we've 'done the right thing"? I understood that the chances anyone really running into this bc are miniscule. Regards Kai ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtMultimedia BIC / header cleanliness issue
On Tue, Jan 22, 2013 at 02:56:27PM +0100, Oswald Buddenhagen wrote: > this isn't about a cover-up, but about not making a fuss about a virtual > non-event (no pun intended). changelog entries, etc. are ok (like for > any bugfix), while changing the soversion seems just a bit over the top. Why is bumping the so-version a lot of fuss? (IMHO this says a lot about QtMultimedia in general, but I'll crawl back under my bridge instead of complain about that...) -- Martin Sandsmark ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtMultimedia BIC / header cleanliness issue
On terça-feira, 22 de janeiro de 2013 11.40.23, Oswald Buddenhagen wrote: > > > I'd say we add the virtual destructors. Better to deal with the fallout > > > now > > > then in the future. > > > > And rename the libraries to libQt5Multimedia.so.6 ? > > imo no. just pretend that it never happened. Sune, especially for you: will you try and patch Qt to change the soname if we don't do it? -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtMultimedia BIC / header cleanliness issue
On Tue, Jan 22, 2013 at 02:29:53PM +0100, Shaw Andy wrote: > > On Tuesday, January 22, 2013 11:40:23 AM Oswald Buddenhagen wrote: > > > imo no. just pretend that it never happened. > > > at this point there aren't many packages which are considered stable, > > > and even fewer packages which depend on this module. other users won't > > > be affected by this change to any noteworthy degree. > > > > That seems most reasonable to me. It's not perfect-by-the-book, but I also > > don't think anyone would notice it given the release just in december. > > I don't think it should be done silently at all, I'm not exactly for > the idea of even breaking BC but I can accept an exception this early > in the Qt 5.x series if there is no other way and it is critical to be > done. But this should be clear in the changelog and release notes > that it was done, better to be open about it than to hope no-one > notices. > this isn't about a cover-up, but about not making a fuss about a virtual non-event (no pun intended). changelog entries, etc. are ok (like for any bugfix), while changing the soversion seems just a bit over the top. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtMultimedia BIC / header cleanliness issue
> -Original Message- > > > On Tuesday, January 22, 2013 11:40:23 AM Oswald Buddenhagen wrote: > > > On Mon, Jan 21, 2013 at 08:14:03AM -0800, Thiago Macieira wrote: > > > > On segunda-feira, 21 de janeiro de 2013 15.33.59, Knoll Lars wrote: > > > > > Finally reading up on some old emails… > > > > > > > > > > I'd say we add the virtual destructors. Better to deal with the > > > > > fallout > > > > > now then in the future. > > > > > > > > And rename the libraries to libQt5Multimedia.so.6 ? > > > > > > imo no. just pretend that it never happened. > > > at this point there aren't many packages which are considered stable, > > > and even fewer packages which depend on this module. other users > won't > > > be affected by this change to any noteworthy degree. > > > however, make sure this gets into 5.0.1, which means bugging the > release > > > team at this point, very soon. > > > > That seems most reasonable to me. It's not perfect-by-the-book, but I also > > don't think anyone would notice it given the release just in december. > > (famous > > last words!) > > I don't think it should be done silently at all, I'm not exactly for the idea > of > even breaking BC but I can accept an exception this early in the Qt 5.x series > if there is no other way and it is critical to be done. But this should be > clear in > the changelog and release notes that it was done, better to be open about it > than to hope no-one notices. I must say I agree with Andy here. Especially as we are actually doing this development in the open. As anyone can watch us, we are setting a standard for how SW development is done, and Qt has had a high star as far as SW quality goes. This is the reason many people choose Qt. And I think that is goes without saying, even though not many people are likely to be using it, that especially as we do something out of the ordinary we need to properly document it. Anything else would be rude. Best regards Nils Christian Roscher-Nielsen – Digia, Qt > ___ > Development mailing list > Development@qt-project.org > http://lists.qt-project.org/mailman/listinfo/development ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtMultimedia BIC / header cleanliness issue
> On Tuesday, January 22, 2013 11:40:23 AM Oswald Buddenhagen wrote: > > On Mon, Jan 21, 2013 at 08:14:03AM -0800, Thiago Macieira wrote: > > > On segunda-feira, 21 de janeiro de 2013 15.33.59, Knoll Lars wrote: > > > > Finally reading up on some old emails… > > > > > > > > I'd say we add the virtual destructors. Better to deal with the fallout > > > > now > > > > then in the future. > > > > > > And rename the libraries to libQt5Multimedia.so.6 ? > > > > imo no. just pretend that it never happened. > > at this point there aren't many packages which are considered stable, > > and even fewer packages which depend on this module. other users won't > > be affected by this change to any noteworthy degree. > > however, make sure this gets into 5.0.1, which means bugging the release > > team at this point, very soon. > > That seems most reasonable to me. It's not perfect-by-the-book, but I also > don't think anyone would notice it given the release just in december. > (famous > last words!) I don't think it should be done silently at all, I'm not exactly for the idea of even breaking BC but I can accept an exception this early in the Qt 5.x series if there is no other way and it is critical to be done. But this should be clear in the changelog and release notes that it was done, better to be open about it than to hope no-one notices. Andy ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtMultimedia BIC / header cleanliness issue
On Tuesday, January 22, 2013 11:40:23 AM Oswald Buddenhagen wrote: > On Mon, Jan 21, 2013 at 08:14:03AM -0800, Thiago Macieira wrote: > > On segunda-feira, 21 de janeiro de 2013 15.33.59, Knoll Lars wrote: > > > Finally reading up on some old emails… > > > > > > I'd say we add the virtual destructors. Better to deal with the fallout > > > now > > > then in the future. > > > > And rename the libraries to libQt5Multimedia.so.6 ? > > imo no. just pretend that it never happened. > at this point there aren't many packages which are considered stable, > and even fewer packages which depend on this module. other users won't > be affected by this change to any noteworthy degree. > however, make sure this gets into 5.0.1, which means bugging the release > team at this point, very soon. That seems most reasonable to me. It's not perfect-by-the-book, but I also don't think anyone would notice it given the release just in december. (famous last words!) Simon ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtMultimedia BIC / header cleanliness issue
On Mon, Jan 21, 2013 at 08:14:03AM -0800, Thiago Macieira wrote: > On segunda-feira, 21 de janeiro de 2013 15.33.59, Knoll Lars wrote: > > Finally reading up on some old emails… > > > > I'd say we add the virtual destructors. Better to deal with the fallout now > > then in the future. > > And rename the libraries to libQt5Multimedia.so.6 ? > imo no. just pretend that it never happened. at this point there aren't many packages which are considered stable, and even fewer packages which depend on this module. other users won't be affected by this change to any noteworthy degree. however, make sure this gets into 5.0.1, which means bugging the release team at this point, very soon. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtMultimedia BIC / header cleanliness issue
On segunda-feira, 21 de janeiro de 2013 15.33.59, Knoll Lars wrote: > Finally reading up on some old emails… > > I'd say we add the virtual destructors. Better to deal with the fallout now > then in the future. And rename the libraries to libQt5Multimedia.so.6 ? -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtMultimedia BIC / header cleanliness issue
Finally reading up on some old emails… I'd say we add the virtual destructors. Better to deal with the fallout now then in the future. Cheers, Lars On Dec 28, 2012, at 1:03 PM, Thiago Macieira wrote: > On sexta-feira, 28 de dezembro de 2012 19.11.32, Sze Howe Koh wrote: >> On 25 December 2012 01:35, Thiago Macieira > wrote: >>> On segunda-feira, 24 de dezembro de 2012 06.30.58, Sascha Cunz wrote: But strictly spoken, I would rather say no: BC is BC no matter what silly mistakes it includes (I have actually had to deal with code that used the above mentioned inheritance layouts). OTOH, it's still very early in the life of released Qt5. >>> >>> We can change the soname for the library. libQt5Multimedia.so.6. >> >> I've never seen Qt DLLs that have additional numbers on Windows; is >> the changing-soname approach acceptable on Windows and Mac? > > It's acceptable on Mac, for the .dylib (non-framework) builds. For framework > builds as well as for Windows, we'd have to invent some new method. > > On Windows, the Qt DLLs have always had the version number: QtCore4.dll and > Qt5Core.dll. However, we're now proposing separating the source version (5) > from the binary version (6). > >> Whatever solution is taken, we'll need to add notes to the >> documentation of both classes, either to say "Binary files that >> include this interface are incompatible between Qt 5.0 and Qt 5.1", or >> "This interface class contains a bug, do not perform a deletion >> through it." > > Agreed. > > -- > Thiago Macieira - thiago.macieira (AT) intel.com > Software Architect - Intel Open Source Technology Center > ___ > Development mailing list > Development@qt-project.org > http://lists.qt-project.org/mailman/listinfo/development ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtMultimedia BIC / header cleanliness issue
On sexta-feira, 28 de dezembro de 2012 19.11.32, Sze Howe Koh wrote: > On 25 December 2012 01:35, Thiago Macieira wrote: > > On segunda-feira, 24 de dezembro de 2012 06.30.58, Sascha Cunz wrote: > >> But strictly spoken, I would rather say no: BC is BC no matter what silly > >> mistakes it includes (I have actually had to deal with code that used the > >> above mentioned inheritance layouts). > >> OTOH, it's still very early in the life of released Qt5. > > > > We can change the soname for the library. libQt5Multimedia.so.6. > > I've never seen Qt DLLs that have additional numbers on Windows; is > the changing-soname approach acceptable on Windows and Mac? It's acceptable on Mac, for the .dylib (non-framework) builds. For framework builds as well as for Windows, we'd have to invent some new method. On Windows, the Qt DLLs have always had the version number: QtCore4.dll and Qt5Core.dll. However, we're now proposing separating the source version (5) from the binary version (6). > Whatever solution is taken, we'll need to add notes to the > documentation of both classes, either to say "Binary files that > include this interface are incompatible between Qt 5.0 and Qt 5.1", or > "This interface class contains a bug, do not perform a deletion > through it." Agreed. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtMultimedia BIC / header cleanliness issue
On 25 December 2012 01:35, Thiago Macieira wrote: > On segunda-feira, 24 de dezembro de 2012 06.30.58, Sascha Cunz wrote: >> But strictly spoken, I would rather say no: BC is BC no matter what silly >> mistakes it includes (I have actually had to deal with code that used the >> above mentioned inheritance layouts). >> OTOH, it's still very early in the life of released Qt5. > > We can change the soname for the library. libQt5Multimedia.so.6. I've never seen Qt DLLs that have additional numbers on Windows; is the changing-soname approach acceptable on Windows and Mac? Whatever solution is taken, we'll need to add notes to the documentation of both classes, either to say "Binary files that include this interface are incompatible between Qt 5.0 and Qt 5.1", or "This interface class contains a bug, do not perform a deletion through it." Sze-Howe ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtMultimedia BIC / header cleanliness issue
On segunda-feira, 24 de dezembro de 2012 06.30.58, Sascha Cunz wrote: > > Adding a new virtual implies changing the layout of the virtual table. If > > we append the virtual, the order of the existing virtuals should not > > change, so the calls should still make through. > > Given the two classes: > > class A { > public: virtual void a(); > }; > > class B : public A { > public: virtual void b(); > }; > > You're effectively saying here that adding a virtual to A doesn't change the > v-table layout of A (which by itself is right), but it implicitly changes > the v-table layout of B (B::b()'s address-slot moves one slot behind). Correct. > > Since the virtual destructor is missing, one can assume that objects of > > those classes are never deleted through those specific classes anyway. > > Moreover, it seems that those classes are often used in multiple > > inheritance and are not in the primary inheritance path. That means the > > addition of virtuals will not change the primary virtual table. > > What is the effect to: > > class UserClass : public QObject, WhereVTableWouldBeExpanded, > AnotherInterface {} > > I think to remember to see that compilers produce code that expects the > vtable of subobjects at a fixed offset from the beginning of the overall > vtable. Correct too. > But strictly spoken, I would rather say no: BC is BC no matter what silly > mistakes it includes (I have actually had to deal with code that used the > above mentioned inheritance layouts). > OTOH, it's still very early in the life of released Qt5. We can change the soname for the library. libQt5Multimedia.so.6. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtMultimedia BIC / header cleanliness issue
On Saturday, December 22, 2012 09:13:05 PM Thiago Macieira wrote: > On domingo, 23 de dezembro de 2012 11.50.18, Sze Howe Koh wrote: > > For consistency with the other interfaces, I suggest simply adding > > empty virtual destructors. I believe this won't break any existing > > code; the only code that should be affected is deletion via pointers > > to QMediaServiceProviderFactoryInterface/QAudioSystemFactoryInterface. > > But in this case, our change would probably fix memory leaks for > > programs which use such code. > > Adding a new virtual implies changing the layout of the virtual table. If we > append the virtual, the order of the existing virtuals should not change, > so the calls should still make through. Given the two classes: class A { public: virtual void a(); }; class B : public A { public: virtual void b(); }; You're effectively saying here that adding a virtual to A doesn't change the v-table layout of A (which by itself is right), but it implicitly changes the v-table layout of B (B::b()'s address-slot moves one slot behind). The above could happen, if someone doesn't inherit the interface directly. > Since the virtual destructor is missing, one can assume that objects of > those classes are never deleted through those specific classes anyway. > Moreover, it seems that those classes are often used in multiple > inheritance and are not in the primary inheritance path. That means the > addition of virtuals will not change the primary virtual table. What is the effect to: class UserClass : public QObject, WhereVTableWouldBeExpanded, AnotherInterface {} I think to remember to see that compilers produce code that expects the vtable of subobjects at a fixed offset from the beginning of the overall vtable. Further someone might be keen enough to use virtual-inheritance with interfaces. > So the question is: is the binary-incompatible change acceptable? As for how those interfaces are _meant to be used_, i would personally say: just go ahead and break BC - In the usual case it doesn't matter. But strictly spoken, I would rather say no: BC is BC no matter what silly mistakes it includes (I have actually had to deal with code that used the above mentioned inheritance layouts). OTOH, it's still very early in the life of released Qt5. Sascha___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtMultimedia BIC / header cleanliness issue
On domingo, 23 de dezembro de 2012 11.50.18, Sze Howe Koh wrote: > For consistency with the other interfaces, I suggest simply adding > empty virtual destructors. I believe this won't break any existing > code; the only code that should be affected is deletion via pointers > to QMediaServiceProviderFactoryInterface/QAudioSystemFactoryInterface. > But in this case, our change would probably fix memory leaks for > programs which use such code. Adding a new virtual implies changing the layout of the virtual table. If we append the virtual, the order of the existing virtuals should not change, so the calls should still make through. Since the virtual destructor is missing, one can assume that objects of those classes are never deleted through those specific classes anyway. Moreover, it seems that those classes are often used in multiple inheritance and are not in the primary inheritance path. That means the addition of virtuals will not change the primary virtual table. So the question is: is the binary-incompatible change acceptable? -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QtMultimedia BIC / header cleanliness issue
On 23 December 2012 06:20, Thiago Macieira wrote: > Hello > > Now that Qt 5.0 is out, I've been doing some clean up tasks I had been putting > off. One of them, to properly do the headersclean test, turned up that > QtMultimedia did not have this test at all. And here's what it produces: > > multimedia/qmediaserviceproviderplugin.h:111:28: error: ‘struct > QMediaServiceProviderFactoryInterface’ has virtual functions and accessible > non-virtual destructor [-Werror=non-virtual-dtor] > > audio/qaudiosystemplugin.h:63:28: error: ‘struct QAudioSystemFactoryInterface’ > has virtual functions and accessible non-virtual destructor [-Werror=non- > virtual-dtor] > > Clearly those two classes cannot be used for deletion. Is that normal? What > are those classes used for, anyway? > > Note that those two classes are exported, but they don't have any out-of-line > virtuals. For QAudioSystemFactoryInterface, the virtual table is emitted in > the library but there isn't one for QMediaServiceProviderFactoryInterface.\ > > Any suggestions on what to do? > -- > Thiago Macieira - thiago.macieira (AT) intel.com > Software Architect - Intel Open Source Technology Center Those interface classes originate from Qt Mobility. I'm guessing that the lack of virtual destructors was an old oversight -- other interfaces (e.g. QMediaServiceSupportedDevicesInterface in qmediaserviceproviderplugin.h) do have empty virtual destructors. For consistency with the other interfaces, I suggest simply adding empty virtual destructors. I believe this won't break any existing code; the only code that should be affected is deletion via pointers to QMediaServiceProviderFactoryInterface/QAudioSystemFactoryInterface. But in this case, our change would probably fix memory leaks for programs which use such code. Sze-Howe P.S. Slightly off-tangent, but I've been unsuccessful in getting an answer so far: Do we still have a maintainer for Qt Multimedia? ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
[Development] QtMultimedia BIC / header cleanliness issue
Hello Now that Qt 5.0 is out, I've been doing some clean up tasks I had been putting off. One of them, to properly do the headersclean test, turned up that QtMultimedia did not have this test at all. And here's what it produces: multimedia/qmediaserviceproviderplugin.h:111:28: error: ‘struct QMediaServiceProviderFactoryInterface’ has virtual functions and accessible non-virtual destructor [-Werror=non-virtual-dtor] audio/qaudiosystemplugin.h:63:28: error: ‘struct QAudioSystemFactoryInterface’ has virtual functions and accessible non-virtual destructor [-Werror=non- virtual-dtor] Clearly those two classes cannot be used for deletion. Is that normal? What are those classes used for, anyway? Note that those two classes are exported, but they don't have any out-of-line virtuals. For QAudioSystemFactoryInterface, the virtual table is emitted in the library but there isn't one for QMediaServiceProviderFactoryInterface.\ Any suggestions on what to do? -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development