https://bugzilla.redhat.com/show_bug.cgi?id=1914689
Richard Shaw <hobbes1069(a)gmail.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
Flags|needinfo?(hobbes1069@gmail. |
|com) |
--- Comment #11 from Richard Shaw <hobbes1069(a)gmail.com> ---
(In reply to code from comment #10)
Issues:
=======
- It is a lot easier to review if you update both the spec and SRPM. This
will
be mandatory for approval, since the SRPM is needed to create the initial
dist-git repo. (Even better, bump the release when you make changes.)
Sorry the intent was to get a quick re-review from Miro as I essentially
rewrote the specfile based on his feedback.
- The LICENSE.md file for the bundled semantic-ui must be installed
with
%license too.
- There is a bundled copy of semantic-ui included. You must follow
https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling. Since
upstream does not *technically* support building with an external copy, you
have the option to publicly contact upstream about a path to doing so, and
then add the appropriate virtual Provides:
Provides: bundled(js-semantic-ui) = 2.4.1
However, all it would take to use a separately-packaged copy would be to
replace this directory with a symbolic link. Then you could package
js-semantic-ui as a dependency in accordance with
https://docs.fedoraproject.org/en-US/packaging-guidelines/JavaScript/ and
https://docs.fedoraproject.org/en-US/packaging-guidelines/Web_Assets/. This
has advantages:
https://fedoraproject.org/wiki/
Bundled_Libraries#Why_Bundled_Libraries_are_a_problem.
Note that if you do package or bundle js-semantic-ui, you must
compile/minify
the CSS and JS yourself from the original sources in the RPM build process,
and you must include the original sources in the RPM, not just the minified
ones.
…but note the next issue, below!
- A new major version, 2.0.0, was released in late January; you should
package
it instead. This affects the semantic-ui situation: “JS/CSS assets are now
copied across by sphinx when builing, rather than being copied by the
extension”. From testing building sphinx-tabs’s own documentation, I don’t
see any reliance on semantic-ui at all. Maybe the problem has gone away.
You
should look into it more closely than I did.
There's not longer anything bundled which is good because I only need this as a
dep for OpenColorIO 2.0. I don't need any more packages :)
- You correctly noted in %check that Sphinx is too old for this
package on
EPEL
(both 7 and 8). Besides, the pyproject-rpm-macros are not available on
EPEL.
So, remove the EPEL-specific cruft from the spec file.
Each “%{python3_pkgversion}” should just become a “3”.
Done.
This
%{?python_provide:%python_provide python3-%{pypi_name}}
should be deleted; even if you were packaging for EPEL, you should
conditionalize it because this macro should not be used on Fedora; see
This was never needed even for EPEL, as both EPEL and Fedora define the
macro:
%{?!python3_pkgversion:%global python3_pkgversion 3}
I cleaned it up. I only plan to build for Fedora. I only need it for Rawhide
but I may build it for f33 as well.
BR’s should be written like:
BuildRequires: python3-devel
BuildRequires: python3dist(setuptools)
Done.
- “%pyproject_save_files sphinx_tabs” could become
“%pyproject_save_files %{python_module_name}”, if you like
Done.
- Instead of manually specifying BR’s for testing, change
%pyproject_buildrequires
to
%pyproject_buildrequires -x testing
You can fix the missing python3dist(bs4) by replacing "bs4" with
"beautifulsoup4" in setup.py; see
https://pypi.org/project/bs4/.
You can loosen the pytest version restriction, which is currently <4;
version
6 does work.
Unfortunately, you still cannot run the tests without
python3dist(pytest-regressions), so choose one of the following:
- Package python-pytest-regressions.
- Figure out how to patch it out of the tests, and run them.
- Go back to just “%pyproject_buildrequires”, and put a comment where the
%check section would be explaining the missing dependency.
If you do get the tests working,
%check
%pytest
should work just fine.
Again, since this is only being used as a build dep for me and I don't need
anymore packages I think I'll leave it alone for now.
- It would be nice if you build the HTML documentation and installed
it in a
-doc subpackage. Just set PYTHONPATH and use sphinx-build.
Done.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component