Bug#1033811: unblock: mariadb/1:10.11.2-2

[Otto Kekäläinen]

Package: release.debian.org
Severity: normal
User: release.d...@packages.debian.org
Usertags: unblock
Control: affects -1 + src:mariadb

Please unblock package 'mariadb' so that the bug fixes in the Debian
revision 2 can be included in the release of Debian 12 "Bookworm".

The package is currently 7/20 days old, but even after 20 days it will
not automatically migrate because it is listed as a key package.

Also, it has autopkgtest failure for armel/armhf. The failure is safe
to ignore because it is from a new test that had a typo[1] on these
two platforms. Autopkgtests fully pass on all other platforms. I
decided to not do a re-upload just for this typo for now, it would not
be frugal use of builders and Debian infra.


This Debian revision has been carefully crafted to only include bug
fixes, test improvements and translations (following the release
policy[2]). There are no risky functional changes, and the package has
not had any new bugs reported for several weeks and confidence that
this is the best version for Bookworm is high.

The changes can be reviewed by reading individual commits (each change
is well documented)[3]. Below summary from changelog[4]:

mariadb (1:10.11.2-2) unstable; urgency=medium
.
[ Otto Kekäläinen ]
* SUMMARY: This version has a lot of bug fixes, quality fixes, documentation
and translation updates and it is tailored for the Debian 12 "Bookworm"
release and all potentially functional changes have been left out and
are pending review and merge post-Bookworm release at
https://salsa.debian.org/mariadb-team/mariadb-server/-/merge_requests,
where contributions both as reviews and new submissions are welcome!
* Update NEWS to summarize what is new in MariaDB 10.11 (compared to 10.6)
* Update and activate configuration tracing in autopkgtests to ensure that
for the Debian 12 "Bookworm" release cycle no upstream server config change
would slip in unnoticed
* Enable mariadb-plugin-rocksdb for riscv64 (fixes autopkgtests for riscv64)
* Sync downstream (where applicable) with upstream 10.11 debian/* contents
so that using diff/meld to compare changes are easier
* Add important upstream 10.11.2+ fixes as packes:
* Stack overflow in pinbox allocator (PR#2541)
* Upgrades from MySQL 5.7 to MariaDB 10.11 (MDEV-30483) (Closes: #866751)
* Misc compiler warnings in upstream build
* Incomplete stack traces if MariaDB crashes
* Make mariadbd emit warnings if mariadb-upgrade was not run
* Binlog failures due to 'character_set_client' (MDEV-30824)
* Prevent mariadb-test-run from using native I/O on ppc64el and s390x due to
Linux kernel bug (Related: #1031656)
* Add patch to better diagnose potential failures in main.order_by_innodb
* Add patch to fix cross-compilation failure on uca-dump (Closes: #1029165)
* Update mariadb-test-run skip test lists to not run tests that are known
to be broken or unstable, and that have been reported upstream
* Limit check of running mysqld/mariadbd to system users (Closes: #1032047)
* Make error more helpful in case server restart fails (Related: #1033234)
* Update Lintian overrides after rigorous review of all Lintian issues
* Remove incorrect Multi-Arch definitions
* Fix man pages syntax issues (Closes: #1032861)
* Fix spelling in MariaDB and components (Closes: #1032860)
* Refresh patches metadata
* Update upstream signing key
* Fix dependency of obsolete libncurses5-dev
.
[ Ekaterine Papava ]
* Add Georgian translation (error messages)
.
[ Tuukka Pasanen ]
* Update README files to correct versions



[1] https://salsa.debian.org/mariadb-team/mariadb-server/-/merge_requests/42
[2] https://release.debian.org/testing/freeze_policy.html
[3] https://salsa.debian.org/mariadb-team/mariadb-server/-/commits/debian/1%2510.11.2-2
[4] https://tracker.debian.org/news/1427834/accepted-mariadb-110112-2-source-into-unstable/

[Paul Gevers]

Control: tags -1 moreinfo

Hi Otto,

On 02-04-2023 05:50, Otto Kekäläinen wrote:
> This Debian revision has been carefully crafted to only include bug
> fixes, test improvements and translations (following the release
> policy[2]). There are no risky functional changes, and the package has
> not had any new bugs reported for several weeks and confidence that
> this is the best version for Bookworm is high.

I hope you realize that you're stretching it:
68 files changed, 11039 insertions(+), 404 deletions(-)

Several of these changes are not really appropriate and should have been
done before the hard freeze. The idea of the freeze schedule is to
stabilize the archive, particularly key packages. mariadb is a key package.

One of the changes I'm not please with is that you are renaming
variables. Really, now?

Why now change from /usr/bin/mysql to /usr/bin/mariadb when the former
is a symlink to the latter. Seems like unnecessary risks. (You even seem
to have missed usr/bin/mysqladmin in mariadb-server.postrm, or is it in
postrm better to have the link?)

Renaming patches (why and why now?) also doesn't help with a review;
it's difficult to see if they are the same.

You drop the Hurd patch, but I couldn't find it documented. (Hurd has
been failing since the first unversioned mariadb, so I can guess, but
the point of unblock requests is that I shouldn't need to).

During my review several days ago I got to the
2464-log-missing-upgrade.patch. I might continue the review later but
for now I'm out of spoons.

Paul

[Otto Kekäläinen]

> I hope you realize that you're stretching it:
> 68 files changed, 11039 insertions(+), 404 deletions(-)

Yes, there are a lot of fixes. There was an extensive push to test and
polish everything from Feb to mid-March, resulting in 39 git commits.
However everything is purely about bug fixes, improved logging to help
future bugfixes, NEWS, translations etc. Everything is documented in
debian/changelog for users and even more detailed in git commit log
for package developers at
https://salsa.debian.org/mariadb-team/mariadb-server/-/commits/debian/latest

> Several of these changes are not really appropriate and should have been
> done before the hard freeze. The idea of the freeze schedule is to
> stabilize the archive, particularly key packages. mariadb is a key package.

Let's review each change now. I am happy to provide additional explanations.

> One of the changes I'm not please with is that you are renaming
> variables. Really, now?

This is about commit
https://salsa.debian.org/mariadb-team/mariadb-server/-/commit/09be495b1ca7811e1c6200215e20de38f26834ec,
right?

The variables were renamed upstream in commit
https://salsa.debian.org/mariadb-team/mariadb-server/-/commit/952af4a1794ea640213c9c3b3931c84349493a9b
that affects branches 10.5+ and was done in December 2022. Not ideal
to do in a stable release, agree, but from Debian packager point of
view this change had been in a mariadb.org release already, proven
stable, and minimizing delta to upstream Debian packaging is best for
the long-term maintenance of the package in Debian.

> Why now change from /usr/bin/mysql to /usr/bin/mariadb when the former
> is a symlink to the latter. Seems like unnecessary risks. (You even seem
> to have missed usr/bin/mysqladmin in mariadb-server.postrm, or is it in
> postrm better to have the link?)

I synced contents by carefully reviewing the full directories of
upstream and downstream in Meld Merge, and I did not miss anything,
the full upstream change is included.

Upstream 10.11 branch has the same MYADMIN="/usr/bin/mysqladmin in
https://salsa.debian.org/mariadb-team/mariadb-server/-/blob/d84a2826290d9676faebba0849d1b9fb7f5efcd8/debian/mariadb-server.postrm#L9
as we have now in Debian.

> Renaming patches (why and why now?) also doesn't help with a review;
> it's difficult to see if they are the same.

To facilitate contributions I prefer to accept occasionally even
sub-par contributions, and then fix them up myself. The case you refer
to is debian/patches/{2477.patch =>
2477-rocksdb-atomic-riscv64.patch}. I did not want to carry a patch
that is just a number from
https://salsa.debian.org/mariadb-team/mariadb-server/-/merge_requests/34,
so I renamed it to have a sensible title, and added proper metadata in
the patch.

Git renders this rename nicely both on command-line and in e.g.
https://salsa.debian.org/mariadb-team/mariadb-server/-/commit/fe0701c4455a6e9f32f9495f286a093ab22c6a26
but I guess you are reviewing just overall patch attached in this bug
report.

> You drop the Hurd patch, but I couldn't find it documented. (Hurd has
> been failing since the first unversioned mariadb, so I can guess, but
> the point of unblock requests is that I shouldn't need to).

https://salsa.debian.org/mariadb-team/mariadb-server/-/commit/fe0701c4455a6e9f32f9495f286a093ab22c6a26
> Drop 2006-kfreebsd-amd64.patch which had already been applied upstream and
> was wrongly being re-applied in Debian.

You are right, I should have included this in the debian/changelog. In
my current workflow I do full documentation in commits (that are
stored permanently on Debian Salsa) and then summarize them in the
debian/changelog to avoid the changelog being too verbose. This one I
should have been more verbose on.

> During my review several days ago I got to the
> 2464-log-missing-upgrade.patch. I might continue the review later but
> for now I'm out of spoons.

This patch adds logging - essentially it makes the server start
nagging if mariadb-upgrade has not been run. Missing the upgrade due
to bugs in maintainer scripts etc occasionally happens, and it is
really hard to get bug submitters to research this themselves or to
provide reproducible steps on what they did. Having the ability for
submitters to copy-paste this message from logs helps tremendously.

I am amazed that you review in detail the full diff attached to the
unblock request. I did not know release managers are that dedicated to
quality, that is great for Debian users!

It awes and humbles me that you are so diligent. At the same time I am
a bit worried will it scale? I sent several emails to co-maintainers,
pkg-mysql-maint list, debian-devel list and tried to rally people to
review MRs at https://salsa.debian.org/mariadb-team/mariadb-server/-/merge_requests
before these commits when in, but few responded. It is too much work
even for people to do in their spare time. The expectation that one
release manager reads all changes in all unblock requests seems
unrealistic.

Would it be OK if I try to get somebody from the LTS team or somebody
else who is considered a senior DD to review the rest of
https://salsa.debian.org/mariadb-team/mariadb-server/-/commits/debian/latest
between tags debian/1%10.11.1-5 and debian/1%10.11.2-1 so you don't
have to do everything? Would you be happy with the rest of the review
being delegated to somebody else?

- Otto

[Otto Kekäläinen]

Hi!

Can you please list the commits you do not accept so I can revert them and upload a 10.11.2-3 which you are then willing to approve?

I think it would be a huge miss if none of a month worth of bugfix work would make into the release.

[Graham Inggs]

Hi Otto

On Thu, 20 Apr 2023 at 16:44, Otto Kekäläinen <ot...@debian.org> wrote:
> Can you please list the commits you do not accept so I can revert them and upload a 10.11.2-3 which you are then willing to approve?

I've had a look at some of the bugs closed in the changelog of
10.11.2-2; #866751, #1029165, #1032047, #1032861 and #1032860, and
none of these seem to be of severity: important or release critical.
I suggest reverting to 10.11.2-1 and then only cherry-picking the
fixes from 10.11.2-2 that are appropriate for the current freeze
stage, i.e. release critical bugs, severity: important bugs, and
translation updates and documentation fixes, etc. Be sure to make it
clear in your unblock request why the changes are important, e.g.
Reason, Impact, Tests, Risks, as per the template generated by
reportbug.

Regards
Graham

[Otto Kekäläinen]

Thanks for the review. Can you please also look at commit log as it has additional information and also commit-by-commit is easier to read?

I would argue that all the work done in Feb-March was targeted for Bookworm and appropriate and low risk. As you can see in the Salsa project all improvement type work was left pending to be merged after Bookworm is out.

Anyway, thanks for the feedback, I will review these and revert a subset of bugfixes done in Feb-March and upload 1:10.11.2-4.

(I already reverted one commit Paul mentioned in 10.11.2-3 upload.)

Otto

[Otto Kekäläinen]

To summarize, the request is to revert these bugfixes:

* Upgrades from MySQL 5.7 to MariaDB 10.11 (MDEV-30483) (Closes: #866751)

* Add patch to fix cross-compilation failure on uca-dump (Closes: #1029165)

* Limit check of running mysqld/mariadbd to system users (Closes: #1032047)

* Fix man pages syntax issues (Closes: #1032861)
* Fix spelling in MariaDB and components (Closes: #1032860)

I made this into a MR at
https://salsa.debian.org/mariadb-team/mariadb-server/-/merge_requests/45
and it is now pending feedback from other contributors.

Note also related
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1036081 about new
upstream maintenance release 10.11.3 and request for decision if it
can be included in Bookworm.