maria-developers team mailing list archive

[Rasmus Johansson <rasmus@xxxxxxxxxxx>]

Hi Serg,

On Thu, Jun 13, 2019 at 11:18 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:

> Hi, Rasmus!
>
> On Jun 13, Rasmus Johansson wrote:
> > revision-id: e35fb718eae (mariadb-10.4.5-44-ge35fb718eae)
> > parent(s): 8e3a4be45c5
> > author: Rasmus Johansson <razze@xxxxxx>
> > committer: Rasmus Johansson <razze@xxxxxx>
>
> Is that an address you want to have in git history?
> You already have two, just making sure the third one is intentional.
>

I changed this for the latest commit. Thanks for pointing it out.


> > timestamp: 2019-06-13 13:20:40 +0000
> > message:
> >
> > MDEV-17591 Create MariaDB named commands/symlinks
> >
> > diff --git a/cmake/install_macros.cmake b/cmake/install_macros.cmake
> > index f28720e97bc..e49f296ed75 100644
> > --- a/cmake/install_macros.cmake
> > +++ b/cmake/install_macros.cmake
> > @@ -111,8 +111,10 @@ FUNCTION(INSTALL_SCRIPT)
> >    ENDIF()
> >
> >    INSTALL(PROGRAMS ${script} DESTINATION ${ARG_DESTINATION} ${COMP})
> > +  string(REPLACE "${CMAKE_BINARY_DIR}/scripts/" "" dest ${script})
>
> better not to assume that the script is in ${CMAKE_BINARY_DIR}/scripts/
> I'd use get_filename_component() here. Like
>
>  get_filename_component(dest "${scripts}" NAME)
>

Changed accordingly.


> > +# Add MariaDB symlinks
> > +macro(CREATE_MARIADB_SYMLINK src)
>
> do you still skip Windows, as in the previous commit? I couldn't find it
> in this one.
>

Yes, Windows will be in a seperate commit.


> > +
> > +    add_custom_target(
> > +      symlink_${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}")
> > +
> > +    set(symlink_install_dir ${INSTALL_BINDIR})
> > +    # adjust install location if needed
> > +    if(${dest} MATCHES "mariadb-install-db")
> > +      set(symlink_install_dir ${INSTALL_SCRIPTDIR})
> > +    endif()
>
> this is very fragile. better to take the path from the ${src}
> also with get_filename_component().
>

In this case it doesn't really make sense to use get_filename_component
because here $dest is already only the filename, no path.


> > +usr/share/man/man1/mysql_find_rows.1.gz
> usr/share/man/man1/mariadb-find-rows.1.gz
> > +usr/share/man/man1/mysql_fix_extensions.1.gz
> usr/share/man/man1/mariadb-fix-extensions.1.gz
>
> manpages aren't symlinked in rpms or bintars.
> and symlinks depends only on MariaDB-client, while
> for example mysql_install_db is in the server package.
>

I've now added the manpage symlinks to bintar and rpms also.


> may be just put symlinks into the same package as symlink targets?
> just like with debs?
>

This also done now for rpm.

The new commit is here: https://github.com/MariaDB/server/commit/270c73b

Rasmus