The private-shared-object-provides can be easily filtered out, see bug #554464 as a reference.

What's a 'pure-Python implementation' for you/upstream?
For me it's a library, written in plain python without any C libraries in it. Am I alonw with this?

Could you please add a %check section and then delete the (then not needed anymore) test folder?
(Currently are 2 tests failing thought...)

(In reply to comment #1)
> The private-shared-object-provides can be easily filtered out, see bug #554464
> as a reference.

fixed
 
> What's a 'pure-Python implementation' for you/upstream?
> For me it's a library, written in plain python without any C libraries in it.
> Am I alonw with this?

The summary is changed now.

> Could you please add a %check section and then delete the (then not needed
> anymore) test folder?
> (Currently are 2 tests failing thought...)

%check section added and the two failing tests commented out.  I will report the issue with the tests upstream.   

Here are the updated files:
Spec URL: http://fab.fedorapeople.org/packages/SRPMS/python-dulwich.spec
SRPM URL:
http://fab.fedorapeople.org/packages/SRPMS/python-dulwich-0.6.0-2.fc13.src.rpm

Here is the full review for the latest package:

* rpmlint: TODO (minor)
rpmlint RPMS/i686/python-dulwich-* SRPMS/python-dulwich-0.6.0-2.fc12.src.rpm SPECS/python-dulwich.spec
 python-dulwich.i686: W: spurious-executable-perm /usr/share/doc/python-dulwich-0.6.0/docs/tutorial/test.py
 python-dulwich.i686: W: no-manual-page-for-binary dul-web
 python-dulwich.i686: W: no-manual-page-for-binary dul-daemon
 python-dulwich.i686: W: no-manual-page-for-binary dulwich

In general the rpmlint output is OK, probably it would be better to remove the +x permission from test.py just to keep rpmlint silent. It should be no problem just to use "python test.py" whenever someone tries out the example from the tutorial.

* naming: TODO
- name does not match upstream: Is there a specific reason why the name was altered to python-dulwich instead of using plain dulwich?
- spec file name matches package name

* License: OK
- license file packaged
- GPLv2+ acceptable
- license matches sources

* specfile in American English and legible: OK

* %description: OK

* Sources: OK
- Source0 URL ok
- spectool -g python-dulwich.spec works
- sources matches upstream - md5sum:
ea3ed7198ce154cf05784a3f75b4013f  dulwich-0.6.0.tar.gz

* Python requirements: 
- BR: python-devel: OK
- set python_sitearch when including arch-specific libraries: OK

* Compilation: TODO
- package does not build in koji
- you have to add python-nose as BR for the tests

* debuginfo sub-package: OK
- non-empty

* BuildRequires: TODO (see Compilation)

* Requires: TODO
- the used command for filtering out unwanted dependencies will probably remove too many provides/requires ("grep -v %{srcname}")
- something like "grep -v -E '(_objects.so|_pack.so)' should be better

* Locales handling: OK (n/a)

* shared/static libs, pkgconfig/header/*.la files: OK (n/a)

* packages must own all directories: OK

* files not listed twice: OK

* permissions of files: OK
- %defattr used
- final file permissions OK

* %clean section: OK

* macro usage: OK

* code vs. content: OK (no content)

* large documentation into subpackage: OK (n/a)

* GUI application needs %{name}.desktop: OK (n/a)

* no directories owned which are already owned by other packages: OK

* rm -rf %{buildroot} at the beginning of %{install}: OK

* all file names UTF8: OK

* functional test: ??
- dulwich itself works fine
- dul-daemon works for some local git clones, but does not for others (I could not clone from them.)
- dul-web did not work for me
Since the client functionality seems to work quite ok, I think these bugs should not block the review. However it would be good if you could report them upstream - with luck upstream can provide some bug fixes short-term.

* Scriptlets: OK (n/a)

Please disregard my comment about the package name. python-dulwich is perfectly fine according to http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Addon_Packages_.28python_modules.29 .

(In reply to comment #3)
> Here is the full review for the latest package:
> 
> * rpmlint: TODO (minor)
> In general the rpmlint output is OK, probably it would be better to remove the
> +x permission from test.py just to keep rpmlint silent. It should be no problem
> just to use "python test.py" whenever someone tries out the example from the
> tutorial.

Exec permission removed
 
> * Compilation: TODO
> - package does not build in koji
> - you have to add python-nose as BR for the tests

Added, package builds in koji
http://koji.fedoraproject.org/koji/taskinfo?taskID=2292934
 
> * Requires: TODO
> - the used command for filtering out unwanted dependencies will probably remove
> too many provides/requires ("grep -v %{srcname}")
> - something like "grep -v -E '(_objects.so|_pack.so)' should be better

Changed

> * functional test: ??
> - dulwich itself works fine

Yepp, it's one of the only working features.

> - dul-daemon works for some local git clones, but does not for others (I could
> not clone from them.)
> - dul-web did not work for me

dul-* seams partially to work but they are not really useful because they lacks to almost every functionality. 

> Since the client functionality seems to work quite ok, I think these bugs
> should not block the review. However it would be good if you could report them
> upstream - with luck upstream can provide some bug fixes short-term.
 
Thanks for the review.

Here are the updated files:
Spec URL: http://fab.fedorapeople.org/packages/SRPMS/python-dulwich.spec
SRPM URL:
http://fab.fedorapeople.org/packages/SRPMS/python-dulwich-0.6.0-3.fc13.src.rpm

Thanks for the new package. Everything is fixed now besides the regexp to filter out the provides/requires:

The used syntax doesn't work:
$ echo "_objects.so"| grep -v -e '_objects.so|_pack.so'
_objects.so
$

It is necessary to use "-E" for "extended regexp":
$ echo "_objects.so"| grep -v -E '_objects.so|_pack.so'
$

If you change the package for this, please just remove also the commented line with "##grep..." in the %build section. ;-)

FWIW the reason Dulwich' description mentions it being pure Python is because the extensions are optional; they are not necessary for Dulwich to work, though they will make it a fair bit faster.

Christian , thanks for the fix.

(In reply to comment #6)
> The used syntax doesn't work:
> $ echo "_objects.so"| grep -v -e '_objects.so|_pack.so'
> _objects.so
> $
> 
> It is necessary to use "-E" for "extended regexp":
> $ echo "_objects.so"| grep -v -E '_objects.so|_pack.so'
> $

* Wed Sep 01 2010 Fabian Affolter <fabian> - 0.6.1-1
- Fixed grep parameter
- Run all test now
- Updated to new upstream version 0.6.1

Here are the updated files:
Spec URL: http://fab.fedorapeople.org/packages/SRPMS/python-dulwich.spec
SRPM URL: http://fab.fedorapeople.org/packages/SRPMS/python-dulwich-0.6.1-1.fc13.src.rpm

Thanks for the new package. It looks good, unfortunately the trick with
"%global __find_provides %{_rpmconfigdir}/find-provides | grep -v -E '_objects.so|_pack.so'" 
does not work (independently of "-E"):

rpm -qp --provides RPMS/i686/python-dulwich-0.6.1-1.fc13.i686.rpm 
_objects.so  
_pack.so  
python-dulwich = 0.6.1-1.fc13
python-dulwich(x86-32) = 0.6.1-1.fc13

I have investigated the issue a little bit and so far I haven't found any references that the suggested syntax (use a pipe in the spec file) is actually used anywhere else. 

Usually it is suggested to use an external script (see http://fedoraproject.org/wiki/PackagingDrafts/FilteringAutomaticDependencies ).

I've tested the following:

- create the following file
SOURCES/dulwich-find-provides.sh:
-----------------------------------
#!/bin/sh

if [ -x /usr/lib/rpm/redhat/find-provides ] ; then
FINDPROV=/usr/lib/rpm/redhat/find-provides
else
FINDPROV=/usr/lib/rpm/find-provides
fi

$FINDPROV $* | sed -e '/\(_objects.so\|_pack.so\)/d'

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

- use the following in the spec file:
%global _use_internal_dependency_generator 0
%global __find_provides %{SOURCE1}
%global __find_requires %{_rpmconfigdir}/find-requires
[...]
%Source1:        dulwich-find-provides.sh

This will finally generate the correct requires / provides:

rpmbuild@:~$ rpm -qp --requires RPMS/i686/python-dulwich-0.6.1-1.fc12.i686.rpm 
rpmlib(PartialHardlinkSets) <= 4.0.4-1
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(CompressedFileNames) <= 3.0.4-1
/usr/bin/python  
libc.so.6  
libc.so.6(GLIBC_2.0)  
libc.so.6(GLIBC_2.1.3)  
libc.so.6(GLIBC_2.4)  
libpthread.so.0  
libpython2.6.so.1.0  
rpmlib(PayloadIsXz) <= 5.2-1
rpmbuild@:~$ rpm -qp --provides RPMS/i686/python-dulwich-0.6.1-1.fc12.i686.rpm 
python-dulwich = 0.6.1-1.fc12
python-dulwich(x86-32) = 0.6.1-1.fc12
rpmbuild@:~$ 

I'm sorry that I haven't fully tested this before...

P.S.: http://fedoraproject.org/wiki/Common_Rpmlint_issues#private-shared-object-provides should be fixed as well - I'll write tomspur a separate mail. ;-)

(In reply to comment #10)
> Thanks for the new package. It looks good, unfortunately the trick with
> "%global __find_provides %{_rpmconfigdir}/find-provides | grep -v -E
> '_objects.so|_pack.so'" 
> does not work (independently of "-E"):
[snip]
> Usually it is suggested to use an external script (see
> http://fedoraproject.org/wiki/PackagingDrafts/FilteringAutomaticDependencies ).
[snip]
> http://fedoraproject.org/wiki/Common_Rpmlint_issues#private-shared-object-provides
> should be fixed as well - I'll write tomspur a separate mail. ;-)

I just found a far better way to solve this.
There are some macros defined in redhat-rpm-config, see:
http://fedorapeople.org/~cweyl/macros.filtering

With that you can simply do:
%filter_provides_in %{python_sitearch}
%filter_setup

Which works like a charm!
I'll update the wiki to suggest using that instead, because we haven't automatic provides for python (yet), so we can exclude the pythonpath completely.
(Might change sometime.)

Because this applies to ALL python-non-noarch packages it would be great if this would be defined somewhere in the python macros.
This way, no python package needs to do that sort of filtering anymore.

(CC'ing Toshio and David for opionions)

Thank you all again for all the help with this package.

(In reply to comment #11)
> I just found a far better way to solve this.
> There are some macros defined in redhat-rpm-config, see:
> http://fedorapeople.org/~cweyl/macros.filtering
> 
> With that you can simply do:
> %filter_provides_in %{python_sitearch}
> %filter_setup

This seems to work

[fab@localhost repos]$ rpm -qp --provides /home/fab/rpmbuild/RPMS/x86_64/python-dulwich-0.6.2-1.fc14.x86_64.rpm 
python-dulwich = 0.6.2-1.fc14
python-dulwich(x86-64) = 0.6.2-1.fc14

Here are the updated files:
Spec URL: http://fab.fedorapeople.org/packages/SRPMS/python-dulwich.spec
SRPM URL:
http://fab.fedorapeople.org/packages/SRPMS/python-dulwich-0.6.2-1.fc14.src.rpm

I have tested the latest package: all reported issues including the filtering of the provides are fixed now.

-> APPROVED

New Package SCM Request
=======================
Package Name: python-dulwich
Short Description: A python implementation of the Git file formats and protocols
Owners: fab
Branches: F-14 F-13
InitialCC:

Git done (by process-git-requests).

python-dulwich-0.6.2-1.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/python-dulwich-0.6.2-1.fc13

python-dulwich-0.6.2-1.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/python-dulwich-0.6.2-1.fc14

python-dulwich-0.6.2-1.fc14 has been pushed to the Fedora 14 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update python-dulwich'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/python-dulwich-0.6.2-1.fc14

Package Change Request
======================
Package Name: python-dulwich
New Branches: el5 el6
Owners: fab

Git done (by process-git-requests).

python-dulwich-0.6.2-1.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

python-dulwich-0.6.2-1.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.