Re: [Maria-developers] cf70893ac9d: MDEV-21303 Make executables MariaDB named
Hi, Rasmus! See my comments/questions below: On Mar 16, Rasmus Johansson wrote:
revision-id: cf70893ac9d (mariadb-10.5.0-391-gcf70893ac9d) parent(s): 56402e84b5b author: Rasmus Johansson <razze@iki.fi> committer: Rasmus Johansson <razze@iki.fi> timestamp: 2020-03-16 11:10:25 +0000 message:
MDEV-21303 Make executables MariaDB named
To change all executables to have a mariadb name I had to: - Do name changes in every CMakeLists.txt that produces executables - CREATE_MARIADB_SYMLINK was removed and GET_SYMLINK added by Wlad to reuse the function in other places also - The scripts/CMakeLists.txt could make use of GET_SYMLINK instead of introducing redundant code, but I thought I'll leave that for next release - A lot of changes to debian/.install and debian/.links files due to swapping of real executable and symlink. I did not however change the name of the manpages, so the real name is still mysql there and mariadb are symlinks. - The Windows part needed a change now when we made the executables mariadb -named. MSI (and ZIP) do not support symlinks and to not break backward compatibility we had to include mysql named binaries also. Done by Wlad
diff --git a/cmake/mysql_add_executable.cmake b/cmake/mysql_add_executable.cmake index eec370d51af..f4d71ae9cef 100644 --- a/cmake/mysql_add_executable.cmake +++ b/cmake/mysql_add_executable.cmake @@ -48,7 +48,7 @@ FUNCTION (MYSQL_ADD_EXECUTABLE) ENDIF()
IF (ARG_WIN32) - SET(WIN32 WIN32) + SET(WIN32 ARG_WIN32)
This looks wrong, see below
ELSE() UNSET(WIN32) ENDIF() @@ -62,6 +62,7 @@ FUNCTION (MYSQL_ADD_EXECUTABLE) ELSE() UNSET(EXCLUDE_FROM_ALL) ENDIF() + ADD_EXECUTABLE(${target} ${WIN32} ${MACOSX_BUNDLE} ${EXCLUDE_FROM_ALL} ${sources})
here it'll pass ARG_WIN32 to ADD_EXECUTABLE. But ADD_EXECUTABLE takes an optional WIN32 keyword, not ARG_WIN32.
# tell CPack where to install @@ -79,16 +80,49 @@ FUNCTION (MYSQL_ADD_EXECUTABLE) IF (COMP MATCHES ${SKIP_COMPONENTS}) RETURN() ENDIF() + IF (WITH_STRIPPED_CLIENT AND NOT target STREQUAL mysqld)
this should be `STREQUAL mariadbd` now, I believe.
INSTALL(CODE "SET(CMAKE_INSTALL_DO_STRIP 1)" COMPONENT ${COMP}) SET(reset_strip ON) ENDIF() - MYSQL_INSTALL_TARGETS(${target} DESTINATION ${ARG_DESTINATION} COMPONENT ${COMP}) + + IF(NOT ${mariadbname} STREQUAL "")
where is mariadbname set?
+ MYSQL_INSTALL_TARGETS(${mariadbname} DESTINATION ${ARG_DESTINATION} COMPONENT ${COMP}) + ELSE() + MYSQL_INSTALL_TARGETS(${target} DESTINATION ${ARG_DESTINATION} COMPONENT ${COMP}) + ENDIF() + IF (reset_strip) INSTALL(CODE "SET(CMAKE_INSTALL_DO_STRIP 0)" COMPONENT ${COMP}) ENDIF() ENDIF()
- # create mariadb named symlink - CREATE_MARIADB_SYMLINK(${target} ${ARG_DESTINATION} ${COMP}) + # create MySQL named "legacy links" + + # Windows note: + # Here, hardlinks are used, because cmake can't install symlinks. + # In packages, there are won't be links, just copies.
better to put the comment below in the ELSE branch, not here, far from the code that it is supposed to explain.
+ GET_SYMLINK(${target} link) + IF(link) + IF(UNIX) + ADD_CUSTOM_COMMAND(TARGET ${target} POST_BUILD + COMMAND ${CMAKE_COMMAND} -E create_symlink + ${target} ${link} + COMMENT "Creating ${link} link" + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}) + INSTALL(PROGRAMS + ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${link} + DESTINATION + ${ARG_DESTINATION} + COMPONENT ${COMP}) + ELSE() + SET(link ${link}.exe) + ADD_CUSTOM_COMMAND(TARGET ${target} POST_BUILD + COMMAND cmake -E remove -f ${link} + COMMAND mklink /H ${link} $<TARGET_FILE_NAME:${target}> + COMMENT "Creating ${link} link" + WORKING_DIRECTORY $<TARGET_FILE_DIR:${target}>) + INSTALL(PROGRAMS $<TARGET_FILE_DIR:${target}>/${link} DESTINATION ${ARG_DESTINATION} COMPONENT ${COMP}) + ENDIF() + ENDIF() ENDFUNCTION() diff --git a/cmake/symlinks.cmake b/cmake/symlinks.cmake index ec638bc82de..e040ff19f77 100644 --- a/cmake/symlinks.cmake +++ b/cmake/symlinks.cmake @@ -9,68 +9,46 @@ macro(REGISTER_SYMLINK from to) endmacro()
# MariaDB names for executables -REGISTER_SYMLINK("mysql_client_test_embedded" "mariadb-client-test-embedded") -REGISTER_SYMLINK("mysql_client_test" "mariadb-client-test") +REGISTER_SYMLINK("mariadb-client-test-embedded" "mysql_client_test_embedded") +REGISTER_SYMLINK("mariadb-client-test" "mysql_client_test") ...
I suppose you need to make sure that everything works with new names, without symlinks. That is all internal tools and scripts invoke other tools and scripts by their mariadb* names. A way to do it could be to comment out all REGISTER_SYMLINK lines and fix whatever that will break. grep is also a useful tool here, but with rather low signal-to-noise ratio.
+ +MACRO(GET_SYMLINK name out) + set(${out}) + list(FIND MARIADB_SYMLINK_FROMS ${name} _index) if (${_index} GREATER -1) - list(GET MARIADB_SYMLINK_TOS ${_index} mariadbname) - endif() - - if (mariadbname) - CREATE_MARIADB_SYMLINK_IN_DIR(${src} ${mariadbname} ${dir} ${comp}) - endif() -endmacro(CREATE_MARIADB_SYMLINK) - -# Add MariaDB symlinks in directory -macro(CREATE_MARIADB_SYMLINK_IN_DIR src dest dir comp) - if(UNIX) - add_custom_target( - SYM_${dest} ALL - DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/${dest} - ) - - add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${dest} POST_BUILD - COMMAND ${CMAKE_COMMAND} -E create_symlink ${src} ${dest} - COMMENT "mklink ${src} -> ${dest}") - - install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${dest} DESTINATION ${dir} COMPONENT ${comp}) + list(GET MARIADB_SYMLINK_TOS ${_index} ${out}) endif() -endmacro(CREATE_MARIADB_SYMLINK_IN_DIR) +ENDMACRO() diff --git a/debian/mariadb-backup.install b/debian/mariadb-backup.install index 2bc61c12079..e1967495b00 100644 --- a/debian/mariadb-backup.install +++ b/debian/mariadb-backup.install @@ -1,5 +1,4 @@ -usr/bin/mariabackup +usr/bin/mariadb-backup usr/bin/mbstream usr/share/man/man1/mariabackup.1 -usr/share/man/man1/mariadb-backup.1
this is strange. Why did you remove mariadb-backup.1 and not mariabackup.1 ?
usr/share/man/man1/mbstream.1 diff --git a/debian/mariadb-client-10.5.install b/debian/mariadb-client-10.5.install index 67c0c2619c3..216e4015851 100644 --- a/debian/mariadb-client-10.5.install +++ b/debian/mariadb-client-10.5.install @@ -1,29 +1,17 @@ debian/additions/innotop/innotop usr/bin/ debian/additions/mysqlreport usr/bin/ +usr/bin/mariadb-access +usr/bin/mariadb-admin usr/bin/mariadb-conv -usr/bin/mysql_find_rows -usr/bin/mysql_fix_extensions -usr/bin/mysql_waitpid -usr/bin/mysqlaccess -usr/bin/mysqladmin -usr/bin/mysqldump -usr/bin/mysqldumpslow -usr/bin/mysqlimport -usr/bin/mysqlshow
you removed mysqlshow here, and added mariadb-show to the server package. Intentional? UPD: I see that before your patch mysqlshow was in the client package, while mariadb-show symlink was in the server package. It's a bug, both should be in the client package.
+usr/bin/mariadb-dump +usr/bin/mariadb-dumpslow +usr/bin/mariadb-find-rows +usr/bin/mariadb-fix-extensions +usr/bin/mariadb-import +usr/bin/mariadb-slap +usr/bin/mariadb-waitpid usr/bin/mysqlslap
you did not remove mysqlslap, but added mariadb-slap
usr/bin/mytop -usr/share/man/man1/mariadb-access.1 -usr/share/man/man1/mariadb-admin.1 -usr/share/man/man1/mariadb-binlog.1 -usr/share/man/man1/mariadb-conv.1 -usr/share/man/man1/mariadb-dump.1 -usr/share/man/man1/mariadb-dumpslow.1 -usr/share/man/man1/mariadb-find-rows.1 -usr/share/man/man1/mariadb-fix-extensions.1 -usr/share/man/man1/mariadb-import.1 -usr/share/man/man1/mariadb-plugin.1 -usr/share/man/man1/mariadb-slap.1 -usr/share/man/man1/mariadb-waitpid.1
same weirdness as elsewhere, mariadb*.1 man pages are symlinks to mysql*.1 manpages. Should be vice versa.
usr/share/man/man1/mysql_find_rows.1 usr/share/man/man1/mysql_fix_extensions.1 usr/share/man/man1/mysql_waitpid.1 diff --git a/debian/mariadb-client-10.5.links b/debian/mariadb-client-10.5.links index 5d966575b76..d1bdfcfa3c8 100644 --- a/debian/mariadb-client-10.5.links +++ b/debian/mariadb-client-10.5.links @@ -1,21 +1,19 @@ -usr/bin/mysql_find_rows usr/bin/mariadb-find-rows -usr/bin/mysql_fix_extensions usr/bin/mariadb-fix-extensions -usr/bin/mysql_plugin usr/bin/mariadb-plugin -usr/bin/mysql_waitpid usr/bin/mariadb-waitpid -usr/bin/mysqlaccess usr/bin/mariadb-access -usr/bin/mysqladmin usr/bin/mariadb-admin -usr/bin/mysqlbinlog usr/bin/mariadb-binlog
you removed mysqlbinlog, but added mariadb-binlog to the server package. Intentional? same for mysql_plugin/mariadb-plugin. UPD: mysqlbinlog was in the server package, while mariadb-binlog symlink was in the client package. So your change is correct, both will be in the server package now. Good. same for mysql_plugin/mariadb-plugin.
-usr/bin/mysqlcheck usr/bin/mariadb-analyze -usr/bin/mysqlcheck usr/bin/mariadb-optimize diff --git a/debian/mariadb-plugin-rocksdb.install b/debian/mariadb-plugin-rocksdb.install index 80987612c30..0fc868a0721 100644 --- a/debian/mariadb-plugin-rocksdb.install +++ b/debian/mariadb-plugin-rocksdb.install @@ -2,6 +2,5 @@ etc/mysql/conf.d/rocksdb.cnf etc/mysql/mariadb.conf.d usr/bin/myrocks_hotbackup usr/bin/mysql_ldb
you didn't replace mysql_ldb with mariadb-ldb?
usr/lib/mysql/plugin/ha_rocksdb.so -usr/share/man/man1/mariadb-ldb.1 usr/share/man/man1/myrocks_hotbackup.1 usr/share/man/man1/mysql_ldb.1 diff --git a/debian/mariadb-server-10.5.install b/debian/mariadb-server-10.5.install index 4a860ff57df..38d9f984276 100644 --- a/debian/mariadb-server-10.5.install +++ b/debian/mariadb-server-10.5.install @@ -14,22 +14,23 @@ usr/bin/aria_pack usr/bin/aria_read_log usr/bin/galera_new_cluster usr/bin/galera_recovery +usr/bin/mariadb-binlog +usr/bin/mariadb-convert-table-format +usr/bin/mariadb-hotcopy +usr/bin/mariadb-plugin +usr/bin/mariadb-secure-installation usr/bin/mariadb-service-convert +usr/bin/mariadb-setpermission +usr/bin/mariadb-show +usr/bin/mariadb-tzinfo-to-sql +usr/bin/mariadbd-multi +usr/bin/mariadbd-safe +usr/bin/mariadbd-safe
duplicate line ^^^
usr/bin/msql2mysql usr/bin/myisam_ftdump usr/bin/myisamchk usr/bin/myisamlog usr/bin/myisampack -usr/bin/mysql_convert_table_format -usr/bin/mysql_plugin -usr/bin/mysql_secure_installation -usr/bin/mysql_setpermission -usr/bin/mysql_tzinfo_to_sql -usr/bin/mysqlbinlog -usr/bin/mysqld_multi -usr/bin/mysqld_safe -usr/bin/mysqld_safe_helper
you don't install mariadbd-safe-helper. Perhaps that's where your duplicate line comes from, it's not a duplicate, it's truncated :)
-usr/bin/mysqlhotcopy usr/bin/perror usr/bin/replace usr/bin/resolve_stack_dump diff --git a/debian/mariadb-server-10.5.links b/debian/mariadb-server-10.5.links index f2d97460371..e3a2d68541a 100644 --- a/debian/mariadb-server-10.5.links +++ b/debian/mariadb-server-10.5.links @@ -1,9 +1,20 @@ -usr/bin/mysql_convert_table_format usr/bin/mariadb-convert-table-format -usr/bin/mysql_secure_installation usr/bin/mariadb-secure-installation -usr/bin/mysql_setpermission usr/bin/mariadb-setpermission -usr/bin/mysql_tzinfo_to_sql usr/bin/mariadb-tzinfo-to-sql -usr/bin/mysqld_multi usr/bin/mariadbd-multi -usr/bin/mysqld_safe usr/bin/mariadbd-safe -usr/bin/mysqld_safe_helper usr/bin/mariadbd-safe-helper -usr/bin/mysqlhotcopy usr/bin/mariadb-hotcopy -usr/bin/mysqlshow usr/bin/mariadb-show +usr/bin/mariadb-binlog usr/bin/mysqlbinlog +usr/bin/mariadb-convert-table-format usr/bin/mysql_convert_table_format +usr/bin/mariadb-hotcopy usr/bin/mysqlhotcopy +usr/bin/mariadb-plugin usr/bin/mysql_plugin +usr/bin/mariadb-secure-installation usr/bin/mysql_secure_installation +usr/bin/mariadb-setpermission usr/bin/mysql_setpermission +usr/bin/mariadb-show /usr/bin/mysqlshow
this is the only line in your whole patch where you symlink by the absolute path. Looks like a typo.
+usr/bin/mariadb-tzinfo-to-sql usr/bin/mysql_tzinfo_to_sql +usr/bin/mariadbd-multi usr/bin/mysqld_multi +usr/bin/mariadbd-safe usr/bin/mysqld_safe +usr/bin/mariadbd-safe-helper usr/bin/mysqld_safe_helper diff --git a/libmariadb b/libmariadb index 3be5897c334..ca68b114ba7 160000 --- a/libmariadb +++ b/libmariadb @@ -1 +1 @@ -Subproject commit 3be5897c3346639fa6d7195480d93108798c4917 +Subproject commit ca68b114ba7e882e8abe8888204d675c4d8751f8
This is clearly wrong, please make sure you've removed it from your commit before pushing!
diff --git a/scripts/CMakeLists.txt b/scripts/CMakeLists.txt index 7be46ac1985..350fbd14cf6 100644 --- a/scripts/CMakeLists.txt +++ b/scripts/CMakeLists.txt @@ -244,8 +232,20 @@ SET(mysqlaccess_COMPONENT COMPONENT Client) SET(mysql_find_rows_COMPONENT COMPONENT Client) SET(mytop_COMPONENT Mytop)
+MACRO(INSTALL_LINK old new destination component) + EXECUTE_PROCESS( + COMMAND ${CMAKE_COMMAND} -E create_symlink ${old} ${new} + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} + ) + INSTALL( + PROGRAMS ${CMAKE_CURRENT_BINARY_DIR}/${new} + DESTINATION ${destination} + COMPONENT ${component} + ) +ENDMACRO() + IF(WIN32) - # On Windows, some .sh and some .pl.in files are configured + # On Windows, some .sh and some .pl.in files are configured # The resulting files will have .pl extension (those are perl scripts)
# Input files with pl.in extension @@ -296,60 +296,73 @@ ELSE() # On Unix, most of the files end up in the bin directory SET(BIN_SCRIPTS msql2mysql - mysql_config - mysql_setpermission - mysql_secure_installation - mysqlaccess - mysql_convert_table_format - mysql_find_rows + mariadb-setpermission + mariadb-secure-installation + mariadb-access + mariadb-convert-table-format + mariadb-find-rows mytop - mysqlhotcopy + mariadb-hotcopy + mariadb-install-db ${SERVER_SCRIPTS} ${WSREP_SCRIPTS} ${SYSTEMD_SCRIPTS} ) + FOREACH(file ${BIN_SCRIPTS}) - IF(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${file}.sh) - CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/${file}.sh - ${CMAKE_CURRENT_BINARY_DIR}/${file} ESCAPE_QUOTES @ONLY) - ELSEIF(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${file}) - CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/${file} - ${CMAKE_CURRENT_BINARY_DIR}/${file} COPYONLY) - ELSE() - MESSAGE(FATAL_ERROR "Can not find ${file}.sh or ${file} in " - "${CMAKE_CURRENT_SOURCE_DIR}" ) - ENDIF() - # TODO: The following EXECUTE could be redundant as INSTALL_SCRIPT - # macro does an INSTALL(PROGRAMS ..) that automatically sets +x on - # the executable. - EXECUTE_PROCESS(COMMAND chmod +x ${CMAKE_CURRENT_BINARY_DIR}/${file}) - IF(NOT ${file}_COMPONENT) - SET(${file}_COMPONENT Server) - ENDIF() - INSTALL_SCRIPT( - ${CMAKE_CURRENT_BINARY_DIR}/${file} - DESTINATION ${INSTALL_BINDIR} - COMPONENT ${${file}_COMPONENT} - ) + # set name of executable + list(FIND MARIADB_SYMLINK_FROMS ${file} _index) + + if(${_index} GREATER -1) + list(GET MARIADB_SYMLINK_TOS ${_index} binname) + else() + set(binname ${file}) + endif()
1. why are you not using GET_SYMLINK ? 2. Why INSTALL_SYMLINK here instead of the old CREATE_MARIADB_SYMLINK ? You avoid creating new targets for script symlinks, but who cares about that?
+ + if(NOT "${binname}" STREQUAL "")
how can ${binname} be "" ?
+ IF(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${binname}.sh) + CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/${binname}.sh + ${CMAKE_CURRENT_BINARY_DIR}/${file} ESCAPE_QUOTES @ONLY) + ELSEIF(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${binname}) + CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/${binname} + ${CMAKE_CURRENT_BINARY_DIR}/${file} COPYONLY) + ELSE() + MESSAGE(FATAL_ERROR "Can not find ${binname}.sh or ${binname} in " + "${CMAKE_CURRENT_SOURCE_DIR}" ) + ENDIF() + + IF(NOT ${file}_COMPONENT) + SET(${file}_COMPONENT Server) + ENDIF() + INSTALL_SCRIPT( + ${CMAKE_CURRENT_BINARY_DIR}/${file} + DESTINATION ${INSTALL_BINDIR} + COMPONENT ${${file}_COMPONENT} + ) + + # Create symlink + if (NOT ${binname} STREQUAL ${file}) + INSTALL_LINK(${file} ${binname} ${INSTALL_BINDIR} ${${file}_COMPONENT}) + + # Place mysql_install_db symlink also in scripts + if (${binname} STREQUAL "mysql_install_db") + SET(bindir ${bindir1}) + SET(prefix ${prefix1}) + SET(sbindir ${sbindir1}) + SET(scriptdir ${scriptdir1}) + SET(libexecdir ${libexecdir1}) + SET(pkgdatadir ${pkgdatadir1}) + SET(pkgplugindir ${pkgplugindir1}) + SET(localstatedir ${localstatedir1}) + CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/mysql_install_db.sh ${CMAKE_CURRENT_BINARY_DIR}/mariadb-install-db ESCAPE_QUOTES @ONLY) + INSTALL(PROGRAMS ${CMAKE_CURRENT_BINARY_DIR}/${file} DESTINATION ${INSTALL_SCRIPTDIR} COMPONENT Server) + INSTALL(PROGRAMS ${CMAKE_CURRENT_BINARY_DIR}/${binname} DESTINATION ${INSTALL_SCRIPTDIR} COMPONENT Server)
I don't understand that at all. 1. What does the comment "Place mysql_install_db symlink also in scripts" even mean? 2. you CONFIGURE_FILE mariadb-install-db twice. 3. Why two INSTALL(PROGRAM ? 4. You set all bindir/etc variables for mysql_install_db and it will affect CONFIGURE_FILE for all following scripts 5. why do you even compare ${binname} with mysql_install_db and not ${file} with mariadb-install-db ?
+ endif() + endif() + endif()
we customary always use uppercase in all cmake keywords.
ENDFOREACH()
- SET (wsrep_sst_rsync_wan ${CMAKE_CURRENT_BINARY_DIR}/wsrep_sst_rsync_wan) - ADD_CUSTOM_COMMAND( - OUTPUT ${wsrep_sst_rsync_wan} - COMMAND ${CMAKE_COMMAND} ARGS -E create_symlink - wsrep_sst_rsync - wsrep_sst_rsync_wan - ) - ADD_CUSTOM_TARGET(symlink_wsrep_sst_rsync - ALL - DEPENDS ${wsrep_sst_rsync_wan} - ) - INSTALL( - FILES ${wsrep_sst_rsync_wan} - DESTINATION ${INSTALL_BINDIR} - COMPONENT Server - ) - + INSTALL_LINK(wsrep_sst_rsync wsrep_sst_rsync_wan ${INSTALL_BINDIR} Server) FOREACH(file ${WSREP_SOURCE}) CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/${file}.sh ${CMAKE_CURRENT_BINARY_DIR}/${file} ESCAPE_QUOTES @ONLY) diff --git a/win/packaging/ca/CMakeLists.txt b/win/packaging/ca/CMakeLists.txt index 99dc7da01fb..326bab47de4 100644 --- a/win/packaging/ca/CMakeLists.txt +++ b/win/packaging/ca/CMakeLists.txt
I didn't review windows packaging, it's between you and wlad. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Hi Serg, I've done fixes to the below comments that you had in: https://github.com/MariaDB/server/commit/2a00390f513ea9ec4d947eb9e68ac44dcf8... Below are some comments to your comments also. On Mon, Mar 16, 2020 at 8:44 PM Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Rasmus!
See my comments/questions below:
On Mar 16, Rasmus Johansson wrote:
revision-id: cf70893ac9d (mariadb-10.5.0-391-gcf70893ac9d) parent(s): 56402e84b5b author: Rasmus Johansson <razze@iki.fi> committer: Rasmus Johansson <razze@iki.fi> timestamp: 2020-03-16 11:10:25 +0000 message:
MDEV-21303 Make executables MariaDB named
To change all executables to have a mariadb name I had to: - Do name changes in every CMakeLists.txt that produces executables - CREATE_MARIADB_SYMLINK was removed and GET_SYMLINK added by Wlad to reuse the function in other places also - The scripts/CMakeLists.txt could make use of GET_SYMLINK instead of introducing redundant code, but I thought I'll leave that for next release - A lot of changes to debian/.install and debian/.links files due to swapping of real executable and symlink. I did not however change the name of the manpages, so the real name is still mysql there and mariadb are symlinks. - The Windows part needed a change now when we made the executables mariadb -named. MSI (and ZIP) do not support symlinks and to not break backward compatibility we had to include mysql named binaries also. Done by Wlad
diff --git a/cmake/mysql_add_executable.cmake b/cmake/mysql_add_executable.cmake index eec370d51af..f4d71ae9cef 100644 --- a/cmake/mysql_add_executable.cmake +++ b/cmake/mysql_add_executable.cmake @@ -48,7 +48,7 @@ FUNCTION (MYSQL_ADD_EXECUTABLE) ENDIF()
IF (ARG_WIN32) - SET(WIN32 WIN32) + SET(WIN32 ARG_WIN32)
This looks wrong, see below
Reverted
ELSE() UNSET(WIN32) ENDIF() @@ -62,6 +62,7 @@ FUNCTION (MYSQL_ADD_EXECUTABLE) ELSE() UNSET(EXCLUDE_FROM_ALL) ENDIF() + ADD_EXECUTABLE(${target} ${WIN32} ${MACOSX_BUNDLE} ${EXCLUDE_FROM_ALL} ${sources})
here it'll pass ARG_WIN32 to ADD_EXECUTABLE. But ADD_EXECUTABLE takes an optional WIN32 keyword, not ARG_WIN32.
Reverted. Works correctly again.
# tell CPack where to install @@ -79,16 +80,49 @@ FUNCTION (MYSQL_ADD_EXECUTABLE) IF (COMP MATCHES ${SKIP_COMPONENTS}) RETURN() ENDIF() + IF (WITH_STRIPPED_CLIENT AND NOT target STREQUAL mysqld)
this should be `STREQUAL mariadbd` now, I believe.
Yes, fixed
INSTALL(CODE "SET(CMAKE_INSTALL_DO_STRIP 1)" COMPONENT ${COMP}) SET(reset_strip ON) ENDIF() - MYSQL_INSTALL_TARGETS(${target} DESTINATION ${ARG_DESTINATION}
COMPONENT ${COMP})
+ + IF(NOT ${mariadbname} STREQUAL "")
where is mariadbname set?
It's not set anymore. It was for an intermediate version. I've removed the change.
+ MYSQL_INSTALL_TARGETS(${mariadbname} DESTINATION ${ARG_DESTINATION} COMPONENT ${COMP}) + ELSE() + MYSQL_INSTALL_TARGETS(${target} DESTINATION ${ARG_DESTINATION} COMPONENT ${COMP}) + ENDIF() + IF (reset_strip) INSTALL(CODE "SET(CMAKE_INSTALL_DO_STRIP 0)" COMPONENT ${COMP}) ENDIF() ENDIF()
- # create mariadb named symlink - CREATE_MARIADB_SYMLINK(${target} ${ARG_DESTINATION} ${COMP}) + # create MySQL named "legacy links" + + # Windows note: + # Here, hardlinks are used, because cmake can't install symlinks. + # In packages, there are won't be links, just copies.
better to put the comment below in the ELSE branch, not here, far from the code that it is supposed to explain.
Fixed
+ GET_SYMLINK(${target} link) + IF(link) + IF(UNIX) + ADD_CUSTOM_COMMAND(TARGET ${target} POST_BUILD + COMMAND ${CMAKE_COMMAND} -E create_symlink + ${target} ${link} + COMMENT "Creating ${link} link" + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}) + INSTALL(PROGRAMS + ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${link} + DESTINATION + ${ARG_DESTINATION} + COMPONENT ${COMP}) + ELSE() + SET(link ${link}.exe) + ADD_CUSTOM_COMMAND(TARGET ${target} POST_BUILD + COMMAND cmake -E remove -f ${link} + COMMAND mklink /H ${link} $<TARGET_FILE_NAME:${target}> + COMMENT "Creating ${link} link" + WORKING_DIRECTORY $<TARGET_FILE_DIR:${target}>) + INSTALL(PROGRAMS $<TARGET_FILE_DIR:${target}>/${link} DESTINATION ${ARG_DESTINATION} COMPONENT ${COMP}) + ENDIF() + ENDIF() ENDFUNCTION() diff --git a/cmake/symlinks.cmake b/cmake/symlinks.cmake index ec638bc82de..e040ff19f77 100644 --- a/cmake/symlinks.cmake +++ b/cmake/symlinks.cmake @@ -9,68 +9,46 @@ macro(REGISTER_SYMLINK from to) endmacro()
# MariaDB names for executables -REGISTER_SYMLINK("mysql_client_test_embedded" "mariadb-client-test-embedded") -REGISTER_SYMLINK("mysql_client_test" "mariadb-client-test") +REGISTER_SYMLINK("mariadb-client-test-embedded" "mysql_client_test_embedded") +REGISTER_SYMLINK("mariadb-client-test" "mysql_client_test") ...
I suppose you need to make sure that everything works with new names, without symlinks. That is all internal tools and scripts invoke other tools and scripts by their mariadb* names.
A way to do it could be to comment out all REGISTER_SYMLINK lines and fix whatever that will break. grep is also a useful tool here, but with rather low signal-to-noise ratio.
I'm saving this for next release.
+ +MACRO(GET_SYMLINK name out) + set(${out}) + list(FIND MARIADB_SYMLINK_FROMS ${name} _index) if (${_index} GREATER -1) - list(GET MARIADB_SYMLINK_TOS ${_index} mariadbname) - endif() - - if (mariadbname) - CREATE_MARIADB_SYMLINK_IN_DIR(${src} ${mariadbname} ${dir} ${comp}) - endif() -endmacro(CREATE_MARIADB_SYMLINK) - -# Add MariaDB symlinks in directory -macro(CREATE_MARIADB_SYMLINK_IN_DIR src dest dir comp) - if(UNIX) - add_custom_target( - SYM_${dest} ALL - DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/${dest} - ) - - add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${dest} POST_BUILD - COMMAND ${CMAKE_COMMAND} -E create_symlink ${src} ${dest} - COMMENT "mklink ${src} -> ${dest}") - - install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${dest} DESTINATION ${dir} COMPONENT ${comp}) + list(GET MARIADB_SYMLINK_TOS ${_index} ${out}) endif() -endmacro(CREATE_MARIADB_SYMLINK_IN_DIR) +ENDMACRO() diff --git a/debian/mariadb-backup.install b/debian/mariadb-backup.install index 2bc61c12079..e1967495b00 100644 --- a/debian/mariadb-backup.install +++ b/debian/mariadb-backup.install @@ -1,5 +1,4 @@ -usr/bin/mariabackup +usr/bin/mariadb-backup usr/bin/mbstream usr/share/man/man1/mariabackup.1 -usr/share/man/man1/mariadb-backup.1
this is strange. Why did you remove mariadb-backup.1 and not mariabackup.1 ?
Fixed
usr/share/man/man1/mbstream.1 diff --git a/debian/mariadb-client-10.5.install b/debian/mariadb-client-10.5.install index 67c0c2619c3..216e4015851 100644 --- a/debian/mariadb-client-10.5.install +++ b/debian/mariadb-client-10.5.install @@ -1,29 +1,17 @@ debian/additions/innotop/innotop usr/bin/ debian/additions/mysqlreport usr/bin/ +usr/bin/mariadb-access +usr/bin/mariadb-admin usr/bin/mariadb-conv -usr/bin/mysql_find_rows -usr/bin/mysql_fix_extensions -usr/bin/mysql_waitpid -usr/bin/mysqlaccess -usr/bin/mysqladmin -usr/bin/mysqldump -usr/bin/mysqldumpslow -usr/bin/mysqlimport -usr/bin/mysqlshow
you removed mysqlshow here, and added mariadb-show to the server package. Intentional?
UPD: I see that before your patch mysqlshow was in the client package, while mariadb-show symlink was in the server package. It's a bug, both should be in the client package.
Fixed
+usr/bin/mariadb-dump +usr/bin/mariadb-dumpslow +usr/bin/mariadb-find-rows +usr/bin/mariadb-fix-extensions +usr/bin/mariadb-import +usr/bin/mariadb-slap +usr/bin/mariadb-waitpid usr/bin/mysqlslap
you did not remove mysqlslap, but added mariadb-slap
Fixed
usr/bin/mytop -usr/share/man/man1/mariadb-access.1 -usr/share/man/man1/mariadb-admin.1 -usr/share/man/man1/mariadb-binlog.1 -usr/share/man/man1/mariadb-conv.1 -usr/share/man/man1/mariadb-dump.1 -usr/share/man/man1/mariadb-dumpslow.1 -usr/share/man/man1/mariadb-find-rows.1 -usr/share/man/man1/mariadb-fix-extensions.1 -usr/share/man/man1/mariadb-import.1 -usr/share/man/man1/mariadb-plugin.1 -usr/share/man/man1/mariadb-slap.1 -usr/share/man/man1/mariadb-waitpid.1
same weirdness as elsewhere, mariadb*.1 man pages are symlinks to mysql*.1 manpages. Should be vice versa.
usr/share/man/man1/mysql_find_rows.1 usr/share/man/man1/mysql_fix_extensions.1 usr/share/man/man1/mysql_waitpid.1 diff --git a/debian/mariadb-client-10.5.links b/debian/mariadb-client-10.5.links index 5d966575b76..d1bdfcfa3c8 100644 --- a/debian/mariadb-client-10.5.links +++ b/debian/mariadb-client-10.5.links @@ -1,21 +1,19 @@ -usr/bin/mysql_find_rows usr/bin/mariadb-find-rows -usr/bin/mysql_fix_extensions usr/bin/mariadb-fix-extensions -usr/bin/mysql_plugin usr/bin/mariadb-plugin -usr/bin/mysql_waitpid usr/bin/mariadb-waitpid -usr/bin/mysqlaccess usr/bin/mariadb-access -usr/bin/mysqladmin usr/bin/mariadb-admin -usr/bin/mysqlbinlog usr/bin/mariadb-binlog
you removed mysqlbinlog, but added mariadb-binlog to the server package. Intentional?
same for mysql_plugin/mariadb-plugin.
UPD: mysqlbinlog was in the server package, while mariadb-binlog symlink was in the client package. So your change is correct, both will be in the server package now. Good.
same for mysql_plugin/mariadb-plugin.
I had to add some conflicts and replaces to the debian/control file to get
Fixed these to work
-usr/bin/mysqlcheck usr/bin/mariadb-analyze -usr/bin/mysqlcheck usr/bin/mariadb-optimize diff --git a/debian/mariadb-plugin-rocksdb.install b/debian/mariadb-plugin-rocksdb.install index 80987612c30..0fc868a0721 100644 --- a/debian/mariadb-plugin-rocksdb.install +++ b/debian/mariadb-plugin-rocksdb.install @@ -2,6 +2,5 @@ etc/mysql/conf.d/rocksdb.cnf etc/mysql/mariadb.conf.d usr/bin/myrocks_hotbackup usr/bin/mysql_ldb
you didn't replace mysql_ldb with mariadb-ldb?
Fixed
usr/lib/mysql/plugin/ha_rocksdb.so -usr/share/man/man1/mariadb-ldb.1 usr/share/man/man1/myrocks_hotbackup.1 usr/share/man/man1/mysql_ldb.1 diff --git a/debian/mariadb-server-10.5.install b/debian/mariadb-server-10.5.install index 4a860ff57df..38d9f984276 100644 --- a/debian/mariadb-server-10.5.install +++ b/debian/mariadb-server-10.5.install @@ -14,22 +14,23 @@ usr/bin/aria_pack usr/bin/aria_read_log usr/bin/galera_new_cluster usr/bin/galera_recovery +usr/bin/mariadb-binlog +usr/bin/mariadb-convert-table-format +usr/bin/mariadb-hotcopy +usr/bin/mariadb-plugin +usr/bin/mariadb-secure-installation usr/bin/mariadb-service-convert +usr/bin/mariadb-setpermission +usr/bin/mariadb-show +usr/bin/mariadb-tzinfo-to-sql +usr/bin/mariadbd-multi +usr/bin/mariadbd-safe +usr/bin/mariadbd-safe
duplicate line ^^^
Fixed
usr/bin/msql2mysql usr/bin/myisam_ftdump usr/bin/myisamchk usr/bin/myisamlog usr/bin/myisampack -usr/bin/mysql_convert_table_format -usr/bin/mysql_plugin -usr/bin/mysql_secure_installation -usr/bin/mysql_setpermission -usr/bin/mysql_tzinfo_to_sql -usr/bin/mysqlbinlog -usr/bin/mysqld_multi -usr/bin/mysqld_safe -usr/bin/mysqld_safe_helper
you don't install mariadbd-safe-helper. Perhaps that's where your duplicate line comes from, it's not a duplicate, it's truncated :)
Fixed
-usr/bin/mysqlhotcopy usr/bin/perror usr/bin/replace usr/bin/resolve_stack_dump diff --git a/debian/mariadb-server-10.5.links b/debian/mariadb-server-10.5.links index f2d97460371..e3a2d68541a 100644 --- a/debian/mariadb-server-10.5.links +++ b/debian/mariadb-server-10.5.links @@ -1,9 +1,20 @@ -usr/bin/mysql_convert_table_format usr/bin/mariadb-convert-table-format -usr/bin/mysql_secure_installation usr/bin/mariadb-secure-installation -usr/bin/mysql_setpermission usr/bin/mariadb-setpermission -usr/bin/mysql_tzinfo_to_sql usr/bin/mariadb-tzinfo-to-sql -usr/bin/mysqld_multi usr/bin/mariadbd-multi -usr/bin/mysqld_safe usr/bin/mariadbd-safe -usr/bin/mysqld_safe_helper usr/bin/mariadbd-safe-helper -usr/bin/mysqlhotcopy usr/bin/mariadb-hotcopy -usr/bin/mysqlshow usr/bin/mariadb-show +usr/bin/mariadb-binlog usr/bin/mysqlbinlog +usr/bin/mariadb-convert-table-format usr/bin/mysql_convert_table_format +usr/bin/mariadb-hotcopy usr/bin/mysqlhotcopy +usr/bin/mariadb-plugin usr/bin/mysql_plugin +usr/bin/mariadb-secure-installation usr/bin/mysql_secure_installation +usr/bin/mariadb-setpermission usr/bin/mysql_setpermission +usr/bin/mariadb-show /usr/bin/mysqlshow
this is the only line in your whole patch where you symlink by the absolute path. Looks like a typo.
Fixed
+usr/bin/mariadb-tzinfo-to-sql usr/bin/mysql_tzinfo_to_sql +usr/bin/mariadbd-multi usr/bin/mysqld_multi +usr/bin/mariadbd-safe usr/bin/mysqld_safe +usr/bin/mariadbd-safe-helper usr/bin/mysqld_safe_helper diff --git a/libmariadb b/libmariadb index 3be5897c334..ca68b114ba7 160000 --- a/libmariadb +++ b/libmariadb @@ -1 +1 @@ -Subproject commit 3be5897c3346639fa6d7195480d93108798c4917 +Subproject commit ca68b114ba7e882e8abe8888204d675c4d8751f8
This is clearly wrong, please make sure you've removed it from your commit before pushing!
I ran git submodule update. Should be ok now.
diff --git a/scripts/CMakeLists.txt b/scripts/CMakeLists.txt index 7be46ac1985..350fbd14cf6 100644 --- a/scripts/CMakeLists.txt +++ b/scripts/CMakeLists.txt @@ -244,8 +232,20 @@ SET(mysqlaccess_COMPONENT COMPONENT Client) SET(mysql_find_rows_COMPONENT COMPONENT Client) SET(mytop_COMPONENT Mytop)
+MACRO(INSTALL_LINK old new destination component) + EXECUTE_PROCESS( + COMMAND ${CMAKE_COMMAND} -E create_symlink ${old} ${new} + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} + ) + INSTALL( + PROGRAMS ${CMAKE_CURRENT_BINARY_DIR}/${new} + DESTINATION ${destination} + COMPONENT ${component} + ) +ENDMACRO() + IF(WIN32) - # On Windows, some .sh and some .pl.in files are configured + # On Windows, some .sh and some .pl.in files are configured # The resulting files will have .pl extension (those are perl scripts)
# Input files with pl.in extension @@ -296,60 +296,73 @@ ELSE() # On Unix, most of the files end up in the bin directory SET(BIN_SCRIPTS msql2mysql - mysql_config - mysql_setpermission - mysql_secure_installation - mysqlaccess - mysql_convert_table_format - mysql_find_rows + mariadb-setpermission + mariadb-secure-installation + mariadb-access + mariadb-convert-table-format + mariadb-find-rows mytop - mysqlhotcopy + mariadb-hotcopy + mariadb-install-db ${SERVER_SCRIPTS} ${WSREP_SCRIPTS} ${SYSTEMD_SCRIPTS} ) + FOREACH(file ${BIN_SCRIPTS}) - IF(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${file}.sh) - CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/${file}.sh - ${CMAKE_CURRENT_BINARY_DIR}/${file} ESCAPE_QUOTES @ONLY) - ELSEIF(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${file}) - CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/${file} - ${CMAKE_CURRENT_BINARY_DIR}/${file} COPYONLY) - ELSE() - MESSAGE(FATAL_ERROR "Can not find ${file}.sh or ${file} in " - "${CMAKE_CURRENT_SOURCE_DIR}" ) - ENDIF() - # TODO: The following EXECUTE could be redundant as INSTALL_SCRIPT - # macro does an INSTALL(PROGRAMS ..) that automatically sets +x on - # the executable. - EXECUTE_PROCESS(COMMAND chmod +x ${CMAKE_CURRENT_BINARY_DIR}/${file}) - IF(NOT ${file}_COMPONENT) - SET(${file}_COMPONENT Server) - ENDIF() - INSTALL_SCRIPT( - ${CMAKE_CURRENT_BINARY_DIR}/${file} - DESTINATION ${INSTALL_BINDIR} - COMPONENT ${${file}_COMPONENT} - ) + # set name of executable + list(FIND MARIADB_SYMLINK_FROMS ${file} _index) + + if(${_index} GREATER -1) + list(GET MARIADB_SYMLINK_TOS ${_index} binname) + else() + set(binname ${file}) + endif()
1. why are you not using GET_SYMLINK ? 2. Why INSTALL_SYMLINK here instead of the old CREATE_MARIADB_SYMLINK ? You avoid creating new targets for script symlinks, but who cares about that?
I re-did this part of the code to use GET_SYMLINK.
+ + if(NOT "${binname}" STREQUAL "")
how can ${binname} be "" ?
It cannot, you're right. I removed the if
+ IF(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${binname}.sh) + CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/${binname}.sh + ${CMAKE_CURRENT_BINARY_DIR}/${file} ESCAPE_QUOTES @ONLY) + ELSEIF(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${binname}) + CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/${binname} + ${CMAKE_CURRENT_BINARY_DIR}/${file} COPYONLY) + ELSE() + MESSAGE(FATAL_ERROR "Can not find ${binname}.sh or ${binname} in " + "${CMAKE_CURRENT_SOURCE_DIR}" ) + ENDIF() + + IF(NOT ${file}_COMPONENT) + SET(${file}_COMPONENT Server) + ENDIF() + INSTALL_SCRIPT( + ${CMAKE_CURRENT_BINARY_DIR}/${file} + DESTINATION ${INSTALL_BINDIR} + COMPONENT ${${file}_COMPONENT} + ) + + # Create symlink + if (NOT ${binname} STREQUAL ${file}) + INSTALL_LINK(${file} ${binname} ${INSTALL_BINDIR} ${${file}_COMPONENT}) + + # Place mysql_install_db symlink also in scripts + if (${binname} STREQUAL "mysql_install_db") + SET(bindir ${bindir1}) + SET(prefix ${prefix1}) + SET(sbindir ${sbindir1}) + SET(scriptdir ${scriptdir1}) + SET(libexecdir ${libexecdir1}) + SET(pkgdatadir ${pkgdatadir1}) + SET(pkgplugindir ${pkgplugindir1}) + SET(localstatedir ${localstatedir1}) + CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/mysql_install_db.sh ${CMAKE_CURRENT_BINARY_DIR}/mariadb-install-db ESCAPE_QUOTES @ONLY) + INSTALL(PROGRAMS ${CMAKE_CURRENT_BINARY_DIR}/${file} DESTINATION ${INSTALL_SCRIPTDIR} COMPONENT Server) + INSTALL(PROGRAMS ${CMAKE_CURRENT_BINARY_DIR}/${binname} DESTINATION ${INSTALL_SCRIPTDIR} COMPONENT Server)
I don't understand that at all.
1. What does the comment "Place mysql_install_db symlink also in scripts" even mean?
2. you CONFIGURE_FILE mariadb-install-db twice.
3. Why two INSTALL(PROGRAM ?
4. You set all bindir/etc variables for mysql_install_db and it will affect CONFIGURE_FILE for all following scripts
5. why do you even compare ${binname} with mysql_install_db and not ${file} with mariadb-install-db ?
Re-written
+ endif() + endif() + endif()
we customary always use uppercase in all cmake keywords.
ENDFOREACH()
- SET (wsrep_sst_rsync_wan ${CMAKE_CURRENT_BINARY_DIR}/wsrep_sst_rsync_wan) - ADD_CUSTOM_COMMAND( - OUTPUT ${wsrep_sst_rsync_wan} - COMMAND ${CMAKE_COMMAND} ARGS -E create_symlink - wsrep_sst_rsync - wsrep_sst_rsync_wan - ) - ADD_CUSTOM_TARGET(symlink_wsrep_sst_rsync - ALL - DEPENDS ${wsrep_sst_rsync_wan} - ) - INSTALL( - FILES ${wsrep_sst_rsync_wan} - DESTINATION ${INSTALL_BINDIR} - COMPONENT Server - ) - + INSTALL_LINK(wsrep_sst_rsync wsrep_sst_rsync_wan ${INSTALL_BINDIR} Server) FOREACH(file ${WSREP_SOURCE}) CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/${file}.sh ${CMAKE_CURRENT_BINARY_DIR}/${file} ESCAPE_QUOTES @ONLY) diff --git a/win/packaging/ca/CMakeLists.txt b/win/packaging/ca/CMakeLists.txt index 99dc7da01fb..326bab47de4 100644 --- a/win/packaging/ca/CMakeLists.txt +++ b/win/packaging/ca/CMakeLists.txt
I didn't review windows packaging, it's between you and wlad.
The commit with the fixes are https://github.com/MariaDB/server/commit/2a00390f513ea9ec4d947eb9e68ac44dcf8...
participants (2)
-
Rasmus Johansson -
Sergei Golubchik