Merge ~ahasenack/ubuntu/+source/cyrus-sasl2:groovy-sasl-disable-doc-build into ubuntu/+source/cyrus-sasl2:ubuntu/devel

[Andreas Hasenack]

Disable doc building during package build. I feel a bit dirty doing this, but at the moment it's the fastest way forward.

cyrus-sasl2 builds docs with sphinx, and upstream has a sphinx extension in the source. That extension no longer works with the current sphinx in groovy, causing the FTBFS.

I filed a bug upstream at https://github.com/cyrusimap/cyrus-sasl/issues/624 highlighting two of the problems I believe I have fixed, but then I ran into a third one that I had no idea how to approach. I'll update the upstream bug with more details about my findings and a diff.

For the time being, this branch disables doc building. I checked the following:
- the two patches we have that touch source docs still end up in the final rendered/installed manpage, and that's because the mdoc source file is installed as is as a manpage in d/rules.
  - 0003-Update-saslauthd.conf-location-in-documentation.patch and 0019-Add-reference-to-LDAP_SASLAUTHD-file-to-the-saslauth.patch touch saslauthd/saslauthd.mdoc which d/rules does this with:
    install -m 644 saslauthd/saslauthd.mdoc \
        $(TMP_MIT)/usr/share/man/man8/saslauthd.8

- the manpages were initially not shipped anymore, with a zero-sized file, after my change. I had to copy them to the temporary build directories for MIT and Heimdal in d/rules
- I diffed the manpages from the build currently in groovy (which was from focal) to the one produced by this branch, and the only change was the timestamp inside each manpage, i.e., things like:
diff -uNr focal/usr/local/share/man/man3/sasl_auxprop.3 groovy/usr/local/share/man/man3/sasl_auxprop.3
--- focal/usr/local/share/man/man3/sasl_auxprop.3 2020-09-29 16:34:29.000000000 -0300
+++ groovy/usr/local/share/man/man3/sasl_auxprop.3 2020-09-29 16:36:27.000000000 -0300
@@ -1,6 +1,6 @@
 .\" Man page generated from reStructuredText.
 .
-.TH "SASL_AUXPROP" "3" "September 29, 2020" "2.1.27" "Cyrus SASL"
+.TH "SASL_AUXPROP" "3" "November 08, 2018" "2.1.27" "Cyrus SASL"
 .SH NAME
 sasl_auxprop \- Cyrus SASL documentation
 .

- I also diffed /usr/share/doc/cyrus-sasl2-doc for both cases, and there was no difference at all.

PPA with test build: https://launchpad.net/~ahasenack/+archive/ubuntu/sasl2-drop-doc-build

[Andreas Hasenack (ahasenack)]

The other branch I have is at https://code.launchpad.net/~ahasenack/ubuntu/+source/cyrus-sasl2/+git/cyrus-sasl2/+ref/groovy-sasl2-fix-sphinx-build. The build progresses a bit further, but still fails. I updated our bug and upstream's.

[Andreas Hasenack (ahasenack)]

In that other branch, where I still try to build with sphinx, I got something that builds, and at first glance the docs look fine. I setup a ppa[1] and will do a diff to see if any artifacts disappeared or changed.

1. https://launchpad.net/~ahasenack/+archive/ubuntu/sasl2-fix-sphinx-build/+packages

[Andreas Hasenack (ahasenack)]

Ok, that attempt to fix the sphinx build failed. The manpages were not built correctly. So this MP here is still plan A.

[Bryce Harrington (bryce)]

I'll take the review for this. Code changes LGTM.

Make sure to open a new bug report for "Re-enable docs for cyrus-sasl2" with link to your upstream bug report (if possible) and server team subscribed, so we can get flagged for followup when that gets fixed.

Sphinx was mentioned in a number of migration problems last week. I didn't investigate any very deeply but clearly its last bump introduced some breaking changes...

Anyway, the PPA appears to have built properly. I will replicate the build myself and run autopkgtests to verify.

[Bryce Harrington (bryce)]

* Changelog:
  - [-] old content and logical tag match as expected
  - [√] changelog entry correct version and targeted codename
  - [√] changelog entries correct
  - [√] update-maintainer has been run

* Actual changes:
  - [-] no upstream changes to consider
  - [-] no further upstream version to consider
  - [√] debian changes look safe

* Old Delta:
  - [-] dropped changes are ok to be dropped
  - [-] nothing else to drop
  - [√] changes forwarded upstream/debian (if appropriate)

* New Delta:
  - [√] no new patches added
  - [-] patches match what was proposed upstream
  - [-] patches correctly included in debian/patches/series
  - [-] patches have correct DEP3 metadata

* Build/Test:
  - [√] build is ok
  - [√] verified PPA package installs/uninstalls
    + But see discussion below
  - [√] autopkgtest against the PPA package passes
    + Skip in lxc; see below
  - [-] sanity checks test fine

Installing sasl2-bin results in some man pages installed to /usr/share/man/man8 and /usr/share/man/man1 that appear to match the bins installed by the package. The man pages display fine and are formatted correctly.

I was also able to install cyrus-sasl2-doc. As expected it's missing a lot of stuff, but the top level framework skeleton is there:

$ dpkg -L cyrus-sasl2-doc
/.
/usr
/usr/share
/usr/share/doc
/usr/share/doc/cyrus-sasl2-doc
/usr/share/doc/cyrus-sasl2-doc/LDAP_SASLAUTHD.gz
/usr/share/doc/cyrus-sasl2-doc/NEWS.Debian.gz
/usr/share/doc/cyrus-sasl2-doc/changelog.Debian.gz
/usr/share/doc/cyrus-sasl2-doc/copyright
/usr/share/doc/cyrus-sasl2-doc/developer.html
/usr/share/doc/cyrus-sasl2-doc/download.html
/usr/share/doc/cyrus-sasl2-doc/genindex.html
/usr/share/doc/cyrus-sasl2-doc/getsasl.html
/usr/share/doc/cyrus-sasl2-doc/index.html
/usr/share/doc/cyrus-sasl2-doc/operations.html
/usr/share/doc/cyrus-sasl2-doc/packager.html
/usr/share/doc/cyrus-sasl2-doc/search.html
/usr/share/doc/cyrus-sasl2-doc/setup.html
/usr/share/doc/cyrus-sasl2-doc/support.html
/usr/share/doc-base
/usr/share/doc-base/cyrus-sasl2-doc

Of course, the links in these html files are largely broken. What I'm wondering is if it might be better to make the cyrus-sasl2-doc binary package uninstallable?

I also ran the autopkgtests but they did not seem to do anything interesting in my lxc container. At least, it seems not to care about the missing docs:

autopkgtest [01:08:35]: testing package cyrus-sasl2 version 2.1.27+dfsg-2ubuntu1~ppa1
autopkgtest [01:08:35]: build not needed
* SKIP no tests in this package
autopkgtest [01:08:35]: @@@@@@@@@@@@@@@@@@@@ summary
* SKIP no tests in this package

Anyway, apart from the question on the docs package installability everything else looks fine to go in.

review: Approve

[Andreas Hasenack (ahasenack)]

Hm, the html links are indeed broken, but are like that in the original groovy package already. Let me see if I can fix that.

About tests, the sssd and krb5 have sasl tests that will be exercised.

[Andreas Hasenack (ahasenack)]

I pushed some commits to include the sasl/ documentation subdirectory, but things still don't look super right. I pushed that to the ppa.

I'm inclined to revert that and go back to db38017450b0f54b7c2abed37ce17693e18b3700 which is what you reviewed, since I think further messing with the docs isn't worth it until we can get them to build again.

[Andreas Hasenack (ahasenack)]

Ah, looks like I missed a subdir _static with needed elements

[Andreas Hasenack (ahasenack)]

Ok, new build up in the ppa, looking much better. Could you take another peek and help me decide whether to just fix the ftbfs (what you reviewed up to db38017450b0f54b7c2abed37ce17693e18b3700) or also include these extra commits?

[Bryce Harrington (bryce)]

This looks great. I verified the build locally and installed the cyrus-sasl2-doc_2.1.27+dfsg-2ubuntu1_all.deb binary package, and randomly lynx-navigated around stuff in /usr/share/doc/cyrus-sasl2-doc. I could find no broken links or problematic behaviors.

Given how minimal the packaging fix was, and that it fixes an easily discerned user-facing bug, I do think this is worth inclusion. Indeed, these user docs are going to be more broadly applicable to users than the sphinx-generated code docs. Since this is documentation there would not be the level of worry if it was a code change for introducing a regrssion. The only thought I have is that it might worth having a bug report LP# associated but I don't think that's a huge deal.

So LGTM, +1.

review: Approve

[Andreas Hasenack (ahasenack)]

How about I reopen https://bugs.launchpad.net/ubuntu/+source/cyrus-sasl2/+bug/1894907 after this upload closes it? It has all the history, and links to upstream and debian bugs

[Bryce Harrington (bryce)]

If there's no chance that the bug could be an SRU candidate, then reopening it is fine. Make sure to update the title.

Otherwise, might be simpler to just make a new bug whose description is just a pointer to LP: #1894907. Then you don't have to remember to reopen.

But either way's fine so long as the tech debt is tracked somewhere.

[Andreas Hasenack (ahasenack)]

Hm, I ran lintian on the new build, and introducing the missing directories also introduces another js library (jquery):
E: cyrus-sasl2 source: source-is-missing doc/html/_static/jquery.js line length is 32014 characters (>512)
W: cyrus-sasl2-doc: embedded-javascript-library usr/share/doc/cyrus-sasl2-doc/_static/jquery-3.1.0.js please use libjs-jquery

This package has so many issues it's sad :(

I'll drop the extra changes I did, fix just the ftbfs, and work on the package in the next dev cycle.

[Andreas Hasenack (ahasenack)]

I filed https://bugs.launchpad.net/ubuntu/+source/cyrus-sasl2/+bug/1898593 to properly fix the doc building with sphinx.

[Andreas Hasenack (ahasenack)]

Tagging and uploading 404f9fbc6c3531e750ce25a90a521316fe2ae6e9

$ git push pkg upload/2.1.27+dfsg-2ubuntu1
Enumerating objects: 19, done.
Counting objects: 100% (19/19), done.
Delta compression using up to 4 threads
Compressing objects: 100% (14/14), done.
Writing objects: 100% (14/14), 1.83 KiB | 69.00 KiB/s, done.
Total 14 (delta 10), reused 0 (delta 0)
To ssh://git.launchpad.net/~usd-import-team/ubuntu/+source/cyrus-sasl2
 * [new tag] upload/2.1.27+dfsg-2ubuntu1 -> upload/2.1.27+dfsg-2ubuntu1

$ dput ubuntu ../cyrus-sasl2_2.1.27+dfsg-2ubuntu1_source.changes
Checking signature on .changes
gpg: ../cyrus-sasl2_2.1.27+dfsg-2ubuntu1_source.changes: Valid signature from AC983EB5BF6BCBA9
Checking signature on .dsc
gpg: ../cyrus-sasl2_2.1.27+dfsg-2ubuntu1.dsc: Valid signature from AC983EB5BF6BCBA9
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading cyrus-sasl2_2.1.27+dfsg-2ubuntu1.dsc: done.
  Uploading cyrus-sasl2_2.1.27+dfsg-2ubuntu1.debian.tar.xz: done.
  Uploading cyrus-sasl2_2.1.27+dfsg-2ubuntu1_source.buildinfo: done.
  Uploading cyrus-sasl2_2.1.27+dfsg-2ubuntu1_source.changes: done.
Successfully uploaded packages.

diff --git a/debian/changelog b/debian/changelog
index 09f5803..8c32f30 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+cyrus-sasl2 (2.1.27+dfsg-2ubuntu1) groovy; urgency=medium
+
+  * Disable sphinx doc build, as the extension bundled in this source is
+    not compatible with current sphinx release (LP: #1894907):
+    - d/control: remove python3-sphinx build-depends
+    - d/rules: disable sphinx doc build, copy pre-built manpages to build dirs
+
+ -- Andreas Hasenack <[e-mail]>  Tue, 29 Sep 2020 19:25:19 +0000
+
 cyrus-sasl2 (2.1.27+dfsg-2) unstable; urgency=medium
 
   [ Salvatore Bonaccorso ]
diff --git a/debian/control b/debian/control
index 285347e..40d7cf2 100644
--- a/debian/control
+++ b/debian/control
@@ -1,7 +1,8 @@
 Source: cyrus-sasl2
 Section: libs
 Priority: standard
-Maintainer: Debian Cyrus Team <[e-mail]>
+Maintainer: Ubuntu Developers <[e-mail]>
+XSBC-Original-Maintainer: Debian Cyrus Team <[e-mail]>
 Uploaders: Fabian Fagerholm <[e-mail]>,
            Roberto C. Sanchez <[e-mail]>,
            Ondřej Surý <[e-mail]>,
@@ -23,7 +24,6 @@ Build-Depends: chrpath,
                libsqlite3-dev,
                libssl-dev,
                po-debconf,
-               python3-sphinx,
                quilt
 Build-Conflicts: heimdal-dev
 Vcs-Browser: https://salsa.debian.org/debian/cyrus-sasl2
diff --git a/debian/rules b/debian/rules
index d30b4d9..e452b45 100755
--- a/debian/rules
+++ b/debian/rules
@@ -101,7 +101,7 @@ CONFIGURE_COMMON_OPTIONS= \
 	--with-plugindir=/usr/lib/$(DEB_HOST_MULTIARCH)/sasl2 \
 	--sysconfdir=/etc \
 	--with-devrandom=/dev/urandom \
-	--with-sphinx-build
+	--without-sphinx-build
 
 # Some convenience variables
 export TMPBUILD_MIT := build-mit
@@ -147,10 +147,16 @@ override_dh_auto_configure:
 	LDFLAGS="$(LDFLAGS) $(HEIMDAL_LDFLAGS)" \
 	CFLAGS="$(CFLAGS)" CPPFLAGS="$(CPPFLAGS) $(HEIMDAL_CPPFLAGS)" \
 	dh_auto_configure -B$(TMPBUILD_HEIMDAL) -- $(CONFIGURE_COMMON_OPTIONS) --with-gss_impl=heimdal
+	# when not building the docs, manpages have to be copied to the temporary
+	# build directory
+	cp -a man $(TMPBUILD_HEIMDAL)
 
 	LDFLAGS="$(LDFLAGS) $(MIT_LDFLAGS)" \
 	CFLAGS="$(CFLAGS)" CPPFLAGS="$(CPPFLAGS) $(MIT_CPPFLAGS)" \
 	dh_auto_configure -B$(TMPBUILD_MIT) -- $(CONFIGURE_COMMON_OPTIONS) --with-gss_impl=mit
+	# when not building the docs, manpages have to be copied to the temporary
+	# build directory
+	cp -a man $(TMPBUILD_MIT)
 
 	# Record the build-time settings for later reference
 	echo 'To build this package, configure was called as follows:' \