Introduce INSTALL_PREFIX_SCRIPT to easily set up prefixes
ClosedPublic

Authored by apol on Dec 12 2017, 3:37 PM.

Details

Summary

If enabled it will install a prefix.sh script to the root of the prefix.
This file will be useful to be able to easily set up a prefix to be used by
integrating the environment variables it exports.

Test Plan
$ cat prefix.sh 
export PATH=/home/apol/devel/kde5/bin:$PATH
export LD_LIBRARY_PATH=/home/apol/devel/kde5/lib64:$LD_LIBRARY_PATH

export XDG_DATA_DIRS=/home/apol/devel/kde5/share:${XDG_DATA_DIRS:-/usr/local/share/:/usr/share/}
export XDG_CONFIG_DIRS=/home/apol/devel/kde5/etc/xdg:${XDG_CONFIG_DIRS:-/etc/xdg}

export QT_PLUGIN_PATH=/home/apol/devel/kde5/lib64/plugins:$QT_PLUGIN_PATH
export QML2_IMPORT_PATH=/home/apol/devel/kde5/lib64/qml:$QML2_IMPORT_PATH

Diff Detail

Repository
R240 Extra CMake Modules
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
apol created this revision.Dec 12 2017, 3:37 PM
Restricted Application added projects: Frameworks, Build System. · View Herald TranscriptDec 12 2017, 3:37 PM
Restricted Application added a subscriber: Build System. · View Herald Transcript
apol requested review of this revision.Dec 12 2017, 3:37 PM
krop added a subscriber: krop.Dec 12 2017, 3:56 PM
krop added inline comments.
kde-modules/KDEInstallDirs.cmake
705–709

missing LD_LIBRARY_PATH ?

712

I'm not sure that's the right location. my installation prefixes only contains subdirectories.
This file will also be overwritten if several modules turn the option on (which is an issue for distributions if they decide to enable the feature)

apol added inline comments.Dec 12 2017, 4:22 PM
kde-modules/KDEInstallDirs.cmake
705–709

Usually isn't necessary as you get the libraries through the rpath. I can add it though.

712

I wouldn't expect distros to pick this up, distros usually operate exclusively on /usr. If for some reason they did, they can always get this file in from a separate package that only installs this file.

It's the right location though, because all projects in the same prefix will need this variables in the same way.

krop added a comment.Dec 12 2017, 4:37 PM

+1. Please add the missing documentation about this new option.

kde-modules/KDEInstallDirs.cmake
705–709

It won't harm and solve potential issues if for any reason the rpath is removed

712

Allright. just add the PERMISSIONS keyword to make it executable.

sitter requested changes to this revision.Dec 12 2017, 5:06 PM
sitter added a subscriber: sitter.
sitter added inline comments.
kde-modules/KDEInstallDirs.cmake
704

From a style perspective, I'd suggest having the prefix.sh live somewhere in the installed ECM tree and get copied, rather than maintained as a glorified heredoc in the cmake code. That's just a suggestion though.

705

This is not correct, the XDG_ vars are not necessarily set, so all code that sets them ought to ensure their default values are appended if necessary.

If $XDG_DATA_DIRS is either not set or empty, a value equal to /usr/local/share/:/usr/share/ should be used.

i.e.

export XDG_DATA_DIRS=${KDE_INSTALL_FULL_DATADIR}:${XDG_DATA_DIRS:-/usr/local/share:/usr/share}
706

Same as for XDG_DATA_DIRS

If $XDG_CONFIG_DIRS is either not set or empty, a value equal to /etc/xdg should be used."

This revision now requires changes to proceed.Dec 12 2017, 5:06 PM

Depending on the end goal of this you may wish to also set the following environment variables:

CMAKE_PREFIX_PATH: to help guide CMake to look here in addition to system prefixes
PKG_CONFIG_PATH: to help pkg-config find *.pc files
QMAKEFEATURES: to help qmake find *.pri files

Details on how those need to be used are documented in the CI environment management code at https://cgit.kde.org/sysadmin/ci-tooling.git/tree/helpers/helperslib/EnvironmentHandler.py

kfunk added a subscriber: kfunk.Dec 13 2017, 10:07 AM

If we'd name this file somewhat less generic then it could be even installed by default, no?

I had the scheme of the QNX setup script in my mind: https://github.com/acklinr/qnx660/blob/master/qnx660-env.sh

Thus: Maybe rename prefix.sh to say 'ecm-env.sh' and install the file by default?

kde-modules/KDEInstallDirs.cmake
704

Definitely, +1.

krop added a comment.Dec 13 2017, 10:14 AM
In D9299#179032, @kfunk wrote:

If we'd name this file somewhat less generic then it could be even installed by default, no?

I had the scheme of the QNX setup script in my mind: https://github.com/acklinr/qnx660/blob/master/qnx660-env.sh

Thus: Maybe rename prefix.sh to say 'ecm-env.sh' and install the file by default?

Files overwritting each others is a no-go for packagers. +1 for a less generic name however.

krop added a comment.Dec 13 2017, 10:22 AM
In D9299#179035, @cgiboudeaux wrote:
In D9299#179032, @kfunk wrote:

If we'd name this file somewhat less generic then it could be even installed by default, no?

I had the scheme of the QNX setup script in my mind: https://github.com/acklinr/qnx660/blob/master/qnx660-env.sh

Thus: Maybe rename prefix.sh to say 'ecm-env.sh' and install the file by default?

Files overwritting each others is a no-go for packagers. +1 for a less generic name however.

Adding files in /usr directly, will also be rejected.

Now that I had a closer look: KDEInstallDirs only defines some paths, I don't think this change shall be part of this file. What about moving it to its own file and sourcing it instead ?

In D9299#179036, @cgiboudeaux wrote:
In D9299#179035, @cgiboudeaux wrote:
In D9299#179032, @kfunk wrote:

If we'd name this file somewhat less generic then it could be even installed by default, no?

I had the scheme of the QNX setup script in my mind: https://github.com/acklinr/qnx660/blob/master/qnx660-env.sh

Thus: Maybe rename prefix.sh to say 'ecm-env.sh' and install the file by default?

Files overwritting each others is a no-go for packagers. +1 for a less generic name however.

Adding files in /usr directly, will also be rejected.

Now that I had a closer look: KDEInstallDirs only defines some paths, I don't think this change shall be part of this file. What about moving it to its own file and sourcing it instead ?

I agree. KDEInstallDirs.cmake seems to be wrong location for this functionality.

What I'm envisioning is a ecm-env.sh-like script which gets installed into $PREFIX/bin as soon as you install ECM.

Pseudo-cmake code:

configure_file(${CMAKE_SOURCE_DIR}/ecm-env.sh ${CMAKE_BINARY_DIR}/ecm-env.sh)
install(FILES ${CMAKE_BINARY_DIR}/ecm-env.sh DESTINATION ${BIN_INSTALL_DIR})
apol marked 5 inline comments as done.Dec 13 2017, 12:29 PM
apol added a comment.Dec 13 2017, 12:35 PM

I agree. KDEInstallDirs.cmake seems to be wrong location for this functionality.

What I'm envisioning is a ecm-env.sh-like script which gets installed into $PREFIX/bin as soon as you install ECM.

Pseudo-cmake code:

configure_file(${CMAKE_SOURCE_DIR}/ecm-env.sh ${CMAKE_BINARY_DIR}/ecm-env.sh)
install(FILES ${CMAKE_BINARY_DIR}/ecm-env.sh DESTINATION ${BIN_INSTALL_DIR})

We won't be necessarily using the same prefix as ECM, in fact it's quite unlikely.

A motivation for me to work on this patch is to ease the ability to develop and deploy project builds into their own prefix rather than dumping it wherever the system is putting everything else. Setting up a separate prefix is easy at build-time (just pass -DCMAKE_INSTALL_PREFIX=/opt/mambo) but at runtime it is complex.

Alternatively we have this documentation to set it up: https://community.kde.org/Guidelines_and_HOWTOs/Build_from_source#Set_up_the_runtime_environment

apol updated this revision to Diff 23856.Dec 13 2017, 12:38 PM

Address issues

apol edited the test plan for this revision. (Show Details)Dec 13 2017, 12:39 PM
apol updated this revision to Diff 23858.Dec 13 2017, 12:40 PM

Added missing file

sitter accepted this revision.Dec 14 2017, 9:36 PM

I currently can't do a testbuild, but the code looks good now.

If we want to iterate on the concept further I think we should do it separately, no sense holding up this diff from landing over discussing potential additional features. The current use case seems perfectly reasonable in of itself.

This revision is now accepted and ready to land.Dec 14 2017, 9:36 PM
This revision was automatically updated to reflect the committed changes.
kfunk added a comment.Dec 15 2017, 3:58 PM

Sorry for being late, but I didn't have time to follow up on this one.

My concerns:

  • If you have that option ON and all frameworks install into the same prefix, prefix.sh will be overwritten. => Bad.
  • I still don't fully see how to adopt the use case apol mentioned: Say each Framework installs into its own unique prefix. => Ok.
    • But how are you supposed to source each prefix.sh? There must be script to do this? Where's the documentation then?
apol added a comment.Dec 15 2017, 4:12 PM
In D9299#179909, @kfunk wrote:

Sorry for being late, but I didn't have time to follow up on this one.

Sorry, maybe I should have waited ^^'

My concerns:

  • If you have that option ON and all frameworks install into the same prefix, prefix.sh will be overwritten. => Bad.

It's not too bad, they will all write the same.

  • I still don't fully see how to adopt the use case apol mentioned: Say each Framework installs into its own unique prefix. => Ok.
    • But how are you supposed to source each prefix.sh? There must be script to do this? Where's the documentation then?

so basically, to develop kate you do:

git clone kde:kate
cd kate
mkdir build
cd build
cmake .. -DCMAKE_INSTALL_PREFIX=/opt/banana
make install -j4

this itself will work as usual, now you can do

source prefix.sh
kate

and use it

If installation really bothers you, we can remove the installation altogether and suggest just calling it from the build directory.

I included because I started with kde:scratch/apol/prefixsetup in mind, which I use to test apps I develop on systems that I have where I just make install ; source /opt/mystuff/prefix.sh. But it's not necessary to do it like that and removing the option also can reduce the complexity.

dfaure added a subscriber: dfaure.Dec 24 2017, 5:28 PM

Please don't set LD_LIBRARY_PATH, it prevents running stuff from the builddir when hacking (I just added a note about that to https://community.kde.org/Guidelines_and_HOWTOs/Making_apps_run_uninstalled)