Some remarks:

- There's a typo in viemtn/__init__.py, generated from the specfile:
  "from viewmtn import assemble_urls, web" instead of
  "from import viewmtn assemble_urls, web".

- Why is viewmtn/__init__.py generated in the specfile? Could be a %source.

- Shouldn't the symlink logic for the user_config not be just the other way
  round? 

  /usr/lib/python2.5/site-packages/viewmtn/user_config.py -> /etc/viewmtn.conf.py
  instead of
  /etc/viewmtn.conf.py -> /usr/lib/python2.5/site-packages/viewmtn/user_config.py

  This way, one could customize the db location via editing the file in /etc,
  which currently doesn't work.
  
- What about interoperability with the 'monotone-server packet?' Would be
  really nice if displaying the db from that package would work out of the
  box. (Would need changing/relaxing the permissions of the server db a bit.)

- Maybe webpy should go into its own package and viewmtn depend on that.

- The inevitable rpmlint output:

viewmtn.noarch: E: 
non-executable-script /usr/lib/python2.5/site-packages/viewmtn/web/__init__.py 
0644
viewmtn.noarch: E: 
non-executable-script /usr/lib/python2.5/site-packages/viewmtn/viewmtn.py 0644
viewmtn.noarch: E: non-standard-uid /var/cache/viewmtn-graph apache
viewmtn.noarch: E: non-standard-gid /var/cache/viewmtn-graph apache
viewmtn.noarch: E: 
non-executable-script /usr/lib/python2.5/site-packages/viewmtn/fdo/xdgbasedir.py 
0644
viewmtn.noarch: E: 
non-executable-script /usr/lib/python2.5/site-packages/viewmtn/mk2.py 0644
viewmtn.noarch: W: 
symlink-should-be-relative /etc/viewmtn.conf.py /usr/lib/python2.5/site-packages/viewmtn/user_config.py
viewmtn.noarch: E: 
non-executable-script /usr/lib/python2.5/site-packages/viewmtn/fdo/sharedmimeinfo.py 
0644
viewmtn.noarch: E: 
non-executable-script /usr/lib/python2.5/site-packages/viewmtn/fdo/icontheme.py 
0644
viewmtn.noarch: E: 
non-executable-script /usr/lib/python2.5/site-packages/viewmtn/tests.py 0644
viewmtn.noarch: E: 
non-executable-script /usr/lib/python2.5/site-packages/viewmtn/genproxy.py 
0644

- typo fixed

- It's 3 lines and extra cvs hooey for a separate file is more trouble than
help.  Mainly it was just because I kept having to change the plan for how that
loading would work.

- I would have thought that too, but I'm following the example of mailman
mm_cfg.py, which is just the same sort of case.

- monotone's poor support for db file sharing makes it extremely hairy to make
anything work out of the box.  The sample configuration in the viewmtn rpm works
for ssh-based db serving as deployed on fedorahosted.org now.  I want to get
viewmtn packaged and running before we try to solve every problem.

- Slicing up the upstream sources to separate webpy is outside the scope of the
packaging effort.  I don't think it's used unmodified, and I'm sure that version
drift will be a big problem if we tried to maintain it separately.

- The #! complaints are stupid, but I worked around them since it would be more
stupid to waste one's life arguing with rpmlint authors.

I don't see what different would be the proper way to handle uid/gid.
The directory has to be owned by the uid/gid httpd runs as.
The "Requires(pre):  httpd" line should make sure it exists before install.

New version of spec and src.rpm in the same place.

(In reply to comment #2)
> - I would have thought that too, but I'm following the example of mailman
> mm_cfg.py, which is just the same sort of case.

The mailman package doing it this way doesn't mean it's correct or even useful 
here. 

With the current setup, ViewMTN doesn't care at all about a config file 
in /etc, so why should the symlink be there at all? Solely as a hint, where to 
look for the real config file? I don't think so.

So, inverting the symlink direction is in fact not enough, but additionally, 
your sniplet added at the end of config.py, should import the file from /etc 
and not from %{python_sitelib}/viewmtn.

Again, I consider it a packaging bug if a file under /usr has to be edited in 
order to configure the package, at least I understand the FHS this way. 
Additionally, because %{python_sitelib}/viewmtn/user_config.py is not 
marked %config, changes made there are easily overwritten during a package 
update.

> - monotone's poor support for db file sharing makes it extremely hairy to
> make anything work out of the box.  The sample configuration in the
> viewmtn rpm works for ssh-based db serving as deployed on fedorahosted.org
> now.  I want to get viewmtn packaged and running before we try to solve
> every problem.

It didn't affect usage on fedorahosted.org, if, in addition 
to /srv/mtn/*/db.mtn, /var/lib/monotone/server.mtn, would be considered for 
the list of dbs to be served, does it? Thought ViewMTN tried to work around 
the sharing problems (in a limited fashion) by restarting the mtn process in 
case of any problem. Otoh, this is not a blocker issue.

I'm not asserting opinions, nor am I any python expert.  I'm just trying to
follow Fedora packaging guidelines.  With the sense of the symlink inverted, I
cannot get the package to build because of unpackaged /etc/*.py[oc] files. 
(Please try it yourself with koji build --scratch.)  As I understand it, the
role of the reviewer is to enforce Fedora packaging policies, not to pick them.
 Please refer to python packaging guidelines that say exactly what I should do,
and have those guidelines changed if you disagree with them.  I could not find
anything clear about a case like this, so the existing Fedora example, and the
facts about what works at all, are the guide.

The database sharing really is not a simple problem, I told you the truth.  If
any obvious suggestion were actually viable, I would already be doing it.
Please do not hold this review hostage to that can of worms.

If you have improvements to suggest, feel free to send concrete .spec patches
that you have tested yourself.  If the improvements you advocate are not
mandated by Fedora packaging policies, then please save them for proper bug
reports once the package exists in Fedora, and do not conflate opinions and
desires for the package's details with a Fedora packaging policy conformance review.

(In reply to comment #4)
> I'm just trying to follow Fedora packaging guidelines.

Which clearly say, that FHS is to follow:
http://fedoraproject.org/wiki/Packaging/Guidelines#head-e1c5548cbbe551c7a43d375c524ab2ea0188557e
and any deviation from that should be rationalized in the review process. 
That's all I'm trying to do.

> With the sense of the symlink inverted, I cannot get the package to build
> because of unpackaged /etc/*.py[oc] files.

You have a point here. We probably should bug upstream (later, that is), to 
find a cleaner solution for configuring ViewMTN. I will ask Grahame.

But you didn't answer my two questions:
- What is the purpose of the symlink in /etc?
- How do you prevent the config file %{python_sitelib}/viewmtn/user_config.py
  from being overwritten during an update?

> The database sharing really is not a simple problem, I told you the truth.

Already said that I'm not insisting on that.

By the way, this section is also very clear about config files:
http://fedoraproject.org/wiki/Packaging/Guidelines#head-72ae42b20a4c2896c9e7e295b94cfc26bf580bef

/etc is where people look for configuration files.  I'm still following the
example of mailman here.

I added %config(noreplace), which I had overlooked before.

Please give concrete changes you want made, not a quiz.

Have a look at the smolt package, this also has a python config file in /etc, 
but in a subdir. Here's how I think it could look like for viewmtn:

http://thm.fedorapeople.org/viewmtn.spec
http://thm.fedorapeople.org/viewmtn-0.10-2.fc8.src.rpm

Builds fine in koji, see 
http://koji.fedoraproject.org/koji/taskinfo?taskID=517435.

I've updated the package following your example.  Thanks for the pointer.

[x],[~] = ok, [!] = problem, [-] = not applicable

[x] package meets naming guidelines
[x] specfile is encoded in ascii or utf-8
[x] specfile matches base package name
[x] specfile uses macros consistently
[x] specfile is written cleanly
[x] specfile is written in AE
[x] changelog is present and has correct format
[x] license matches actual license
[x] license is open source-compatible
[x] license text is included in package
[x] source tag has correct url
[x] source files match upstream
    md5sum: 1758f8b6340cca7d191b21fc25daf283
[x] latest version is packaged
[x] summary is concise
[x] dist tag is present
[x] buildroot is correct
[x] buildroot is prepped
[x] %clean is present
[x] proper build requirements
[!] proper requirements
    python-paste doesn't seem to be needed.
[-] uses %{?_smp_mflags}
[-] uses %{optflags}
[x] doesn't use %makeinstall
[x] package builds at least on one architecture
    tested on: f8/x86_64
[x] packages installs and runs at least on one architecture
    tested on: f8/x86_64
[~] rpmlint is quiet
    viewmtn.noarch: E: non-standard-uid /var/cache/viewmtn-graph apache
    Seems necessary though to give apache write permissions to the cache.
[~] final provides/requires look sane
    see above, python-paste doesn't seem to be needed.
[-] ldconfig called in %post and %postun if required
[x] code, not content
[x] file permissions are appropriate
[-] debuginfo package looks usable
[x] config files marked as %config(noreplace)
[x] owns all the directories it creates
[-] static libraries in -devel subpackage
[-] header files in -devel subpackage
[-] development .so files in -devel subpackage
[-] pkgconfig files in -devel subpackage, requires pkgconfig
[-] no .la files
[x] doesn't need a -docs subpackage
[!] relevant docs are included
    You should include the original INSTALL file, together with a
    README.fedora file explaining how, in this package, viewmtn is
    hooked to the webserver.
[x] doc files are not needed at runtime
[-] provides a .desktop file, build-requires desktop-file-utils
[-] uses %find_lang, build-requires gettext

APPROVED.

Please fix the two (minor) issues before releasing the package.

Thomas, you should assign the bug to yourself if you do the review...

New Package CVS Request
=======================
Package Name: viewmtn
Short Description: Web interface for Monotone version control system
Owners: roland
Branches: F-8 EL-5
InitialCC: 
Cvsextras Commits: yes

cvs done.

go