| Summary: | wxsvg - C++ library to create, manipulate and render SVG files | ||
|---|---|---|---|
| Product: | Package Reviews | Reporter: | Stewart Adam <s.adam> |
| Component: | Review Request | Assignee: | Nicolas Chauvet <kwizart> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | dominik, kevin.kofler, kwizart, om, pwaldenlinux, rpmfusion-package-review |
| Priority: | P5 | ||
| Version: | Current | ||
| Hardware: | All | ||
| OS: | GNU/Linux | ||
| namespace: | free | ||
| Bug Depends on: | |||
| Bug Blocks: | 4, 148 | ||
| Attachments: |
rpmsodiff between fedora/rpmfusion 1.0-2
rpmsodiff between fedora (1.0-4) and rpmfusion (1.0-3) without soname patch |
||
|
Description
Stewart Adam
2008-09-27 19:51:45 CEST
Made a bunch of changes, the package should work standalone now. Also added README.Fedora which included instructions on how to use wxsvg-freeworld when building other packages. SPEC: http://downloads.diffingo.com/rpmfusion/wxsvg-freeworld.spec SRPM: http://downloads.diffingo.com/rpmfusion/wxsvg-freeworld-1.0-0.9.b11.fc10.src.rpm There is lot of unused-direct-shlib-dependency from the wxsvg library on wxGTK. I think it should be the same with the binaries as nothing suggest from the code that it should use all thoses libraries (look at /usr/bin/calculette-freeworld ) My guess is that (as freetype-freeworld), you should only provide the library. Nothing suggest that ffmpeg should be required from the -devel. And even that the devel should be provided. It is also a really bad idea to rename the headers name (which result as an API break). Would be better to install headers in /usr/include/wxsvg-freeworld and then having them in /usr/include/wxsvg-freeworld/wxSVG/svg.h and /usr/include/wxsvg-freeworld/wxSVGXML/svgxml.h so that can only be overridden by the addition of a -I/usr/include/wxsvg-freeworld CFLAGS. Your export CFLAGS failed to set RPM_OPT_FLAGS. It is better to avoid this kind of bad hack and set the FFMPEG_CFLAGS accurately (discovered by libavcodec.pc if not already done). Okay, the package no longer include the headers or the binaries. Instead, I've added a requirement for wxsvg and wxsvg-devel where applicable to pull in the original binaries and headers.
I patched in LIBAVFORMAT_{CFLAGS,LDFLAGS} into Makefile.am as discussed on IRC. The ffmpeg-devel requirement is gone as well. One thing is at the moment, wxsvg in Fedora isn't updated to beta11 yet so unless --nodeps is used, this won't build for now.
SPEC: http://downloads.diffingo.com/rpmfusion/wxsvg-freeworld.spec
SRPM:
http://downloads.diffingo.com/rpmfusion/wxsvg-freeworld-1.0-0.10.b11.fc10.src.rpm
Is the -devel package really needed? In other words, is it possible to just build against the Fedora wxsvg-devel and override it at runtime only? As kwizart explained, that's how freetype-freeworld works: there's no -devel package, instead you build against the regular freetype-devel, freetype-freeworld is binary compatible with it. But that won't work if there are symbols (functions or global variables) in the .so which are not available if ffmpeg support is disabled. I took another look and realized the one file packaged in -devel was a symlink .so. The -devel subpackage is gone now: SPEC: http://downloads.diffingo.com/rpmfusion/wxsvg-freeworld.spec SRPM: http://downloads.diffingo.com/rpmfusion/wxsvg-freeworld-1.0-0.11.b11.fc10.src.rpm I just build wxsvg 1.0beta11 for Fedora, so rebuilding this in mock should work tomorrow or the day after without and dep errors. Erm, that should read "I just built wxsvg 1.0beta11 for Fedora, so rebuilding this in mock should work tomorrow or the day after without any dep errors." * BuildRequires: pkgconfig : This isn't needed (bring but freetype-devel at least).
* Requires: /etc/ld.so.conf.d : uneeded, we can assume this directory is here.
* Requires: wxsvg >= %{version}: <This would be needed if the dependent package would need the binaries along with the library recompiled. Actually the binaries are failing (each ones output a backstrace or a segmentation fault)
I cannot investigate this at this time but the original Fedora package need some fixes... (failing on i686 In my case).
* The unused-direct-shlib-dependency aren't fixed in the original fedora package
* Why this package is building an internal version of libs/libexpat.a ?
(whereas it can use the shared one?)
* We don't use %{?_smp_mflags} at %install step. (uneeded)
* Some headers/libraries are missing from your ffmepg patch.
checking for FFMPEG...
yes
checking ffmpeg/avutil.h usability...
no
checking ffmpeg/avutil.h presence...
no
checking for ffmpeg/avutil.h...
no
* Package is now built with out RPM_OPT_FLAGS - good.
Okay, I noticed dvdstyler 1.7.1 was out, and it requires wxsvg version 1.0 final (it's finally out of beta). Here are the updates packages: SPEC: http://downloads.diffingo.com/rpmfusion/wxsvg-freeworld.spec SRPM: http://downloads.diffingo.com/rpmfusion/wxsvg-freeworld-1.0-1.fc10.src.rpm Changes: * Removed interal build of expat * Removed dependency on wxsvg * Removed requires on /etc/ld.so.conf.d * Removed BR for pkgconfig * Fix %{?smp_mflags} typo on %install line Worth mentioning: * rpmlint shows no unused-direct-shlib-dependency after update to 1.0 on the Fedora packages. 1.0 has been tagged and is building now for rawhide. * Unfortunately, the *.so file is needed to enable programs to build properly against wxsvg-freeworld. --> wxsvg-freeworld.x86_64: W: devel-file-in-non-devel-package /usr/lib64/wxsvg-freeworld/libwxsvg.so Despite the warning, I decided to put all into one package since it would be overkill to put the single symlink into its own -devel subpackage imho. * As for the ffmpeg headers, it does two checks - one using pkgconfig (new way) and one using the header check (old way)... Since the new way works, we can just ignore it. Oops, I should have checked the terminal output first... dist-f10 is locked, so I'll build 1.0 for Fedora after the release. For review purposes, this is the SRPM of wxsvg-1.0-1.fc10 for Fedora: http://downloads.diffingo.com/rpmfusion/wxsvg-1.0-1.fc10.src.rpm > Despite the warning, I decided to put all into one package since it would be
> overkill to put the single symlink into its own -devel subpackage imho.
You can't do that. Even if it contains just a single .so symlink, devel symlinks MUST be in a -devel package, no exceptions (well, if it's a compiler or something similar where the entire package is effectively a devel package, that's an exception, but not for a library!).
Also: * How does one build against the .so symlink in the private directory? Manual -L flag hackery? * Are you SURE there's binary compatibility in the sense that packages linked against the Fedora wxsvg will work with this one? (I'm asking because the necessity to link against the .so from wxsvg-freeworld is indicative of some binary incompatibility, but it can still be compatible in the direction which is actually relevant.) Please test some packages using wxsvg in Fedora with wxsvg-freeworld installed to make sure they still work with the replaced library. If they don't, you'll have to rename the library (to something like libwxsvg.so.0fw), not move it and have it override the Fedora version. (In reply to comment #11) > You can't do that. Even if it contains just a single .so symlink, devel > symlinks MUST be in a -devel package, no exceptions We'll have to ignore rpmlint's error about no binary files in usr/lib then. Besides being more practical, I figured error > warning so it was better to put it into the main package ;) Here it is with a split -devel: SPEC: http://downloads.diffingo.com/rpmfusion/wxsvg-freeworld.spec SRPM: http://downloads.diffingo.com/rpmfusion/wxsvg-freeworld-1.0-2.fc10.src.rpm (In reply to comment #12) > Also: > * How does one build against the .so symlink in the private directory? Manual > -L flag hackery? Yup, AFAIK there's no other way. It's not great, but it's better than conflicting with the Fedora installation of wxsvg (in which case nothing would work at all). We have the same situation for the nvidia libs vs mesa libs for example. > * Are you SURE there's binary compatibility in the sense that packages linked > against the Fedora wxsvg will work with this one? (I'm asking because the > necessity to link against the .so from wxsvg-freeworld is indicative of some > binary incompatibility, but it can still be compatible in the direction which > is actually relevant.) Please test some packages using wxsvg in Fedora with > wxsvg-freeworld installed to make sure they still work with the replaced > library. If they don't, you'll have to rename the library (to something like > libwxsvg.so.0fw), not move it and have it override the Fedora version. Other than the addition of some (ffmpeg-related) functions, everything should be identical. I've tested using svgview from the Fedora package while the overrides are active and everything still functions as expected. Oh, by the way, you can build the Fedora wxsvg-1.0-1.fc10 for dist-f10-updates-candidate right now (just cvs up, including your common/ directory, and you'll get an F-10 branch which builds to dist-f10-updates-candidate) and also queue it as a post-release update in Bodhi. Then you could also provide a Koji link here rather than just the SRPM. wxsvg 1.0 was just pushed into stable for F10, F9 and F8 so the package's requirements should be satisfiable now without using --nodeps. Created attachment 37 [details]
rpmsodiff between fedora/rpmfusion 1.0-2
this log show the diff between symbols available in the fedora elf library and the rpmfusion one. The rpmfusion one has symbols added, which is what we could expect with the ffmpeg-devel addition as BR (along with the expat-devel fix that seems to be enought to link to the system shared expat and added the XML symbols).
Now that seems a little more scary about symbols that are removed; since a third part library/binary might rely on these symbols from the fedora package and hence, that's what we call Application Binary Interface to collapse,( aka ABI break.)
The question is then:
Are theses symbols exposed to the API or only used internaly from the wxsvg library. At this time I'm not sure about how to check (there is a need to check the wxsvg-devel header at least). But still the symbols removal is suspect. (why adding BR will remove symbols?). And maybe some code is conditionalized.
for now that would be fine to have the expat-devel added to the fedora package and the expat sources removed between configure and make (so we are sure not to use the internals headers at fedora wxsvg built time).
Then we will compare the two built more easily.
If we cannot assure ABI from the fedora to the rpmfusion package to be preserved, then we might work on another solution with a library soname change (rename).but fedora packages that will use fedora wxsvg won't be able to use wxsvg-freeworld.
That makes me wonder which package "can" use wxsvg and if a ffmpeg-less version of wxsvg is "at the end" usable.
In reply to comment #15, the wxSVG release has broken installed livna dvdstyler instances. An RPMfusion package for dvdstyler is not available yet. So dvdstyler F8-10 users are dead in the water. Sorry, I forgot to add the details [pwalden@walden3 ~]$ dvdstyler dvdstyler: symbol lookup error: dvdstyler: undefined symbol: _ZN9wxSVGCtrl13sm_eventTableE An ABI change without a soname bump? Looks like upstream deserves some beating. ;-( Between b7 and b10: $ abicheck /usr/bin/dvdstyler /usr/bin/dvdstyler: PRIVATE: (libc.so.6:GLIBC_2.4) __stack_chk_fail /usr/bin/dvdstyler: UNBOUND_SYMBOLS: 17 (run ldd -r on binary for more info) $ ldd -r /usr/bin/dvdstyler | grep ^\s undefined symbol: _ZN9wxSVGCtrl13sm_eventTableE (/usr/bin/dvdstyler) /usr/bin/dvdstyler: Symbol `_ZTV16wxSVGRectElement' has different size in shared object, consider re-linking /usr/bin/dvdstyler: Symbol `_ZTV13wxXmlDocument' has different size in shared object, consider re-linking /usr/bin/dvdstyler: Symbol `_ZTV16wxSVGDefsElement' has different size in shared object, consider re-linking /usr/bin/dvdstyler: Symbol `_ZTV15wxSVGUseElement' has different size in shared object, consider re-linking /usr/bin/dvdstyler: Symbol `_ZTV16wxSVGLineElement' has different size in shared object, consider re-linking /usr/bin/dvdstyler: Symbol `_ZTV15wxSVGSVGElement' has different size in shared object, consider re-linking /usr/bin/dvdstyler: Symbol `_ZTV12wxSVGElement' has different size in shared object, consider re-linking /usr/bin/dvdstyler: Symbol `_ZTV17wxSVGImageElement' has different size in shared object, consider re-linking /usr/bin/dvdstyler: Symbol `_ZTV13wxSVGGElement' has different size in shared object, consider re-linking /usr/bin/dvdstyler: Symbol `_ZTV17wxSVGVideoElement' has different size in shared object, consider re-linking undefined symbol: _Z17XmlFindNodeSimpleP9wxXmlNodeRK8wxString (/usr/bin/dvdstyler) undefined symbol: _ZN9wxSVGCtrl5ClearEv (/usr/bin/dvdstyler) undefined symbol: _Z12XmlReadValueP9wxXmlNodeRK8wxString (/usr/bin/dvdstyler) undefined symbol: _Z11XmlFindNodeP9wxXmlNodeRK8wxString (/usr/bin/dvdstyler) undefined symbol: _ZN13wxXmlDocument4LoadER13wxInputStreamRK8wxString (/usr/bin/dvdstyler) undefined symbol: _ZN9wxSVGCtrlC2EP8wxWindowiRK7wxPointRK6wxSizelRK11wxValidatorRK8wxString (/usr/bin/dvdstyler) undefined symbol: _ZN13wxXmlDocument7SetRootEP9wxXmlNode (/usr/bin/dvdstyler) undefined symbol: _ZN9wxSVGCtrlD2Ev (/usr/bin/dvdstyler) undefined symbol: _ZNK9wxSVGCtrl8GetScaleEv (/usr/bin/dvdstyler) undefined symbol: _ZN9wxSVGCtrl6SetSVGEP13wxSVGDocument (/usr/bin/dvdstyler) undefined symbol: _ZNK13wxXmlDocument4SaveER14wxOutputStream (/usr/bin/dvdstyler) undefined symbol: _ZNK9wxXmlNode18GetPreviousSiblingEv (/usr/bin/dvdstyler) undefined symbol: _Z13XmlWriteValueP9wxXmlNodeRK8wxStringS3_ (/usr/bin/dvdstyler) undefined symbol: _ZNK13wxXmlDocument4SaveERK8wxString (/usr/bin/dvdstyler) undefined symbol: _ZN9wxSVGCtrl7RefreshEbPK6wxRect (/usr/bin/dvdstyler) undefined symbol: _ZN13wxXmlDocument4LoadERK8wxStringS2_ (/usr/bin/dvdstyler) (In reply to comment #16) > this log show the diff between symbols available in the fedora elf library and > the rpmfusion one. The rpmfusion one has symbols added, which is what we could > expect with the ffmpeg-devel addition as BR (along with the expat-devel fix > that seems to be enought to link to the system shared expat and added the XML > symbols). This has been done on the Fedora version - expat is required by svgview, which we remove from the package after %build, so for wxsvg-freeworld we only need a BR. > The question is then: > Are theses symbols exposed to the API or only used internaly from the wxsvg > library. At this time I'm not sure about how to check (there is a need to check > the wxsvg-devel header at least). But still the symbols removal is suspect. > (why adding BR will remove symbols?). And maybe some code is conditionalized. The only files which contain '#ifdef USE_FFMPEG' are: * include/wxSVG/SVGCanvasItem.h * src/SVGCanvasItem.cpp * src/SVGDocument.cpp I've taken a look but can't see any immediate reason as to why symbols would be removed... There's lots of code within functions that have been conditionality, but apart from this: #ifdef USE_FFMPEG class wxFfmpegMediaDecoder; #endif not much should be changing. > for now that would be fine to have the expat-devel added to the fedora package > and the expat sources removed between configure and make (so we are sure not to > use the internals headers at fedora wxsvg built time). > Then we will compare the two built more easily. done on rawhide, I'm requesting a F-10 update as soon as it's done building. (Koji build for rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=993919) > If we cannot assure ABI from the fedora to the rpmfusion package to be > preserved, then we might work on another solution with a library soname change > (rename). but fedora packages that will use fedora wxsvg won't be able to use > wxsvg-freeworld. > That makes me wonder which package "can" use wxsvg and if a ffmpeg-less version > of wxsvg is "at the end" usable. As long as a program uses the non-ffmpeg parts of wxsvg, it works fine - I've tested svgview and it displays a few test SVGs (mostly) OK, but that seems to be a bug with the program itself and not because of the lack of ffmpeg. I've created a package with a different soname here: SPEC: http://downloads.diffingo.com/rpmfusion/wxsvg-freeworld.spec SRPM: http://downloads.diffingo.com/rpmfusion/wxsvg-freeworld-1.0-3.fc10.src.rpm I've tested it with dvdstyler and it works well. It would be really nice if this became available fairly soon on rpmfusion. As noted, availability of dvdstyler-1.7.1 depends on this. can you produce the updated rpmsodiff with fedora and this package (then, using the same SONOME this time so we can compare). Created attachment 66 [details]
rpmsodiff between fedora (1.0-4) and rpmfusion (1.0-3) without soname patch
The attachment I just created is the rpmsodiff between the lastest Fedora wxsvg in updates-testing (1.0-4) and the current RPM Fusion one (1.0-3), but without the soname patch enabled so that rpmsodiff would work. I double-checked manually anyways, and with the soname patch enabled the same symbols are added/removed as expected. Okay, we can move ahead with the package removal now. If you still think this is the best option, I'll start the package removal process. I've asked upstream about any software which does not require the ffmpeg extensions, you can view the thread here: https://sourceforge.net/forum/forum.php?thread_id=2850496&forum_id=424987 At the moment, none exists. One of the developers writes some programs which don't use the ffmpeg extensions, but he's going to move those to the wxsvg-python once it's ready. OK so advice #fedora-devel mailing list that wxsvg will be removed for F-11 (we cannot remove it from the Everything repository so it will be always available in F-9/F10) (1) Then you can build the wxsvg-freeworld version (with the SONAME change), for F-9/10. We might need another review for the F-11 version (wxsvg) without the SONAME change. If needed, you can ask on the rpmfusion mailing list if wxsvg can have a common agreament to be introduced in F-9/F10 without the soname change and hence will replace the deprecated fedora wxsvg as a special exception. (1) This is the EndOfLife procedure https://fedoraproject.org/wiki/PackageMaintainers/PackageEndOfLife re-open, I've made an error, sorry As I earlier said, it would be really nice if this became available fairly soon on rpmfusion, availability of dvdstyler-1.7.1 depends on this. Package removal process in Fedora is done now, so the -freeworld suffix can be removed from the package (along with the soname patch). SPEC: http://downloads.diffingo.com/rpmfusion/wxsvg.spec SRPM: http://downloads.diffingo.com/rpmfusion/wxsvg-1.0-5.fc10.src.rpm Description: wxSVG is C++ library to create, manipulate and render SVG files. (In reply to comment #31) > Package removal process in Fedora is done now, One note: Can you please put a small note in the wiki somewhere that explains the whole situation in a few words? Maybe we should send the text and a link to that page to a few mailing lists as well, to make sure people are aware of the whole situation as well. That should mostly prevent that people start to think "RPM Fusion replaces packages from Fedora" (In reply to comment #32) > (In reply to comment #31) > > Package removal process in Fedora is done now, > > One note: Can you please put a small note in the wiki somewhere that explains > the whole situation in a few words? Maybe we should send the text and a link to > that page to a few mailing lists as well, to make sure people are aware of the > whole situation as well. That should mostly prevent that people start to think > "RPM Fusion replaces packages from Fedora" Done - http://rpmfusion.org/Package/wxsvg Well, I've missed something with 1.0-5: You seem to have removed the SONAME change but also dropped the binaries and the headers ? Do you plan to rely on the fedora package for the -devel and F9/F10 ? - libtool is missing as BR as you are calling autogen.sh (libtool will bring automake and autoconf). - Please remove #Pulled by ffmpeg #BuildRequires: pkgconfig As pkg-config will be bring by nearly all BuildRequires that are mentioned. - Headers and binaries aren't provided # Headers are the same (for now at least); don't dup them. Binaries too - Requirements for the -devel subpackage: Are you sure that Requires: wxGTK-devel is needed ? * Package has built fine with mock on: - Rawhide i386 (once libtool is added as BR). - Fedora 10 i386 (once libtool is added as BR). (In reply to comment #34) > Well, I've missed something with 1.0-5: > You seem to have removed the SONAME change but also dropped the binaries and > the headers ? Do you plan to rely on the fedora package for the -devel and > F9/F10 ? > > - libtool is missing as BR as you are calling autogen.sh (libtool will bring > automake and autoconf). Fixed > - Please remove > #Pulled by ffmpeg > #BuildRequires: pkgconfig > As pkg-config will be bring by nearly all BuildRequires that are mentioned. Done > - Headers and binaries aren't provided > # Headers are the same (for now at least); don't dup them. Binaries too Sorry, I forgot to remove the rm -rf... Fixed now > - Requirements for the -devel subpackage: > Are you sure that Requires: wxGTK-devel is needed ? > Yup - some of the wxGTK files are included in the wxSVG headers, and there's also a check for in configure: "checking for wxWidgets version >= 2.8.7... yes (version 2.8.9)" SPEC: http://downloads.diffingo.com/rpmfusion/wxsvg.spec SRPM: http://downloads.diffingo.com/rpmfusion/wxsvg-1.0-6.fc10.src.rpm So, indeed, the wxGTK-devel package is needed from wxsvg-devel Since the wxsvg-devel headers contains some includes from the wxGTK-devel package (like #include <wx/string.h> , etc) ----------------- This package (wxsvg) is APPROVED by me ----------------- Package CVS request ====================== Package Name: wxsvg Short Description: C++ library to create, manipulate and render SVG files Owners: firewing Branches: F-9 F-10 InitialCC: ---------------------- License tag: free (In reply to comment #37) > Package CVS request > ====================== > Package Name: wxsvg Done ------8<------- 1 size_t fwrite(const void * __restrict ptr, size_t size, http://www-look-4.com/category/travel/ 2 size_t nmemb, register FILE * __restrict stream) 3 { 4 size_t retval; https://komiya-dental.com/category/technology/ 5 __STDIO_AUTO_THREADLOCK_VAR; 6 http://www.iu-bloomington.com/category/technology/ 7 > __STDIO_AUTO_THREADLOCK(stream); 8 9 retval = fwrite_unlocked(ptr, size, nmemb, stream); 10 https://waytowhatsnext.com/category/technology/ 11 __STDIO_AUTO_THREADUNLOCK(stream); 12 http://www.wearelondonmade.com/category/travel/ 13 return retval; 14 } ------>8------- http://www.jopspeech.com/category/travel/ Here, we are at line 7. Using the "next" command leads no where. However, setting a breakpoint on line 9 and issuing "continue" works. http://joerg.li/category/travel/ Looking at the assembly instructions reveals that we're dealing with the critical section entry code [1] that should never be interrupted, in this case by the debugger's implicit breakpoints: http://connstr.net/category/travel/ ------8<------- ... http://embermanchester.uk/category/travel/ 1 add_s r0,r13,0x38 2 mov_s r3,1 3 llock r2,[r0] <-. 4 brne.nt r2,0,14 --. | http://www.slipstone.co.uk/category/travel/ 5 scond r3,[r0] | | 6 bne -10 --|--' 7 brne_s r2,0,84 <-' http://www.logoarts.co.uk/category/travel/ ... ------>8------- http://www.acpirateradio.co.uk/category/travel/ Lines 3 until 5 (inclusive) are supposed to be executed atomically. Therefore, GDB should never (implicitly) insert a breakpoint on lines 4 and 5, else the http://www.compilatori.com/category/travel/ program will try to acquire the lock again by jumping back to line 3 and gets stuck in an infinite loop. https://www.webb-dev.co.uk/category/technology/ The solution is to make GDB aware of these patterns so it inserts breakpoints after the sequence -- line 6 in this example. |