Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 184331
Summary: | Review Request: k3d - 3D modeling and rendering system | ||||||
---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Denis Leroy <denis> | ||||
Component: | Package Review | Assignee: | Paul F. Johnson <paul> | ||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||
Severity: | medium | Docs Contact: | |||||
Priority: | medium | ||||||
Version: | rawhide | CC: | gemi | ||||
Target Milestone: | --- | ||||||
Target Release: | --- | ||||||
Hardware: | All | ||||||
OS: | Linux | ||||||
Whiteboard: | |||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2006-08-11 08:53:06 UTC | Type: | --- | ||||
Regression: | --- | Mount Type: | --- | ||||
Documentation: | --- | CRM: | |||||
Verified Versions: | Category: | --- | |||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
Cloudforms Team: | --- | Target Upstream Version: | |||||
Embargoed: | |||||||
Bug Depends On: | |||||||
Bug Blocks: | 163779 | ||||||
Attachments: |
|
Description
Denis Leroy
2006-03-07 23:56:31 UTC
(In reply to comment #0) > rpmlint will complain about header files in the main RPM (in /usr/share): as far as i understand this is a false positive, though if a K3D power-user could confirm i'd appreciate. These are included by for shaders and thus and integral part of the package, I would say. I did not look closely at your spec file, but here is the spec file I used for building the package. May be you find something useful. http://math.ifi.unizh.ch/fedora/spec/k3d.spec * Could you update to 0.5.10.0? * gts can be used, but is not yet in Extras I suggest you submit gts for review and make it a dependency for k-3d The review process of this package will be quick, I think. * There is also a dependency on SuperLU, I don't know how crucial it is. gts was just submited a few hours ago as https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=192363 gts is now released in Extras. Please update the srpm to the new version with gts included. Did you see Fedora packages at http://www.planetsaphire.com/rpms/k3d/? Will look at it. My development system is in a container in a boat in the middle of the atlantic, so I won't be able to work on this package for another month or so. K3D would take about 24 hours to compile on this old laptop :-( Can you post the URLs of both the spec and srpm here so I can just download, test, review and we can get this one off into FE? Spec: http://www.poolshark.org/src/k3d.spec SRPM: http://www.poolshark.org/src/k3d-0.5.15.0-1.src.rpm - Updated to latest version - I folded the devel package into the main package since there's not much sense in having the split (since otherwise the main package would have to Require it). - Added gts support Comments: %post /sbin/ldconfig You don't have ldconfig in the requires. You can get around this with %post -p /sbin/ldconfig %{_libdir}/*.so.* Not overly happy with this. If the package is generating some libs which all start with a common name (such a libfoo), then the libdir entry can be changed to libfoo*.so.* The same applies for the regex. Having %bindir/k3d* is enough (though you will need %exclude %{_bindir}/k3d-config I'm slightly confused by %{_libdir}/*.so.* %{_libdir}/k3d Is the application putting libraries into both libdir and libdir/k3d? In the -devel you have %{_libdir}/*.so As before, can you alter this to be a real name? (In reply to comment #8) > - I folded the devel package into the main package since there's not much sense > in having the split (since otherwise the main package would have to Require it). Very bad move. Please revert this change and split into *-devel and *non-devel, again. (In reply to comment #10) > (In reply to comment #8) > > - I folded the devel package into the main package since there's not much sense > > in having the split (since otherwise the main package would have to Require it). > > Very bad move. Please revert this change and split into *-devel and *non-devel, > again. If the main package would have to require it, it sounds like the files in the devel package (at least some of them) aren't really devel files. What was in the devel package that's needed by the main package? Is there anything that was in the devel package that's *not* needed by the main package? (In reply to comment #11) > (In reply to comment #10) > > (In reply to comment #8) > > > - I folded the devel package into the main package since there's not much sense > > > in having the split (since otherwise the main package would have to Require it). > > > > Very bad move. Please revert this change and split into *-devel and *non-devel, > > again. > > If the main package would have to require it, it sounds like the files in the > devel package (at least some of them) aren't really devel files. This package seems to contain shared libs, dll-modules/plugins and headers. > What was in the devel package that's needed by the main package? Is there > anything that was in the devel package that's *not* needed by the main > package? I presume the OP had mixed up *.so symlinks and dll-modules/plugins. Apologies for the delay - my buildsys is somewhat elderly! It built fine in mock (x86). I'll go over everything else tonight. Created attachment 133264 [details]
rpmlint output once installed
You need to fix the problems highlighted in #14 before the review goes any further SPEC: http://www.poolshark.org/src/k3d.spec SRPM: http://www.poolshark.org/src/k3d-0.5.15.0-2.src.rpm - I cleaned up the %files section a bit, as you requested. The package does indeed have both libraries in /usr/lib and /usr/lib/k3d (those are plugins). - For the undefined-non-weak-symbol warnings. I believe this is intentional. See here: http://www.redhat.com/archives/fedora-extras-list/2006-July/msg00569.html Essentially you are getting the warnings because the package dependencies are not installed. As for the weak linking, I believe it is intentional to allow k3d libs to link to either libGL.so from Xorg or from Nv***a. - For the devel vs non-devel. Part of the package can be considered a devel package, but I dont think it does what a traditional devel package would do. The reasons has to do with what K-3D does: it's merely a modeler, the first step before calling the main rendering engine (e.g. aqsis, which I'll try to package next), and that step involves using the devel-type files. So the devel files are required by the main package. This is explicitely mentioned in the project page at http://www.k-3d.org/wiki/GettingStarted , as well as in comment #1 above. I brought up the issue on a fedora mailing list a while ago, but I can't seem to track it down in the archives. I remember the consensus was that it made little sense to split a package if both parts require each other. OTOH I can't say I have a strong opinion on the matter... The devel package are files required to build or develop the package. These are headers, .so files and compiling documentation. This package requires a -devel package SPEC: http://www.poolshark.org/src/k3d.spec SRPM: http://www.poolshark.org/src/k3d-0.5.15.0-3.src.rpm Okay then, I resplit the package, and added 'Requires: k3d-devel'. There are still some header files in the main package, under /usr/share/, but those are false positives (see comment #1). --mode 644 \ Is this required for the desktop icon? Have you asked upstream if these "false positives" are exactly that or if they should be held in the -devel package? rpmlint is giving in the main package E: k3d devel-dependency k3d-devel This will cause a lot of problems. How can the binary require the devel? Also has the devel-file-in-non-devel warnings The devel package contains a pile of warnings about dangling-relative-symlinks and more importantly E: k3d-devel only-non-binary-in-usr-lib The debuginfo package has lots of errors E k3d-debuginfo script-without-shellbang /usr/src/debug/k3d-0.5.15.0/k3dsdk/nodes.cpp (as an example) These errors are normally a permissions issue, they need to be chmod to 0644 in the after the archive has been dearchived. rpmlint reports the SRPM to be clean. I've not built it in mock yet due to these errors. Spec: http://www.poolshark.org/src/k3d.spec SRPM: http://www.poolshark.org/src/k3d-0.5.16.0-1.src.rpm Paul, thanks for your patience and for your comments. I was able to borrow a build system and do a little more testing, and I also exchanged a few emails with upstream developers. Comments embedded below. > --mode 644 : Is this required for the desktop icon? No, about 5% of extras packages specify it explicitely. I removed it. > E: k3d devel-dependency k3d-devel Ok, I checked with upstream and the good news is that this requirement can be dropped (they updated the wiki to clarify). So this is fixed. > Also has the devel-file-in-non-devel warnings Those are false positives, as confirmed by upstream: Timothy M. Shead <tshead> Wrote: "The .h files in /share/shaders are part of the shader source code, not the K-3D source code. They are there so that shaders can be compiled *at runtime* by the user's RenderMan engine. So they belong in the runtime package." > E debuginfo script-without-shellbang > /usr/src/debug/k3d-0.5.15.0/k3dsdk/nodes.cpp (as an example) Fixed. > The devel package contains a pile of warnings about > dangling-relative-symlinks Right, this occurs when a package uses library symlinks in a subdirectory of /usr/lib (those in /usr/lib are ignored by rpmlint). For example, 'graphviz' is another package that has the same problem. Anyways, this actually was an error in the spec as the run-time package requires the .so files, so I had to move them into the main package. K-3D will not load the plugins correctly if only the .so.0.0.0 files are present. There are a couple of libs in /usr/lib that also need their .so files in the main package, because they are dlopened directly by k3d-bin. I think those really belong in /usr/lib/k3d and I've reported the problem upstream, a fix would require a pretty intrusive autoconf/automake patch which i think is not worth the effort. > E: k3d-devel only-non-binary-in-usr-lib Should no longer occur, as /usr/lib/k3d is in the main package now. Should build cleanly in mock, for devel and FC-5. FC-4 requires a small BR change, but otherwise also builds cleanly. Okay Good Builds fine and actually works rpmlint is clean on all packages (with the exceptions listed above which I'm willing to accept) No dupes in the BRs Includes documentation No issues with directory ownership Bad Needs pkgconfig adding to the R: on the devel You've packaged the same README in the main and devel package Unsure find -type f -regex '.*\.\(cpp\|h\|svg\)' -perm +111 -exec chmod -x {} ';' - seems like overkill to me. Fix the two bads (which aren't that bad!), and it's good to go SPEC: http://www.poolshark.org/src/k3d.spec SRPM: http://www.poolshark.org/src/k3d-0.5.16.0-2.src.rpm > Needs pkgconfig adding to the R: on the devel Actually, k3d doesn't provide any pkgconfig .pc file, it provides its own 'k3d-config' executable. > You've packaged the same README in the main and devel package Fixed. > find -type f -regex '.*\.\(cpp\|h\|svg\)' -perm +111 -exec chmod -x {} ';' - Ah the lovely find command. Well I'd still rather do that than do a massive chmod -x over the whole tree since it contains all sorts of executable scripts and whatnot. Or did you have something even simpler in mind ? Okay, I'm happy with that. Is it worth adding the tests directory into the -devel directory as well as the docs/xml/sample_document.k3d file? Might be a useful addition. Not a bad idea but the test directory is not easily packagable, we'd have to provide custom Makefiles... We could add the gzip'ed sample file to the devel package, or even to the main package, i think it's a good idea. Is this a must item or did you approve the package ? Add it to the devel package and it's good to go. SPEC: http://www.poolshark.org/src/k3d.spec SRPM: http://www.poolshark.org/src/k3d-0.5.16.0-3.src.rpm Paul, thanks for your review! Thanks for making the addition APPROVED Built :-) Changing the Summary for tracking purposes. |