maria-developers team mailing list archive

[Daniel Black <daniel@xxxxxxxxxxx>]

updated as bb-11.0-danielblack-MDEV-30203-pkgtest-symlink-packages on top
of bb-11.0-serg branch

On Tue, Dec 20, 2022 at 2:16 AM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:

> 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)
>
> ack


> * may be, better to use ${SYM_COMPONENT}-symlinks ?
>   Like in MariaDB-server-symlinks ?
>

Ack, at some point it was assumed to be finicky about "-", but really it
was the case.


>
> > +  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
>
>
ack


> > +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
>
>
ok. Moved to direct link creation as REGISTGER_SYMLINK is used to determine
compat links.


> >  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.
>
>
converged to -compat



> >  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).
>
>

sure, lets try - pushed
bb-11.0-danielblack-MDEV-30203-pkgtest-symlink-packages

https://salsa.debian.org/grooverdan/mariadb-server/-/pipelines/472980




> > --- a/debian/mariadb-backup.install
> > +++ b/debian/mariadb-backup.install
> > @@ -1,6 +1,4 @@
> > -usr/bin/mariabackup
>
> same here. needs mariadb-backup-compat
>
>
for the time being I've kept mariadb-backup as the only package (no
mariadb-backup-compat) and it still has the exe symlink



> >  usr/bin/mariadb-backup
> >  usr/bin/mbstream
> > -usr/share/man/man1/mariabackup.1
>

but this man link is removed.


> >  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
>

opps. Late push coming.


> > 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?
>

 $ rpm -qlp
/home/dan/repos/build-mariadb-server-10.11/_CPack_Packages/Linux/RPM/RPMS/noarch/MariaDB-client-compat-11.0.0-1.fc36.noarch.rpm
/usr/bin/mysql
/usr/bin/mysql_embedded
/usr/bin/mysql_find_rows
/usr/bin/mysql_plugin
/usr/bin/mysql_tzinfo_to_sql
/usr/bin/mysql_waitpid
/usr/bin/mysqlaccess
/usr/bin/mysqladmin
/usr/bin/mysqlbinlog
/usr/bin/mysqlcheck
/usr/bin/mysqldump
/usr/bin/mysqlimport
/usr/bin/mysqlshow
/usr/bin/mysqlslap
/usr/share/man/man1/mysql.1.gz
/usr/share/man/man1/mysql_convert_table_format.1.gz
/usr/share/man/man1/mysql_embedded.1.gz
/usr/share/man/man1/mysql_find_rows.1.gz
/usr/share/man/man1/mysql_plugin.1.gz
/usr/share/man/man1/mysql_secure_installation.1.gz
/usr/share/man/man1/mysql_setpermission.1.gz
/usr/share/man/man1/mysql_tzinfo_to_sql.1.gz
/usr/share/man/man1/mysql_waitpid.1.gz
/usr/share/man/man1/mysqlaccess.1.gz
/usr/share/man/man1/mysqladmin.1.gz
/usr/share/man/man1/mysqlbinlog.1.gz
/usr/share/man/man1/mysqlcheck.1.gz
/usr/share/man/man1/mysqldump.1.gz
/usr/share/man/man1/mysqldumpslow.1.gz
/usr/share/man/man1/mysqlhotcopy.1.gz
/usr/share/man/man1/mysqlimport.1.gz
/usr/share/man/man1/mysqlshow.1.gz
/usr/share/man/man1/mysqlslap.1.gz

$ rpm -qlp
/home/dan/repos/build-mariadb-server-10.11/_CPack_Packages/Linux/RPM/RPMS/noarch/MariaDB-server-compat-11.0.0-1.fc36.noarch.rpm

/usr/bin/mysql_fix_extensions
/usr/bin/mysql_install_db
/usr/bin/mysql_upgrade
/usr/bin/mysqld_multi
/usr/bin/mysqld_safe
/usr/bin/mysqld_safe_helper
/usr/sbin/mysqld
/usr/share/man/man1/mysql.server.1.gz
/usr/share/man/man1/mysql_fix_extensions.1.gz
/usr/share/man/man1/mysql_install_db.1.gz
/usr/share/man/man1/mysql_upgrade.1.gz
/usr/share/man/man1/mysqld_multi.1.gz
/usr/share/man/man1/mysqld_safe.1.gz
/usr/share/man/man1/mysqld_safe_helper.1.gz
/usr/share/man/man8/mysqld.8.gz

I didn't see where manpages are moved to -symlinks packages in rpms.
>

Addressed now in man/CMakeList.txt


>
> > +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.
>


sure. Dropped test-compat.


>
> > 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
>

Ack. MDEV-30205 WIP for this.