Hi, Rasmus! On Jun 14, Rasmus Johansson wrote: > revision-id: b4cdf20dd15 (mariadb-10.4.5-44-gb4cdf20dd15) > parent(s): 8e3a4be45c5 > author: Rasmus Johansson <ras...@mariadb.com> > committer: Rasmus Johansson <ras...@mariadb.com> > timestamp: 2019-06-14 14:55:20 +0000 > message: > > MDEV-17591 Create MariaDB named commands/symlinks > > diff --git a/cmake/symlinks.cmake b/cmake/symlinks.cmake > new file mode 100644 > index 00000000000..32cf24e3e3b > --- /dev/null > +++ b/cmake/symlinks.cmake > @@ -0,0 +1,76 @@ > +# MariaDB names for executables > +list(APPEND MARIADB_SYMLINK_NAMES "mysql" "mariadb") > +list(APPEND MARIADB_SYMLINK_NAMES "mysqlaccess" "mariadb-access") > +list(APPEND MARIADB_SYMLINK_NAMES "mysqladmin" "mariadb-admin") > +list(APPEND MARIADB_SYMLINK_NAMES "mariabackup" "mariadb-backup") ... > +# Add MariaDB symlinks > +macro(CREATE_MARIADB_SYMLINK src comp) > + # Find the MariaDB name for executable > + list(FIND MARIADB_SYMLINK_NAMES ${src} _index) > + > + if (${_index} GREATER -1) > + MATH(EXPR _index "${_index}+1") > + list(GET MARIADB_SYMLINK_NAMES ${_index} _name) > + set(mariadbname ${_name}) > + endif() > + > + if (mariadbname) > + set(dest ${mariadbname}) > + set(symlink_install_dir ${INSTALL_BINDIR}) > + > + # adjust install location if needed > + if(${dest} MATCHES "mariadb-install-db") > + set(symlink_install_dir ${INSTALL_SCRIPTDIR}) > + endif()
I still think it's very fragile and difficult to maintain. Why not to pass the destination as a third argument to CREATE_MARIADB_SYMLINK? As far as I can see, everywhere where you use CREATE_MARIADB_SYMLINK the destination is available. The rest is okay. I'd still suggest to simplify by splitting the list in two, like macro(REGISTER_SYMLINK from to) list(APPEND MARIADB_SYMLINK_FROMS from) list(APPEND MARIADB_SYMLINK_TOS to) endmacro() REGISTER_SYMLINK(mysql mariadb) REGISTER_SYMLINK(mysqladmin mariadb-access) ...etc... Then you won't need to do math on indexes, you simply search in MARIADB_SYMLINK_FROMS and get the same element in MARIADB_SYMLINK_TOS. Also you will need to search only in MARIADB_SYMLINK_FROMS, not like now, when you search in the complete list, risking to get spurious matches on mariadb* names. Same in manpages, you'll only iterate over MARIADB_SYMLINK_FROMS list. > + > + CREATE_MARIADB_SYMLINK_IN_DIR(${src} ${dest} ${symlink_install_dir} > ${comp}) > + endif() > +endmacro(CREATE_MARIADB_SYMLINK) Regards, Sergei Chief Architect MariaDB and secur...@mariadb.org _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp