sysutils/rdiff-backup: Update to 2.0.0 and undeprecate

[tagattie]

Summary

• Update 2.0.0
• Add PKGNAMEPREFIX
• Switch to python 3

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=246250

Test Plan

• portlint -AC: looks fine
• poudriere testport: OK ({121,113}amd64, {121,113}i386)

[ehaupt]

As this port becomes flavored PKGBASE and PKGNAME changes:

old:

$ make -VPKGBASE
rdiff-backup
$ make -VPKGNAME
rdiff-backup-1.2.8_3,1

new:

$ make -VPKGBASE
py37-rdiff-backup
$ make -VPKGNAME
py37-rdiff-backup-2.0.0

So it seems this becomes a case for ports/MOVED.

But how would this be correctly mapped in ports/MOVED. Maybe like this?

sysutils/rdiff-backup|sysutils/rdiff-backup@py37|2020-05-06|Flavorize

Maybe the port needs to be renamed to sysutils/py-rdiff-backup?

We'd appreciate a review and advise on how to handle the this situation correctly.

[tagattie]

There is another question regarding the change of package name.

The port currently has PORTEPOCH=1. Could PORTEPOCH be removed because of the package name change?

Section 5.2.3.2 says:

Dropping or resetting PORTEPOCH incorrectly leads to no end of grief. If the discussion above was not clear enough, please consult the FreeBSD ports mailing list.

So PORTEPOCH can be removed in some cases but I'm not sure it can be removed in this case.

We'd appreciate your review and advise.

[koobs]

Python Porting Policy

We've tried to clearly cover as many aspects of Python package porting as possible in the following policy guidelines:

https://wiki.freebsd.org/Python/PortsPolicy

If there's anything there not mentioned, please let us know so we can cover those areas

If anything is unclear, or you think additional clarity can be provided, please also let us know.

Apart from the inline review items:

• The PyPI source distribution (sdist/tar.gz) provides tests (obviously the github sources do to). When a python package provides tests, add TEST_DEPENDS and a (do)test: target to run the tests. This greatly improves QA ability speed, and coverage.

Note: Dont just depend on and run tox/virtualenvs, run the commands those tools run. In this case see tox.ini deps = and commands = sections, .travis.yml and Makefile for hints.

Given this is (provides) a C extension, you may need to override PYDISTUTILS_BUILDARGS to make it testable without installation first. See the following commit as an example:

https://svnweb.freebsd.org/changeset/ports/503350

On the MOVED entry

yes that's how its been done in the past:

accessibility/py3-speech-dispatcher|accessibility/py-speech-dispatcher@py36|2017-11-30|Moved to a flavored, generic, version
devel/boost-python3-libs|devel/boost-python-libs@py36|2018-02-15|Flavorize

However, its a flawed and broken system, since one can't know in advance what the users/systems default_python version is. I also don't know how or if existing tools handle this discrepancy.

On naming (py- or not)

This is mostly covered by the Naming::Prefixing section of the policy

https://wiki.freebsd.org/Python/PortsPolicy#Prefixing_.28py-.2A.29

I can see myself making an argument that 'rdiff-backup' is "sufficiently" (maybe kinda) a "complete end-user product", but the connection is tenuous at best, and probably not sufficient to supplant Python: PEP20 (as grounding principles):

"""
Special cases aren't special enough to break the rules.
Although practicality beats purity.
"""

The complete end user product mentioned in the guidelines were designed to be reserved to say for example, an IDE, or other "product" (commercial or otherwise), not listed in PyPI, that just happened to be implemented in Python

sysutils/rdiff-backup/Makefile

8 ↗

(On Diff #71675)

Add 'python' to CATEGORIES as a virtual category (particularly for those packages listed in PyPI)

12 ↗

(On Diff #71675)

Set COMMENT to match setup.py:description (modulo portlint/length rules) where possible:

in rdiff-backups case: Backup and Restore utility, easy to use, efficient, locally and remotely usable

14 ↗

(On Diff #71675)

• License stipulated upstream is license="GPLv2+",
• Add LICENSE_FILE where one is provided with the distribution files. Using github, the file is COPYING. It's also contained in the CHEESESHOP (PyPI) sdist as the same filename

16–17 ↗

(On Diff #71675)

These don't appear to be declared dependencies. Where did they come from?

The only setup_requires I see listed for 2.0.0 is setuptools_scm [1]

[1] setup.py: setup_requires=['setuptools_scm'],

21 ↗

(On Diff #71675)

Use CHEESESHOP (PyPI) for MASTER_SITES unless there is a compelling reason not to

[1] https://wiki.freebsd.org/Python/PortsPolicy#MASTER_SITES

23 ↗

(On Diff #71675)

Use autoplist unless there's a compelling case note to.

Note also: autoplist can be complemented by a pkg-plist for manual entries

This package appears to install files into common (not python version specific) locations (like /usr/local/bin/) and as such must be made concurrent safe:

https://wiki.freebsd.org/Python/PortsPolicy#concurrent

25 ↗

(On Diff #71675)

The other ports mentioned here probably need to be leveled up in the same manner this one does

29–30 ↗

(On Diff #71675)

Likely handled already (not needed) with USE_PYTHON=autoplist, if not leave them here or add them to pkg-plist

35–36 ↗

(On Diff #71675)

Use prefix-safe PYTHONPREFIX_SITELIBDIR variable here

sysutils/rdiff-backup/files/patch-setup.py

5–17 ↗

(On Diff #71675)

I wouldn't bother patching this out. It serves little purpose and can be brittle as other features of setuptools_scm may get leveraged in the future, which if not used, prevent the port from packaging correctly on updates.

sysutils/rdiff-backup/pkg-plist

29–33 ↗

(On Diff #71675)

This package list would only work for builds with python 3.7. When using a static pkg-plist you must use %%VARIABLES%% that get replaced with the appropriate python version

Using autoplist handles this

[koobs]

I'd ask portmgr explicitly about PORTEPOCH. It would be nice to remove if it can be. There are few opportunities to do that, so worth taking that time while other change are being made.

[tagattie]

sysutils/rdiff-backup/Makefile

12 ↗

(On Diff #71675)

Will change the comments as follows. This is a bit different from what is is the description to not exceed 70 characters limit.

COMMENT=	Easy to use and efficient backup and restore utility

14 ↗

(On Diff #71675)

Will change the license block as follows:

LICENSE=	GPLv2+
LICENSE_FILE=	${WRKSRC}/COPYING

16–17 ↗

(On Diff #71675)

Will delete those two dependencies and add setuptools_scm to BUILD_DEPENDS.

BUILD_DEPENDS=	${PYTHON_PKGNAMEPREFIX}setuptools_scm>0:devel/py-setuptools_scm@${PY_FLAVOR}

21 ↗

(On Diff #71675)

Will switch from GITHUB to CHEESESHOP.
Delete:

USE_GITHUB=	yes

Add:

MASTER_SITES=	CHEESESHOP

23 ↗

(On Diff #71675)

Will change USE_PYTHON as follows:

USE_PYTHON=	autoplist concurrent distutils

25 ↗

(On Diff #71675)

There is no port named rdiff-backup-devel and this line seems some leftover from the past.

So I guess this line can be removed.

29–30 ↗

(On Diff #71675)

Will delete those 2 lines and let USE_PYTHON=autoplist handle those files.

sysutils/rdiff-backup/files/patch-setup.py

5–17 ↗

(On Diff #71675)

Will delete this file.

sysutils/rdiff-backup/pkg-plist

29–33 ↗

(On Diff #71675)

Will delete pkg-plist and let USE_PYTHON=autoplist handle the packing list.

[tagattie]

Update the patch to address the inline comments.

Regarding testing, the port seems to require pyxattr and pylibacl. However, pyxattr cannot seem to compile (on FreeBSD 12 amd64).

[koobs]

In D24816#546375, @tagattie wrote:

Update the patch to address the inline comments.

Regarding testing, the port seems to require pyxattr and pylibacl. However, pyxattr cannot seem to compile (on FreeBSD 12 amd64).

Nice work!

Regarding testing, just add the test_depends and (do)-test target, as nothing runs tests automatically.

Let's also create an issue regarding pyxattr so we can fix it

Did autoplist handle all the docs/man pages below?

MAN1S=		rdiff-backup.1 rdiff-backup-statistics.1
DOCS=		CHANGELOG README COPYING FAQ.html

sysutils/rdiff-backup/Makefile

24 ↗

(On Diff #71712)

This can be removed yeh?

[tagattie]

Regarding testing, just add the test_depends and (do)-test target, as nothing runs tests automatically.

I will add py-tox to TEST_DEPENDS and do-test target.

Let's also create an issue regarding pyxattr so we can fix it

Did autoplist handle all the docs/man pages below?

MAN1S=		rdiff-backup.1 rdiff-backup-statistics.1
DOCS=		CHANGELOG README COPYING FAQ.html

Yes, I checked ${WRKDIR}/.PLIST.mktmp and all the files are included in the list.

sysutils/rdiff-backup/Makefile

24 ↗

(On Diff #71712)

It seems I was wrong. Those ports may be installed on a machine and may conflict with the current version.
So I would leave the line as it is.

[tagattie]

Add TEST_DEPENDS and do-test target

[koobs]

Nice work.

While we prefer not to use tox/venv for tests in almost all cases, as most are trivial wrappers of other test tools (pytest), I note that rdiff-backup has no 'single command' referenced in upstream documentation or test files for running the tests so we'd have to replicate multiple commands that are run. In this case, running tox is easier, but I also anticipate that running pytest in the tests dir will probably also 'just work'.

We should try to remove use of tox as a test dependency in the long term.

[koobs]

Oh btw, did the tests pass? :)

[tagattie]

Oh btw, did the tests pass? :)

No, style check with flake8 fails...

Functional tests cannot run because a dependent module (pyxattr) does not build...

[tagattie]

Additional Note: I will change port name from sysutils/rdiff-backup to sysutils/py-rdiff-backup following the Python Ports Policy.

I look forward to a reply from portmgr regarding PORTEPOCH.

[bapt]

the point of portepoch is to allow a package to move forward even if the versionning upstream changes and seems to go backward.

If the package name has changed, then there is no comparison possible with any previous version, so yes it is the right time to remove portepoch

[tagattie]

If the package name has changed, then there is no comparison possible with any previous version, so yes it is the right time to remove portepoch

Thanks a lot for the answer! Now we can move forward for this port changes committed.