Bug 257496 - [NEW PORT] security/py-django-pam: PAM backend for Django authentication
Summary: [NEW PORT] security/py-django-pam: PAM backend for Django authentication
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 (Nobody)
Keywords: feature, needs-qa
Depends on:
Reported: 2021-07-30 02:28 UTC by Alberto Mijares
Modified: 2021-08-20 00:35 UTC (History)
3 users (show)

See Also:
koobs: maintainer-feedback+

shar file for port (3.17 KB, text/plain)
2021-07-30 02:28 UTC, Alberto Mijares
no flags Details
shar file for port with fixed issues (1.76 KB, text/plain)
2021-07-30 17:10 UTC, Alberto Mijares
no flags Details
shar file for port (3rd version) (1.79 KB, text/plain)
2021-08-01 15:29 UTC, Alberto Mijares
no flags Details
shar file for port (4th rev) (1.82 KB, text/plain)
2021-08-02 19:13 UTC, Alberto Mijares
no flags Details
shar file for port (1.87 KB, text/plain)
2021-08-03 05:31 UTC, Alberto Mijares
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alberto Mijares 2021-07-30 02:28:47 UTC
Created attachment 226791 [details]
shar file for port

I made this port with pytoport. I think it's very useful for python devs.
Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 2021-07-30 02:40:38 UTC
Thank you for submitting a new port Alberto.

Review items:

  - We try to use devel as a last report, unless the best and only category is devel. I'd suggest security/ here for this port, which is where other PAM related ports exist. 

  - COMMENT is too long (Running QA with portlint would pick this up. use upstreams shorter description: "Django PAM authentication backend implementation". Bonus: Ask upstream to change their setup.py:description to the shorter version :)

  - Uncomment LICENSE (portlint would have picked this up)
  - Add LICENSE_FILE if one is shipped with the PyPI sdist
  - Clean up and format the pkg-descr (78 char columns). The following is enough:

  Django PAM can be used in an SSO (Single Sign On) environment or just with a 
  single box where you want to log into a Django app with your UNIX login.

Highly recommend running the port through QA (portlint and poudriere at least). For information on how to do this, see: 


Jump on #freebsd-ports or #freebsd-python on Libera Chat IRC if you need help :)
Comment 2 Alberto Mijares 2021-07-30 17:10:41 UTC
Created attachment 226805 [details]
shar file for port with fixed issues

I fixed everything from koobs review suggestions. portlint complains only about only 2 lines in pkg-descr but koobs proposed the current text. I'm not sure if LICENSE_FILE is needed because the Makefile already says the right one.

poudriere passes on 11.4-RELEASE with latest ports tree.
Comment 3 Kubilay Kocak freebsd_committer freebsd_triage 2021-07-31 00:18:10 UTC
(In reply to Alberto Mijares from comment #2)

What are the portlint warnings?

The guidelines on LICENSE_FILE are to include it when a file is provided in the DISTFILES. While license texts can be similar identical, often times they are not, and copyrights are unique
Comment 4 Alberto Mijares 2021-08-01 15:29:52 UTC
Created attachment 226846 [details]
shar file for port (3rd version)

This is the output of portlint on this last version attached. A LICENSE_FILE was included.

WARN: /usr/home/amijares/myports/security/py-django-pam/pkg-descr: contains less than 3 lines, make it longer if possible.(currently 2 lines)
WARN: Makefile: you may want directory for dependency ${PYTHON_PKGNAMEPREFIX}python-pam>0 to be security/py-python-pam@${PY_FLAVOR}
WARN: Makefile: you may want directory for dependency ${PYTHON_PKGNAMEPREFIX}six>0 to be devel/py-six@${PY_FLAVOR}
0 fatal errors and 3 warnings found.

I´d like some hints on how to fix the last 2 warnings.
Comment 5 Alberto Mijares 2021-08-02 19:13:08 UTC
Created attachment 226888 [details]
shar file for port (4th rev)

With some help from #freebsd-ports IRC channel, I managed to solve the issue with directory dependencies.

Now portlint only warns about only 2 lines in pkg-descr, which I think is acceptable.

poudriere passes too.
Comment 6 Alberto Mijares 2021-08-02 19:29:01 UTC
(In reply to Kubilay Kocak from comment #1)
This is just to let you know that I asked upstream to change setup.py:description and they already did it. See https://github.com/cnobile2012/django-pam/issues/13.
Comment 7 Guangyuan Yang freebsd_committer 2021-08-02 20:13:06 UTC
(In reply to Alberto Mijares from comment #6)

Hi! You seem to have forgotten the WWW field in the pkg-descr for this port. Also, it is recommended to have a detailed description in pkg-descr. Otherwise looks good to me, thanks!
Comment 8 Kubilay Kocak freebsd_committer freebsd_triage 2021-08-03 00:51:29 UTC
Great job Alberto!

Highly recommend adding TEST_DEPENDS and a (do-)test: target for proper QA. Also note that upstream only tests up to Python 3.6, so it is unclear if higher versions are actually supported. In these cases, tests should be confirmed to pass for Python versions higher than what is tested, otherwise capping USES=python:<version-spec> to the highest tested version (-3.6 in this case)
Comment 9 Alberto Mijares 2021-08-03 05:31:07 UTC
Created attachment 226902 [details]
shar file for port

I included the WWW line in pkg-descr.

Regarding the QA, I ran the tests with python 3.8 and all good except for one test I had to disable.
Ran 20 tests in 29.017s

OK (skipped=1)
Destroying test database for alias 'default'...
Name                           Stmts   Miss     Cover   Missing
django_pam/accounts/forms.py      31      2    93.55%   54-55
TOTAL                            202      2    99.01%

6 files skipped due to complete coverage.

Explaining why this one test fails is out of my scope at the moment. However, I think the package works overall.

They released a new version upstream and soon I'll have to update this port for getting up-to-date with them. So, what I propose is to commit this port as it is and I will manage to include the QA settings for the upcoming update. Need some time to figure out a few things.
Comment 10 Guangyuan Yang freebsd_committer 2021-08-17 08:03:12 UTC
Currently don't have time to take care of this - back to the pool.
Comment 11 Kubilay Kocak freebsd_committer freebsd_triage 2021-08-20 00:35:30 UTC
^Triage: Nothing for maintainer to do/ack at the moment, set flag state accordingly. This issue is ready to review and QA for commit