dfaure added a comment.
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)
REPOSITORY
R240 Extra CMake Modules
REVISION DETAIL
apol added a comment.
In https://phabricator.kde.org/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
kfunk added a comment.
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
This revision was automatically updated to reflect the committed changes.
Closed by commit R240:24134c972e37: Introduce INSTALL_PREFIX_SCRIPT to easily
set up prefixes (authored by apol).
REPOSITORY
R240 Extra CMake Modules
CHANGES SINCE LAST UPDATE
sitter accepted this revision.
sitter added a comment.
This revision is now accepted and ready to land.
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
apol updated this revision to Diff 23858.
apol added a comment.
Added missing file
REPOSITORY
R240 Extra CMake Modules
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D9299?vs=23856=23858
BRANCH
prefix
REVISION DETAIL
https://phabricator.kde.org/D9299
AFFECTED FILES
apol edited the test plan for this revision.
REPOSITORY
R240 Extra CMake Modules
REVISION DETAIL
https://phabricator.kde.org/D9299
To: apol, #frameworks, sitter
Cc: kfunk, bcooksley, ngraham, sitter, cgiboudeaux, #build_system
apol updated this revision to Diff 23856.
apol added a comment.
Address issues
REPOSITORY
R240 Extra CMake Modules
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D9299?vs=23817=23856
BRANCH
prefix
REVISION DETAIL
https://phabricator.kde.org/D9299
AFFECTED FILES
apol added a comment.
> 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:
>
>
apol marked 5 inline comments as done.
REPOSITORY
R240 Extra CMake Modules
REVISION DETAIL
https://phabricator.kde.org/D9299
To: apol, #frameworks, sitter
Cc: kfunk, bcooksley, ngraham, sitter, cgiboudeaux, #build_system
kfunk added a comment.
In https://phabricator.kde.org/D9299#179036, @cgiboudeaux wrote:
> In https://phabricator.kde.org/D9299#179035, @cgiboudeaux wrote:
>
> > In https://phabricator.kde.org/D9299#179032, @kfunk wrote:
> >
> > > If we'd name this file somewhat less generic then
cgiboudeaux added a comment.
In https://phabricator.kde.org/D9299#179035, @cgiboudeaux wrote:
> In https://phabricator.kde.org/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
cgiboudeaux added a comment.
In https://phabricator.kde.org/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:
kfunk added inline comments.
INLINE COMMENTS
> sitter wrote in KDEInstallDirs.cmake:699
> 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
kfunk added a comment.
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
bcooksley added a comment.
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
sitter requested changes to this revision.
sitter added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> KDEInstallDirs.cmake:699
> +if(INSTALL_PREFIX_SCRIPT)
> +file(WRITE ${CMAKE_BINARY_DIR}/prefix.sh "
> +export
cgiboudeaux added a comment.
+1. Please add the missing documentation about this new option.
INLINE COMMENTS
> apol wrote in KDEInstallDirs.cmake:700-704
> Usually isn't necessary as you get the libraries through the rpath. I can add
> it though.
It won't harm and solve potential issues if
apol added inline comments.
INLINE COMMENTS
> cgiboudeaux wrote in KDEInstallDirs.cmake:700-704
> missing LD_LIBRARY_PATH ?
Usually isn't necessary as you get the libraries through the rpath. I can add
it though.
> cgiboudeaux wrote in KDEInstallDirs.cmake:707
> I'm not sure that's the right
cgiboudeaux added inline comments.
INLINE COMMENTS
> KDEInstallDirs.cmake:700-704
> +export XDG_DATA_DIRS=${KDE_INSTALL_FULL_DATADIR}:$XDG_DATA_DIRS
> +export XDG_CONFIG_DIRS=${KDE_INSTALL_FULL_CONFDIR}:$XDG_CONFIG_DIRS
> +export PATH=${KDE_INSTALL_FULL_BINDIR}:$PATH
> +export
apol created this revision.
apol added a reviewer: Frameworks.
Restricted Application added projects: Frameworks, Build System.
Restricted Application added a subscriber: Build System.
REVISION SUMMARY
If enabled it will install a prefix.sh script to the root of the prefix.
This file will be
21 matches
Mail list logo
