Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=483025 Michael Schwendt <mschwendt@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mschwendt@xxxxxxxxx --- Comment #11 from Michael Schwendt <mschwendt@xxxxxxxxx> 2010-01-14 06:57:33 EST --- * spectool -g imms-3.1.0-0.6.rc8.fc11.src/imms.spec retrieves an index.html file instead of the source tarball. Project hosting has moved to Google Code. * Currently fails to build (F-12 i686): | audplugin.o: In function `imms_get_playlist_item(int)': | /home/qa/mnt/work/tmp/rpm/BUILD/imms-3.1.0-rc8/build/../clients/audacious/audplugin.cc:81: undefined reference to `filename_to_utf8' | collect2: ld returned 1 exit status | make[1]: *** [libaudaciousimms.so] Error 1 Seems it doesn't link libaudcore and would need an update for Audacious >= 2.1 on F-12 (for F-12 there is a koji buildroot override tag for Audacious 2.2 currently, btw). * The spec file isn't pretty due to questionable sed substitutions. What do you do if some of the matches fail silently? Only if that results in a failing build, the sed usage is acceptable. Else you ought to prefer patch files. (or add guards in the spec file) Examples: 1) Using sed to modify install paths => upstream release modifies the build framework => your sed subst fails => rpmbuild fails because %install fails or the buildroot contents don't match the %files section => ACCEPTABLE, because you cannot proceed without fixing your package so it will build again. 2) Using sed to modify the contents of files => upstream release modifies the files => your sed subst fails => rpmbuild succeeds because no build error is found => NOT ACCEPTABLE, because either your sed substs are obsolete OR they fail silently => in case of the latter they don't apply your fixes anymore. In %build you use sed to adjust various CPPFLAGS/CXXFLAGS prior to running %configure. Nothing verifies that your sed substitutions have been applied actually. Bad. In %install you use sed to adjust install paths to point them into the buildroot. Consider yourself lucky that if the sed substitutions fail, most likely the rpmbuild fails too, because %files doesn't match buildroot anymore. Generally, however, there is no guarantee. And you use wildcards in the %files section, which means you don't even require specific files to be present in the buildroot ('*' = any file that's found will match). In the combination with sed substs, this is unsafe. Safer would be to hardcode the names of important files (e.g. immsd and immstool) and effectively require them to be found. * BuildRequires: sox What about run-time? I see in the source that it tries to popen() sox at run-time. Does that result in a run-time requirement of sox? Or is it fully optional and with a clear error/warning message if sox cannot be found? * As a hint for the %files section: where you include entire directories recursively, such as %{_datadir}/imms it's more readable to append a slash like %{_datadir}/imms/ to make explicit that it's a directory and not a single file. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review