Spec URL: http://endoframe.com/openvrml.spec SRPM URL: http://endoframe.com/openvrml-0.16.1-1.src.rpm Description: OpenVRML is a VRML/X3D runtime library and Web browser plug-in.
I know Braden's work on openvrml for a long time and had invited him to contribute this package to FE some time ago. Therefore I am going to review this package and am willing to sponsor him.
Basically, the spec is in good shape for an initial submission. Nevertheless, it needs some work: Let's start with what rpmlint has to say: 1) W: openvrml-debuginfo no-version-in-last-changelog W: openvrml-devel no-version-in-last-changelog W: openvrml-gl no-version-in-last-changelog W: openvrml-gl-devel no-version-in-last-changelog W: openvrml-gtkplug no-version-in-last-changelog W: openvrml-mozilla-plugin no-version-in-last-changelog Please add them. 2) W: openvrml-devel no-version-dependency-on openvrml 0.16.1 openvrml-devel lacks Requires: openvrml = %{version}-%{release} 3) W: openvrml-gl-devel summary-ended-with-dot OpenVRML OpenGL renderer headers and static library. W: openvrml-gtkplug summary-ended-with-dot VRML browser GtkPlug component for embedding in GTK+ applications. W: openvrml-mozilla-plugin summary-ended-with-dot VRML browser plug-in for Mozilla-based browsers. Please remove the '.' at the end of the corresponding Summary: lines. 4) E: openvrml-gl library-without-ldconfig-postin /usr/lib/libopenvrml-gl.so.6.0.3 E: openvrml-gl library-without-ldconfig-postun /usr/lib/libopenvrml-gl.so.6.0.3 Please add ldconfig calls, analogous to what you apply to the main package to openvrml-gl 5) E: openvrml-gtkplug info-files-without-install-info-postin /usr/share/info/openvrml-gtkplug.info.gz E: openvrml-gtkplug info-files-without-install-info-postun /usr/share/info/openvrml-gtkplug.info.gz You need to add appropriate calls to /usr/sbin/install-info to openvrml-gtkplug 5) E: openvrml-gtkplug explicit-lib-dependency libXmu This requirement is redundant and should be removed (The dependency on libXmu is already being picked up by rpm). In the same boat is this package's "Requires: gtk2". It also probably is redundant. 5) Any particular reason you explicitly "Requires: firefox" at several places? I thought mozilla-plugins being installed into .../mozilla/plugins were supposed to be usable by all mozilla-compatible browsers (mozilla, firefox, seamonkey, etc.), so an explicit "Requires: firefox" seems unnecessarily restrictive to me. 6) Please "Requires: libGLU-devel" instead of "mesa-libGLU" in *-devel packages. 7) Please use "%{version}-%{release}"-style dependencies instead of "%{version}" dependencies inside of your package.
(In reply to comment #2) > 6) Please "Requires: libGLU-devel" instead of "mesa-libGLU" in *-devel packages. Oops, of cause I meant to write: ... "Requires: libGLU-devel" instead of "mesa-libGLU-devel" ...
(In reply to comment #2) > Basically, the spec is in good shape for an initial submission. Nevertheless, it > needs some work: > > Let's start with what rpmlint has to say: > > 1) > W: openvrml-debuginfo no-version-in-last-changelog > W: openvrml-devel no-version-in-last-changelog > W: openvrml-gl no-version-in-last-changelog > W: openvrml-gl-devel no-version-in-last-changelog > W: openvrml-gtkplug no-version-in-last-changelog > W: openvrml-mozilla-plugin no-version-in-last-changelog > > Please add them. Done, I think. > 2) > W: openvrml-devel no-version-dependency-on openvrml 0.16.1 > openvrml-devel lacks > Requires: openvrml = %{version}-%{release} Check. > 3) > W: openvrml-gl-devel summary-ended-with-dot OpenVRML OpenGL renderer headers and > static library. > W: openvrml-gtkplug summary-ended-with-dot VRML browser GtkPlug component for > embedding in GTK+ applications. > W: openvrml-mozilla-plugin summary-ended-with-dot VRML browser plug-in for > Mozilla-based browsers. > Please remove the '.' at the end of the corresponding Summary: lines. Check. > 4) > E: openvrml-gl library-without-ldconfig-postin /usr/lib/libopenvrml-gl.so.6.0.3 > E: openvrml-gl library-without-ldconfig-postun /usr/lib/libopenvrml-gl.so.6.0.3 > Please add ldconfig calls, analogous to what you apply to the main package to > openvrml-gl Check. > 5) > E: openvrml-gtkplug info-files-without-install-info-postin > /usr/share/info/openvrml-gtkplug.info.gz > E: openvrml-gtkplug info-files-without-install-info-postun > /usr/share/info/openvrml-gtkplug.info.gz > You need to add appropriate calls to /usr/sbin/install-info to openvrml-gtkplug Fixed, I think. > 5) > E: openvrml-gtkplug explicit-lib-dependency libXmu > This requirement is redundant and should be removed (The dependency on libXmu is > already being picked up by rpm). > > In the same boat is this package's "Requires: gtk2". > It also probably is redundant. Okay. > 5) Any particular reason you explicitly "Requires: firefox" at several places? > I thought mozilla-plugins being installed into .../mozilla/plugins were supposed > to be usable by all mozilla-compatible browsers (mozilla, firefox, seamonkey, > etc.), so an explicit "Requires: firefox" seems unnecessarily restrictive to me. For openvrml, it's because libopenvrml links with libmozjs. Hopefully rpm picks this up and this one isn't necessary. For openvrml-mozilla-plugin, it's a holdover since the plug-in used to use XPCOM. It doesn't anymore, so hopefully it's unnecessary, too. > 6) Please "Requires: libGLU-devel" instead of "mesa-libGLU" in *-devel packages. Okay. How come? > 7) Please use "%{version}-%{release}"-style dependencies instead of "%{version}" > dependencies inside of your package. Check. spec: http://endoframe.com/openvrml.spec srpm: http://endoframe.com/openvrml-0.16.1-2.src.rpm
Sorry for not having responded earlier, but ... I couldn't manage to look into this earlier. There still are some issues needing to be addressed: * Please '%%' instead of '%' in references to rpm-tags in %changelog entries. This is to work around rpm bogusly expanding '%' in %changelogs * Please add a blank line at the end of each %post*/%pre block. Some versions of rpm suffer from a bug which bogusly concatenate %post*/%pre otherwise. * openvrml-gl-devel and openvrml-devel contain *.pc's => each of them must Requires: pkgconfig * Building in mock fails: ... error: Installed (but unpackaged) file(s) found: /usr/share/info/dir => Add rm -f %{_infodir}/dir near to the end of %install
I'm interested in having this in Extras, too. A few comments: 1. redundant BuildRequires: pkgconfig (required by freetype-devel and libpng-devel), libpng-devel (required by cairo-devel, which is required by gtk2-devel), fontconfig-devel (required by cairo-devel), freetype-devel (required by fontconfig-devel) 2. use BuildRequires: gecko-devel instead of firefox-devel and libGLU-devel instead of mesa-libGLU-devel 3. reduntant Requires: mesa-libGLU for -gl subpackage 4. use %setup -q instead of %setup 5. use %configure --disable-dependency-tracking to speed up build process 6. use %{__make} instead of make in %install for consistency 7. avoid unnecessary docs duplication in every subpackage 8. rename mozilla-plugin to plugin
BTW: if it's Assigned to you, Ralf, why is the status still NEW?
Judging by the contents of the .pc file, I suggest adding the following for -devel: Requires: freetype-devel Requires: libjpeg-devel Requires: libpng-devel
(In reply to comment #6) > I'm interested in having this in Extras, too. A few comments: > 1. redundant BuildRequires: pkgconfig (required by freetype-devel and > libpng-devel), libpng-devel (required by cairo-devel, which is required by > gtk2-devel), fontconfig-devel (required by cairo-devel), freetype-devel > (required by fontconfig-devel) Okay. Presumably making openvrml-devel Require freetype-devel would moot Ralf's suggestion that openvrml-devel should Require pkgconfig. > 2. use BuildRequires: gecko-devel instead of firefox-devel and libGLU-devel > instead of mesa-libGLU-devel Re: gecko-devel, I don't think I can do that. I think the change that does need to be made is for openvrml to Require firefox = 1.5.0.8. Note that libopenvrml links with libmozjs, which lives in a versioned subdirectory. Consequently, openvrml-devel would Require firefox-devel = 1.5.0.8. > 3. reduntant Requires: mesa-libGLU for -gl subpackage Why is that redundant? > 4. use %setup -q instead of %setup > 5. use %configure --disable-dependency-tracking to speed up build process > 6. use %{__make} instead of make in %install for consistency Okay. > 7. avoid unnecessary docs duplication in every subpackage Users shouldn't need to be aware of the package dependency chain in order to find the README or license for the package. Would it be acceptable to install symlinks? > 8. rename mozilla-plugin to plugin No. There are lots of kinds of plug-ins. This package makes a Mozilla/Gecko one. I might entertain changing it to "gecko-plugin"; but since "Mozilla plug- in" or "Netscape plug-in" is what everyone calls these things, that's going to be a tough sell.
(In reply to comment #9) > (In reply to comment #6) > > I'm interested in having this in Extras, too. A few comments: > > 1. redundant BuildRequires: pkgconfig (required by freetype-devel and > > libpng-devel), libpng-devel (required by cairo-devel, which is required by > > gtk2-devel), fontconfig-devel (required by cairo-devel), freetype-devel > > (required by fontconfig-devel) > > Okay. Presumably making openvrml-devel Require freetype-devel would moot > Ralf's suggestion that openvrml-devel should Require pkgconfig. As a result, yes. > > 2. use BuildRequires: gecko-devel instead of firefox-devel and libGLU-devel > > instead of mesa-libGLU-devel > > Re: gecko-devel, I don't think I can do that. I think the change that does > need to be made is for openvrml to Require firefox = 1.5.0.8. Note that > libopenvrml links with libmozjs, which lives in a versioned subdirectory. > Consequently, openvrml-devel would Require firefox-devel = 1.5.0.8. OK, I guess this is moot, because the only package providing gecko-devel is firefox-devel. > > 3. reduntant Requires: mesa-libGLU for -gl subpackage > > Why is that redundant? $ rpm -qpR openvrml-gl-0.16.1-1.x86_64.rpm ... libGLU.so.1()(64bit) ... $ rpm -q --whatprovides "libGLU.so.1()(64bit)" mesa-libGLU-6.5.1-7.fc6.x86_64 That's why. > > 7. avoid unnecessary docs duplication in every subpackage > > Users shouldn't need to be aware of the package dependency chain in order to > find the README or license for the package. Would it be acceptable to install > symlinks? Well, I'll leave that for Ralf to decide. IMHO people are clueful enough to search in openvrml-%version if they can't find it in openvrml-mozilla-plugin-%version. > > 8. rename mozilla-plugin to plugin > > No. There are lots of kinds of plug-ins. This package makes a Mozilla/Gecko > one. I might entertain changing it to "gecko-plugin"; but since "Mozilla plug- > in" or "Netscape plug-in" is what everyone calls these things, that's going to > be a tough sell. OK.
One more thing. Looks like there is a test suite that can be executed using make check. It seems to run properly even if $DISPLAY is not set, so please include it in the specfile, i.e.: %check %{__make} check
(In reply to comment #9) > (In reply to comment #6) > > I'm interested in having this in Extras, too. A few comments: > > 1. redundant BuildRequires: pkgconfig (required by freetype-devel and > > libpng-devel), libpng-devel (required by cairo-devel, which is required by > > gtk2-devel), fontconfig-devel (required by cairo-devel), freetype-devel > > (required by fontconfig-devel) > > Okay. Presumably making openvrml-devel Require freetype-devel would moot Ralf's > suggestion that openvrml-devel should Require pkgconfig. Any package that contains a *.pc MUST directly "Require: pkgconfig" to make sure %{_libdir}/pkgconfig has an owner. > > 2. use BuildRequires: gecko-devel instead of firefox-devel I fail to understand this. > > 4. use %setup -q instead of %setup Optional. > > 5. use %configure --disable-dependency-tracking to speed up build process Optional, left to the packager's preference. This doesn't make much sense for this package, because this speed up is marginal for this package. > > 7. avoid unnecessary docs duplication in every subpackage > > Users shouldn't need to be aware of the package dependency chain in order to > find the README or license for the package. Would it be acceptable to install > symlinks? No. > > 8. rename mozilla-plugin to plugin > > No. There are lots of kinds of plug-ins. This package makes a Mozilla/Gecko > one. I might entertain changing it to "gecko-plugin"; but since "Mozilla plug- > in" or "Netscape plug-in" is what everyone calls these things, that's going to > be a tough sell. ACK.
I tried actually using it and it seems that the firefox plugin requires -gtkplug subpackage to run at all. Also, it's extremely slow without hardware accelerated OpenGL, but that's to be expected.
(In reply to comment #13) > I tried actually using it and it seems that the firefox plugin requires -gtkplug > subpackage to run at all. I don't understand what you are trying to say. Rpm-wise, the mozilla-plugin already depends on the *-gtkplug package. (In reply to comment #7) > BTW: if it's Assigned to you, Ralf, why is the status still NEW? Because bugzilla hadn't changed it, when I assigned it to me?
(In reply to comment #11) > One more thing. Looks like there is a test suite that can be executed using make > check. It seems to run properly even if $DISPLAY is not set, so please include > it in the specfile, i.e.: > > %check > %{__make} check Except there's a crash-on-exit bug lurking somewhere that one of the test cases triggers *some* of the time. (In reply to comment #12) > Any package that contains a *.pc MUST directly "Require: pkgconfig" to make sure > %{_libdir}/pkgconfig has an owner. Understood. > > > 2. use BuildRequires: gecko-devel instead of firefox-devel > I fail to understand this. Not sure I do either; but for the reason I already mentioned, I think it's moot. > > > 4. use %setup -q instead of %setup > Optional. What's it do? > > > 5. use %configure --disable-dependency-tracking to speed up build process > > Optional, left to the packager's preference. This doesn't make much sense for > this package, because this speed up is marginal for this package. True; I won't bother with it, at least for now. > > > 7. avoid unnecessary docs duplication in every subpackage > > > > Users shouldn't need to be aware of the package dependency chain in order to > > find the README or license for the package. Would it be acceptable to install > > symlinks? > No. Okay. I'll leave it as-is. (In reply to comment #13) > I tried actually using it and it seems that the firefox plugin requires -gtkplug > subpackage to run at all. Yes; there *is* a Requires to that effect. > Also, it's extremely slow without hardware accelerated > OpenGL, but that's to be expected. There's also a bug that makes the plug-in soak up CPU cycles, which doesn't help matters. *** spec: http://endoframe.com/openvrml.spec srpm: http://endoframe.com/openvrml-0.16.1-3.src.rpm I hope I haven't missed anything.
(In reply to comment #15) > (In reply to comment #11) > > > > 4. use %setup -q instead of %setup > > Optional. > > What's it do? -q silences the untarring of source tarball (tar xf instead of tar xvf). As verbose untarring increases the size of log files, some people prefer "%setup -q". However given the size of the log files building openvrml produces, this probably is negligible ;) > > > > 5. use %configure --disable-dependency-tracking to speed up build process > > > > Optional, left to the packager's preference. This doesn't make much sense for > > this package, because this speed up is marginal for this package. > > True; I won't bother with it, at least for now. OK with me. > > > > 7. avoid unnecessary docs duplication in every subpackage > > > > > > Users shouldn't need to be aware of the package dependency chain in order to > > > find the README or license for the package. Would it be acceptable to install > > > symlinks? > > No. > > Okay. I'll leave it as-is. Feel free to do as you like. Rule of thumb should be "docs should accompany those components they document" and "ship them when you feel it's necessary". You're the upstream author, so feel free to add "ChangeLogs", READMEs, License files where you feel them to be appropriate. > spec: http://endoframe.com/openvrml.spec > srpm: http://endoframe.com/openvrml-0.16.1-3.src.rpm > > I hope I haven't missed anything. Will try to look into this later today.
(In reply to comment #16) > > I hope I haven't missed anything. Unfortunately, you did :( > Will try to look into this later today. Some MUSTFIXES remain: * Some %post/%pre sections still lack a blank line at their end. * The infos do not install/uninstall correctly: - "info openvrml-gtkplug" doesn't jump to the corresponding info pages - "rpm -e openvrml-gtkplug" fails: install-info: warning: no entries found for `/usr/share/info/openvrml-gtkplug.info.gz'; nothing deleted The origin is a missing direntry inside of the *.info (cf. the openvrml-gtkplug entry in /usr/share/info/dir after installing the rpm). Normally this indicates broken/incomplete texinfo sources. Standard work-around would be to either patch/fix the sources, or to pass the necessary info to install-info inside of the specs. RECOMMENDATION: * I recommend using %{?dist} in %release. In your case this would be Release: 3%{?dist} This eases a package's maintenance in longer terms. There exist other minor issue, which IMO don't have to be addressed now and are better be addressed after importing the packaging in FE.
(In reply to comment #17) > (In reply to comment #16) > > > I hope I haven't missed anything. > > Unfortunately, you did :( D'oh. > > Will try to look into this later today. > > Some MUSTFIXES remain: > > * Some %post/%pre sections still lack a blank line at their end. Damn. > * The infos do not install/uninstall correctly: > - "info openvrml-gtkplug" doesn't jump to the corresponding info pages > - "rpm -e openvrml-gtkplug" fails: > install-info: warning: no entries found for > `/usr/share/info/openvrml-gtkplug.info.gz'; nothing deleted > > The origin is a missing direntry inside of the *.info (cf. the openvrml-gtkplug > entry in /usr/share/info/dir after installing the rpm). > Normally this indicates broken/incomplete texinfo sources. > > Standard work-around would be to either patch/fix the sources, or to pass the > necessary info to install-info inside of the specs. I'm stumped here so far. There is a direntry in openvrml-gtkplug.texi; and if there's something wrong with it, it's not apparent to me. I do see the message you're getting upon uninstalling openvrml-gtkplug. If I then try to reinstall it, I get this: install-info: menu item `openvrml-gtkplug' already exists, for file `(none)' error: %post(openvrml-gtkplug-0.16.1-3.x86_64) scriptlet failed, exit status 1 I'll play around with this a bit more; however, if this is not something that can be repaired by passing arguments to install-info, I'm inclined to punt and simply not package the info for 0.16.1. I'll fix it for 0.16.2.
Created attachment 141222 [details] fix dirently in openvrml-gtkplug.texi Braden, check this diff. It should solve the direntry issue. When testing, first, manually remove any line referring to openvrml from /usr/share/info/dir. Otherwise any attempts to run "rpm -e" will fail.
Thanks. I've buckled and converted this to a patch that is applied to the pristine sources. spec: http://endoframe.com/openvrml.spec patch: http://endoframe.com/openvrml-gtkplug-info.patch srpm: http://endoframe.com/openvrml-0.16.1-4.src.rpm
APPROVED - Packaging-wise, this package now looks sufficently well to me. I am going to sponsor you, so you should now proceed with step 10. of http://fedoraproject.org/wiki/Extras/Contributors Functionally, I am facing an issue with openvrml-mozilla-plugin and firefox, I don't have an explaination for: After installing "openvrml-mozilla-plugin" and restarting firefox, firefox doesn't show the plugin in "about:plugins", nor does seem to know how to handle *.wrl files. I recall, earlier versions of openvrml and firefox/mozilla did, so I am wondering about what might have happened.
Yea! Thanks for your feedback, gentlemen. Ralf, I'm not observing the problem you describe with openvrml-mozilla-plugin using the 0.16.1-4 RPMs. Certainly the first thing to check is the presence of /usr/lib/mozilla/plugins/openvrml.so.
(In reply to comment #22) > Ralf, I'm not observing the problem you describe with openvrml-mozilla-plugin > using the 0.16.1-4 RPMs. I still observe it and haven't got the faintest clue about what might be going on :(. ATM, I am inclined to blame (wild guesses!!) a firefox upgrade having happened since having built the openvrml rpms (I have seen this happening several times with other plugins and former upgrades of firefox), my very friend SELinux (It's currently nagging me all over the place) and/or prelink .( > Certainly the first thing to check is the presence of > /usr/lib/mozilla/plugins/openvrml.so. Of cause it's present. I imported the rpms into a local yum repository and used a standard "yum install" to install the plugin. Anyway, I officially sponsored you. The next step would be to import your srpm into FE's CVS and to request a build. Unfortunately, AFAICT, right now FE's CVS is broken and awaiting fixing. According to it's maintainer "on Monday" (US-time).
IME, it can be useful to copy the firefox launcher script to your home directory and edit it such that useful console output doesn't get sent to /dev/null.
Confirming absence of OpenVRML plugin in 'about:plugins' and subsequent failure to handle .wrl files, as reported in comment #21. - firefox-1.5.0.8-1.fc6 - No improvement with new created profile - strace reveals openvrml.so being loaded when consulting "about:plugins" - ldd /usr/lib/mozilla/plugins/openvrml.so shows no obvious problems
Please report any outstanding issues as new bugs. Also note my suggestion in comment #24. (And see bug 157279.)
(In reply to comment #26) > Please report any outstanding issues as new bugs. I'd bugzilla them, but there is still no openvrml component in bugzilla. > Also note my suggestion in comment #24. (And see bug 157279.) Been there, tried it (and many other things), but I can't spot anything obvious. "about:plugins" and accessing the plugin from inside firefox continue not to work for me, on different machines, on different accounts, with locally built rpms and with the official ones ;)
I realize STATUS=CLOSED, but for the sake of completeness : the problem originally reported in comment #21 is fixed in openvrml-mozilla-plugin-0.16.1-5.fc6.
NB: openvrml-mozilla-plugin-0.17.6-2.fc8 is still broken as per comment #21.
It's not "still" broken. You're observing a new bug. This may be the result of the recent change that added -fvisibility=hidden when compiling. This works on F9; but possibly the firefox/xulrunner headers on F8 don't specify the symbol visibility for symbols that require external linkage. Please file a new bug. And please include the output of $ nm -D /usr/lib/mozilla/plugins/openvrml.so | grep NP_ Substitute "lib64" if you're on x86_64.