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 203217
Summary: | Review Request: csound - music synthesis system | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Dan Williams <dcbw> | ||||||||||||||||||||||||
Component: | Package Review | Assignee: | Paul F. Johnson <paul> | ||||||||||||||||||||||||
Status: | CLOSED RAWHIDE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||||||||||||||||||||||
Severity: | medium | Docs Contact: | |||||||||||||||||||||||||
Priority: | medium | ||||||||||||||||||||||||||
Version: | rawhide | CC: | green, pbrobinson, seg | ||||||||||||||||||||||||
Target Milestone: | --- | Flags: | kevin:
fedora-cvs+
|
||||||||||||||||||||||||
Target Release: | --- | ||||||||||||||||||||||||||
Hardware: | All | ||||||||||||||||||||||||||
OS: | Linux | ||||||||||||||||||||||||||
Whiteboard: | |||||||||||||||||||||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||||||||||||||||||||
Doc Text: | Story Points: | --- | |||||||||||||||||||||||||
Clone Of: | Environment: | ||||||||||||||||||||||||||
Last Closed: | 2006-09-05 18:13:05 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
Dan Williams
2006-08-19 02:23:07 UTC
Couple of quick items: 1. Source should be full URL. 2. Package fails to build in Mock. I'll attach the build log for it. Created attachment 134832 [details]
Mock Build Log
Looking at the .spec alone reveals several orphaned/unincluded directories: %{_libdir}/%{name}/ %{_libdir}/%{name}/plugins/ %{_datadir}/%{name}/ # -devel %{_includedir}/%{name}/ # -tk %{_libdir}/%{name}/tcl/ Created attachment 134843 [details]
Updated spec file
We're trying to base our audio packages off othe PlanetCCRMA packages, but
PlanetCCRMA's Csound is a fair bit older than this one.
I'm attaching an updated spec file, but it still isn't perfect. It adds
support for java, jack, OSC, DSSI and the Csound GUI. Please include
incorporate these changes.
I think we should also have a -tutorial subpackage, and solve the missing
manual problem.
(In reply to comment #4) > Created an attachment (id=134843) [edit] > Updated spec file > > We're trying to base our audio packages off othe PlanetCCRMA packages, but > PlanetCCRMA's Csound is a fair bit older than this one. > > I'm attaching an updated spec file, but it still isn't perfect. It adds > support for java, jack, OSC, DSSI and the Csound GUI. Please include > incorporate these changes. > > I think we should also have a -tutorial subpackage, and solve the missing > manual problem. > I'll incorporate your patch. BTW, do you know if enabling Jack makes it required at runtime, or is csound smart enough to not use it? I don't think OLPC won't have Jack and we don't want to fork... (In reply to comment #5) > I'll incorporate your patch. BTW, do you know if enabling Jack makes it > required at runtime, or is csound smart enough to not use it? I don't think > OLPC won't have Jack and we don't want to fork... Try packaging %{_libdir}/csound/plugins/librtjack.so into a subpackage. The same may work for the libdssi4cs.so and libosc.so if you want to make it really modular. I'm a tad concerned that it's redefining printf for some reason in a HELL of a lot of files. It seems to be coming from h/csoundCore.h line 119 rpmlint warnings. src.rpm is clean csound-5.03.0-2 : W: csound no-soname %{_libdir}/lib_csnd.so (should this not be in the -devel package? csound-devel : W: csound-devel no-docs (can be ignored) csound-python: W: dangling-symlink %{_libdir}/python2.4/site-packages/_csnd.so %{_libdir}/lib_csnd.so csound-python: W: symlink-should-be-relative %{_libdir}/python2.4/site-packages/_csnd.so %{_libdir}/lib_csnd.so csound-java: W: no-soname %{_libdir}/lib_jsound.so W: no-docs W: symlink-should-be-relative %{_datadir}/java/csnd.jar %{_libdir}/csound/java/csnd.jar csound-tk: E: only-non-binary-in-usr-lib W: no docs csound-debuginfo: clean For the missing .so files, these really should be in the csound-devel package (and java-devel package). The tk Error must be attended to. Your specfile is also missing Requires(post and postun): /sbin/ldconfig Does the package not come with other vocabulary languages? If this is for the OLPC programme, then it makes sense for translations to be in there! In %files... The %{_bindir} entries can be globbed to make things easier to read (or better still, a simple for loop on the package names. In %files devel %{_includedir}/%{name}/* Need to be changed to %{includedir}/%{name}/ to take ownership of the directory and files In %files tk I'm not sure about the .tk files in %{_libdir}. Is this the correct place for them? csound is currently building in mock - I'll report back when it's done. (In reply to comment #7) > I'm a tad concerned that it's redefining printf for some reason in a HELL of a > lot of files. It seems to be coming from h/csoundCore.h line 119 I asked about that, upstream is now aware that we dislike it :) It was added to make sure that all plugins use the Csound logging facilities. I think it may go away in the near future, and we can likely remove it if we wish. > > rpmlint warnings. > > src.rpm is clean > > csound-5.03.0-2 : W: csound no-soname %{_libdir}/lib_csnd.so (should this not be > in the -devel package? No, it's the python module code, and it should be in /usr/lib/python2.4/site-packages/. I'll move it. > csound-devel : W: csound-devel no-docs (can be ignored) > > csound-python: W: dangling-symlink %{_libdir}/python2.4/site-packages/_csnd.so > %{_libdir}/lib_csnd.so > csound-python: W: symlink-should-be-relative > %{_libdir}/python2.4/site-packages/_csnd.so %{_libdir}/lib_csnd.so Will be fixed when that lib moves to python's site-packages. > csound-java: W: no-soname %{_libdir}/lib_jsound.so > W: no-docs > W: symlink-should-be-relative %{_datadir}/java/csnd.jar > %{_libdir}/csound/java/csnd.jar This one shouldn't be a symlink either, should probably move csnd.jar to %{_datadir} too. > csound-tk: E: only-non-binary-in-usr-lib > W: no docs > > csound-debuginfo: clean > > For the missing .so files, these really should be in the csound-devel package > (and java-devel package). The tk Error must be attended to. Except that most aren't devel libs; they are the java and python module libraries and should live in those packages, not in the main csound packages. > > Your specfile is also missing Requires(post and postun): /sbin/ldconfig Will add. > Does the package not come with other vocabulary languages? If this is for the > OLPC programme, then it makes sense for translations to be in there! I don't think so... I couldn't see a switch for it at build-time, but I'll ask. > In %files... > > The %{_bindir} entries can be globbed to make things easier to read (or better > still, a simple for loop on the package names. Can that be done even if some stuff from %{_bindir} is owned by a sub-package? > In %files devel > %{_includedir}/%{name}/* Need to be changed to %{includedir}/%{name}/ to take > ownership of the directory and files Will do. > In %files tk > > I'm not sure about the .tk files in %{_libdir}. Is this the correct place for them? Yeah, that's odd. I'm not sure. > csound is currently building in mock - I'll report back when it's done. mock fails - you need to add BR libpng-devel (failed building the GUI) 8--->
> In %files...
>
> The %{_bindir} entries can be globbed to make things easier to read (or better
> still, a simple for loop on the package names.
Can that be done even if some stuff from %{_bindir} is owned by a sub-package?
<---8
I know I've done it in other packages - but only where the main package has
owned the root directory. Remember, you can always use %dir
(In reply to comment #7) > csound-java: W: no-soname %{_libdir}/lib_jsound.so I think this is ignoreable, as lib_jcsound.so is only ever ldopened (btw, what happened to the 'c' in lib_jcsound.so? cut-n-paste error?) (In reply to comment #8) > (In reply to comment #7) > > W: symlink-should-be-relative %{_datadir}/java/csnd.jar > > %{_libdir}/csound/java/csnd.jar > > This one shouldn't be a symlink either, should probably move csnd.jar to > %{_datadir} too. I think we should keep this as a symlink (although I suppose it can be relative). Upstream wants it in %{_libdir}/csound/java, but our fedora standard is to place .jar files in %{_javadir} (== %{_datadir}/java). We should leave it in both places to satisfy both our requirement and any upstream packages that expect to find it in csound's %{_libdir}/csound/java directory. Created attachment 134864 [details]
Specfile v3
Splits out the jack, dssi, and osc bits. We want this _really_ modular so OLPC
can use it. Also incorporates cleanup suggestions from above.
Created attachment 134870 [details]
Update spec file (v3++)
This version adds -javadoc and -tutorial subpackages.
Created attachment 134885 [details]
spec v4
Incorporate v3++ changes and revert change that moved the jarfile.
I just realized that the gcj related stuff in %post and %postun need to move to %post and %postun for the -java subpackage. My bad. Sorry about that! Created attachment 134889 [details]
csound manual sources extracted from cvs repository.
These manual sources come from the csound cvs repository and correspond to the
5.03 release. Details are in the spec file I'm about to upload.
Created attachment 134891 [details]
spec v4++
This version creates a -manual subpackage from the csound manual sources I've
uploaded (%{SOURCE1} in the spec file).
csound5gui's Help menu should be fixed to look for HTML from this package
instead of from somewhere in /usr/share. I can do this tomorrow unless
somebody beats me to it.
Now that I think of it, csound5gui should get an application desktop icon.
Sorry for all of these changes -- I just want this package to be all that it
can be (and not have any feature regressions from the older PlanetCCRMA csound
package).
I've got an specfile that does the doc redirect, and also fixes up Requires. Turns out I can't glob %{_bindir} files in the main package, because then RPM thinks that the main package owns %{_bindir} files that it should not. I'm talking about: %files %{_bindir}/ %files tk %{_bindir}/brkpt That makes the main package think it owns brkpt, which means both a perl and a tk dep in the main package... Created attachment 134917 [details]
specfile v5
This specfile re-adds the non-globbed bindir and libdir files. I folded in the
documentation changes and added bits to change the documentation path in
csound5gui to the correct path for the manual package, and also to use
'xdg-open' rather than the hardcoded 'firefox'.
Created attachment 134940 [details]
spec file v5++
This adds a -fluidsynth subpackage. I didn't realize this was available until
I built on a machine with fluidsynth-devel installed.
Are there any translations available for this package? Okay, from the latest... fails to build in mock - you need to add libjpeg-devel to the BRs rpmlint is mostly clean (I'm ignoring packages without docs). The exceptions are csound-java : W: no-soname %{_libdir}/lib_jcsound.so W: symlink-should-be-relative %{_datadir}/java/csnd.jar %{_libdir}/%{name}/java.csnd.jar csound-osc: E: explicit-lib-dependency liblo csound-manual: E: zero-length %{_docdir}/%{name}-manual-%{version}/examples/ifthen.csd Other things... The Dist: number doesn't get increased with each new release. The problem with that is that when I'm tracking, it's a bitch to find a difference when you have to check datestamps instead of something nice like the Dist number. Can you please increment with each alteration? When you add a new tarball, can you please respin the src.rpm - it makes life a damned sight easier for review and testing purposes. I don't have a problem with minor things like spec files and patches, but whopping huge addins - that's another thing. Please change the printf redefinition. Not interested in upstream, it's wrong and is also a possible security problem (printf is well defined in terms of what it does and it's problems, the new printf is a different kettle of fish). If it helps, rename the function cs_printf. It will probably take a minute to do a glob|regex search and replace on the source. Is it sufficient to fix the others, but leave the two warnings for csound-java? Not worried about the symlink, not happy with the no-soname. However, if it's because the library is a java one masquerading as a real one for some reason, then I'm okay with it. Fix the others and so-name and we're good to go. The biggest killer IMO is the printf redefinition - I'd not allow it through if it's still complaining. (In reply to comment #25) > Not worried about the symlink, not happy with the no-soname. However, if it's > because the library is a java one masquerading as a real one for some reason, > then I'm okay with it. It's a JNI library. Nobody ever links against it - it is dlopen'd by the JVM running the java code shipped in this package. Fedora Core/Extras includes many such libraries with no SONAME. This should be OK. Shouldn't the JNI library be in %{_libdir}/java/, or %{_libdir}/java-ext/? (In reply to comment #27) > Shouldn't the JNI library be in %{_libdir}/java/, or > %{_libdir}/java-ext/? What makes you think that? #26 - fair enough. Once the other aspects are fixed and new srpm is uploaded (complete with any sub-package tarballs), I'll review it and hopefully, it'll pass and the world will carry one, but with a lot of people on the OLPC enjoying the benefits :-) (In reply to comment #28) > (In reply to comment #27) > > Shouldn't the JNI library be in %{_libdir}/java/, or > > %{_libdir}/java-ext/? > > What makes you think that? It is a wild guess that dlopened files are searched for by the JVM in these directories, but I may be wrong, since I don't know java packaging at all... Isn't there a directory more suitable that %_libdir? perl and python don't drop the similar object files in %_libdir, I guessed it was the same with java, but I may be missing something... (as a side note I couldn't rebuild csound, I would have liked to investigate more, but rpmbuild -ba stops at the very beginning with: Checking for C header file stdio.h... no *** Failed to compile a simple test program. The compiler is *** possibly not set up correctly, or is used with invalid flags. *** Check config.log to find out more about the error. trying the gcc command line that appears in config.log by hand, there is no error??? I am on up to date rawhide.) Created attachment 135230 [details]
specfile v6
Latest specfile; changes in changelog entry at bottom
All the latest files are also at: http://people.redhat.com/dcbw/csound/ #30 - I've only seen this once and that was when my system was seriously screwed up (mixed x86 and x86_64 rpms causing conflicts). Are you on a 64 bit box? If you are, get rid of the i386 versions and see if that's happier. #32 - Have you merged the two tarballs [source + manual] together? If so, you either need to document it or preferably have Source0 for the package and Source1 for the docs - I can't really verify md5sums if they're merged here and not upstream. You never did say if there were any translations for the package. Building the packages now... rpmlint comes up with the usual no-docs for a couple of packages and two warnings for the java package (symlink and so-name). I'll build it in mock and if all goes well, will do the full review on it tomorrow at some point. I do need the point above about if you have merged the tarballs together clarifying though. No, tarballs haven't been merged. They are separate Source lines in the specfile, since they are separate tarballs upstream. There appear to be no translations at this time, I'm still trying to get word from upstream about that though. failed to build in mock RPM builde errors: File not found: %{buildroot}/%{name}-%{version}.fc6-root-mockbuild/usr/lib/gcj/csound I'm also concerned with the file attached to the next entry when building the manual. Created attachment 135307 [details]
mock build log
This log is repeated for most of the chapters with some having more errors than
others, but in general, they follow the format here.
It shouldn't constitute a problem, but it may make the manual look a bit odd
(In reply to comment #35) > failed to build in mock > > RPM builde errors: > > File not found: > %{buildroot}/%{name}-%{version}.fc6-root-mockbuild/usr/lib/gcj/csound You need to update your java-1.4.2-gcj-compat-devel to the -104 release. (In reply to comment #36) > This log is repeated for most of the chapters with some having more errors than > others, but in general, they follow the format here. > > It shouldn't constitute a problem, but it may make the manual look a bit odd The manual build process requires that we build it multiple times. Forward references are only resolved in the last build. The errors your pointing out happen prior to the final build, so they are ignoreable. Right, it's happy in mock (i386) and I'm not overly concerned about the manual. Review time: Good : Consistent use of macros throughout the spec file Clear US-English builds cleanly in mock main package md5 matches upstream (cvs cannot be checked) Main package has docs, others report no docs (not worried) No problems with file permissions No dupes in the rpms Licences fine Doesn't have any locale stuff or desktop icon Compiles fine (x86) No ownership problems Needs work CVS archive doesn't match the guidelines (see http://fedoraproject.org/wiki/Packaging/NamingGuidelines#NonNumericRelease). It should reflect the date on which it was grabbed (so it really should be csound-manual-CVS20060816-disttag The files section can be globbed %{_bindir}/c* grabs all starting with c (etc) makes the spec file a lot simpler to read (IMO) Fix the CVS datestamp for the manual and I'm happy to let this one go in. I'd be happy if the file globbing in the spec file was done, but it's not a blocker if you don't (In reply to comment #39) > Needs work > CVS archive doesn't match the guidelines (see > http://fedoraproject.org/wiki/Packaging/NamingGuidelines#NonNumericRelease). It > should reflect the date on which it was grabbed (so it really should be > csound-manual-CVS20060816-disttag This applies to pre-release snapshots, but not in this case. In this case we pulled the sources out using the release tag upstream used in their cvs repository, as descripted in the spec file. The only reason we had to pull it out of cvs is because they only made the resulting PDF/html available, not the original source. In that case, I'm happy :-) APPROVED imported. Thanks! Package Change Request ====================== Package Name: csound New Branches: EL-6 Owners: pbrobinson sdz cvs done. |