Skip to content
This repository has been archived by the owner on Apr 15, 2020. It is now read-only.

Add 'tcl' package #1515

Merged
merged 3 commits into from Aug 9, 2018
Merged

Add 'tcl' package #1515

merged 3 commits into from Aug 9, 2018

Conversation

drodin
Copy link
Contributor

@drodin drodin commented Aug 7, 2018


if(NOT TCL_LIBRARY)
message(FATAL_ERROR "TCL_LIBRARY not found!")
endif()
add_library("tcl" UNKNOWN IMPORTED)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For imported libraries it's better to use tcl::tcl.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

IMPORTED_LOCATION "${TCL_LIBRARY}"
IMPORTED_LINK_INTERFACE_LIBRARIES "${interface_libraries}"
)
get_filename_component(TCL_LIB_DIR "${TCL_ORIG_LIBRARY}" DIRECTORY CACHE)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

@@ -0,0 +1,333 @@
# Copyright (c) 2015 Ruslan Baratov, Alexandre Pretyman
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to update original module instead of creating new and maintaining duplicate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cmake/modules/hunter_autotools_project.cmake updated

EXTRA_FLAGS
${extra_flags}
PATCH_COMMAND
${patch_command}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cleaned

list(APPEND cflags "--include android_def.h")
list(APPEND extra_flags "--enable-langinfo=no")
list(APPEND patch_command
&& ${CMAKE_COMMAND} -E copy "@HUNTER_PACKAGE_SCRIPT_DIR@/android_def.h" "@HUNTER_PACKAGE_SOURCE_DIR@"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please try not to use &&:

You can extend ExternalProject_Add step by COMMAND, I think it should work here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

substituted && by COMMAND
build ok https://travis-ci.com/drodin/hunter/builds/81224211

@@ -0,0 +1,41 @@
find_path(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are using pkgconfig-export-targets.cmake.in (see below) then this module is not needed, package can be imported by PkgConfig::tcl, see example:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tried it, and it's not working for this project at least without a complete rewrite of tcl.pc

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not working for this project

What are the errors? May be hunter_pkgconfig_export_target need to be improved?

Copy link
Contributor Author

@drodin drodin Aug 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a tcl.pc file after running ./configure on a Mac, building static lib

# tcl pkg-config source file

prefix=/Users/user/workspace/mobile/hunter/_Base/xxxxxxx/2dec4d1/c0825ff/Build/tcl/Install
exec_prefix=/Users/user/workspace/mobile/hunter/_Base/xxxxxxx/2dec4d1/c0825ff/Build/tcl/Install
libdir=/Users/user/workspace/mobile/hunter/_Base/xxxxxxx/2dec4d1/c0825ff/Build/tcl/Install/lib
includedir=${prefix}/include

Name: Tool Command Language
Description: Tcl is a powerful, easy-to-learn dynamic programming language, suitable for a wide range of uses.
URL: http://www.tcl.tk/
Version: 8.6.8
Requires.private: zlib >= 1.2.3
Libs: -L${libdir} -ltcl8.6 -ltclstub8.6
Libs.private:  -lz  -lpthread 
Cflags: -I${includedir}
  1. Requires.private: zlib >= 1.2.3 if I don't add package zlib, I get error -- Package 'zlib', required by 'tcl', not found otherwise Target "tcl_test" links to target "PkgConfig::zlib" but the target was not found. But there is really no need for hunter zlib, as configure tests for system zlib and if not found can use one provided with the package.

  2. Dependencies are not in Libs: thet are in Libs.private: so they are not included in _LDFLAGS or INTERFACE_LINK_LIBRARIES

  3. There is no dependency on the libm, that breaks testing on Linux

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But there is really no need for hunter zlib, as configure tests for system zlib and if not found can use one provided with the package

It should be always zlib from Hunter:

Target "tcl_test" links to target "PkgConfig::zlib" but the target was not found

I think this part is already handled quite well for autotools, take a look at DEPENDS_ON_PACKAGES variable, e.g.:

  • hunter_pick_scheme(DEFAULT url_sha1_autotools)
    set(x11_dependencies
    xproto
    xextproto
    xtrans
    xcb
    kbproto
    inputproto
    )
    hunter_cmake_args(
    x11
    CMAKE_ARGS # do not use double quotes on CMAKE_ARGS
    DEPENDS_ON_PACKAGES=${x11_dependencies}
    PKGCONFIG_EXPORT_TARGETS=x11-xcb;x11
    )

Dependencies are not in Libs: thet are in Libs.private

I don't know the details but I guess private libraries should not be used anywhere, if you got linker errors related to such libraries then they are not private - tcl.pc issue.

There is no dependency on the libm, that breaks testing on Linux

Yes, that's a tcl.pc issue too.

@@ -0,0 +1,3 @@
#if __ANDROID_API__ < 21
#undef HAVE_STRUCT_DIRENT64
#endif
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@@ -0,0 +1,41 @@
find_path(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not working for this project

What are the errors? May be hunter_pkgconfig_export_target need to be improved?

@ruslo ruslo merged commit 0aa7dbe into ruslo:master Aug 9, 2018
@ruslo
Copy link
Owner

ruslo commented Aug 9, 2018

@drodin drodin deleted the pr.tcl branch August 10, 2018 09:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants