Bug 800284

Summary: Review Request: AtomicParsley - Command-Line Program to Read and Set iTunes-style Metadata Tags
Product: [Fedora] Fedora Reporter: Avi Alkalay <avibrazil>
Component: Package ReviewAssignee: Petr Pisar <ppisar>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: rawhideCC: dwmw2, ktdreyer, mavit, package-review, ppisar
Target Milestone: ---Flags: ppisar: fedora-review+
gwync: 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: 2014-07-02 13:45:28 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:

Description Avi Alkalay 2012-03-06 08:36:59 UTC
RPM, SPEC and SRPM: http://avi.alkalay.net/software/atomicparsley/

AtomicParsley is a lightweight command line program that can read and set
iTunes-style metadata tags in MPEG-4 files & 3gp assets in 3GPP/3GPP2 files.

This package started to be reviewed in RPMFusion (https://bugzilla.rpmfusion.org/show_bug.cgi?id=2190) but since MP4 metadata and the software code has no intellectual property issues, I'm submitting to main Fedora.

Comment 1 Petr Pisar 2012-05-07 11:53:32 UTC
Please provide new SPEC file that reflect changes requested by the rpmfussion review. Namely not touching COPYING, not using macros for coreutils commands etc.

Especially the COPYING is fatal for this review.

Comment 2 Avi Alkalay 2012-05-07 13:20:37 UTC
Please check new version 0.9.0-9.fc16 on same URL.

Thank you for reviewing.

Comment 3 Petr Pisar 2012-05-09 08:05:17 UTC
Source file is original. Ok.

TODO: The summary emphasis a service, but the tool manipulates MPEG-4 atoms regardless of the origin of a file. Please change the summary to be more descriptive (e.g. `Manipulate MPEG-4 metadata').

TODO: The same applies to the description. Copying first paragraph with the list of metadata profiles from project web page sounds better for me.

FIX: All files are distributed under GPLv2 or later version. Change the License tag to GPLv2+.

TODO: Remove the useless commented code from SPEC file (Source1, etc.).

Source0 and URL are usable Ok.

TODO: Append a slash to the URL to provide normalized URL.

TODO: The `rm -rf __MACOSX' command in %prep section should go after %setup.

TODO: Replace %__sed with plain `sed'.
TODI: Replace %__install with plain `install'.

TODO: Setting CXX variable is useless. It's not used anywhere in the build script. Maybe you want to replace `g++' with `$CXX' in the build script.

$ rpmlint AtomicParsley.spec ../SRPMS/AtomicParsley-0.9.0-9.fc18.src.rpm ../RPMS/x86_64/AtomicParsley-*
AtomicParsley.spec:8: W: macro-in-comment %{name}
AtomicParsley.spec:8: W: macro-in-comment %{version}
AtomicParsley.spec:13: W: macro-in-comment %{_tmppath}
AtomicParsley.spec:13: W: macro-in-comment %{name}
AtomicParsley.spec:13: W: macro-in-comment %{version}
AtomicParsley.spec:13: W: macro-in-comment %{release}
AtomicParsley.spec:23: W: macro-in-comment %{S
AtomicParsley.spec:39: W: macro-in-comment %clean
AtomicParsley.spec:40: W: macro-in-comment %{buildroot}
AtomicParsley.spec:44: W: macro-in-comment %defattr
AtomicParsley.spec:13: W: mixed-use-of-spaces-and-tabs (spaces: line 2, tab: line 13)
AtomicParsley.src:8: W: macro-in-comment %{name}
AtomicParsley.src:8: W: macro-in-comment %{version}
AtomicParsley.src:13: W: macro-in-comment %{_tmppath}
AtomicParsley.src:13: W: macro-in-comment %{name}
AtomicParsley.src:13: W: macro-in-comment %{version}
AtomicParsley.src:13: W: macro-in-comment %{release}
AtomicParsley.src:23: W: macro-in-comment %{S
AtomicParsley.src:39: W: macro-in-comment %clean
AtomicParsley.src:40: W: macro-in-comment %{buildroot}
AtomicParsley.src:44: W: macro-in-comment %defattr
AtomicParsley.src:13: W: mixed-use-of-spaces-and-tabs (spaces: line 2, tab: line 13)
AtomicParsley.x86_64: W: incoherent-version-in-changelog 0.9.0-9.fc16 ['0.9.0-9.fc18', '0.9.0-9']
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: empty-debuginfo-package
3 packages and 1 specfiles checked; 2 errors, 24 warnings.

FIX: Remove the dead code from spec file, or escape percent characters with another percent. The SPEC macros are evaluated even inside shell comments
FIX: Remove the `.fc16' from changelog entries.

$ rpm -q -lv -p ../RPMS/x86_64/AtomicParsley-0.9.0-9.fc18.x86_64.rpm
-rwxr-xr-x    1 root    root                   167944 May  9 09:36 /usr/bin/AtomicParsley
drwxr-xr-x    2 root    root                        0 May  9 09:36 /usr/share/doc/AtomicParsley-0.9.0
-rw-r--r--    1 root    root                     4524 Sep 16  2006 /usr/share/doc/AtomicParsley-0.9.0/AP buglist.txt
-rw-r--r--    1 root    root                    15123 Sep 30  1999 /usr/share/doc/AtomicParsley-0.9.0/COPYING
-rw-r--r--    1 root    root                    24412 Sep 26  2006 /usr/share/doc/AtomicParsley-0.9.0/Using AtomicParsley.rtf
File permissions and layout is Ok.

$ rpm -q --requires -p ../RPMS/x86_64/AtomicParsley-0.9.0-9.fc18.x86_64.rpm |sort |uniq -c
      1 libc.so.6()(64bit)
      1 libc.so.6(GLIBC_2.14)(64bit)
      1 libc.so.6(GLIBC_2.2.5)(64bit)
      1 libc.so.6(GLIBC_2.3.4)(64bit)
      1 libc.so.6(GLIBC_2.4)(64bit)
      1 libgcc_s.so.1()(64bit)
      1 libm.so.6()(64bit)
      1 libm.so.6(GLIBC_2.2.5)(64bit)
      1 libstdc++.so.6()(64bit)
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsXz) <= 5.2-1
      1 rtld(GNU_HASH)
Binary requires are Ok.

$ rpm -q --provides  -p ../RPMS/x86_64/AtomicParsley-0.9.0-9.fc18.x86_64.rpm |sort |uniq -c
      1 AtomicParsley = 0.9.0-9.fc18
      1 AtomicParsley(x86-64) = 0.9.0-9.fc18
Binary provides are Ok.

$ resolvedeps rawhide ../RPMS/x86_64/AtomicParsley-0.9.0-9.fc18.x86_64.rpm 
Binary dependencies resolvable. Ok.

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

I sent a request for Fedora legal department to bless this package because I'm not sure about MPEG-4 container patents (http://lists.fedoraproject.org/pipermail/legal/2012-May/thread.html).

Otherwise the package is in line with Fedora packaging guidelines.


Please correct all `FIX' issues, consider fixing `TODO' items, and provide new spec file.

Resolution: Package NOT approved.

Comment 4 Avi Alkalay 2012-09-26 18:55:17 UTC
Here are the updates based on last comments:

http://avi.alkalay.net/software/atomicparsley/AtomicParsley-0.9.0-10.fc17.src.rpm
http://avi.alkalay.net/software/atomicparsley/AtomicParsley.spec
http://avi.alkalay.net/software/atomicparsley/AtomicParsley-0.9.0-10.fc17.x86_64.rpm




(In reply to comment #3)
> TODO: The `rm -rf __MACOSX' command in %prep section should go after %setup.

This observation can't be changed as requested. I put a comment on the spec
file to be clear why it is like this.

Comment 5 Petr Pisar 2012-10-01 11:48:13 UTC
Spec file changes:
--- AtomicParsley.spec.old      2012-05-09 07:36:25.372000202 +0000
+++ AtomicParsley.spec  2012-09-26 18:50:26.000000000 +0000
@@ -1,31 +1,37 @@
-Summary:   Command-Line Program to Read and Set iTunes-style Metadata Tags
+Summary:   Command-line program to read and set MPEG-4 tags and metadata compatible with iPod/iTunes 
 URL:       http://atomicparsley.sourceforge.net
 Name:      AtomicParsley
 Version:   0.9.0
-Release:   9%{?dist}
-License:   GPLv2
+Release:   10%{?dist}
+License:   GPLv2+
 Group:     Applications/Multimedia
-#Source0: http://prdownloads.sourceforge.net/atomicparsley/%{name}-source-%{version}.zip
 Source0:   http://downloads.sourceforge.net/project/atomicparsley/atomicparsley/%{name}%20v%{version}/%{name}-source-%{version}.zip
-#Source1:   http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt
 Patch0:    %{name}-fix_bad_math.patch
 
 #BuildRoot:    %{_tmppath}/%{name}-%{version}-%{release}-buildroot
 
 
 %description
-Command line program that can read and set iTunes-style meta data
-tags in MPEG-4 files (M4V, M4A, MP4, MOV) & 3gp assets in 3GPP/3GPP2 files.
+AtomicParsley is a command line program for reading, parsing and setting
+tags and metadata into MPEG-4 files supporting these styles of metadata:
+
+. iTunes-style metadata 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
+
 
 %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}"
-#cp %{S:1} COPYING
 %patch0
 
-%__sed -i 's/-O2/$OPTFLAGS/g;' build
-
-%__sed -i '1aset -e' build
+sed -i 's/-O2/$OPTFLAGS/g;' build
+sed -i 's/g++/$CXX/g;' build
+sed -i '1aset -e' build
 
 %build
 CXX="%__cxx" \
@@ -33,23 +39,22 @@
 ./build
 
 %install
-%__install -D -s -m0755 AtomicParsley "%{buildroot}%{_bindir}/AtomicParsley"
+install -D -s -m0755 AtomicParsley "%{buildroot}%{_bindir}/AtomicParsley"
 
 
-#%clean
-#rm -rf "%{buildroot}"
-
 %files
 
-#%defattr(-,root,root)
 %doc COPYING AP\ buglist.txt Using\ AtomicParsley.rtf
 %{_bindir}/AtomicParsley
 
 %changelog
-* Fri Mar 02 2012 Avi Alkalay <avi> 0.9.0-9.fc16
+* Tue Sep 25 2012 Avi Alkalay <avi> 0.9.0-10
+- Editing with comments from https://bugzilla.redhat.com/show_bug.cgi?id=800284#c3
+
+* Fri Mar 02 2012 Avi Alkalay <avi> 0.9.0-9
 - Editing with comments from https://bugzilla.rpmfusion.org/show_bug.cgi?id=2190#c1
 
-* Wed Feb 22 2012 Avi Alkalay <avi> 0.9.0-7.fc16
+* Wed Feb 22 2012 Avi Alkalay <avi> 0.9.0-7
 - RPM and patches adapted and built for Fedora 16 based on Madriva SRPM
 
 * Thu Jul 22 2010 pascal


> TODO: The summary emphasis a service, but the tool manipulates MPEG-4 atoms regardless
> of the origin of a file. Please change the summary to be more descriptive (e.g.
> `Manipulate MPEG-4 metadata').
-Summary:   Command-Line Program to Read and Set iTunes-style Metadata Tags
+Summary:   Command-line program to read and set MPEG-4 tags and metadata compatible with iPod/iTunes
Ok.

> TODO: The same applies to the description. Copying first paragraph with the list of
> metadata profiles from project web page sounds better for me.
%description
-Command line program that can read and set iTunes-style meta data
-tags in MPEG-4 files (M4V, M4A, MP4, MOV) & 3gp assets in 3GPP/3GPP2 files.
+AtomicParsley is a command line program for reading, parsing and setting
+tags and metadata into MPEG-4 files supporting these styles of metadata:
+
+. iTunes-style metadata 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
+
Ok.

> FIX: All files are distributed under GPLv2 or later version. Change the License tag to
> GPLv2+.
-License:   GPLv2
+Release:   10%{?dist}
+License:   GPLv2+
Ok.

> TODO: Remove the useless commented code from SPEC file (Source1, etc.).
It's better now.
TODO: Remove #BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-buildroot.

> TODO: Append a slash to the URL to provide normalized URL.
Not addressed.

> TODO: The `rm -rf __MACOSX' command in %prep section should go after %setup.
 %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.
I still can't understand how removing directory before unpacking sources can remove the directory delivered with sources.
TODO: If the directory is created in %build or %install section, you should remove it
there. Or better fix the build script.

> TODO: Replace %__sed with plain `sed'.
> TODI: Replace %__install with plain `install'.
-%__sed -i 's/-O2/$OPTFLAGS/g;' build
-
-%__sed -i '1aset -e' build
+sed -i 's/-O2/$OPTFLAGS/g;' build
+sed -i 's/g++/$CXX/g;' build
+sed -i '1aset -e' build
Ok.

> TODO: Setting CXX variable is useless. It's not used anywhere in the build script.
> Maybe you want to replace `g++' with `$CXX' in the build script.
Ok.

$ rpmlint AtomicParsley.spec ../SRPMS/AtomicParsley-0.9.0-10.fc19.src.rpm ../RPMS/x86_64/AtomicParsley-*0.9.0-10.*
AtomicParsley.spec:11: W: macro-in-comment %{_tmppath}
AtomicParsley.spec:11: W: macro-in-comment %{name}
AtomicParsley.spec:11: W: macro-in-comment %{version}
AtomicParsley.spec:11: W: macro-in-comment %{release}
AtomicParsley.spec:26: W: macro-in-comment %setup
AtomicParsley.spec:27: W: macro-in-comment %clean
AtomicParsley.spec:11: W: mixed-use-of-spaces-and-tabs (spaces: line 2, tab: line 11)
AtomicParsley.src: W: spelling-error Summary(en_US) metadata -> meta data, meta-data, metatarsal
AtomicParsley.src: E: summary-too-long C Command-line program to read and set MPEG-4 tags and metadata compatible with iPod/iTunes
AtomicParsley.src: W: spelling-error %description -l en_US metadata -> meta data, meta-data, metatarsal
AtomicParsley.src: W: spelling-error %description -l en_US uuid -> quid
AtomicParsley.src: E: description-line-too-long C . uuid private user extension text & file embedding for MPEG-4 & derivative files
AtomicParsley.src:11: W: macro-in-comment %{_tmppath}
AtomicParsley.src:11: W: macro-in-comment %{name}
AtomicParsley.src:11: W: macro-in-comment %{version}
AtomicParsley.src:11: W: macro-in-comment %{release}
AtomicParsley.src:26: W: macro-in-comment %setup
AtomicParsley.src:27: W: macro-in-comment %clean
AtomicParsley.src:11: W: mixed-use-of-spaces-and-tabs (spaces: line 2, tab: line 11)
AtomicParsley.x86_64: W: spelling-error Summary(en_US) metadata -> meta data, meta-data, metatarsal
AtomicParsley.x86_64: E: summary-too-long C Command-line program to read and set MPEG-4 tags and metadata compatible with iPod/iTunes
AtomicParsley.x86_64: W: spelling-error %description -l en_US metadata -> meta data, meta-data, metatarsal
AtomicParsley.x86_64: W: spelling-error %description -l en_US uuid -> quid
AtomicParsley.x86_64: E: description-line-too-long C . uuid private user extension text & file embedding for MPEG-4 & derivative files
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: empty-debuginfo-package
3 packages and 1 specfiles checked; 6 errors, 21 warnings.

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.

> FIX: Remove the dead code from spec file, or escape percent characters with another
> percent. The SPEC macros are evaluated even inside shell comments
FIX: Remove commented BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-buildroot.

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

> FIX: Remove the `.fc16' from changelog entries.
Ok.

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

Please correct all `FIX' issues, consider fixing `TODO' items and provide new spec file.

Resolution: Package NOT approved.

Comment 6 Avi Alkalay 2012-10-01 21:59:46 UTC
Here are the updates based on last comments:

http://avi.alkalay.net/software/atomicparsley/AtomicParsley.spec
http://avi.alkalay.net/software/atomicparsley/AtomicParsley-0.9.0-11.fc17.src.rpm
http://avi.alkalay.net/software/atomicparsley/AtomicParsley-0.9.0-11.fc17.x86_64.rpm

rpmlint now is as clean as possible.

I moved the 'rm __MACOSX' to %build and changed the comment to be more clear:

%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

Thank you for reviewing.

Comment 7 Petr Pisar 2012-10-02 14:27:55 UTC
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> 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> 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.

Comment 8 Avi Alkalay 2012-10-02 15:22:24 UTC
Fixed all, I think.

http://avi.alkalay.net/software/atomicparsley/AtomicParsley.spec
http://avi.alkalay.net/software/atomicparsley/AtomicParsley-0.9.0-12.fc17.src.rpm
http://avi.alkalay.net/software/atomicparsley/AtomicParsley-0.9.0-12.fc17.x86_64.rpm

I think this is the best and most clear way:

%prep
# 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 fails in the unzipping process.
# We will (try to) remove this useless leftover directory and the new one that
# will be generated by unzipping right now to avoid problems in subsequent RPM
# builds of the same package and to keep the build directory clean.
rm -rf __MACOSX
%setup -q -n "%{name}-source-%{version}"
rm -rf ../__MACOSX
chmod a-x *.cpp *.h
%patch0

Comment 9 Peter Oliver 2014-01-14 00:05:00 UTC
Hello, folks.  This review was approved in 2012, so is there any reason the package hasn't made it in to Fedora yet?

Comment 10 Avi Alkalay 2014-01-14 00:21:14 UTC
(In reply to Peter Oliver from comment #9)
> Hello, folks.  This review was approved in 2012, so is there any reason the
> package hasn't made it in to Fedora yet?

I have no idea. I have to confess I never fully understood the approval process, it is very convoluted and never ends. I have no single package proposal that passed all steps and finally entered the main distribution. Very frustrating and disappointing.

Comment 11 Peter Oliver 2014-01-14 00:49:20 UTC
The fedora-review flag is set to "+", and that means that this package has passed review (described at https://fedoraproject.org/wiki/Package_Review_Process).

FAS shows you as being in the "Fedora Packager GIT Commit Group", so that means you've been sponsored.  (https://admin.fedoraproject.org/accounts/user/view/aviram)

So, I think you're up to here: https://fedoraproject.org/wiki/Join_the_package_collection_maintainers?rd=PackageMaintainers/Join#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner.  From here it's just a matter of requesting a git repository for the package, checking in the code, and getting the package built!

By the way, I notice AtomicParsley seems to have a de-facto new upstream at https://bitbucket.org/wez/atomicparsley/ (see https://sourceforge.net/p/atomicparsley/discussion/514419/thread/083240f5/ for discussion), but for now perhaps it's better to concentrate on releasing what you've got so far.

Comment 12 Avi Alkalay 2014-01-14 00:57:14 UTC
Thank you for the update, I didn't know much about it.

I'll come back to this as soon as I finish the upgrade of my home and build system to F20.

Comment 13 Christopher Meng 2014-01-14 01:54:36 UTC
Please update this package to 2.1.4, I will review it again.

Comment 14 Christopher Meng 2014-01-14 01:58:05 UTC
Sorry, not 2.1.4, but svn version.

This software stopped development in 2006, last commit in SVN is 2007, you'd better checkout from svn and build the latest snapshot package.

Comment 15 Petr Pisar 2014-01-14 12:14:11 UTC
This package has been approved. As Peter Oliver wrote, you should now request for a git repository and do all the other stuff to put the approved packaged into the Fedora.

Comment 16 Avi Alkalay 2014-01-28 16:46:50 UTC
Since this is approved, I'll get it into Fedora 20 first and then I'll use new code from https://bitbucket.org/wez/atomicparsley/ as found by Peter Oliver. The updated Fedora 20 packages are here:

http://avi.alkalay.net/software/atomicparsley/

Comment 17 Avi Alkalay 2014-01-28 16:50:24 UTC
New Package SCM Request
=======================
Package Name: AtomicParsley
Short Description: Command-line program to read and set MPEG-4 tags compatible with iPod/iTunes
Owners: aviram
Branches: f20
InitialCC:

Comment 18 Gwyn Ciesla 2014-01-28 16:59:16 UTC
Git done (by process-git-requests).

Comment 19 Avi Alkalay 2014-01-29 17:12:43 UTC
I'm having problems building on Koji. This is what I have after a 'fedpkg scratch-build':

http://koji.fedoraproject.org/koji/taskinfo?taskID=6468675

"BuildError: error retrieving sources, mock exited with status 1; see build.log for more information"

I can't see build.log, the zip file is on GIT and a new zip can be downloaded from the Source0 line on the spec file, which is:

http://downloads.sourceforge.net/project/atomicparsley/atomicparsley/%{name}%20v%{version}/%{name}-source-%{version}.zip

Can anybody help? Thank you in advance.

Comment 20 Ken Dreyer 2014-01-29 17:44:19 UTC
Hi Avi,

From http://kojipkgs.fedoraproject.org//work/tasks/8676/6468676/root.log:

DEBUG util.py:281:  Could not execute sources: Command curl -H Pragma: -o ./AtomicParsley-source-0.9.0.zip -R -S --fail http://pkgs.fedoraproject.org/repo/pkgs/AtomicParsley/AtomicParsley-source-0.9.0.zip/681e6ecec2921c98e07a9262bdcd6cf2/AtomicParsley-source-0.9.0.zip returned code 22 with error:   % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
DEBUG util.py:281:                                   Dload  Upload   Total   Spent    Left  Speed
DEBUG util.py:281:  
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
DEBUG util.py:281:  curl: (22) The requested URL returned error: 404 Not Found


Did you run "fedpkg new-sources AtomicParsley-source-0.9.0.zip" first, in order to upload the zip file to the lookaside cache?

When you run "fedpkg scratch-build" without the --srpm flag, Koji will try to build from Git and the lookaside cache, and if there's no zip file in the lookaside cache, the build will fail.

Alternatively, if you want to do a scratch build without uploading a zip file with "fedpkg new-sources", you should use the "--srpm" flag instead. "fedpkg scratch-build --srpm" means "create an SRPM locally, then upload that to koji and build that."

In this case I would recommend that you first do a scratch build with --srpm to get familiar with it, then run "fedpkg new-sources AtomicParsley-source-0.9.0.zip", then do a second scratch build (wihtout --srpm) to see how that works, and then finally do your real "fedpkg build".

Comment 21 Avi Alkalay 2014-01-29 18:45:13 UTC
No, I didn't make any "new-sources".
No documentation that I was pointed to or that I could find mentioned it.

This build infrastructure seems very powerful but documentation is too diffuse, too high level and not good for whom is starting, as me. Where is the starting point to understand all these commands and the whole process? I mean, with clear command examples and entire life cycle of a package until it gets available to plain "yum install". I'm trying for years with no success.

BTW, how did you get to that root.log URL?

Thank you Ken, will try again as soon as I get to the command line.

Comment 23 Ken Dreyer 2014-01-29 19:44:10 UTC
(In reply to Avi Alkalay from comment #21)
> No, I didn't make any "new-sources".
> No documentation that I was pointed to or that I could find mentioned it.

Typically for new packages, I will run "fedpkg clone", then "fedpkg import PATH_TO_SRPM". The "import" task takes care of running "new-sources". Then, for future updates, I will just run "new-sources".


> This build infrastructure seems very powerful but documentation is too
> diffuse, too high level and not good for whom is starting, as me. Where is
> the starting point to understand all these commands and the whole process? I
> mean, with clear command examples and entire life cycle of a package until
> it gets available to plain "yum install". I'm trying for years with no
> success.

It is a lot to digest. Feel free to ask the Fedora devel mailing list for advice. I've sponsored you in FAS so you can also ask me directly (ktdreyer), although I can't promise I can always respond promptly :)


> BTW, how did you get to that root.log URL?

1) Click on your koji task: http://koji.fedoraproject.org/koji/taskinfo?taskID=6468675
2) Click on the "buildSRPMFromSCM" descendent task,
3) In the "Output" section you'll see a list of files.

Generally either the "build.log" file or the "root.log" file will contain the relevant error for the task failure.

Comment 24 Avi Alkalay 2014-01-29 20:18:45 UTC
Thank you all for the help. I got it done:

http://koji.fedoraproject.org/koji/taskinfo?taskID=6469904

Now, before I dive into these docs, how to make the package available on a plain "yum install" for Fedora 20 ?

Thank you !

Comment 26 Avi Alkalay 2014-01-29 20:51:51 UTC
Thank you Peter. I guess I got it built to F20 in an indirect way:

koji build f20 `fedpkg giturl`

Of course the method on your link is better. Now I'll continue from that point in the documentation.

Comment 27 Peter Oliver 2014-02-10 22:31:59 UTC
A couple more tips.

Don't forget to merge your changes to the master branch; otherwise they will be missing from rawhide and future Fedora releases.  There's a discussion of this on the devel mailing list at the moment: https://lists.fedoraproject.org/pipermail/devel/2014-February/195399.html

When submitting an update, you're asked for a bug number.  Including this is usually a good idea, because then followers of the bug get automatically notified as the update progresses.

Hope this helps.  Thanks for sticking with it!

Comment 28 Avi Alkalay 2014-02-12 17:37:45 UTC
I think it is done now, merged changes to master. I used documentation from https://fedoraproject.org/wiki/Join_the_package_collection_maintainers?rd=PackageMaintainers/Join#Update_Your_Branches_.28if_desired.29

fedpkg switch-branch master
git merge f20
git push
fedpkg build

And it is building.

Comment 30 Christopher Meng 2014-02-13 02:06:50 UTC
Yes, I can see the updates in bodhi:

https://admin.fedoraproject.org/updates/AtomicParsley

------------------

But upgradepath check seems not good, I found a 0.9.0 version update still rolling in testing:

https://admin.fedoraproject.org/updates/FEDORA-2014-1812/AtomicParsley-0.9.0-13.fc20

You can delete that update to prevent yourself from accidental push-stable click.

Others are fine.

Welcome to Fedora.

Comment 31 Avi Alkalay 2014-02-13 02:20:12 UTC
I unpushed it and deleted. Done.
Thanks for the tip.