Bug 2006555 - Review Request: python-autodocsumm - Extending your autodoc API docs with a summary
Summary: Review Request: python-autodocsumm - Extending your autodoc API docs with a s...
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Hunor Csomortáni
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-09-21 21:31 UTC by Ben Beasley
Modified: 2021-09-27 13:34 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-09-26 15:09:51 UTC
Type: ---
hcsomort: fedora-review?


Attachments (Terms of Use)

Description Ben Beasley 2021-09-21 21:31:41 UTC
Spec URL: https://music.fedorapeople.org/python-autodocsumm.spec
SRPM URL: https://music.fedorapeople.org/python-autodocsumm-0.2.7-1.fc34.src.rpm
Description:

This sphinx extension provides some useful extensions to the Sphinx autodoc
extension. Those are

   1. It creates a Table of Contents in the style of the autosummary extension
      with methods, classes, functions and attributes
   2. As you can include the __init__ method documentation for via the
      autoclass_content configuration value, we provide the autodata_content
      configuration value to include the documentation from the __call__ method
   3. You can exclude the string representation of specific objects. E.g. if
      you have a large dictionary using the not_document_data configuration
      value.

Fedora Account System Username: music

This will improve the documentation, and allow a patch to be dropped, in python-marshmallow. It will also improve the (yet-to-be-packaged) documentation in python-trimesh.

Koji scratch builds:

F36: https://koji.fedoraproject.org/koji/taskinfo?taskID=76076972
F35: https://koji.fedoraproject.org/koji/taskinfo?taskID=76077242
F34: https://koji.fedoraproject.org/koji/taskinfo?taskID=76077466
F33: https://koji.fedoraproject.org/koji/taskinfo?taskID=76077671

Notes on selected rpmlint messages:

> python-autodocsumm-doc.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/python-autodocsumm-doc/html/objects.inv
> python-autodocsumm-doc.noarch: W: file-not-utf8 /usr/share/doc/python-autodocsumm-doc/html/objects.inv

The interpshinx inventory file objects.inv is not a text file, but it contains enough text to fool rpmlint into complaining about it.

> python-autodocsumm.src:87: W: macro-in-%changelog %autochangelog

rpmlint does not understand rpmautospec

Comment 1 Hunor Csomortáni 2021-09-24 07:42:30 UTC
Hey Ben o/

Here is my review:


Issues:
=======

'python-autodocsumm-doc' includes a few JS libraries, originating from
sphinx-build, which have a different license and should be marked as
bundled.

I see a few options to fix this:
- Don't generate and include the docs (drop the doc subpackage).
- Update the doc subpackage and add 'Provides: bundled(<lib>)' and 'License: '
  tags as required to mark the bundled JS libraries and their respective
  licenses.
- (Maybe?) Generate the docs in a text format. The usefulness of this is
  going to be somewhat limited, as links would be entirely missing.


===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
[x]: License file installed when any subpackage combination is installed.

[!]: Package contains no bundled libraries without FPC exception.
     - Sphinx includes quite a few JS libraries in python-autodocsumm-doc

[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[-]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

Python:
[x]: Python eggs must not download any dependencies during the build
     process.
[x]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Packages MUST NOT have dependencies (either build-time or runtime) on
     packages named with the unversioned python- prefix unless no properly
     versioned package exists. Dependencies on Python packages instead MUST
     use names beginning with python2- or python3- as appropriate.
[x]: Python packages must not contain %{pythonX_site(lib|arch)}/* in %files
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[-]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[-]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
     python3-autodocsumm
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[-]: Package should compile and build into binary rpms on all supported
     architectures.
[x]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.

Comment 2 Ben Beasley 2021-09-24 13:26:29 UTC
> 'python-autodocsumm-doc' includes a few JS libraries, originating from
> sphinx-build, which have a different license and should be marked as
> bundled.

Hmm—this is an interesting point. It seems to make sense under a strict interpretation of the packaging rules.

At the same time, this is a very typical Sphinx-generated -doc subpackage, of which there are about 600 in the distribution, and only a handful do even part of what you’ve suggested. The situation with Doxygen is similar, with nearly 400 extant packages. I’ve therefore started a discussion on the packaging mailing list (https://lists.fedoraproject.org/archives/list/packaging@lists.fedoraproject.org/thread/LLUAURXZVADATHK65HBPPBHKF4EM4UC3/) about how this should be handled in general, as changing a single package (or even every package I control) is a drop in an ocean of similar documentation packages.

Still, I’m willing and able to make the changes you’ve requested. I’ll follow up with an updated submission after considering any responses on the packaging list.

Comment 3 Ben Beasley 2021-09-26 14:38:20 UTC
For future reference, a copy of my posting to the packaging mailing list (linked above) follows:

-----


It was recently brought up in a package review (https://bugzilla.redhat.com/show_bug.cgi?id=2006555) that a Sphinx-generated HTML documentation package contained several JavaScript libraries, and that:

  1. their licenses should be accounted for and added to the License
     field for the -doc subpackage
  2. they should be treated as bundled libraries and marked with virtual
     Provides

Note that the JavaScript is inserted by the Sphinx documentation build system, and is not present in the source tarball. Your “typical” Sphinx documentation has minified and unminified copies of JQuery (js-jquery) and Underscore (js-underscore), both MIT-licensed, plus several unminified utility libraries that belong to Sphinx itself (doctools.js, language_data.js, searchtools.js). The overall Sphinx project is BSD-licensed. The details vary by theme, but this is the most common case.

To me, it seems that this feedback is a reasonable strict reading of the relevant packaging guidelines. If this is the right interpretation of Fedora policy, what should be done about it in general?

-----

Based on looking for installed files named “searchtools.js” in packages named -doc or -docs, there are about 600 documentation packages generated with Sphinx. This might miss some that have exotic themes.

As far as I can tell, only one existing package in the distribution, python-murano-package-check, tries to use virtual Provides to track the libraries that belong to Sphinx itself:

  Provides:       bundled(js-doctools)
  Provides:       bundled(js-jquery) = 3.1.0
  Provides:       bundled(js-searchtools)
  Provides:       bundled(js-underscore) = 1.3.1
  Provides:       bundled(js-websupport)

and this package does not actually build its documentation subpackage.

Only the following packages have virtual Provides for js-jquery and js-underscore. In most cases these lack the version numbers.

  - arb-doc
  - mpdecimal-doc
  - python-BTrees-doc
  - python-latexcodec-doc
  - python-networkx-doc
  - python3-persistent-doc
  - sympy-doc

Of the packages in the list above, only mpdecimal-doc has a License field that seems to try to account for the licenses of the JavaScript libraries.

-----

Personally, I can add virtual Provides and adjust the License on the -doc subpackage for my package under review, and it’s even possible for me to replace the Underscore and JQuery libraries with symbolic links to those installed by js-jquery and js-underscore. However, changing one package, or even every package I control, is a tiny drop in a vast ocean of documentation packages. Consider, for example, that Doxygen also includes JQuery and several Doxygen-specific libraries in its output, so the same arguments about licenses and virtual Provides apply to it. A search for “dynsections.js” in packages named *-doc or *-docs gives nearly 400 Doxygen-based packages.

If this is a real problem, it seems like it needs to be handled and/or documented in a way that can potentially scale to the rest of those 600 Sphinx-based packages, to the nearly 400 Doxygen-based packages, and beyond that to the output of other documentation systems. I am not sure what that would look like.

All thoughts are welcome.

Comment 4 Ben Beasley 2021-09-26 14:58:33 UTC
I just followed up on the packaging mailing list with:

-----

I’ve used cairomm as a case study for trying to bring a Doxyen-based HTML documentation package into compliance, and python-autodocsumm as a case study for Sphinx.

-----

For cairomm, I have unbundled js-jquery and altered the License for the -doc subpackage. I am not treating the Doxygen-generated JavaScript files as bundled libraries because they are templated and project-dependent, not simply copied in as fully-reusable libraries.

While Doxygen does not insert a standalone license file for its MIT license, the text is at least present in the JavaScript file headers. I might have missed mentioning some non-JavaScript files inserted by Doxygen that are potentially MIT-licensed but do not reference their license.

Another solution would be to ask Doxygen to build a PDF instead of HTML. The result would have no visible bundled assets, but in my opinion would be less convenient for users.

There are some judgement calls here, but I think the result (https://src.fedoraproject.org/rpms/cairomm/c/948e04f946d95612948d81d9090e2bcfa95abe56?branch=rawhide) is reasonable.

-----

The situation for python-autodocsumm’s Sphinx documentation is more dire. I can unbundle the Sphinx-inserted copies of js-jquery and js-underscore. I can add virtual Provides for Sphinx’s libraries doctools.js and searchtools.js:

Provides:       bundled(js-sphinx-doctools)
Provides:       bundled(js-sphinx-searchtools)

although I cannot version them, because the version could only be that of Sphinx itself, and that changes regularly without alterations to my package’s sources; I don’t know a way to handle that automatically, and I could not reasonably keep it synchronized manually. These libraries were never intended to be handled separately from the documentation they support.

I can claim that language_data.js and documentation_options.js are not bundled libraries, because they are generated from templates using project-dependent options.

Now I have the problem that all of the Sphinx-inserted files in _static have the same BSD license as Sphinx itself, but they only reference the Sphinx LICENSE file in their headers; they do not contain license text at all, and BSD is one of the licenses where license text is required.

I could possibly handle this by copying in the license file from the Sphinx package in the build environment. I’d better make sure I don’t miss a hypothetical Sphinx license change, so I should probably grep that license file in %check for a key phrase to make sure its contents are what I expect.

Now I’m left with the theme. The autodocsumm documentation uses sphinx_rtd_theme, which is a very common choice. Theme files are mostly in _static/css/ and _static/js/.

In _static/js/, there are two JavaScript files, badge_only.js and theme.js. Both are minified and contain no license information. Okay, I can check the license of python3-sphinx_rtd_theme, and maybe I can copy in the unminified versions manually to comply with that part of the guidelines. Oh, it turns out only the minified versions are packaged, so there’s nothing I can do. (Well—shipping only minified files isn’t compliant with guidelines, so I could send a PR or file an issue on python-sphinx_rtd_theme and expect the maintainer to do something about it. I’m not trying to call out this particular package or its maintainers, as it’s very hard to find and deal with all the bits of JavaScript and web content that are tucked away in Python packages these days, and such oversights are rampant in existing packages.)

In _static/css/, there are two corresponding CSS files, badge_only.css and theme.css. Both have the same problems (minified only, no license information), but *also*, theme.css has a bundled copy of the CSS from Font Awesome 4.7.0. That’s also MIT-licensed, with only a URL for the license and no included license text.

-----

This is the point at which I say that the burden of trying to package this near-minimal example of Sphinx-generated documentation in strict compliance with the guidelines is unreasonable, and I drop it, along with the documentation in all other Python packages for which I am responsible.

I would be surprised if there is even one case where HTML documentation for a Python package is currently packaged in a way that truly satisfies all of these requirements around license text, web assets, JavaScript, and bundling; and I suspect the broader situation for Doxygen and other documentation generators is not much better.

As a user who has often needed to work offline for various reasons, it would be a great loss if the solution really has to be that HTML documentation should not be packaged in general.

Comment 5 Ben Beasley 2021-09-26 15:09:51 UTC
The effect of all of the above for this package is:

   1. It is impractical to correct all of the issues you pointed out, so
      the documentation will not be packaged.
   2. Since these issues apply to *all* Sphinx-generated documentation
      packages, I plan to start dropping all documentation from Python
      packages I control.

Since this package is only useful for generating documentation with Sphinx,
I no longer have a need for it, and I’m closing the review request as WONTFIX.

Thanks again for your review.

Comment 6 Ben Beasley 2021-09-27 13:34:42 UTC
One last follow-up:

It turns out that even the situation with Doxygen is worse. The file jquery.js inserted by Doxygen was not just a copy of JQuery (js-jquery) to be unbundled; it also contained in the same file:

- JQuery UI (js-jquery-ui)
- jquery.ScrollTo (https://github.com/flesler/jquery.scrollTo/)
- PowerTip (https://stevenbenner.github.io/jquery-powertip/)
- JQuery UI Touch Punch (js-jquery-ui-touch-punch)
- SmartMenus jQuery Plugin (http://www.smartmenus.org)

It’s possible to account for all of these, package the missing ones separately, and reconstruct the bundle using unminified sources (or patch the generated HTML to use separate script tags, yuck), but the burden is getting ridiculous again. It looks like dropping all Doxygen-based HTML documentation, or building a less-useful PDF instead, will be the sensible (but disappointing) outcome.


Note You need to log in before you can comment on or make changes to this bug.