[Bug 800284] Review Request: AtomicParsley - Command-Line Program to Read and Set iTunes-style Metadata Tags

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=800284

Petr Pisar <ppisar@xxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|fedora-review?              |
              Flags|                            |fedora-review+

--- Comment #7 from Petr Pisar <ppisar@xxxxxxxxxx> ---
Spec file changes:
--- AtomicParsley.spec.old      2012-09-26 18:50:26.000000000 +0000
+++ AtomicParsley.spec  2012-10-01 21:57:04.000000000 +0000
@@ -1,31 +1,28 @@
-Summary:   Command-line program to read and set MPEG-4 tags and metadata
compatible with iPod/iTunes
-URL:       http://atomicparsley.sourceforge.net
+Summary:   Command-line program to read and set MPEG-4 tags compatible with
iPod/iTunes
+URL:       http://atomicparsley.sourceforge.net/
 Name:      AtomicParsley
 Version:   0.9.0
-Release:   10%{?dist}
+Release:   11%{?dist}
 License:   GPLv2+
 Group:     Applications/Multimedia
 Source0:  
http://downloads.sourceforge.net/project/atomicparsley/atomicparsley/%{name}%20v%{version}/%{name}-source-%{version}.zip
 Patch0:    %{name}-fix_bad_math.patch

-#BuildRoot:    %{_tmppath}/%{name}-%{version}-%{release}-buildroot


 %description
 AtomicParsley is a command line program for reading, parsing and setting
-tags and metadata into MPEG-4 files supporting these styles of metadata:
+tags and meta-data into MPEG-4 files supporting these styles of meta-data:

-. iTunes-style metadata into .mp4, .m4a, .m4p, .m4v, .m4b files
-. 3gp-style assets (3GPP TS 26.444 version 6.4.0 Release 6 specification
+* iTunes-style meta-data into .mp4, .m4a, .m4p, .m4v, .m4b files
+* 3gp-style assets (3GPP TS 26.444 version 6.4.0 Release 6 specification
   conforming) in 3GPP, 3GPP2, MobileMP4 & derivatives
-. ISO copyright notices at movie & track level for MPEG-4 & derivative files
-. uuid private user extension text & file embedding for MPEG-4 & derivative
files
+* ISO copyright notices at movie & track level for MPEG-4 & derivative files
+* uuid private user extension text & file embedding for MPEG-4 & derivative
+  files


 %prep
-# This 'rm' must be *before* %setup because the __MACOSX directory is outside
the
-# main source directory, so a %clean will not actually clean it.
-rm -rf __MACOSX
 %setup -q -n "%{name}-source-%{version}"
 %patch0

@@ -34,12 +31,18 @@
 sed -i '1aset -e' build

 %build
+# The source zip file includes a top level directory called __MACOSX which
doesn't
+# get removed by %%clean, so in a multiple RPM build scenario this directory
will
+# not get removed, making a subsequent RPM build fail in the unzipping
process.
+# We will remove this useless directory right now to avoid problems in
subsequent
+# RPM builds of the same package.
+rm -rf ../__MACOSX
 CXX="%__cxx" \
 OPTFLAGS="%{optflags} -Wall -Wno-deprecated -fno-strict-aliasing" \
 ./build

 %install
-install -D -s -m0755 AtomicParsley "%{buildroot}%{_bindir}/AtomicParsley"
+install -D -m0755 AtomicParsley "%{buildroot}%{_bindir}/AtomicParsley"


 %files
@@ -48,6 +51,9 @@
 %{_bindir}/AtomicParsley

 %changelog
+* Mon Oct 01 2012 Avi Alkalay <avi@xxxxxxx> 0.9.0-11
+- Editing with comments from
https://bugzilla.redhat.com/show_bug.cgi?id=800284#c5
+
 * Tue Sep 25 2012 Avi Alkalay <avi@xxxxxxx> 0.9.0-10
 - Editing with comments from
https://bugzilla.redhat.com/show_bug.cgi?id=800284#c3


> TODO: Remove #BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-buildroot.
-#BuildRoot:    %{_tmppath}/%{name}-%{version}-%{release}-buildroot
Ok.

> TODO: Append a slash to the URL to provide normalized URL.
+URL:       http://atomicparsley.sourceforge.net/
Ok.

> TODO: If the directory is created in %build or %install section, you should remove it
> there. Or better fix the build script.
 %build
+# The source zip file includes a top level directory called __MACOSX which
doesn't
+# get removed by %%clean, so in a multiple RPM build scenario this directory
will
+# not get removed, making a subsequent RPM build fail in the unzipping
process.
+# We will remove this useless directory right now to avoid problems in
subsequent
+# RPM builds of the same package.
+rm -rf ../__MACOSX

I see. The thing that %prep does not prune BUILDROOT before executing its
content is unfortunate. The original approach with rm before %setup was better.

Or maybe the best way is not creating the directory at all like this:

%prep
rm -rf "%{name}-source-%{version}"
unzip -qq '%{SOURCE0}' '%{name}-source-%{version}/*'
%setup -q -T -D -n "%{name}-source-%{version}"
%patch0


$ rpmlint AtomicParsley.spec ../SRPMS/AtomicParsley-0.9.0-11.fc19.src.rpm
../RPMS/x86_64/AtomicParsley-*-11.*
AtomicParsley.src: W: spelling-error %description -l en_US uuid -> quid
AtomicParsley.x86_64: W: spelling-error %description -l en_US uuid -> quid
AtomicParsley.x86_64: E: incorrect-fsf-address
/usr/share/doc/AtomicParsley-0.9.0/COPYING
AtomicParsley.x86_64: W: no-manual-page-for-binary AtomicParsley
AtomicParsley-debuginfo.x86_64: E: incorrect-fsf-address
/usr/src/debug/AtomicParsley-source-0.9.0/main.cpp
AtomicParsley-debuginfo.x86_64: W: spurious-executable-perm
/usr/src/debug/AtomicParsley-source-0.9.0/AtomicParsley.h
AtomicParsley-debuginfo.x86_64: E: incorrect-fsf-address
/usr/src/debug/AtomicParsley-source-0.9.0/AtomicParsley.h
AtomicParsley-debuginfo.x86_64: E: incorrect-fsf-address
/usr/src/debug/AtomicParsley-source-0.9.0/AP_AtomExtracts.cpp
AtomicParsley-debuginfo.x86_64: E: incorrect-fsf-address
/usr/src/debug/AtomicParsley-source-0.9.0/APar_uuid.cpp
AtomicParsley-debuginfo.x86_64: E: incorrect-fsf-address
/usr/src/debug/AtomicParsley-source-0.9.0/AP_commons.h
AtomicParsley-debuginfo.x86_64: E: incorrect-fsf-address
/usr/src/debug/AtomicParsley-source-0.9.0/AP_commons.cpp
AtomicParsley-debuginfo.x86_64: W: spurious-executable-perm
/usr/src/debug/AtomicParsley-source-0.9.0/AtomicParsley.cpp
AtomicParsley-debuginfo.x86_64: E: incorrect-fsf-address
/usr/src/debug/AtomicParsley-source-0.9.0/AtomicParsley.cpp
AtomicParsley-debuginfo.x86_64: E: incorrect-fsf-address
/usr/src/debug/AtomicParsley-source-0.9.0/AP_AtomDefinitions.h
AtomicParsley-debuginfo.x86_64: E: incorrect-fsf-address
/usr/src/debug/AtomicParsley-source-0.9.0/AP_AtomExtracts.h
AtomicParsley-debuginfo.x86_64: E: incorrect-fsf-address
/usr/src/debug/AtomicParsley-source-0.9.0/AP_iconv.h
AtomicParsley-debuginfo.x86_64: E: incorrect-fsf-address
/usr/src/debug/AtomicParsley-source-0.9.0/AP_iconv.cpp
AtomicParsley-debuginfo.x86_64: E: incorrect-fsf-address
/usr/src/debug/AtomicParsley-source-0.9.0/APar_uuid.h
AtomicParsley-debuginfo.x86_64: E: incorrect-fsf-address
/usr/src/debug/AtomicParsley-source-0.9.0/AtomicParsley_genres.h
AtomicParsley-debuginfo.x86_64: E: incorrect-fsf-address
/usr/src/debug/AtomicParsley-source-0.9.0/AtomicParsley_genres.cpp
3 packages and 1 specfiles checked; 15 errors, 5 warnings.

TODO: Unset executable bit on AtomicParsley.h and AtomicParsley.cpp in %prep
section.

> TODO: Expand tabulators to spaces (you can use `expand -t4' command).
> TODO: Shorten summary text. E.g. by removing the iTunes words.
> TODO: Wrap description text to 80 columns.
> TODO: Replace the ASCII stop `bullets' with better approximation. E.g. an asterisk.
Ok.

> FIX: Remove commented BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-buildroot.
Ok.

> FIX: The debug file is empty because you install the binary with `-s' option.
> Don't do that.
Ok.

Package builds in koji
(http://koji.fedoraproject.org/koji/taskinfo?taskID=4550433). Ok.


Please consider fixing the TODO items before building this package.

Resolution: Package APPROVED.

-- 
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



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]