Spec Url: http://www.cgsecurity.org/fcextra/testdisk.spec SRPM Url: http://www.cgsecurity.org/fcextra/testdisk-5.9-1.src.rpm It's my first FC Extra package and I am seeking a sponsor. Description: Tool to check and undelete partition. Works with the following partitions: - DOS/Windows FAT12, FAT16 and FAT32 - NTFS ( Windows NT/2K/XP ) - Linux Ext2 and Ext3 - BeFS ( BeOS ) - BSD disklabel ( FreeBSD/OpenBSD/NetBSD ) - CramFS (Compressed File System) - HFS, Hierarchical File System - JFS, IBM's Journaled File System - Linux Raid - Linux Swap (versions 1 and 2) - LVM and LVM2, Linux Logical Volume Manager - Netware NSS - ReiserFS 3.5 and 3.6 - UFS (Sun/BSD/...) - XFS, SGI's Journaled File System
Updated to version 6.2 SRPM Url: http://www.cgsecurity.org/fcextra/testdisk-6.2-1.src.rpm
>It's my first FC Extra package and I am seeking a sponsor. I can do that. No full review yet, just the things I saw on a first sight: -- Using these macros seems unnescaccary and confusing to me: %define name testdisk %define ver 6.2 %define rel 1 You IMHO should remove them -- Could you remove this: #Packager: Christophe GRENIER <grenier> -- Please change BuildRoot: %{_tmppath}/%{name}-%{version}-buildroot to BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) -- The description is IMHO way to long. Why not something like: Tool to check and undelete partition. Works with FAT12, FAT16, FAT32, NTFS, EXT2, EXT3, BeFS .... And the question is wether ntfs is allowed to be supported if this porgramm hits fedora extras. What exactly does it to support NTFS? Could is be in conflict with patents around NTFS? -- during configure I noticed: >checking for ntfs_device_mount in -lntfs... no >configure: WARNING: No ntfs library detected >checking for libreiserfs_get_version in -lreiserfs... no >configure: WARNING: No reiserfs library detected Is that correct? There are several other checks that result in "no" -- Could you check if they can be ignored safely?
I have updated the spec file according to your remarks. Be supporting NTFS, it only means TestDisk can decode the NTFS boot sector, it doesn't mean it can read/write NTFS partition. ntfs and reiserfs library aren't shipped in fedora. TestDisk can work without them, they are only necessary if you want to be able to list the files from ReiserFS 3.5/2.6 and NTFS partition. So there is no problem between TestDisk and potential patents around NTFS. It's not a problem if there are a lot of "no" during configure. TestDisk can be compiled on a lot of architecture and plateforme, so it has to check a lot of thinks.
(In reply to comment #3) > I have updated the spec file according to your remarks. And you did not increase the Release -- please do this the next time. That's easier and avoids confusion, even during review. > Be supporting NTFS, it only means TestDisk can decode the NTFS boot sector, > it doesn't mean it can read/write NTFS partition. That might be okay. But I'll recheck with Fesco/Legal. > ntfs and reiserfs library aren't shipped in fedora. TestDisk can work without > them, they are only necessary if you want to be able to list the files from > ReiserFS 3.5/2.6 and NTFS partition. Okay. > It's not a problem if there are a lot of "no" during configure. Sure, I just wanted to be sure that you checked them. ;-) One other thing I just noticed that I'm not sure about myself. The spec file has: %doc %{_mandir}/man1/testdisk.1* %doc %{_mandir}/man1/photorec.1* Uhh, do we ship man pages as doc? To answer my own question: Seems we sometimes do according to a grep through a extras cvs checkout [rpmbuild@truhe devel]$ grep /man./ */*.spec | grep \%doc | wc -l 35 [rpmbuild@truhe devel]$ grep /man./ */*.spec | wc -l 1071 @fedora-extras-list: Any option on this? IMHO man pages should not be marked as %doc.
(In reply to comment #4) > One other thing I just noticed that I'm not sure about myself. The spec file has: > %doc %{_mandir}/man1/testdisk.1* > %doc %{_mandir}/man1/photorec.1* > > Uhh, do we ship man pages as doc? To answer my own question: Seems we sometimes > do according to a grep through a extras cvs checkout > [rpmbuild@truhe devel]$ grep /man./ */*.spec | grep \%doc | wc -l > 35 > [rpmbuild@truhe devel]$ grep /man./ */*.spec | wc -l > 1071 > > @fedora-extras-list: Any option on this? IMHO man pages should not be marked as %doc. rpmbuild in Fedora automatically marks manpages as %doc so there is no need to do it explicitly like this. I think the same goes for info files.
Where is the lastet SRPM? http://www.cgsecurity.org/fcextra/testdisk-6.2-1.src.rpm seems to be the old one. And if you need to re-upload/create it: could you remove the %doc as per comment #5 ? tia
Done, %doc as been removed as per comment #5 Spec Url: http://www.cgsecurity.org/fcextra/testdisk.spec SRPM Url: http://www.cgsecurity.org/fcextra/testdisk-6.2-2.src.rpm
Review of d92f0ba26cf4e1c8bb276a38b9dbe376d6599004 testdisk-6.2-2.src.rpm $ rpmlint rpmbuild/*PMS/{,*/}testdisk*6.2-2* W: testdisk incoherent-version-in-changelog 5.0 6.2-2 This is harmless, but you should do it correctly in the future ;-) Review: - Souce testdisk-6.2.tar.bz2 is the same as upstream - Builds fine in mock - License is allowed in FE, correct in the spec file and shipped as %doc - name follows PackageNamingGuidelines - Spec looks good - File list looks good - Works for me - dir ownership OK - permissions OK - clean OK APPROVED; Create an account in the accounts system -- I'll sponsor you