Bug 809086
Summary: | Review Request: mp3val - Tool for Validating and Fixing MPEG Audio Files | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Leonid Podolny <leonid> |
Component: | Package Review | Assignee: | Nobody's working on this, feel free to take it <nobody> |
Status: | CLOSED DEFERRED | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | unspecified | ||
Version: | rawhide | CC: | j, lemenkov, msuchy, package-review, tcallawa |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | AwaitingSubmitter | ||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2015-07-21 13:30:18 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: | 201449 |
Description
Leonid Podolny
2012-04-02 12:59:34 UTC
Fedora doesn't ship any mp3 related stuff: http://fedoraproject.org/wiki/Multimedia/MP3 That's why there is no real chance to get your package into the official Fedora repositories. You can try to get it into RPMfusion, but because you are new to the packagers group and not sponsored yet, it could become somewhat difficult. For your first package, you should choose a software which *not* possibly violates patents or has any other restrictions which avoids an integration in Fedora. (In reply to comment #1) > Fedora doesn't ship any mp3 related stuff: > http://fedoraproject.org/wiki/Multimedia/MP3 > > That's why there is no real chance to get your package into the official Fedora > repositories. You can try to get it into RPMfusion, but because you are new to > the packagers group and not sponsored yet, it could become somewhat difficult. > For your first package, you should choose a software which *not* possibly > violates patents or has any other restrictions which avoids an integration in > Fedora. On the contrary it is possible to include this into Fedora. This app does NOT do actual MPEG frame decoding, so I suspect that no nasty MPEG patents are violated. I could be wrong here - I just quickly looked through the sources and I didn't see any MPEG decoding components. Also take a look at the contents of gstreamer-plugins-bad-free - you'll see the demuxers for MPEG files as well. So demuxing and cheching the integrity of MPEG frames is OK - encoding/decoding isn't. True. I didn't spot any of the maths for decoding the audio data either. Just the parsing of the file format to find frames. [...] Other than that, skimming over the file, I see several items that are unusual (and not typical for Fedora packages). Such as, but possibly not limited to: > License: GPL GPLv2+ according to the source files. > BuildRoot: %{_tmppath}/build-%{name}-%{version} Please consult the related section in the packaging guidelines: https://fedoraproject.org/wiki/Packaging:Guidelines > BuildRequires: gcc-c++ libstdc++ libstdc++-devel glibc-devel These are all not needed, https://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions_2 and make it less easy to rebuild packages in an environment with a replaced minimum build environment. > %build > cd %{_builddir}/%{name}-%{version}-src This 'cd' command is not needed, because the source code's top-level directory is entered automatically. Verify by taking a look at the rpmbuild output. > %clean > %{__rm} -rf "%{buildroot}" > %files > %defattr(-,root,root) > Source: http://prdownloads.sourceforge.net/mp3val/%{name}-%{version}-src.tar.gz There's a section in the packaging guidelines about these items. [...] Visiting the Fedora Packaging related Wiki pages is highly recommended. https://fedoraproject.org/wiki/PackageMaintainers The Review Guidelines are not just for reviewers, but for package submitters and existing packagers, too: https://fedoraproject.org/wiki/Packaging:ReviewGuidelines Michael, Thank you for taking time and reviewing this spec file. I believe I have fixed all your remarks. I also run rpmlint, which I didn't do on the original spec. The updated submission is at: http://dl.dropbox.com/u/6441389/mp3val-0.1.8-2.fc16.src.rpm http://dl.dropbox.com/u/6441389/mp3val.spec URLs are 404. To be honest, I'd want to pass this by legal just in case, but with no actual package to look at I guess there's no point. Please clear the Whiteboard above if providing something to review. Otherwise I'll close this ticket out. (In reply to comment #5) > URLs are 404. To be honest, I'd want to pass this by legal just in case, > but with no actual package to look at I guess there's no point. > > Please clear the Whiteboard above if providing something to review. > Otherwise I'll close this ticket out. After several months of silence, I gave up and deleted the files from dropbox. I will be happy if someone does, in fact, review these packages. Thanks a lot! The files are at: https://dl.dropbox.com/s/rlgt3poa5arhxld/mp3val-0.1.8-2.fc17.src.rpm?dl=1 https://dl.dropbox.com/s/c1bx39s8mqvh9f9/mp3val.spec?dl=1 OK, adding this to the legal blocker. Honestly I don't think there's any real problem here but we try to be very careful with this kind of thing. Does not encode or decode, just parses and checks. Lifting FE-Legal. OK, I grabbed and built the package. rpmlint has a lot of complaints. I'll go over some of them, but I'm not sure why you didn't see and fix them when you ran rpmlint yourself. Also, it's bit of a stretch to call two months and one day "several months". mp3val.x86_64: E: description-line-too-long C integrity. It can be useful for finding corrupted files (e.g. incompletely downloaded, mp3val.x86_64: E: description-line-too-long C truncated, containing garbage). MP3val is also able to fix most of the problems. Being mp3val.x86_64: E: description-line-too-long C a multiplatform application, MP3val can be runned both under Windows and under Linux mp3val.x86_64: E: description-line-too-long C The most common MPEG audio file type is MPEG 1 Layer III (mp3), but MP3val supports also mp3val.x86_64: E: description-line-too-long C other MPEG versions and layers. The tool is also aware of the most common types of tags Please wrap your description to 72 characters. mp3val.x86_64: W: incoherent-version-in-changelog 0.1.8-1 ['0.1.8-2.fc18', '0.1.8-2'] You should have a changelog entry each time you update the version/release. mp3val.x86_64: W: spurious-executable-perm /usr/share/doc/mp3val-0.1.8/manual.html mp3val.x86_64: E: wrong-script-end-of-line-encoding /usr/share/doc/mp3val-0.1.8/manual.html Why is the manual executable? DOS-style line endings need to be converted. mp3val.x86_64: W: no-manual-page-for-binary mp3val This is OK; you should ask upstream to provide a manual page, or perhaps see if Debian has written one, but it's not expected that you will provide one yourself. mp3val-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/mp3val-0.1.8-src/main.cpp mp3val-debuginfo.x86_64: E: wrong-script-end-of-line-encoding /usr/src/debug/mp3val-0.1.8-src/main.cpp There are loads of these. The DOS line endings of the source files aren't such a big deal, but why on earth are they all executable. mp3val-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/mp3val-0.1.8-src/main.cpp You should tell your upstream that they need to update the license to reflect the fact that the FSF moved quite some time ago. It does look as if your upstream hasn't done much in three years, though, so this might not be something you can fix but you should still inform them. (trimmed several repeat versions of the above three complaints) As for the spec file, it looks quite clean. However, you should not use %{__install}. Instead just call "install" normally. It's not as if we're ever going to rename the install executable or remove it from the path. See http://fedoraproject.org/wiki/Packaging:Guidelines#Macros Closing due long inactivity. Feel free to reopen if you want to continue. |