Bug 530288 - Review Request: stonevpn - easy OpenVPN certificate and configuration management
Summary: Review Request: stonevpn - easy OpenVPN certificate and configuration management
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-10-22 07:58 UTC by Leon Keijser
Modified: 2010-02-19 21:47 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-11-17 16:01:25 UTC
Type: ---
Embargoed:
mtasaka: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)
updated SPEC file (2.25 KB, text/plain)
2009-11-06 10:15 UTC, Leon Keijser
no flags Details

Description Leon Keijser 2009-10-22 07:58:47 UTC
Spec URL: http://cloud.github.com/downloads/lkeijser/stonevpn/stonevpn.spec
SRPM URL: http://cloud.github.com/downloads/lkeijser/stonevpn/stonevpn-0.4.3-0.fc11.src.rpm
Description: 
StoneVPN allows you to manage OpenVPN certificates and create
configurations for Windows and Linux machines based on a
template. It can package everything into a zipfile and mail
it to a user.

This is my first package for Fedora and as such, i am in need of a sponsor.

Comment 1 Mamoru TASAKA 2009-11-06 08:07:56 UTC
Some notes:

* EVR (Epoch-Version-Release)
  - For released tarball, the version tag should begin with
    1%{?dist} (not 0), should be incremented every time you change your
    spec file, and be reset to 1%{?dist} when version is upgraded:
    https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Package_Release

* License tag
  - The license tag for this package should be "GPLv2+".
    Ref:
    https://fedoraproject.org/wiki/Licensing/FAQ#How_do_I_figure_out_what_version_of_the_GPL.2FLGPL_my_package_is_under.3F

* SourceURL
  - For sourceforge hosted tarball, follow:
    https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net

* Seemingly unneeded files
-----------------------------------------------------------
/usr/share/StoneVPN/COPYING
/usr/share/StoneVPN/Changelog
/usr/share/StoneVPN/README
/usr/share/StoneVPN/TODO
-----------------------------------------------------------
  - These files are also installed under /usr/share/doc/stonevpn-0.4.3
-----------------------------------------------------------
/usr/share/StoneVPN/patches/pyOpenSSL-0.9-crl_and_revoked.patch
/usr/share/StoneVPN/rpm/stonevpn.spec
-----------------------------------------------------------
  - Are these files really needed?

* Usability
-----------------------------------------------------------
[tasaka1@localhost ~]$ stonevpn 
File /etc/stonevpn.conf does not exist!
-----------------------------------------------------------
  - Please install /etc/stonevpn. Note that we usually expect that the 
    installed application works as it is.

Comment 2 Leon Keijser 2009-11-06 10:15:13 UTC
Created attachment 367803 [details]
updated SPEC file

Updated SPEC file according to comment #2

Comment 3 Leon Keijser 2009-11-06 10:22:21 UTC
Hi Mamoru, thanks for your notes. My comments are below, prefixed with '+':

* EVR (Epoch-Version-Release)
+ fixed

* License tag
+ fixed

* SourceURL
+ fixed. Entire SourceURL pointed to the wrong address.

* Seemingly unneeded files
+ fixed: i've removed them. Although now rpmlint complains about missing documentation. Shouldn't i at least include the README file?

* Usability
+ fixed by adding an {__install} method to %install and including it in %files as %config(noreplace) %{_sysconfdir}/%{name}.conf 


Updated SPEC file is attached to this bug report (since github fails to recognize it as a new file and stupidly shows the older version).
Updated SRPM: http://cloud.github.com/downloads/lkeijser/stonevpn/stonevpn-0.4.3-1.fc11.src.rpm

Thanks!

Comment 4 Mamoru TASAKA 2009-11-06 16:14:11 UTC
Well, what I meant is that for me the following 6 files
---------------------------------------------------------
/usr/share/StoneVPN/COPYING
/usr/share/StoneVPN/Changelog
/usr/share/StoneVPN/README
/usr/share/StoneVPN/TODO
/usr/share/StoneVPN/patches/pyOpenSSL-0.9-crl_and_revoked.patch
/usr/share/StoneVPN/rpm/stonevpn.spec
---------------------------------------------------------
seems unneeded (and should be removed), because
- The former 4 files are also installed (on -0 spec file) under
  /usr/share/doc/stonevpn-0.4.3 via %doc (i.e. what I am saying is
  that while /usr/share/doc/stonevpn-0.4.3/COPYING should be kept,
  /usr/share/StoneVPN/COPYING or so seems unneeded)
- Currently I don't understand why a patch or a rpm spec file
  have to be installed under /usr/share/StoneVPN .

All these 6 files are still installed with -1 srpm.

Comment 5 Leon Keijser 2009-11-06 16:57:18 UTC
You're right and the setup.py installed these in /usr/share/StoneVPN. Since i am the author of stonevpn i modified setup.py so the documentation files, patches and spec file are not installed but kept in the tarball only. 

I have updated the spec file (github _does_ work, although it takes some time for the changes to take effect) and SRPM.

SPEC: http://cloud.github.com/downloads/lkeijser/stonevpn/stonevpn.spec
SRPM: http://cloud.github.com/downloads/lkeijser/stonevpn/stonevpn-0.4.3-2.fc11.src.rpm

Since the %doc section now includes the files again, rpmlint showed no warning/errors anymore. Files that are now installed are:

/etc/stonevpn.conf
/usr/bin/stonevpn
/usr/lib/python2.6/site-packages/StoneVPN
/usr/lib/python2.6/site-packages/StoneVPN/__init__.py
/usr/lib/python2.6/site-packages/StoneVPN/__init__.pyc
/usr/lib/python2.6/site-packages/StoneVPN/__init__.pyo
/usr/lib/python2.6/site-packages/StoneVPN/app.py
/usr/lib/python2.6/site-packages/StoneVPN/app.pyc
/usr/lib/python2.6/site-packages/StoneVPN/app.pyo
/usr/lib/python2.6/site-packages/stonevpn-0.4.3-py2.6.egg-info
/usr/share/doc/stonevpn-0.4.3
/usr/share/doc/stonevpn-0.4.3/COPYING
/usr/share/doc/stonevpn-0.4.3/Changelog
/usr/share/doc/stonevpn-0.4.3/README
/usr/share/doc/stonevpn-0.4.3/TODO

Comment 6 Mamoru TASAKA 2009-11-06 18:10:17 UTC
Some questions:
-------------------------------------------------------------------
29147 2009-10-22 16:50 stonevpn-0.4.3-0.fc11.src/stonevpn-0.4.3.tar.gz
29320 2009-11-07 01:41 stonevpn-0.4.3-2.fc11.src/stonevpn-0.4.3.tar.gz
-------------------------------------------------------------------
- Source tarball changed. Would you explain what happened?
  We expect that if a static URL is used as Source0, its source archive
  does not change (as long as %version or so does not change).

  If the tarball is not the one formally released as "0.4.3" but
  one created from git, please follow:
  https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Post-Release_packages
  https://fedoraproject.org/wiki/Packaging/SourceURL#Using_Revision_Control
  i.e.
  - Include the revision string of git tree or the date when the tarball
    was created to the tarball name and the release number of the srpm.

- Would you have any plan to submit your patch 
  "pyOpenSSL-0.9-crl_and_revoked.patch" to pyOpenSSL upstream?
  * At least I guess Fedora's pyOpenSSL won't accept this patch
    unless pyOpenSSL upstream accepts it and if you want to support
    CRL with this software on Fedora submitting your patch to
    pyOpenSSL upstream is really needed:
  https://fedoraproject.org/wiki/PackageMaintainers/WhyUpstream

Comment 7 Leon Keijser 2009-11-09 13:22:19 UTC
- Source tarball changed. Would you explain what happened?
+ Yes, i accidentally uploaded a modified version under the same name. Stupid mistake. Since there were several other outstanding changes, i made a new version (0.4.4) and created an updated SPEC/SRPM/tarball.

- Would you have any plan to submit your patch
+ It's not my patch, but from Rick Dean. It's already proposed for merge into main. See https://bugs.launchpad.net/pyopenssl/+bug/404436  

Updated SPEC: http://cloud.github.com/downloads/lkeijser/stonevpn/stonevpn.spec
Updated SRPM: http://cloud.github.com/downloads/lkeijser/stonevpn/stonevpn-0.4.4-1.fc11.src.rpm

Comment 8 Mamoru TASAKA 2009-11-09 17:49:09 UTC
Well, I think that latest srpm is okay, so:

-------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few work. 
But before I accept this package, someone (I am a candidate) 
must sponsor you.

Once you are sponsored, you have the right to review other 
submitters' review requests and approve the packages formally. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines" as is described
on :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other 
person's review request, please write the bug number on this bug report 
so that I can check your comments or review request.

Fedora package collection review requests which are waiting for someone to
review can be checked on:
http://fedoraproject.org/PackageReviewStatus/NEW.html
(NOTE: please don't choose "Merge Review")


Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
------------------------------------------------------------

It seems that you are the upstream of this software, however
still I want to see at least one pre-review (by you) or another
review request.

Comment 9 Leon Keijser 2009-11-10 08:52:52 UTC
Okay, thanks. I will do so as soon as time allows.

Comment 10 Leon Keijser 2009-11-10 14:07:06 UTC
Created a pre-review: https://bugzilla.redhat.com/show_bug.cgi?id=494292

Comment 11 Leon Keijser 2009-11-12 18:14:21 UTC
Btw, i thought it would be nice if i added some manpages to it. Do you think this would be good to have in Fedora or does the --help function + README suffice? I've added stonevpn (1) and stonevpn.conf (5).

Comment 12 Mamoru TASAKA 2009-11-13 17:22:43 UTC
Well, I will approve this package.

-------------------------------------------------------------
   This package (stonevpn) is APPROVED by mtasaka
-------------------------------------------------------------

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Install the Client Tools (Koji)".

Now I am sponsoring you.

If you want to import this package into Fedora 11/12, you also have
to look at
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
(after once you rebuilt this package on koji Fedora rebuilding system).

If you have questions, please ask me.

Removing NEEDSPONSOR.


(In reply to comment #11)
> Btw, i thought it would be nice if i added some manpages to it. Do you think
> this would be good to have in Fedora or does the --help function + README
> suffice? I've added stonevpn (1) and stonevpn.conf (5). 
Adding man files is not mandatory, however adding useful documents
are always recommended.

Comment 13 Leon Keijser 2009-11-15 19:52:32 UTC
New Package CVS Request
=======================
Package Name: stonevpn
Short Description: Easy OpenVPN certificate and configuration management
Owners: leon
Branches: F-11 F-12 EL-5
InitialCC:

Comment 14 Jason Tibbitts 2009-11-16 17:45:49 UTC
CVS done.

Comment 15 Leon Keijser 2009-11-17 11:05:06 UTC
One question though: i'm building the package now through koji for F-11, F-12 and rawhide. However, when i want to build for EL-5 (epel), it fails on a missing egg-info file that probably won't be installed in epel but is listed in the %files section of the spec file. Now i could make an if-then statement that checks for the distro and only includes the egg-info file for F-* but what is the proper way?

Comment 16 Mamoru TASAKA 2009-11-17 15:54:07 UTC
(In reply to comment #15)
You can use %if macro, or you can modify EL-5 spec file only if you
don't want to use %if macro. You can do either way.

Comment 17 Mamoru TASAKA 2009-11-17 16:01:25 UTC
Now closing.

Comment 18 Leon Keijser 2010-02-19 21:26:39 UTC
Package Change Request
======================
Package Name: stonevpn
New Branches: F-13
Owners: leon

Comment 19 Jason Tibbitts 2010-02-19 21:31:34 UTC
An F-13 branch already exists for this package.

Comment 20 Leon Keijser 2010-02-19 21:47:36 UTC
I didn't know a new branch would be added automatically. Re-read the CVS FAQ and 'cvs update -d' did the trick.


Note You need to log in before you can comment on or make changes to this bug.