Bug 240375 - [NEW PORT] www/py-isso: Commenting server similar to Disqus
Summary: [NEW PORT] www/py-isso: Commenting server similar to Disqus
Status: Open
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-ports-bugs mailing list
URL:
Keywords: feature, needs-patch, needs-qa
Depends on:
Blocks:
 
Reported: 2019-09-06 19:46 UTC by René Thümmler
Modified: 2019-09-09 08:29 UTC (History)
3 users (show)

See Also:
koobs: maintainer-feedback? (rt)


Attachments
the port directory (4.49 KB, text/plain)
2019-09-06 19:46 UTC, René Thümmler
no flags Details
the port directory (4.77 KB, text/plain)
2019-09-07 11:25 UTC, René Thümmler
no flags Details
the port directory (7.80 KB, text/plain)
2019-09-09 08:29 UTC, René Thümmler
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description René Thümmler 2019-09-06 19:46:37 UTC
Created attachment 207247 [details]
the port directory

Isso is a lightweight commenting server similar to Disqus. It allows anonymous comments, maintains identity and is simple to administrate. It uses JavaScript and cross-origin ressource sharing for easy integration into (static) websites.
Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 2019-09-06 22:47:17 UTC
Thank you for your new port submission René

There's a number of review items that need to be taken care of, so it would be great if you could run the port through QA (portlint, and poudriere in particular). 

For more details and instructions see: https://www.freebsd.org/doc/en/books/porters-handbook/testing.html

At a quick glance:

- Remove "A " from COMMENT (portlint should complaint about this)
- COMMENT for Python ports should match setup.py:description with portlint compliant changes where necessary (COMMENT=Lightweight Disqus alternative)
- Use GH_* variables (eg: GH_TAGNAME) not COMMIT_ID
- Use MASTER_SITES=CHEESESHOP unless there is a compelling temporary reason to use an alternative source (like GitHub), such as missing test or test data files.
- Add python as a secondary CATEGORIES
- NO_ARCH is out of order (put it after USES, portlint should complain about this)
- Dont hardcode paths (like /usr/local/etc), as people can change their PREFIX. Use ${ETCDIR} instead)
- cffi is missing as a BUILD_DEPENDS (setup_requires=["cffi>=1.3.0"],)
- conditional Python 2.7 dependencies are missing (':python_version=="2.7"': ['ipaddr>=2.1', 'configparser']) - Note: these should be moved in upstream code to install_requires with environment markers, they are not optional for python2.7)
- For all *_DEPENDS, match the version specifiers are closely as possible. RUN_DEPENDS currently specify >0, but setup.py says, for example:

misaka>=2.0,<3.0'
werkzeug>=0.9
ipaddr>=2.1'
Comment 2 Kubilay Kocak freebsd_committer freebsd_triage 2019-09-06 22:49:04 UTC
Also, the package has a test suite, it would be great to add a do-test target and RUN_DEPENDS, like:

RUN_DEPENDS = 4{PYTHON_PKGNAMEPREFIX}nose>0:devel/py-nose@${PY_FLAVOR}
...
do-test:
 @cd ${WRKSRC} && ${PYTHON_CMD} -m nosetests -v
Comment 3 René Thümmler 2019-09-07 11:25:33 UTC
Created attachment 207256 [details]
the port directory

Thanks Kubilay for pointing me into the right direction. I think I managed all of your points so far. 

I checked the port with portlint (looks fine.) and the service script with rclint (OK). The port was successfully build here with poudriere (testport) for the the two flavors py27 and py36.

Anything else to check on my side?
Comment 4 Kubilay Kocak freebsd_committer freebsd_triage 2019-09-07 11:52:17 UTC
(In reply to René Thümmler from comment #3)

My pleasure René, you're welcome & very nice work

A few nits:

- No need for a PORTREVISION for a new (not yet committed port)

- misaka has a max version pin, one must either declare that, or patch it out. misaka>=2.0<3.0 should work

- Upstream specifies ==2.7 as the environment marker, so match that (instead of (PYTHON_REL < 3000) where possible. In these "==" cases, one can use <flavor>_RUN_DEPENDS helpers, like: py27_RUN_DEPENDS= foo \ bar

- I'd use ${ETCDIR} rather than ${PREFIX}/etc in post-patch (it covers more of the path), unless it makes the REINPLACE search/replace 'too' challenging or less readable. Likewise for post-install (use ETCDIR)

- For sqlite3, does this actually need the "sqlite module in Python" ? If so, depend on databases/py-sqlite3 directly (we split that module out from the stdlib/language ports). If this is the case, remove sqlite:3 from USES
Comment 5 René Thümmler 2019-09-08 08:37:56 UTC
(In reply to Kubilay Kocak from comment #4)

- you are right for the sqlite3. I was not fully aware of the python port split. Fixed that locally already.

- the py27_RUN_DEPENDS= works like a charm. Was not aware of this but I am improving :)

- the misaka max version pin is fixed.

- PORTREVISION is removed already

- ${ETCDIR} vs ${PREFIX}/etc isnt easy to decide. My Problem here is ${ETCDIR} uses also the python flavor prefix which is a little problem for my service script. (I am working on making all the pathes (bin, conf etc.) configurable within rc.conf for the service script.) In order to provide the script according to ETRCDIR i would need to patch it for every flavor, or is there something like py27_USE_RC_SUBR= I could use? I d like to avoid patching around, so honestly Id prefer ${PREFIX}/etc over ${ETCDIR}. Any further ideas? Or do you know any other ports i can look into?

When I know how to handle the service script the right way with the flavors I'll upload a new (and hopefully  final) version.
Comment 6 René Thümmler 2019-09-08 19:33:50 UTC
I found another problem. After investigating the build packages it turned out some files (js files) are missing. isso has a Makefile (needed to tweak a little bit) with some targets (init, js and man). So i tried

USES=         gmake python
USE_PYTHON=   autoplist distutils
ALL_TARGET=   init man js
MAKEFILE=     ${FILESDIR}/Makefile.in

But then it looks like gmake doesnt run at all. So what is needed then is

do-build:
	@cd ${WRKSRC} && ${MAKE_CMD} -f ${MAKEFILE} init man js

to get a first package (needs more testing). That leads to the conclusion ALL_TARGET is useless here.

Is this wanted/known behaviour?
Comment 7 René Thümmler 2019-09-09 08:29:36 UTC
Created attachment 207315 [details]
the port directory

This is my final version for now.

portlint: looks fine.
rclint: no complains.
make test: OK.
poudriere is OK for py27/py36 as long as you have this options set

RESTRICT_NETWORKING=no
ALLOW_NETWORKING_PACKAGES="py-isso"
BUILD_AS_NON_ROOT=no 

During the build it downloads some npm / js stuff (i was not aware of that the first time). poudriere restricts the network access, so the port will fail. Would it be a good idea to put that as a message (i.e. pkg-message) into the port?