Details
-
Task
-
Status: Closed (View Workflow)
-
Major
-
Resolution: Won't Fix
-
None
Description
The patches can be found here: https://anonscm.debian.org/cgit/pkg-mysql/mariadb-10.3.git/tree/debian/patches
- scripts_mysql_create_system_tables_no_test.patch
Yes, we should do something like this along with https://github.com/MariaDB/server/pull/536 - scripts_mysqld_safe.sh_signals.patch
Probably. - scripts_mysql_install_db.sh_no_test.patch
Yes, we should do something like this along with https://github.com/MariaDB/server/pull/536 - mysql-test__db_test.patch
Yes, we should do something like this along with https://github.com/MariaDB/server/pull/536 - replace_dash_with_bash_mbug675185.patch
Yes. - extend_default_test_timeout_for_tokudb.patch
Does it really take 9 hours to run the whole test suite? - innodb_simulate_comp_test_speedup.patch
Yes, looks nice. - mysqld_multi_confd.patch
Probably, we need to make sure it works well with RPM/tarball. - mysqld_multi.server_lsb-header.patch
This looks Debian specific. Do we still need this script? - mips-groonga-atomic.patch
This is most probably fixed. - mips-connect-unaligned.patch
The problem is valid, but we already have routines for loading/storing unalgined data. - mips-machine.patch
Don't we have this since quite a while already? - hurd_socket.patch
Yes. - armhf_mroonga_storage_fail.patch
Why? - remove-systemd-obsolete-target.patch
Yes, likely. IIRC there was PR fixing it. - kFreeBSD-gettid.patch
Yes, but the patch seem to be wrong: shouldn't it do "return thr_self()" instead of "thr_self(&lwpid)?" - 0018-Fix-the-awk-path-on-Debian.patch
Yes, but we just need to remove "#!/usr/bin/awk". - mytop-merge_src:mytop_improvements.patch
Probably. - Add_default_ExecStartPre_to_mariadb@.service.patch
Yes, it is likely a bug. - 0021-Revert-to-using-system-pcre-library.patch
Yes, but we need to check why did we bondle it in first place. - 0025-Change-the-default-optimization-from-O3-to-O2-in-mys.patch
Why?
Attachments
Issue Links
- relates to
-
MDEV-16318 mysqld_safe: use sh not bash
-
- Closed
-
-
-
- Open
-
-
-
- Closed
-
-
-
- Closed
-
-
-
- Open
-
Activity
33,41,50 - all fixed by https://github.com/MariaDB/server/pull/536
61 - seem related to MDEV-3279 which was a dash bug fixed in 2009 - https://git.kernel.org/pub/scm/utils/dash/dash.git/commit/?id=3800d4934391b144fd261a7957aea72ced7d47ea - think that can be ignored. Was there another reason? For FreeBSD's lets keep it as sh if at all possible.
38 - is there some explanation as to why these changes are needed?
Sorry, I don't have any additional knowledge of the patches. I don't fully understand them.
For 38 the history does not reveal anything useful either: https://github.com/MariaDB/server/commits/10.3/debian/patches/38_scripts__mysqld_safe.sh__signals.dpatch
By the date on the 38 patch's changelog, signal handling I suspect its all handled within mysqld now.
Certainly:
1 (HUP) - handled as reload https://github.com/MariaDB/server/blob/10.3/sql/mysqld.cc#L3582
2 (INT) - https://github.com/MariaDB/server/blob/10.3/sql/mysqld.cc#L3554
3 (QUIT) / 15 (TERM) - https://github.com/MariaDB/server/blob/10.3/sql/mysqld.cc#L3554
The 'cmd & wait' looks like it might be trying to become a sub-process early so manually killing the mysqld_safe script doesn't affect mysqld. As odd as it looks this bit might be useful.
https://anonscm.debian.org/cgit/pkg-mysql/mariadb-10.3.git/tree/debian/patches/remove-systemd-obsolete-target.patch and https://anonscm.debian.org/cgit/pkg-mysql/mariadb-10.3.git/tree/debian/patches/Add_default_ExecStartPre_to_mariadb@.service.patch - is https://github.com/MariaDB/server/pull/387
svoj Please take a look at this commit https://anonscm.debian.org/cgit/pkg-mysql/mariadb-10.3.git/commit/?id=11384b0f3c1273e37e4a77dc09779bd3d71dce98
In that commit oerdnj changed the patch format to use a more modern Debian tooling. As a side effect, the Description fields of each patch was lost. If you look at that commit you can still see the descriptions and reviewing is easier. Sorry for not noticing this earlier.
Here is the patch you asked about the purpose with decription: https://anonscm.debian.org/cgit/pkg-mysql/mariadb-10.3.git/diff/debian/patches/armhf_mroonga_storage_fail.patch?id=11384b0f3c1273e37e4a77dc09779bd3d71dce98 More info about it can be asked from cvicentiu
If you eventually provide me a list of patches you think are upstreamed as such or in another implementation, I will bluntly remove them from downstream and see what happens. I don't have expertise to judge the patches if they have some aspect that is still needed and not in upstream already.
Wow - what a lost in useful information.
https://anonscm.debian.org/cgit/pkg-mysql/mariadb-10.3.git/tree/debian/patches/kFreeBSD-gettid.patch should be pushed https://github.com/percona/PerconaFT they do get to merging patches eventually.
mysqld_multi_confd.patch seems based on the mysql bugs to be about not processing include/includedir. mysqd_multi does now and should probably be dropped.
mysqld_multi.server_lsb-header.patch no point updating this if you don't install it. There's a few prs about improving the systemd multi instance service definition.
innodb_simulate_comp_test_speedup.patch - depending on this patch is wrong. innodb has changed a lot so remove and see if this test fails again.
hurd_socket.patch - seems harmless to include
extend_default_test_timeout_for_tokudb.patch - MTR_FORCE is used by a number of targets not directly used by debian/rules. Could be dropped. If I've missed something the testcase timeout and suite-timeout can be controlled by environment variables in the debian build infrastructure. Hopefully improved hardware since 2014 means this won't be required.
armhf_mroonga_storage_fail.patch as cvicentiu said, should be fixed upstream. Can't see a reason to drop/rework this at the moment. Unconditionally applying it and letting the complier is probably an ok call too for the same reasons mentioned in the former header of the mips patches.
mips patches. can/should be merged upstream. seem harmless enough.
unstable -tests - couldn't see any in 10.3 that where disabled for mips. multi_source.gtid masked due to MDEV-14202, rpl.rpl_mdev6020 not currently masked - does it still fail on MIPs? - if so MDEV needed. Seems excessive to maintain this file its well maintained by Elena anyway.
svoj So oerdnj removed some patches and all patch descriptions in commit https://anonscm.debian.org/cgit/pkg-mysql/mariadb-10.3.git/commit/debian/patches?h=master&id=11384b0f3c1273e37e4a77dc09779bd3d71dce98
So I recovered that commit into a branch you can easily browse at https://anonscm.debian.org/cgit/pkg-mysql/mariadb-10.3.git/tree/debian/patches?h=patches-with-descriptions
mysqld_multi_confd.patch - just needs to use INSTALL_SYSCONF2DIR
remove-systemd-obsolete-target.patch & Add_default_ExecStartPre_to_mariadb@.service.patch - part of https://github.com/MariaDB/server/pull/387 (FROM May 8, 2017) - getting sick of rebasing this one.
replace_dash_with_bash_mbug675185.patch - er, NO, see above.
kFreeBSD-gettid.patch
> Yes, but the patch seem to be wrong: shouldn't it do "return thr_self()" instead of "thr_self(&lwpid)?"
NO, thd_self on FreeBSD requires an argument.
( https://www.freebsd.org/cgi/man.cgi?query=thr_self&apropos=0&sektion=0&manpath=FreeBSD+11.0-RELEASE+and+Ports&arch=default&format=html )
Is anyone going to push this upstream to https://github.com/percona/PerconaFT ? I think I've done enough to fix the lack of Debian up-streaming already.
Here are steps how to get the patch files from the new salsa server:
git clone https://salsa.debian.org/mariadb-team/mariadb-10.1.git |
cd mariadb-10.1 |
git checkout 2d711f6c02fcc964d56bdeed65660f489910dc18
|
cd debian/patches
|
ls
|
|
|
armhf_mroonga_storage_fail.patch mips-unstable-tests.patch
|
c11_atomics.patch mysqld_multi_confd.patch
|
extend_default_test_timeout_for_tokudb.patch mysqld_multi.server_lsb-header.patch
|
fix-spelling-errors.patch mysql-test__db_test.patch
|
hurd_socket.patch remove_rename_mariadb-server_files_in.patch
|
innodb_simulate_comp_test_speedup.patch remove-systemd-obsolete-target.patch
|
kFreeBSD-gettid.patch replace_dash_with_bash_mbug675185.patch
|
libmariadbclient-rename.patch scripts__mysql_create_system_tables__no_test.patch
|
man_pages_wsrep.patch scripts__mysqld_safe.sh__signals.patch
|
mips-connect-unaligned.patch scripts__mysql_install_db.sh__no_test.patch
|
mips-groonga-atomic.patch series
|
mips-machine.patch
|
Some are already upstreamed, but most not. This is the revision where the "Description" fields are intact (before Ondrej removed them) and thus easiest to review/decide on if the patch should go upstream or not.
Goal: the downstream directory debian/patches should be empty. All patches should be either upstreamed (if they are OK), or deleted (if the fix was wrong and upstream will produce a better fix).
This is the latest list of patches we have for MariaDB 10.1 in Debian Stretch (current stable release):
- 0024-Revert-to-using-system-pcre-library.patch
- There should be a build flag for this upstream so no patching would be required in Debian?
- 0025-Change-the-default-optimization-from-O3-to-O2-in-mys.patch
- Is this change incorrect, should the patch be removed from Debian?
- 0026-Mroonga-fix-ice-arm64.patch
- Written by cvicentiu
- Add_default_ExecStartPre_to_mariadb@.service.patch
- This should probably be upstreamed?
- armhf_mroonga_storage_fail.patch
- Patch with description intact: https://salsa.debian.org/mariadb-team/mariadb-10.1/blob/b5d41be0a6bac84708b44d8188042f17d44c061e/debian/patches/armhf_mroonga_storage_fail.patch
- Written by cvicentiu
- c11_atomics.patch
- extend_default_test_timeout_for_tokudb.patch
- Patch with description intact: https://salsa.debian.org/mariadb-team/mariadb-10.1/blob/b5d41be0a6bac84708b44d8188042f17d44c061e/debian/patches/extend_default_test_timeout_for_tokudb.patch
- This modifies how tests run in Debian and the patch is OK to keep.
- fix-FTBFS-on-GNU-Hurd.patch
- Patch for patch in https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=882062
- psergey is merging this upstream
- hurd_socket.patch
- innodb_simulate_comp_test_speedup.patch
- Patch with description intact: https://salsa.debian.org/mariadb-team/mariadb-10.1/blob/b5d41be0a6bac84708b44d8188042f17d44c061e/debian/patches/innodb_simulate_comp_test_speedup.patch
- Remove it once there is a confirmed fix upstream.
- kFreeBSD-gettid.patch
- Patch with description intact: https://salsa.debian.org/mariadb-team/mariadb-10.1/blob/b5d41be0a6bac84708b44d8188042f17d44c061e/debian/patches/kFreeBSD-gettid.patch
- This should be upstreamed?
- libmariadbclient-rename.patch
- Patch with description intact: https://salsa.debian.org/mariadb-team/mariadb-10.1/blob/b5d41be0a6bac84708b44d8188042f17d44c061e/debian/patches/libmariadbclient-rename.patch
- Remove this when upstream or obsolete?
- mips-connect-unaligned.patch
- mips-groonga-atomic.patch
- mips-innobase-atomic.patch
- mips-machine.patch
- mysqld_multi_confd.patch
- mysqld_multi.server_lsb-header.patch
- mysql-test__db_test.patch
- This has been merged in 10.3
- mytop-merge_src:mytop_improvements.patch
- I don't get this nor its context at all.
- remove_rename_mariadb-server_files_in.patch
- remove-systemd-obsolete-target.patch
- replace_dash_with_bash_mbug675185.patch
- Dash has probably been fixed since. Do not upstream it an try removing it from Debian as well.
- scripts_mysql_create_system_tables_no_test.patch
- This has been merged in 10.3: https://github.com/MariaDB/server/commit/9a8498066865b508239b36853403f2700800af2b
- scripts_mysqld_safe.sh_signals.patch
- This has been merged in 10.3: https://github.com/MariaDB/server/commit/64094e12c0e265e4e41ff1d780598ba945344dc6
- scripts_mysql_install_db.sh_no_test.patch
- This has been merged in 10.3: https://github.com/MariaDB/server/commit/9a8498066865b508239b36853403f2700800af2b
extend_default_test_timeout_for_tokudb.patch — not applicable anymore
scripts_mysql_create_system_tables_no_test.patch — already done
fix-FTBFS-on-GNU-Hurd.patch - adjusted and applied (patch-to-that patch, fix for debian bug# 882062 is applied as well)
> remove-systemd-obsolete-target.patch — not applied
Why? Widely removed from fedora and systemd > 202 (http://opensuse.14.x6.nabble.com/reminder-for-systemd-services-do-not-use-syslog-target-anymore-td4991467.html). Even RHEL/Centos7 is version 219.
Considering https://github.com/MariaDB/server/pull/387 instead?
> kFreeBSD-gettid.patch
> This should be upstreamed?
Yes. To: https://github.com/percona/PerconaFT
Why? Because I'm no systemd expert and "not needed in Debian 9" argument didn't sound particularly reassuring that the change won't blow up anywhere else.
But you seem to know that these lines are not needed in (at least some) other distros as well. And you've seen buildbot so you know on what distros we build. So if you think the patch is safe to apply, I can do it.
> "not needed in Debian 9" argument didn't sound particularly reassuring that the change won't blow up anywhere else.
True. Having recently pushed two reverts of debian patches I certainly follow that.
Yes the remove-systemd-obsolete-target.patch is safe (10.2 branch). Oldest systemd version is systemd 210 as found in SLES12 looking in the kernel logs in buildbot. Older SLES/RHEL/Centos versions don't have systemd.
Changed topic to be about upstreamin 10.4 patches in Debian. For latest status, see https://salsa.debian.org/mariadb-team/mariadb-10.4/-/tree/master/debian/patches
It would be perfect if some of these patches landed in 10.5 before it goes GA. Any change cvicentiu you could take a look in the following weeks?
Related:
- kFreeBSD building https://github.com/mariadb-corporation/mariadb-connector-c/pull/139 (merged)
- mytop: https://jira.mariadb.org/browse/MDEV-4476?focusedCommentId=156605&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-156605
- -O2 vs -O3 https://jira.mariadb.org/browse/MDEV-19734?focusedCommentId=156606&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-156606
- use system PCRE when possible https://jira.mariadb.org/browse/MDEV-22892
Documented in downstream patches the latest status in https://salsa.debian.org/mariadb-team/mariadb-10.4/-/commit/d5b206fc7dba8fab2f92c2c09796ffdc5d48a0ab
Kinda fixed as much as possible. Remaining outstanding bits are in new MDEVs.
Making won't fix here because to mark as fixed would somehow involve a version to be specified which isn't possible given the spread of of fixed versions.
Current status for https://salsa.debian.org/mariadb-team/mariadb-10.5/-/tree/master/debian/patches is pretty good and indeed all patches are in their own MDEVs now or I have sent email to their authors urging them to submit upstream themselves.
We are also dragging along patches inside our own repository. Still in 10.3 branch we have files under debian/patches/. It does not really make sense for us as upstream to have patches on ourselves. Please also review them.
If the patches inside our own source repository serves a purpose that isn't in any way fulfilled by current master sources, then I think we need to factor in them some how. Maybe have some CMake switch that activates the alternative behaviour or something.