Spec URL: https://amigadave.fedorapeople.org/python-psautohint-2.3.1-1.fc36.src.rpm SRPM URL: https://amigadave.fedorapeople.org/python-psautohint-2.3.1-1.fc36.src.rpm Description: A standalone version of AFDKO’s autohinter. Fedora Account System Username: amigadave
Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=81230885
Spec URL: https://amigadave.fedorapeople.org/python-psautohint.spec
This is not quite true: > # Test suite needs dependencies not in Fedora. ---- In fact, we can add: > %global data_commit 1e4c5061d328105c4dcfcb6fdbc27ec49b3e9d23 > # This contains bundled fonts for testing, but they are *not* installed > Source1: https://github.com/adobe-type-tools/psautohint-testdata/archive/%{data_commit}/psautohint-testdata-%{data_commit}.tar.gz and > BuildRequires: /usr/bin/tx (which reduces the number of skipped tests) and then to make sure all the functionality is available by default, in python3-psautohint: > Recommends: /usr/bin/tx Then, in %prep: > %autosetup -p1 -n %{tarball_name}-%{version} > rm -vrf tests/integration/data > %setup -q -n %{tarball_name}-%{version} -T -D -b 1 > mv ../psautohint-testdata-%{data_commit}/ tests/integration/data > # Allow newer versions of testing dependencies > sed -r -i 's/("pytest[^[:blank:]]*) >=[^"]+/\1/' setup.py > # Drop pytest-randomly, which doesn’t seem to be mandatory, > # and pytest-cov, since we do not need coverage analysis > sed -r -i '/pytest-(randomly|cov)/d' setup.py The only missing dependency was python3dist(pytest-randomly), which as mentioned above doesn’t actually seem to be required. Then: > %pyproject_buildrequires -x testing and > k="${k-}${k+ and }not test_hashmap_old_version' > %pytest -k "${k-}" At least in a local mock build on x86_64, that results in all the tests running and passing, except for the one I skipped. I haven’t looked into why it fails.
> k="${k-}${k+ and }not test_hashmap_old_version' > %pytest -k "${k-}" This could just be > %pytest -k 'not test_hashmap_old_version' I’m used to using the shell variable pattern so it’s easy to add and remove skips in packages where this is frequently required.
I would simplify this a bit and call the package just 'psautohint' and not python-psautohint because it's just an executable and not a python library. See https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_application_naming
(In reply to Kalev Lember from comment #5) > I would simplify this a bit and call the package just 'psautohint' and not > python-psautohint because it's just an executable and not a python library. > See > https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/ > #_application_naming This is a fair point. There actually is a usable Python API, e.g. [1], but there is a good argument that the package exists primarily to provide the executable. I think I would find it acceptable either way, but lean toward application naming. [1] https://github.com/adobe-type-tools/psautohint/blob/0a1e514a3cb86c6593e4f05814d70afc390f7e69/python/psautohint/__init__.py
David, are you still working on this package?
(In reply to Ben Beasley from comment #6) > (In reply to Kalev Lember from comment #5) > > I would simplify this a bit and call the package just 'psautohint' and not > > python-psautohint because it's just an executable and not a python library. > > See > > https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/ > > #_application_naming > > This is a fair point. There actually is a usable Python API, e.g. [1], but > there is a good argument that the package exists primarily to provide the > executable. I think I would find it acceptable either way, but lean toward > application naming. > > [1] > https://github.com/adobe-type-tools/psautohint/blob/ > 0a1e514a3cb86c6593e4f05814d70afc390f7e69/python/psautohint/__init__.py Oh, that's true, and it's actually installed as well, I think -- I totally missed it at first because the python library must be hidden behind %files -n python3-psautohint -f %{pyproject_files} and not explicitly listed in %files. Anyway, maybe in that case it would make sense to have a split where: Source package name: psautohint Two binary packages: psautohint (/usr/bin/*) python3-psautohint (python library)
(In reply to Kalev Lember from comment #8) > Oh, that's true, and it's actually installed as well, I think -- I totally > missed it at first because the python library must be hidden behind %files > -n python3-psautohint -f %{pyproject_files} and not explicitly listed in > %files. > > Anyway, maybe in that case it would make sense to have a split where: > > Source package name: psautohint > > Two binary packages: > psautohint (/usr/bin/*) > python3-psautohint (python library) This is good advice in general (especially coming from a C/C++ perspective) but I am not sure if it is worth separating them in this case. The implementations of the binaries live within the library and the actual scripts are trivial generated entry points of only a few lines each. There is little to be saved by not installing them when only the library is required. I suppose one could %exclude the __main__.py file from the library and package it with the binaries. That’s probably the right way to separate the two, but I don’t know if it would be worth it; that file is only ~30kB. It’s pretty common for primarily-application Python packages to also provide a Python library. The Provides like “python3dist(psautohint)” are generated automatically (assuming proper dist-info metadata), and the rest can be handled by adding an explicit “%py_provides python3-psautohint” if there is not an explicit library subpackage. It’s also pretty common for primarily-library Python packages to provide executable entry points without a separate subpackage—usually without a virtual Provides for the equivalent application name, although I don’t believe it would be incorrect to add one in cases where it’s clear what that name would be. Obviously, the dividing line between these two types of packages is fuzzy and subjective. There are plenty of cases where there doesn’t seem to be one clearly-correct answer, and there are plenty of examples of important existing packages that probably should have chosen the other naming scheme.
Fair enough. I guess in this case it's mainly an executable package then? Upstream's naming (psautohint instead of python-psautohint) seems to hint that this may be the case. I'm pretty sure David is packaging it for cantarell fonts and cantarell fonts build system uses the executable there and not the library.
This is an automatic action taken by review-stats script. The ticket submitter failed to clear the NEEDINFO flag in a month. As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews we consider this ticket as DEADREVIEW and proceed to close it.