Builds in mock for epel5 and Rawhide. RPMLint silent.

While using this myself for some time I've found experience with this tool quite painful -- feels very "Alpha" and even experienced a couple of server-triggered crashes, which may be exploitable.

Unless I'm only person using this I'm willing to work on this and improve it, otherwise I won't let it enter Fedora and will close this in some time.

So please review this only if you're interested in using it :)

I'm user #2, so we may continue. I added 2 lines to specfile (to add another headers to devel package)
Updated spec is under: http://www.benhur.prf.cuni.cz/medved-7/wydobitki/?path=fedora/extra
JFCh

Thanks, Jan. Good to know this will be of some use -- I can eventually spend some time getting the sick think into shape.

Do you think you can review the package?

OK source files match upstream:
   688560de1cde57ab8d9e0ef7dc6436dbf0267fe8884f9014e50ff92b297b01a8  afpfs-ng-0.8.1.tar.bz2
OK package meets naming and versioning guidelines.
OK specfile is properly named, is cleanly written and uses macros consistently.
FAIL Summary for %package -n fuse-afs is wrong
OK dist tag is present.
OK build root is correct, though I'd prefer:
   BuildRoot:   %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
OK license field matches the actual license.
OK license is open source-compatible.
OK license text included in package.
FAIL no duplicates in %files.
	#fedora-devel 17:11 < tibbs|h> You can include COPYING exactly once.
   Do not duplicate doc files, remove %doc lines from subpackages.
OK latest version is being packaged.
OK BuildRequires are proper.
OK compiler flags are appropriate.
OK %clean is present.
OK package builds in mock.
OK package installs properly.
OK debuginfo package looks complete.
OK rpmlint is silent.
OK final provides and requires are sane, attached at the end of the review
OK no %check because the package contains no testsuite
OK shared libraries present, ldconfig is run
OK owns the directories it creates.
OK doesn't own any directories it shouldn't.
OK file permissions are appropriate.
OK scriptlets look fine
OK code, not content.
OK documentation is small, so no -docs subpackage is necessary.
OK %docs are not necessary for the proper functioning of the package.
OK headers in -devel.
OK no pkgconfig files.
OK no libtool .la droppings.
FAIL Please use %configure --disable-static and drop %exclude *.a
OK desktop files valid and installed properly.

Fix the three details marked FAIL, and put an updated spec to the martyr.

==== afpfs-ng-0.8.1-1.fc11.x86_64.rpm:
   --provides:
libafpclient.so.0()(64bit)  
afpfs-ng = 0.8.1-1.fc11
afpfs-ng(x86-64) = 0.8.1-1.fc11
   --requires:
/sbin/ldconfig  
/sbin/ldconfig  
libafpclient.so.0()(64bit)  
libc.so.6()(64bit)  
libc.so.6(GLIBC_2.2.5)(64bit)  
libc.so.6(GLIBC_2.3)(64bit)  
libc.so.6(GLIBC_2.3.4)(64bit)  
libc.so.6(GLIBC_2.4)(64bit)  
libfuse.so.2()(64bit)  
libgcrypt.so.11()(64bit)  
libgcrypt.so.11(GCRYPT_1.2)(64bit)  
libgmp.so.3()(64bit)  
libncurses.so.5()(64bit)  
libpthread.so.0()(64bit)  
libpthread.so.0(GLIBC_2.2.5)(64bit)  
libpthread.so.0(GLIBC_2.3.2)(64bit)  
libreadline.so.5()(64bit)  
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rtld(GNU_HASH)  
==== afpfs-ng-devel-0.8.1-1.fc11.x86_64.rpm:
   --provides:
afpfs-ng-devel = 0.8.1-1.fc11
afpfs-ng-devel(x86-64) = 0.8.1-1.fc11
   --requires:
afpfs-ng = 0.8.1
libafpclient.so.0()(64bit)  
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
==== fuse-afp-0.8.1-1.fc11.x86_64.rpm:
   --provides:
fuse-afp = 0.8.1-1.fc11
fuse-afp(x86-64) = 0.8.1-1.fc11
   --requires:
/bin/bash  
libafpclient.so.0()(64bit)  
libc.so.6()(64bit)  
libc.so.6(GLIBC_2.2.5)(64bit)  
libc.so.6(GLIBC_2.3.4)(64bit)  
libc.so.6(GLIBC_2.4)(64bit)  
libfuse.so.2()(64bit)  
libfuse.so.2(FUSE_2.2)(64bit)  
libfuse.so.2(FUSE_2.6)(64bit)  
libgcrypt.so.11()(64bit)  
libgmp.so.3()(64bit)  
libncurses.so.5()(64bit)  
libpthread.so.0()(64bit)  
libpthread.so.0(GLIBC_2.2.5)(64bit)  
libpthread.so.0(GLIBC_2.3.2)(64bit)  
libreadline.so.5()(64bit)  
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rtld(GNU_HASH)

Thanks for the review, Stepan.

(In reply to comment #5)
> FAIL Summary for %package -n fuse-afs is wrong

Fixed.

> FAIL no duplicates in %files.
>  #fedora-devel 17:11 < tibbs|h> You can include COPYING exactly once.
>    Do not duplicate doc files, remove %doc lines from subpackages.

I removed it from -devel since it drags in the main package, but did not remove it from the fuse driver. Documentation files are here on purpose, and I don't see why shouldn't they be duplicated (coincidentally, I asked on #fedora-devel too, and got a response COPYING should be in each subpackage). Do you strongly object this?

> FAIL Please use %configure --disable-static and drop %exclude *.a

Done, thanks for noticing.

SPECS: http://v3.sk/~lkundrak/SPECS/afpfs-ng.spec
SRPMS: http://v3.sk/~lkundrak/SRPMS/afpfs-ng-0.8.1-2.fc11.src.rpm

please add ownership of directory /usr/include/afpsa-ng to devel package
 %defattr(-,root,root,-)
+%dir %{_includedir}/afpfs-ng
 %{_includedir}/afpfs-ng

> (In reply to comment #6)
> I removed it from -devel [...], but did not remove
> it from the fuse driver.  [...] Do you strongly
> object this?

Who am I to disagree?  I would not care at all.  But I follow a process to follow a guidelines set up by a comittee.

Actually, I was not the only one who were not able to decipher the terse sentence about duplicate files.  And there seems to be no clarification available:
http://thread.gmane.org/gmane.linux.redhat.fedora.extras.packaging/5425/focus=5435

> (coincidentally, I asked on #fedora-devel
> too, and got a response COPYING should be in each subpackage).

The thread above doesn't seem to indicate that.  It seems clear that if the subpackage requires the main one, the licence text shall go only to the main one.

<offtopic>
While talking about it, it seems crazy that the doc dir is named after the subpackage, not after the main package.  I would prefer using /usr/share/doc/<srcname>, without the version number.</offtopic>

> Thanks for the review, Stepan.

Thanks to you, Ľubo, for the packaging, finxing and mind-reading (you managed to fix a bug with #includes before JFCh asked for it.  ;-)

APPROVED

(In reply to comment #7)
> please add ownership of directory /usr/include/afpsa-ng to devel package
>  %defattr(-,root,root,-)
> +%dir %{_includedir}/afpfs-ng
>  %{_includedir}/afpfs-ng  

Hi Jan,
sorry but this is a misunderstanding.  The current spec means that the dir and all its contents gets packed.  With your change, the dir itself would get packed twice.

An alternate form would be:
%dir %{_includedir}/afpfs-ng
%{_includedir}/afpfs-ng/*.h
This way, rpm would complain if the directory contained anything but the *.h files.

But I'm perfectly happy with the current form as well.
Stepan

Thanks for the review Stepan!

New Package CVS Request
=======================
Package Name: afpfs-ng
Short Description: Apple Filing Protocol client
Owners: lkundrak jfch2222
Branches: EL-5 F-10

(In reply to comment #2)
> While using this myself for some time I've found experience with this tool
> quite painful -- feels very "Alpha" and even experienced a couple of
> server-triggered crashes, which may be exploitable.
> 
> Unless I'm only person using this I'm willing to work on this and improve it,
> otherwise I won't let it enter Fedora and will close this in some time.
> 
> So please review this only if you're interested in using it :)  

I'm the author of afpfs-ng, and there definitely are stability problems with it.  I am currently working on these bugs (mostly related to making it thread safe). 

That said, please report your afpfs-ng bugs so I can ensure that they are fixed.

I'll leave it up to you to decide if you want to include it in FC10, but everyone will be happier with an upcoming release of afpfs-ng.

- Alex

(In reply to comment #11)
> I'm the author of afpfs-ng, and there definitely are stability problems with
> it.  I am currently working on these bugs (mostly related to making it thread
> safe).
> 
> That said, please report your afpfs-ng bugs so I can ensure that they are
> fixed.

I'm having trouble capturing useful debugging information, since the problems I've encountered seem to be very racy. What I've been able to track down is included in the patch that's in the package (I think I've already mailed it to you some time ago).

> I'll leave it up to you to decide if you want to include it in FC10, but
> everyone will be happier with an upcoming release of afpfs-ng.

Is there a timeframe? In any case, an update can be pushed.

(In reply to comment #12)

> I'm having trouble capturing useful debugging information, since the problems
> I've encountered seem to be very racy. What I've been able to track down is
> included in the patch that's in the package (I think I've already mailed it to
> you some time ago).

I had a look through my mail and couldn't find anything from you, but I went through and added the overflow patch to my tree.

> > I'll leave it up to you to decide if you want to include it in FC10, but
> > everyone will be happier with an upcoming release of afpfs-ng.
> 
> Is there a timeframe? In any case, an update can be pushed.  

Probably a month or two.

Thanks!

- Alex

cvs done.

Thank you all.
Imported and built.