[MDEV-9209] [PATCH] scripts: Do not prepend the prefix to absolute paths Created: 2015-11-30  Updated: 2015-12-22  Resolved: 2015-12-22

Status: Closed
Project: MariaDB Server
Component/s: Scripts & Clients
Affects Version/s: 5.5, 10.0, 10.1, 10.2
Fix Version/s: 10.1.10

Type: Bug Priority: Major
Reporter: Sergey Vojtovich Assignee: Sergey Vojtovich
Resolution: Fixed Votes: 0
Labels: contribution

Sprint: 10.1.10

 Description   

The INSTALL_* variables can be absolute or relative to the prefix path. The current code only handles the case where they are relative. With the patch, the old behaviour is used as long as they are relative, but it fixes the case when they are absolute.

This is important for Exherbo since we have to use absolute paths because of our file system layout. Exherbo has /usr/share for arch-independent files and /usr/$ARCH/

{bin,include,lib,libexec}

for arch-dependent files, so that files for multiple architectures can be installed in parallel.

 Comments   
Comment by Sergey Vojtovich [ 2015-12-18 ]

serg, please review patch for this bug. Note that it doesn't completely fix original issue, since we have special handling for STANDALONE in mysql_install_db. I don't like changing that, instead I'd suggest to create separate layout for Exherbo.

We may clean-up these *dir variables and use INSTALL_*DIRABS instead.

I'm not sure how this should work with DESTDIR trick, so I kept original behavior.

In my first patch I modified INSTALL_*DIR to store absolute path. This confused TGZ generator.

Comment by Sergei Golubchik [ 2015-12-20 ]

In my first patch I modified INSTALL_*DIR to store absolute path. This confused TGZ generator.

Could you elaborate on that, please?

Comment by Sergey Vojtovich [ 2015-12-20 ]

make package failed to perform installation, since it attempted to install to /usr/local (or whatever) instead of $TMPDIR/usr/local

Comment by Sergei Golubchik [ 2015-12-21 ]

I think this is ok for make package to fail, and, may be, even the desired behavior — bintar packages don't support absolute paths, so one should not be able to set INSTALL_*DIR to absolute paths.

I would've preferred cmake not to accept absolute paths in the first place when CPACK_GENERATOR is TGZ, but this would prohibit a very valid cmake . && make install use case.

Comment by Sergei Golubchik [ 2015-12-21 ]

Although I'd use a macro like this:

MACRO(SET_PATH VAR VAL)
 
  IF(IS_ABSOLUTE ${VAL})
 
    SET(${VAR} ${VAL})
 
  ELSE()
 
    SET(${VAR} ${prefix}/${VAL})
 
  ENDIF()
 
ENDMACRO(SET_PATH)

Comment by Sergey Vojtovich [ 2015-12-22 ]

serg, could you clarify outcome of your review?

As for the macro - I don't mind, though I don't understand practical benefit.
Macro name should be changed at least.

Comment by Sergei Golubchik [ 2015-12-22 ]

Macro — to avoid duplicating the logic IF (IS_ABSOLUTE) use-as-is ELSE() prepend-prefix ENDIF().

Anyway, the outcome is — don't try to make TGZ (or ZIP, fwiw) generator to work with absolute path names. So the logic of the contributed patch is fine.

Comment by Sergey Vojtovich [ 2015-12-22 ]

serg, but there is no duplication in my patch. Also in the committed patch I don't try to feed absolute paths to TGZ generator, absolute paths are only used to substitute variables in scripts.

Compared to original contribution my patch also fixes this issue in:
cmake/cpack_rpm.cmake
CMakeLists.txt
support-files

Generated at Thu Feb 08 07:32:58 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.