Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=435842
Summary: Review Request: viewmtn - Web interface for Monotone version control system Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: roland@redhat.com QAContact: extras-qa@fedoraproject.org CC: fedora-package-review@redhat.com,notting@redhat.com
Spec URL: http://people.redhat.com/roland/tmp/viewmtn.spec SRPM URL: http://people.redhat.com/roland/tmp/viewmtn-0.10-1.fc9.src.rpm Description: ViewMTN is a web interface to the Monotone distributed version control system. It aims to provide a convenient and useful web interface to Monotone. If you've used interfaces to other version control systems, ViewMTN will be immediately familiar.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: viewmtn - Web interface for Monotone version control system
https://bugzilla.redhat.com/show_bug.cgi?id=435842
thomas.moschny@gmx.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |thomas.moschny@gmx.de
------- Additional Comments From thomas.moschny@gmx.de 2008-03-13 06:56 EST ------- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: viewmtn - Web interface for Monotone version control system
https://bugzilla.redhat.com/show_bug.cgi?id=435842
------- Additional Comments From roland@redhat.com 2008-03-13 20:48 EST ------- - 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: viewmtn - Web interface for Monotone version control system
https://bugzilla.redhat.com/show_bug.cgi?id=435842
------- Additional Comments From thomas.moschny@gmx.de 2008-03-14 05:16 EST ------- (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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: viewmtn - Web interface for Monotone version control system
https://bugzilla.redhat.com/show_bug.cgi?id=435842
------- Additional Comments From roland@redhat.com 2008-03-14 14:21 EST ------- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: viewmtn - Web interface for Monotone version control system
https://bugzilla.redhat.com/show_bug.cgi?id=435842
------- Additional Comments From thomas.moschny@gmx.de 2008-03-14 15:17 EST ------- (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-e1c5548cbbe551c7a43d... 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: viewmtn - Web interface for Monotone version control system
https://bugzilla.redhat.com/show_bug.cgi?id=435842
------- Additional Comments From thomas.moschny@gmx.de 2008-03-14 15:23 EST ------- By the way, this section is also very clear about config files: http://fedoraproject.org/wiki/Packaging/Guidelines#head-72ae42b20a4c2896c9e7...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: viewmtn - Web interface for Monotone version control system
https://bugzilla.redhat.com/show_bug.cgi?id=435842
------- Additional Comments From roland@redhat.com 2008-03-14 15:46 EST ------- /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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: viewmtn - Web interface for Monotone version control system
https://bugzilla.redhat.com/show_bug.cgi?id=435842
------- Additional Comments From thomas.moschny@gmx.de 2008-03-14 20:12 EST ------- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: viewmtn - Web interface for Monotone version control system
https://bugzilla.redhat.com/show_bug.cgi?id=435842
------- Additional Comments From roland@redhat.com 2008-03-14 21:27 EST ------- I've updated the package following your example. Thanks for the pointer.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: viewmtn - Web interface for Monotone version control system
https://bugzilla.redhat.com/show_bug.cgi?id=435842
thomas.moschny@gmx.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-review+
------- Additional Comments From thomas.moschny@gmx.de 2008-03-21 12:34 EST ------- [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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: viewmtn - Web interface for Monotone version control system
https://bugzilla.redhat.com/show_bug.cgi?id=435842
thomas.moschny@gmx.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: viewmtn - Web interface for Monotone version control system
https://bugzilla.redhat.com/show_bug.cgi?id=435842
------- Additional Comments From wolfy@nobugconsulting.ro 2008-03-21 15:22 EST ------- Thomas, you should assign the bug to yourself if you do the review...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: viewmtn - Web interface for Monotone version control system
https://bugzilla.redhat.com/show_bug.cgi?id=435842
roland@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
------- Additional Comments From roland@redhat.com 2008-03-22 15:52 EST ------- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: viewmtn - Web interface for Monotone version control system
https://bugzilla.redhat.com/show_bug.cgi?id=435842
kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|nobody@fedoraproject.org |thomas.moschny@gmx.de Flag|fedora-cvs? |fedora-cvs+
------- Additional Comments From kevin@tummy.com 2008-03-22 21:12 EST ------- cvs done.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: viewmtn - Web interface for Monotone version control system
https://bugzilla.redhat.com/show_bug.cgi?id=435842
roland@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |WORKSFORME
------- Additional Comments From roland@redhat.com 2008-03-23 17:33 EST ------- go
package-review@lists.fedoraproject.org