Hi all,
I noticed one problem regarding the cmake build of libraries with versioning and tdelfeditor. Tdelfeditor is used not only on file with the appropriate version, but also on base 'so' file without a version number. However, this is a symlink to the file with the appropriate version. Use tdelfeditor causes that instead of this symlink is created a regular file. Results are then two full 'so' files, which is incorrect. For example, in tdebase (current stable R14.0.3 Wheezy@amd64):
libkonq.so - size 809232 (package libkonq4-trinity-dev) libkonq.so.4.2.0 - size 809240 (package libkonq4-trinity)
Solutions should be simple: in the common cmake module run tdelfeditor only on libraries with the appropriate version and on base files only if the library is not versioned. In some cases tdelfeditor is used, although library is not shared == tdelfeditor is started without valid arguments. See proposed patch.
I believe that there is no reason against to push the patch. There will be only one consequence - the patch causes rebuild almost all packages.
On 03/14/2016 10:18 PM, Slávek Banko wrote:
Hi all,
I noticed one problem regarding the cmake build of libraries with versioning and tdelfeditor. Tdelfeditor is used not only on file with the appropriate version, but also on base 'so' file without a version number. However, this is a symlink to the file with the appropriate version. Use tdelfeditor causes that instead of this symlink is created a regular file. Results are then two full 'so' files, which is incorrect. For example, in tdebase (current stable R14.0.3 Wheezy@amd64):
libkonq.so - size 809232 (package libkonq4-trinity-dev) libkonq.so.4.2.0 - size 809240 (package libkonq4-trinity)
Solutions should be simple: in the common cmake module run tdelfeditor only on libraries with the appropriate version and on base files only if the library is not versioned. In some cases tdelfeditor is used, although library is not shared == tdelfeditor is started without valid arguments. See proposed patch.
I believe that there is no reason against to push the patch. There will be only one consequence - the patch causes rebuild almost all packages.
Hi Slavek, no problem if the patch needs to be pushed. Anyhow I do not see this problem on my system. See attached screenshot, tdebase rebuilt 3 days ago. Maybe something else? Cheers Michele
2016-03-15 0:18 GMT+03:00 Slávek Banko slavek.banko@axis.cz:
Hi all,
I noticed one problem regarding the cmake build of libraries with versioning and tdelfeditor. Tdelfeditor is used not only on file with the appropriate version, but also on base 'so' file without a version number. However, this is a symlink to the file with the appropriate version. Use tdelfeditor causes that instead of this symlink is created a regular file. Results are then two full 'so' files, which is incorrect. For example, in tdebase (current stable R14.0.3 Wheezy@amd64):
libkonq.so - size 809232 (package libkonq4-trinity-dev) libkonq.so.4.2.0 - size 809240 (package libkonq4-trinity)
Solutions should be simple: in the common cmake module run tdelfeditor only on libraries with the appropriate version and on base files only if the library is not versioned. In some cases tdelfeditor is used, although library is not shared == tdelfeditor is started without valid arguments. See proposed patch.
I believe that there is no reason against to push the patch. There will be only one consequence - the patch causes rebuild almost all packages.
-- Slávek
Index: b/cmake/modules/TDEMacros.cmake
--- a/cmake/modules/TDEMacros.cmake +++ b/cmake/modules/TDEMacros.cmake @@ -835,13 +835,7 @@ # embed name and metadata set( ELF_EMBEDDING_METADATA ""${_target}" "${_description}" "${_license}" "${_copyright}" "${_authors}" "${_product}" "${_organization}" "${_version}" "${_datetime}" "x-sharedlib" "${TDE_SCM_MODULE_NAME}" "${TDE_SCM_MODULE_REVISION}" "${_notes}"" ) separate_arguments( ELF_EMBEDDING_METADATA )
- if( EXISTS ${CMAKE_INSTALL_PREFIX}/bin/tdelfeditor )
- add_custom_command(
TARGET ${_target}POST_BUILDCOMMAND ${CMAKE_INSTALL_PREFIX}/bin/tdelfeditor -m ${CMAKE_CURRENT_BINARY_DIR}/${_soname} ${ELF_EMBEDDING_METADATA} || trueCOMMAND ${CMAKE_INSTALL_PREFIX}/bin/tdelfeditor -e ${CMAKE_CURRENT_BINARY_DIR}/${_soname} || true- )
- if( EXISTS ${CMAKE_INSTALL_PREFIX}/bin/tdelfeditor AND _soname ) if( _version ) add_custom_command( TARGET ${_target}
@@ -849,6 +843,13 @@ COMMAND ${CMAKE_INSTALL_PREFIX}/bin/tdelfeditor -m ${CMAKE_CURRENT_BINARY_DIR}/${_soname}.${_version} ${ELF_EMBEDDING_METADATA} || true COMMAND ${CMAKE_INSTALL_PREFIX}/bin/tdelfeditor -e ${CMAKE_CURRENT_BINARY_DIR}/${_soname}.${_version} || true )
- else( )
add_custom_command(TARGET ${_target}POST_BUILDCOMMAND ${CMAKE_INSTALL_PREFIX}/bin/tdelfeditor -m ${CMAKE_CURRENT_BINARY_DIR}/${_soname} ${ELF_EMBEDDING_METADATA} || trueCOMMAND ${CMAKE_INSTALL_PREFIX}/bin/tdelfeditor -e ${CMAKE_CURRENT_BINARY_DIR}/${_soname} || true)
Few notes about the patch: - At least you should use ${BIN_INSTALL_DIR} rather ${CMAKE_INSTALL_PREFIX}/bin. - It is generally a bad practice to search for a file on an installation path. - Starting cmake 2.8.4 you may refer a ${_target} in a commands and cmake should replace it with the appropriate filepath so you may get rid of the if.
2016-03-15 10:59 GMT+03:00 Michele Calgaro michele.calgaro@yahoo.it:
On 03/14/2016 10:18 PM, Slávek Banko wrote: Hi Slavek, no problem if the patch needs to be pushed. Anyhow I do not see this problem on my system. See attached screenshot, tdebase rebuilt 3 days ago. Maybe something else? Cheers Michele
Do you have tdelfeditor installed on the building system? Note that It is getting build only if you build tdelibs WITH_ELFICON
Dne út 15. března 2016 Fat-Zer napsal(a):
Few notes about the patch:
- At least you should use ${BIN_INSTALL_DIR} rather
${CMAKE_INSTALL_PREFIX}/bin.
Now is quite useful, in connection with the CMake conversion of tdegames, that is not used ${BIN_INSTALL_DIR} but ${CMAKE_INSTALL_PREFIX}/bin.
For some reason the games is installed into the 'games' instead of 'bin' (at least on Debian and Ubuntu). Therefore, on Debian and Ubuntu is for build used -DBIN_INSTALL_DIR="/opt/trinity/games". If for tdelfeditor should be used ${BIN_INSTALL_DIR}, it would not be found. Thanks to the use ${CMAKE_INSTALL_PREFIX}/bin it also works for tdegames.
- It is generally a bad practice to search for
a file on an installation path.
Your observation is correct. Perhaps it would be appropriate to use find_program with hints "${TDE_PREFIX}/bin" and ${BIN_INSTALL_DIR} - just like is in search for tde-config. And then use the value ${TDELFEDITOR_EXECUTABLE}. What do you think?
- Starting cmake 2.8.4 you may refer a
${_target} in a commands and cmake should replace it with the appropriate filepath so you may get rid of the if.
For r14.0.x branch is still supported: + Debian 6.x (Squeeze) == cmake 2.8.2 + Ubuntu 10.04 (Lucid) == cmake 2.8.0 + Ubuntu 10.10 (Maverick) == cmake 2.8.2 + Ubuntu 11.04 (Natty) == cmake 2.8.3
Thank you for your good comments!
Dne út 15. března 2016 Slávek Banko napsal(a):
Your observation is correct. Perhaps it would be appropriate to use find_program with hints "${TDE_PREFIX}/bin" and ${BIN_INSTALL_DIR} - just like is in search for tde-config. And then use the value ${TDELFEDITOR_EXECUTABLE}. What do you think?
Proposed patch attached.
2016-03-15 17:03 GMT+03:00 Slávek Banko slavek.banko@axis.cz:
Dne út 15. března 2016 Slávek Banko napsal(a):
Your observation is correct. Perhaps it would be appropriate to use find_program with hints "${TDE_PREFIX}/bin" and ${BIN_INSTALL_DIR} - just like is in search for tde-config. And then use the value ${TDELFEDITOR_EXECUTABLE}. What do you think?
Proposed patch attached.
-- Slávek
Looks good... Note that you may still get rid of the "if (_version)" with something like «get_target_property ( _lib_location ${_target} LOCATION )» if you wish...
Dne út 15. března 2016 Fat-Zer napsal(a):
2016-03-15 17:03 GMT+03:00 Slávek Banko slavek.banko@axis.cz:
Dne út 15. března 2016 Slávek Banko napsal(a):
Your observation is correct. Perhaps it would be appropriate to use find_program with hints "${TDE_PREFIX}/bin" and ${BIN_INSTALL_DIR} - just like is in search for tde-config. And then use the value ${TDELFEDITOR_EXECUTABLE}. What do you think?
Proposed patch attached.
-- Slávek
Looks good... Note that you may still get rid of the "if (_version)" with something like «get_target_property ( _lib_location ${_target} LOCATION )» if you wish...
A few lines above in the TDEMacros is the part that uses:
get_target_property( _output ${_target} LOCATION )
However, even in this section is also used ${_version}. It believe that LOCATION returns the name of the library without version.
2016-03-15 22:22 GMT+03:00 Slávek Banko slavek.banko@axis.cz:
Dne út 15. března 2016 Fat-Zer napsal(a): A few lines above in the TDEMacros is the part that uses:
get_target_property( _output ${_target} LOCATION )
However, even in this section is also used ${_version}. It believe that LOCATION returns the name of the library without version.
My bad... so nevermind...
Slavek, hi, your patch was malformed — sorry, noticed it just now... It breaks build of tdebase if tdelibs was built without libr/-DWITH_ELFICON=OFF. Build log with error: https://bpaste.net/show/5e4c40ea2dc5
You shouldn't use __internal_find_program here. Proposed patch in the attach. Also a small fix to error syntax...
On Wednesday 13 of April 2016 19:35:40 Fat-Zer wrote:
Slavek, hi, your patch was malformed — sorry, noticed it just now... It breaks build of tdebase if tdelibs was built without libr/-DWITH_ELFICON=OFF. Build log with error: https://bpaste.net/show/5e4c40ea2dc5
You shouldn't use __internal_find_program here. Proposed patch in the attach. Also a small fix to error syntax...
Thank you for the patch - pushed right now.
On 2016/03/15 01:43 PM, Fat-Zer wrote:
Do you have tdelfeditor installed on the building system? Note that It is getting build only if you build tdelibs WITH_ELFICON
All my builds are clean chroot environment builds. Since both Slavek and I use the tde-packaging building scripts, the build should be mostly the same. Only known difference is the packaging version numbering, since I use a different convention for that. But that should not affect the way packages are built. Cheers Michele
Dne út 15. března 2016 Michele Calgaro napsal(a):
On 2016/03/15 01:43 PM, Fat-Zer wrote:
Do you have tdelfeditor installed on the building system? Note that It is getting build only if you build tdelibs WITH_ELFICON
All my builds are clean chroot environment builds. Since both Slavek and I use the tde-packaging building scripts, the build should be mostly the same. Only known difference is the packaging version numbering, since I use a different convention for that. But that should not affect the way packages are built. Cheers Michele
You can test it by this:
1) Run: pbuilder login - to get clean chroot - and next in this chroot
2) Run: aptitude install libkonq4-trinity-dev
3) Verify, whether /opt/trinity/lib/libkonq.so is a symlink
4) Run: /opt/trinity/bin/tdelfeditor -m /opt/trinity/lib/libkonq.so "konq-shared" "" "" "" "" "Trinity Desktop Environment" "" "4.2.0" "03/15/2016 00:50:10" "x-sharedlib" "tdebase" "-1c64345fe4da851982f69ebf6e92bc88cebbad46" ""
5) Verify, whether /opt/trinity/lib/libkonq.so is still a symlink
On my builder and also on build farm, after call "tdelfeditor -m ..." is symlink replaced by regular file.
You can test it by this:
- Run: pbuilder login
- to get clean chroot
- and next in this chroot
Run: aptitude install libkonq4-trinity-dev
Verify, whether /opt/trinity/lib/libkonq.so is a symlink
Yes, it is a symlink
- Run: /opt/trinity/bin/tdelfeditor -m /opt/trinity/lib/libkonq.so
"konq-shared" "" "" "" "" "Trinity Desktop Environment" "" "4.2.0" "03/15/2016 00:50:10" "x-sharedlib" "tdebase" "-1c64345fe4da851982f69ebf6e92bc88cebbad46" ""
- Verify, whether /opt/trinity/lib/libkonq.so is still a symlink
No, it is no longer a symlink
On my builder and also on build farm, after call "tdelfeditor -m ..." is symlink replaced by regular file.
Cheers Michele
Dne út 15. března 2016 Michele Calgaro napsal(a):
You can test it by this:
- Run: pbuilder login
- to get clean chroot
- and next in this chroot
Run: aptitude install libkonq4-trinity-dev
Verify, whether /opt/trinity/lib/libkonq.so is a symlink
Yes, it is a symlink
- Run: /opt/trinity/bin/tdelfeditor -m /opt/trinity/lib/libkonq.so
"konq-shared" "" "" "" "" "Trinity Desktop Environment" "" "4.2.0" "03/15/2016 00:50:10" "x-sharedlib" "tdebase" "-1c64345fe4da851982f69ebf6e92bc88cebbad46" ""
- Verify, whether /opt/trinity/lib/libkonq.so is still a symlink
No, it is no longer a symlink
On my builder and also on build farm, after call "tdelfeditor -m ..." is symlink replaced by regular file.
Cheers Michele
Interestingly. It is not clear to me why this does not happen to you during standard builds!
-
Fat-Zer -
Michele Calgaro -
Slávek Banko