CMake: Install wxBUILD_FILE_ID into wxBUILD_INSTALL_LIBRARY_DIR (PR #25746)
Chris Mayo
I've tried this on 3.3.1 and 3.2.8.1 + f5ee95f.
This plus #25736 is enough for me to install a copy of wxWidgets that I can build applications on with existing scripting.
Some cautions:
DESTINATION "lib/wx/config"was a relative path - the change doesn't seem to matter- There are still some uses of
lib/wx/config/inwx-config-inplace.in,build/tools/build-wxwidgets.pyandbuild/cmake/config.cmake
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/25746
Commit Summary
- 5f82283 CMake: Install wxBUILD_FILE_ID into wxBUILD_INSTALL_LIBRARY_DIR
File Changes
(1 file)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/25746.patch
- https://github.com/wxWidgets/wxWidgets/pull/25746.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
MaartenBent left a comment (wxWidgets/wxWidgets#25746)DESTINATION "lib/wx/config" was a relative path - the change doesn't seem to matter
All install destinations should be relative paths to the CMAKE_INSTALL_PREFIX directory.
Only install(CODE ...) needs full paths.
You removed ${CMAKE_INSTALL_PREFIX} from the command creating the symlink. Is this intended? It seems wrong to me because then it creates a symlink to a target that will only exists when installing directly into /.
Instead $ENV{DESTDIR} should be removed. After reading again about DESTDIR, I think I erroneously added it (in #25466) so f5ee95f should be reverted.
There are still some uses of lib/wx/config/ in wx-config-inplace.in, build/tools/build-wxwidgets.py and build/cmake/config.cmake
The first one is used by configure too, is that broken as well?
I don't know about build-wxwidgets.py, but based on its git history it is an unmaintained file from before wxWidgets 2.9.
build/cmake/config.cmake should indeed be fixed. I think the following lines should also use the result of wx_get_install_dir:
set(libdir "\${exec_prefix}/lib") set(bindir "\${exec_prefix}/bin")
Is a trailing / guaranteed in library_dir or was that just me?
No, it is not guaranteed. wx_get_install_dir only returns a folder name. By default it is the 'lib' or 'bin' argument that is given as second argument. If you specify a different folder using wxBUILD_INSTALL_[RUNTIME/LIBRARY/ARCHIVE]_DIR options, this folder is returned.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Chris Mayo
cjmayo left a comment (wxWidgets/wxWidgets#25746)DESTINATION "lib/wx/config" was a relative path - the change doesn't seem to matter
All install destinations should be relative paths to the
CMAKE_INSTALL_PREFIXdirectory. Onlyinstall(CODE ...)needs full paths.
You removed
${CMAKE_INSTALL_PREFIX}from the command creating the symlink. Is this intended? It seems wrong to me because then it creates a symlink to a target that will only exists when installing directly into/.
I had just copied my configuration over from using --libdir with autotools and hence
-DwxBUILD_INSTALL_LIBRARY_DIR=/usr/lib64 which then lead me to remove ${CMAKE_INSTALL_PREFIX}.
After switching to -DwxBUILD_INSTALL_LIBRARY_DIR=lib64 all the libraries are now installing in my build directory - so I need to investigate further and then update this PR.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Chris Mayo
@cjmayo pushed 2 commits.
- bf58888 Revert "CMake: Include DESTDIR in symlink target"
- 8730c6e CMake: Install wxBUILD_FILE_ID into wxBUILD_INSTALL_LIBRARY_DIR
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Chris Mayo
cjmayo left a comment (wxWidgets/wxWidgets#25746)Some updates - I force pushed because the change is so small and it seemed simpler.
-
Added the revert of f5ee95f first.
-
Put
${CMAKE_INSTALL_PREFIX}back.
I believe I've solved the installing in the build directory #25756
The second part of wx_get_install_dir was adding the trailing /:
if(wxBUILD_INSTALL_PLATFORM_SUBDIR) if(${artifact}_dir) wx_string_append(${artifact}_dir ${GEN_EXPR_DIR}) endif() wx_string_append(${artifact}_dir "${wxPLATFORM_LIB_DIR}") endif()
I can prevent that by specifying -DwxBUILD_INSTALL_PLATFORM_SUBDIR=OFF
I'm not sure how to proceed with wx-config-inplace.in and build/cmake/config.cmake. I haven't really understood what they are doing yet.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
MaartenBent left a comment (wxWidgets/wxWidgets#25746)I can prevent that by specifying -DwxBUILD_INSTALL_PLATFORM_SUBDIR=OFF
That should not be necessary. I see it indeed adds a trailing slash via ${GEN_EXPR_DIR} when wxPLATFORM_LIB_DIR is empty (i.e. for non-msvc builds).
I think the best fix would be to only add it when wxPLATFORM_LIB_DIR is used, by changing the first if to:
if(wxBUILD_INSTALL_PLATFORM_SUBDIR AND wxPLATFORM_LIB_DIR)
I'm not sure how to proceed with wx-config-inplace.in and build/cmake/config.cmake. I haven't really understood what they are doing yet.
build/cmake/config.cmake set all the variables that are needed for wx-config.in and wx-config-inplace.in. All the @variable_name@ in these files are placeholders that get replaced by its actual value when wx-config and wx-config-inplace is created.
So if the wrong lib folder is used in these files, the variable in CMake needs to get the correct value and then that will be filled in in the generated file.
In the other pull request you mentioned you compared the output againts autotools/configure. The goal for CMake should be to have the same output.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
MaartenBent left a comment (wxWidgets/wxWidgets#25746)After seeing the errors in #25757, I think GEN_EXPR_DIR is not needed at all. It is only used for the build output directory. It is not relevant for installing.
if(wxBUILD_INSTALL_PLATFORM_SUBDIR AND wxPLATFORM_LIB_DIR) wx_string_append(${artifact}_dir "/${wxPLATFORM_LIB_DIR}") endif()
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Chris Mayo
@cjmayo pushed 2 commits.
- 7c97f1d CMake: Support wxBUILD_INSTALL_LIBRARY_DIR in wx_write_config
- b640ccd CMake: Use wxPLATFORM_LIB_DIR in wx-config-inplace.in
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Chris Mayo
cjmayo left a comment (wxWidgets/wxWidgets#25746)Some more added. That achieves a few things for me:
--- gtk3-unicode-3.3 +++ gtk3-unicode-3.3 @@ -404,7 +404,7 @@ # Determine the base directories we require. prefix=${input_option_prefix-${this_prefix:-/usr}} exec_prefix=${input_option_exec_prefix-${input_option_prefix-${this_exec_prefix:-${prefix}}}} -wxconfdir="${exec_prefix}/lib/wx/config" +wxconfdir="${exec_prefix}/lib64/wx/config" installed_configs=`cd "$wxconfdir" 2> /dev/null && ls | grep -v "^inplace-"` @@ -940,7 +940,7 @@ exec_prefix=${this_exec_prefix-$exec_prefix} includedir="${prefix}/include" -libdir="${exec_prefix}/lib" +libdir="${exec_prefix}/lib64" bindir="${exec_prefix}/bin" # Trivial queries we can answer now.
I don't understand wx-config-inplace.in so that commit is a guess. With autotools does it reflect --libdir='${prefix}'/64 in wx-config-inplace.in?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Chris Mayo
@cjmayo pushed 0 commits.
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Chris Mayo
cjmayo left a comment (wxWidgets/wxWidgets#25746)I've got my head round inplace now (I was 100% focussed on what ended up in DESTDIR and hadn't looked into the build directory).
In the build directory when using -DwxBUILD_INSTALL_LIBRARY_DIR=lib64 both files are still under lib/wx/config:
$ ls lib/wx/config/
gtk3-unicode-3.3 inplace-gtk3-unicode-3.3
No need for changes related to inplace. Deleted the last commit.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25746)@MaartenBent I'd like a confirmation from you before merging this please. It looks fine to me FWIW but this doesn't mean much concerning CMake...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.