Spec URL: https://kvolny.fedorapeople.org/mp3gain.spec SRPM URL: https://kvolny.fedorapeople.org/mp3gain-1.6.2-2.fc29.src.rpm Description: MP3Gain analyzes and adjusts mp3 files so that they have the same volume. It does not just do peak normalization, as many normalizers do. Instead, it does some statistical analysis to determine how loud the file actually sounds to the human ear. Also, the changes MP3Gain makes are completely lossless. There is no quality lost in the change because the program adjusts the mp3 file directly, without decoding and re-encoding. Fedora Account System Username: kvolny
From a cursory glance, the spec looks generally clean. A couple comments though: - Drop Group: tag - Add comments for Source1, Source2 and Patch2 explaining where they come from. - Does the manpage need to be pre-compressed ? The build could handle the compression and thus the .gz suffix can be replaced by a wildcard in %files in case the compression method changes someday.
You can avoid to manually edit the following line for each new release: %define tarball_version 1_6_2 Just change it to: %define tarball_version %(echo %version|sed s/\\\\\./_/g)
Thanks to Andrea's comment, I noticed I missed another nit-pick ;-) - Replace %define by %global An alternative to sed would be to use tr : %global tarball_version %(echo %version | tr _ .) In any case, you need to either BR: coreutils (for tr) or BR: sed
(In reply to Xavier Bachelot from comment #1) > - Add comments for Source1, Source2 and Patch2 explaining where they come > from. if only I knew :-) ... these predate git history, and I had touched the package only three years _after_ it was imported you-know-where the manpage seems to be extracted from the Debian package and the same for the README.method, probably - the only trace I was able to found is here: https://github.com/rbrito/pkg-mp3gain/blob/master/debian/README.method IMHO, we might as well drop the file, as it doesn't convey any important information about the usage; these things are described on the project's web ...? > - Does the manpage need to be pre-compressed ? The build could handle the > compression and thus the .gz suffix can be replaced by a wildcard in %files > in case the compression method changes someday. to be honest, I don't know how this works ... simply copying uncompressed manpage to workdir in %prep will trigger its compression in %build? how does rpmbuild know about the manpage file? - I'd prefer not to touch it if it ain't broke (In reply to Andrea Musuruane from comment #2) > You can avoid to manually edit the following line for each new release: > %define tarball_version 1_6_2 ... (In reply to Xavier Bachelot from comment #3) > In any case, you need to either BR: coreutils (for tr) or BR: sed the package already uses sed so let's stick to it ... and rpmbuild requires it, so no need to put that into BR(*) (*) btw, it won't help anyways: "Because SRPMs are built without the package’s BuildRequires installed" [https://docs.fedoraproject.org/en-US/packaging-guidelines/#_source_rpm_buildtime_macros] I've uploaded the new version with these changes (I don't think we need to update release and add changelog entries for this WIP): --- mp3gain.spec~ 2019-01-08 16:47:00.000000000 +0100 +++ mp3gain.spec 2019-01-09 12:18:01.136284939 +0100 @@ -1,14 +1,15 @@ -%define tarball_version 1_6_2 - Name: mp3gain Version: 1.6.2 + +%global tarball_version %(echo %version|%{__sed} 's/\\./_/g') + Release: 2%{?dist} Summary: Lossless MP3 volume adjustment tool -Group: Applications/Multimedia License: LGPLv2+ URL: http://mp3gain.sourceforge.net/ Source0: https://sourceforge.net/projects/%{name}/files/%{name}/%{version}/%{name}-%{tarball_version}-src.zip +# copied from Debian Source1: %{name}.1.gz Source2: README.method Patch2: %{name}-cflags-1.5.2.patch @@ -53,7 +54,9 @@ %changelog * Tue Jan 08 2019 Karel Volný <kvolny> - 1.6.2-2 - Import to Fedora (rhbz#1664399) -- Fixed license tag +- Fixed License tag +- Dropped Group tag +- Improved tarball_version definition * Wed Jan 02 2019 Sérgio Basto <sergio> - 1.6.2-1 - Update to 1.6.2
- %{__sed} → sed - Provide the manpaqe unzipped: the build will take care of compressing it. Also glob the .gz extension as the compression method might change in the future. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Header files in -devel subpackage, if present. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "GNU Lesser General Public License (v2.1 or later)", "Unknown or generated". 16 files have unknown license. Detailed output of licensecheck in /home/bob/packaging/review/mp3gain/review- mp3gain/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [-]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [-]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 10240 bytes in 1 files. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package requires other packages for directories it uses. [x]: Package does not own files or directories owned by other packages. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [-]: Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in mp3gain- debuginfo , mp3gain-debugsource [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Patches link to upstream bugs/comments/lists or are otherwise justified. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: Package should compile and build into binary rpms on all supported architectures. [-]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on debuginfo package(s). Note: No rpmlint messages. [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: mp3gain-1.6.2-2.fc30.x86_64.rpm mp3gain-debuginfo-1.6.2-2.fc30.x86_64.rpm mp3gain-debugsource-1.6.2-2.fc30.x86_64.rpm mp3gain-1.6.2-2.fc30.src.rpm mp3gain.x86_64: W: spelling-error Summary(en_US) Lossless -> Loss less, Loss-less, Loveless mp3gain.x86_64: W: spelling-error %description -l en_US normalizers -> normalizes, normalize rs, normalize-rs mp3gain.src: W: spelling-error Summary(en_US) Lossless -> Loss less, Loss-less, Loveless mp3gain.src: W: spelling-error %description -l en_US normalizers -> normalizes, normalize rs, normalize-rs 4 packages and 0 specfiles checked; 0 errors, 4 warnings.
Ping, it seems this review got forgotten.
ah, sorry, thanks for ping new files: https://kvolny.fedorapeople.org/mp3gain.spec https://kvolny.fedorapeople.org/mp3gain-1.6.2-2.fc30.src.rpm changes: --- mp3gain.spec~ 2019-01-09 12:18:01.000000000 +0100 +++ mp3gain.spec 2019-08-16 10:54:26.930040545 +0200 @@ -1,7 +1,7 @@ Name: mp3gain Version: 1.6.2 -%global tarball_version %(echo %version|%{__sed} 's/\\./_/g') +%global tarball_version %(echo %version|sed 's/\\./_/g') Release: 2%{?dist} Summary: Lossless MP3 volume adjustment tool @@ -10,7 +10,7 @@ URL: http://mp3gain.sourceforge.net/ Source0: https://sourceforge.net/projects/%{name}/files/%{name}/%{version}/%{name}-%{tarball_version}-src.zip # copied from Debian -Source1: %{name}.1.gz +Source1: %{name}.1 Source2: README.method Patch2: %{name}-cflags-1.5.2.patch @@ -32,7 +32,7 @@ %setup -q -c %{name}-%{tarball_version} %patch2 -p0 -b .cflags install -p -m 644 %{SOURCE2} . -%{__sed} -i 's/\r//' lgpl.txt +sed -i 's/\r//' lgpl.txt %build @@ -41,22 +41,23 @@ %install install -Dp -m 755 %{name} $RPM_BUILD_ROOT%{_bindir}/%{name} -install -Dp -m 644 %{SOURCE1} $RPM_BUILD_ROOT%{_mandir}/man1/%{name}.1.gz +install -Dp -m 644 %{SOURCE1} $RPM_BUILD_ROOT%{_mandir}/man1/%{name}.1 %files %doc README.method %license lgpl.txt %{_bindir}/%{name} -%{_mandir}/man1/%{name}.1.gz +%{_mandir}/man1/%{name}.1* %changelog -* Tue Jan 08 2019 Karel Volný <kvolny> - 1.6.2-2 +* Fri Aug 16 2019 Karel Volný <kvolny> - 1.6.2-2 - Import to Fedora (rhbz#1664399) - Fixed License tag - Dropped Group tag - Improved tarball_version definition +- Unzipped manpage * Wed Jan 02 2019 Sérgio Basto <sergio> - 1.6.2-1 - Update to 1.6.2
LGTM, package approved.
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/mp3gain
FEDORA-2019-239774aa43 has been submitted as an update to Fedora 31. https://bodhi.fedoraproject.org/updates/FEDORA-2019-239774aa43
damn, this one day late :-( https://bodhi.fedoraproject.org/updates/FEDORA-2019-239774aa43#comment-1015474
mp3gain-1.6.2-2.fc31 has been pushed to the Fedora 31 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-239774aa43
mp3gain-1.6.2-2.fc31 has been pushed to the Fedora 31 stable repository. If problems still persist, please make note of it in this bug report.
Hi Karel, The repo that was originally shipping mp3gain when it was not allowed in Fedora currently has mp3gain-1.6.2-3.fc31, while Fedora has mp3gain-1.6.2-2.fc31. Could you please bump the release to 4 in order to have a proper upgrade path ? Also, the package needs to be properly retired in that other repo. I can take care of that if you wish, just let me know. Regards, Xavier
(In reply to Xavier Bachelot from comment #15) > The repo that was originally shipping mp3gain when it was not allowed in > Fedora currently has mp3gain-1.6.2-3.fc31, while Fedora has > mp3gain-1.6.2-2.fc31. > Could you please bump the release to 4 in order to have a proper upgrade > path ? > Also, the package needs to be properly retired in that other repo. I can > take care of that if you wish, just let me know. Hi, we're still in beta ... if you retire it[*] before final release then everything will be okay even without bumping the release, wouldn't it? [*] yes please
mp3gain has been retired for both Rawhide and F31 in that other famous repo. I guess it's ok to not bump the release, there are likely very few people who have this package installed for F31 or Rawhide, and people running such a setup likely know how to yum/dnf distro-sync anyway. If I may abuse you once more, maybe you could also take care of the other branches in Fedora so it can be completely removed on the other side? The remaining branches are F30, F29, EL7 and EL6.
(In reply to Xavier Bachelot from comment #17) > If I may abuse you once more, maybe you could also take care of the other > branches in Fedora so it can be completely removed on the other side? The > remaining branches are F30, F29, EL7 and EL6. I thought we'll just let it rot until the old versions are phased out ... is there any special reason to move it? - well, yes, I can do it, it just doesn't fit into my POV that the realeased distro version shouldn't change (that much)
No particular reason to move except keeping all pieces at the same place. Imho, it would be less confusing, shall any need to update arise for older branches. F29 and F30 both have mp3gain 1.6.2, so it would be mostly a no op. EL6 and EL7 have the older 1.5.2 r2, so this would indeed be a change. Looking a bit closer, this version is affected by CVE-2017-12911, which is fixed in 1.6.0 and later. I think that brings incentive for the update.
Builds fine for EL7: https://koji.fedoraproject.org/koji/taskinfo?taskID=37689931 Fails on EL6 because of missing mpg123-devel: https://koji.fedoraproject.org/koji/taskinfo?taskID=37689933
ok, let's do it halfway - no EL6 as it lacks mpg123, and no F29 as it's going to die in two months
Works for me. EL6 was a moot point anyway, as it's been branched but never built. I'll retire the EL7 and F30 branches in the other repo once it's been branched and built in Fedora/EPEL. Thanks again.
FEDORA-EPEL-2019-8effe82ec2 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2019-8effe82ec2
FEDORA-2019-f34acb828e has been submitted as an update to Fedora 30. https://bodhi.fedoraproject.org/updates/FEDORA-2019-f34acb828e
FEDORA-EPEL-2019-19eeb2b3ce has been submitted as an update to Fedora EPEL 8. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2019-19eeb2b3ce
mp3gain-1.6.2-2.fc30 has been pushed to the Fedora 30 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-f34acb828e
mp3gain-1.6.2-2.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2019-8effe82ec2
mp3gain-1.6.2-2.el8 has been pushed to the Fedora EPEL 8 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2019-19eeb2b3ce
mp3gain-1.6.2-2.fc30 has been pushed to the Fedora 30 stable repository. If problems still persist, please make note of it in this bug report.
mp3gain-1.6.2-2.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.
mp3gain-1.6.2-2.el8 has been pushed to the Fedora EPEL 8 stable repository. If problems still persist, please make note of it in this bug report.