Hi, Daniel, This is a review of `git diff 12786f0e779f 5f76981eb649`, that is, for commits: 5f76981eb64 rpm: packages that dont match component name 25eaa0aebfa MDEV-30205: [Cmtr: suite look in mariadb-test path 0981201f027 mtr - wsrep use mariadb executables eba0b35035a mtr: maradb names for *embedded and mariadb-client-test 974ca88f0fe deb: fix autotest - exclude for MDEV-30205 b19d75408bb Deb: Breaks/Replaces/COnflicts c60f0341950 MDEV-30203 deb breaks/replaces on compat packages f984f49fba7 MDEV-30203 - deb fix piuparts 310d025ff0d MDEV-30205: fix deb autopkgtest 89f6cf47ee2 rpm: fix packaging of compat/non-compat scripts links 494ed76d15a deb: mysqlhotcopy to mariadb-client-compat d03075f3a1f deb: server-core mysql* -> server-compat 9453812193d wsrep_sst_mariabackup to use mariadb-backup (and message accordingly) a4f843a7a5a Fix uncollected symlink eb9a469bf9a Add package recommended dependencies c135be62274 Remove duplicate entry 84c7cd29c61 Fix Debian packaging for mariabackup 5dbc7ae8bb4 MDEV-30203: compat packages - RPM remove mariabackup link eb4fa5d750f MDEV-30203: debian - make compat packages d4be0b9be73 MDEV-30203: Create mysql symlink packages (RPM) - scripts 441879fd493 MDEV-30203: Create mysql symlink packages (RPM) On Dec 19, Daniel Black wrote:
diff --git a/cmake/cpack_rpm.cmake b/cmake/cpack_rpm.cmake index 339363b2169..7b48fd64f4c 100644 --- a/cmake/cpack_rpm.cmake +++ b/cmake/cpack_rpm.cmake @@ -85,6 +85,35 @@ SET(CPACK_RPM_server_PACKAGE_DESCRIPTION "${CPACK_RPM_PACKAGE_DESCRIPTION}") SET(CPACK_RPM_test_PACKAGE_SUMMARY "MariaDB database regression test suite") SET(CPACK_RPM_test_PACKAGE_DESCRIPTION "${CPACK_RPM_PACKAGE_DESCRIPTION}")
+# "set/append array" - append a set of strings, separated by a space +MACRO(SETA var) + FOREACH(v ${ARGN}) + SET(${var} "${${var}} ${v}") + ENDFOREACH() +ENDMACRO(SETA) + +FOREACH(SYM_COMPONENT Server Client Test) + SET(SYM ${SYM_COMPONENT}_symlinks) + string(TOLOWER ${SYM} SYM)
* why all cmake commands are uppercase, but `string` is lowercase? * SET is redundant, you can do STRING(TOLOWER ${SYM_COMPONENT}_symlinks SYM) * may be, better to use ${SYM_COMPONENT}-symlinks ? Like in MariaDB-server-symlinks ?
+ SET(SYMCOMP ${SYM_COMPONENT}Symlinks) + string(TOUPPER ${SYMCOMP} SYMCOMP_UPPER) + SET(CPACK_COMPONENT_${SYMCOMP_UPPER}_GROUP "${SYM}") + SET(CPACK_COMPONENTS_ALL "${CPACK_COMPONENTS_ALL}" "${SYMCOMP}") + SET(CPACK_RPM_${SYM}_PACKAGE_SUMMARY "MySQL compatible symlinks for MariaDB database ${SYM_COMPONENT} binaries/scripts") + SET(CPACK_RPM_${SYM}_PACKAGE_DESCRIPTION "${CPACK_RPM_PACKAGE_DESCRIPTION}") + SET(CPACK_RPM_${SYM}_PACKAGE_ARCHITECTURE "noarch") + SETA(CPACK_RPM_${SYM_COMPONENT}_PACKAGE_RECOMMENDS "MariaDB-${SYM}")
You forgot to say that symlink package DEPENDS on the non-symlink package
+ENDFOREACH() + +# TODO change to 11.0 on rebase +SETA(CPACK_RPM_client_symlinks_PACKAGE_CONFLICTS + "MariaDB-client < 10.11.2" + "MariaDB-server < 10.11.2") +SETA(CPACK_RPM_server_symlinks_PACKAGE_CONFLICTS + "MariaDB-server < 10.11.2") +SETA(CPACK_RPM_test_symlinks_PACKAGE_CONFLICTS + "MariaDB-test < 10.11.2") + # libmariadb3 SET(CPACK_RPM_shared_PACKAGE_SUMMARY "LGPL MariaDB database client library") SET(CPACK_RPM_shared_PACKAGE_DESCRIPTION "This is LGPL MariaDB client library that can be used to connect to MySQL diff --git a/cmake/symlinks.cmake b/cmake/symlinks.cmake index 3f3b4e4a9b5..a4eb6c2b42c 100644 --- a/cmake/symlinks.cmake +++ b/cmake/symlinks.cmake @@ -12,7 +12,6 @@ endmacro() REGISTER_SYMLINK("mariadb" "mysql") REGISTER_SYMLINK("mariadb-access" "mysqlaccess") REGISTER_SYMLINK("mariadb-admin" "mysqladmin") -REGISTER_SYMLINK("mariadb-backup" "mariabackup")
better not. there're scripts that might be using it. This is why I suggested `strncmp(my_progname, "mariadb", 7)` and not `strncmp(my_progname, "maria", 5)` in your PR
REGISTER_SYMLINK("mariadb-binlog" "mysqlbinlog") REGISTER_SYMLINK("mariadb-check" "mysqlcheck") REGISTER_SYMLINK("mariadb-client-test-embedded" "mysql_client_test_embedded") diff --git a/debian/control b/debian/control index 81418c13656..6b225ca4b3d 100644 --- a/debian/control +++ b/debian/control @@ -548,7 +548,8 @@ Provides: default-mysql-client, virtual-mysql-client Recommends: libdbd-mariadb-perl | libdbd-mysql-perl, libdbi-perl, - libterm-readkey-perl + libterm-readkey-perl, + mariadb-client-compat
Please, no. deb and rpm packages are different enough already. don't use different suffixes for them. Choose either -symlink or -compat (or something else) and use it consistently both in deb and in rpm.
Description: MariaDB database client binaries MariaDB is a fast, stable and true multi-user, multi-threaded SQL database server. SQL (Structured Query Language) is the most popular database query @@ -558,6 +559,110 @@ Description: MariaDB database client binaries This package includes the client binaries and the additional tools innotop and mariadb-report (mysqlreport).
+Package: mariadb-client-compat +Architecture: all +Depends: mariadb-client +Multi-Arch: foreign +Description: MySQL compatibility links to mariadb-client binaries/scripts. +Conflicts: mariadb-client (<< ${source:Version}), + mariadb-client-10.0,
I wonder, would it work, if you won't set any Conflicts/Breaks, but only set Depends? It'd be a lot simpler that way (if it'll work).
+ mariadb-client-10.1, + mariadb-client-10.2, + mariadb-client-10.3, + mariadb-client-10.4, + mariadb-client-10.5, + mariadb-client-10.6, + mariadb-client-10.7, + mariadb-client-10.8, + mariadb-client-5.1, + mariadb-client-5.2, + mariadb-client-5.3, + mariadb-client-5.5, + mariadb-client-core (<< ${source:Version}), + mariadb-client-core-10.0, + mariadb-client-core-10.1, + mariadb-client-core-10.2, + mariadb-client-core-10.3, + mariadb-client-core-10.4, + mariadb-client-core-10.5, + mariadb-client-core-10.6, + mariadb-client-core-10.7, + mariadb-client-core-10.8, + mariadb-server (<< ${source:Version}), + mariadb-server-10.0, + mariadb-server-10.1, + mariadb-server-10.2, + mariadb-server-10.3, + mariadb-server-10.4, + mariadb-server-10.5, + mariadb-server-10.6, + mariadb-server-10.7, + mariadb-server-10.8, + mysql-client (<< 5.0.51), + mysql-client-5.0, + mysql-client-5.1, + mysql-client-5.5, + mysql-client-5.6, + mysql-client-5.7, + mysql-client-8.0, + mysql-client-core-5.0, + mysql-client-core-5.1, + mysql-client-core-5.5, + mysql-client-core-5.6, + mysql-client-core-5.7, + mysql-client-core-8.0, + mysql-server-core-5.7, + mysql-server-core-8.0, + percona-server-server-5.6, + percona-server-server, + percona-xtradb-cluster-server-5.6, + percona-xtradb-cluster-server-5.7, + percona-xtradb-cluster-server +Breaks: mariadb-client (<< ${source:Version}), + mariadb-client-10.0, + mariadb-client-10.1, + mariadb-client-10.2, + mariadb-client-10.3, + mariadb-client-10.4, + mariadb-client-10.5, + mariadb-client-10.6, + mariadb-client-10.7, + mariadb-client-10.8, + mariadb-client-5.1, + mariadb-client-core (<< ${source:Version}), + mariadb-client-core-10.0, + mariadb-client-core-10.1, + mariadb-client-core-10.2, + mariadb-client-core-10.3, + mariadb-client-core-10.4, + mariadb-client-core-10.5, + mariadb-client-core-10.6, + mariadb-client-core-10.7, + mariadb-client-core-10.8, + mariadb-server (<< ${source:Version}), + mariadb-server-10.0, + mariadb-server-10.1, + mariadb-server-10.2, + mariadb-server-10.3, + mariadb-server-10.4, + mariadb-server-10.5, + mariadb-server-10.6, + mariadb-server-10.7, + mariadb-server-10.8, + mysql-server-5.5, + mysql-server-5.6, + mysql-server-5.7, + mysql-server-8.0, + mysql-server-core-5.5, + mysql-server-core-5.6, + mysql-server-core-5.7, + mysql-server-core-8.0, + percona-server-server-5.6, + percona-server-server, + percona-xtradb-cluster-server-5.6, + percona-xtradb-cluster-server-5.7, + percona-xtradb-cluster-server + Package: mariadb-server-core Architecture: any Depends: mariadb-common (>= ${source:Version}), diff --git a/debian/mariadb-backup.install b/debian/mariadb-backup.install index e450f8f46a0..a288377ddd0 100644 --- a/debian/mariadb-backup.install +++ b/debian/mariadb-backup.install @@ -1,6 +1,4 @@ -usr/bin/mariabackup
same here. needs mariadb-backup-compat
usr/bin/mariadb-backup usr/bin/mbstream -usr/share/man/man1/mariabackup.1 usr/share/man/man1/mariadb-backup.1 usr/share/man/man1/mbstream.1 diff --git a/debian/mariadb-client.links b/debian/mariadb-client.links index 62e3651daf5..8ccce12c609 100644 --- a/debian/mariadb-client.links +++ b/debian/mariadb-client.links @@ -2,11 +2,6 @@ usr/bin/mariadb-check usr/bin/mariadb-analyze usr/bin/mariadb-check usr/bin/mariadb-optimize usr/bin/mariadb-check usr/bin/mariadb-repair usr/bin/mariadb-check usr/bin/mariadbcheck -usr/bin/mariadb-check usr/bin/mysqlanalyze -usr/bin/mariadb-check usr/bin/mysqlcheck -usr/bin/mariadb-check usr/bin/mysqloptimize -usr/bin/mariadb-check usr/bin/mysqlrepair -usr/bin/mariadb-report usr/bin/mysqlreport usr/share/man/man1/mariadb-check.1.gz usr/share/man/man1/mariadb-analyze.1.gz usr/share/man/man1/mariadb-check.1.gz usr/share/man/man1/mariadb-optimize.1.gz usr/share/man/man1/mariadb-check.1.gz usr/share/man/man1/mariadb-repair.1.gz
you forgot to move symlinks to man pages
diff --git a/debian/mariadb-server-compat.install b/debian/mariadb-server-compat.install new file mode 100644 index 00000000000..36701d8e0c6 --- /dev/null +++ b/debian/mariadb-server-compat.install @@ -0,0 +1,12 @@ +usr/bin/mysqld_multi +usr/bin/mysqld_safe +usr/bin/mysqld_safe_helper +usr/bin/mysql_install_db +usr/bin/mysql_upgrade +usr/sbin/mysqld +usr/share/man/man1/mysqld_multi.1 +usr/share/man/man1/mysqld_safe.1 +usr/share/man/man1/mysqld_safe_helper.1
great. do you do the same for rpms? I didn't see where manpages are moved to -symlinks packages in rpms.
+usr/share/man/man1/mysql_install_db.1 +usr/share/man/man1/mysql_upgrade.1 +usr/share/man/man8/mysqld.8 diff --git a/debian/mariadb-test-compat.install b/debian/mariadb-test-compat.install new file mode 100644 index 00000000000..7b498d7d1c8 --- /dev/null +++ b/debian/mariadb-test-compat.install @@ -0,0 +1 @@ +usr/share/mysql/mysql-test/mysql-test-run.pl
You forgot mysql-test-run, mysqltest, mysql_client_test, and their *embedded variants. But, you know, it's probably not worth a new package at all. It's the *test* package (not interesting for users, never used in production, so no immutability requirements). I suggest to keep those symlinks in the mariadb-test package. Your other PR will add a warning. And eventually we drop them. Optional/Recommended/Suggested - sounds like too much toubles for a *test* package.
diff --git a/mysql-test/lib/mtr_cases.pm b/mysql-test/lib/mtr_cases.pm index fe202279ac7..ad1ac6fdf88 100644 --- a/mysql-test/lib/mtr_cases.pm +++ b/mysql-test/lib/mtr_cases.pm @@ -387,14 +387,14 @@ sub collect_suite_name($$) else { my @dirs = my_find_dir(dirname($::glob_mysql_test_dir), - ["mysql-test/suite", @plugin_suitedirs ], + ["mariadb-test/suite", "mysql-test/suite", @plugin_suitedirs ],
Eh, no. That is, yes, but not in this set of commits, not in this MDEV
$suitename); # # if $suitename contained wildcards, we'll have many suites and # their overlays here. Let's group them appropriately. # for (@dirs) { - m@^.*/(?:mysql-test/suite|$plugin_suitedir_regex)/(.*)$@o or confess $_; + m@^.*/(?:mariadb-test/suite|mysql-test/suite|$plugin_suitedir_regex)/(.*)$@o or confess $_; push @{$suites{$1}}, $_; } }
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org