Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clean up pkg-config files #1138

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
5 participants
@junghans
Copy link
Contributor

commented May 27, 2014

  • prefix contained build dir
  • rpath are Linux-specific and should not be in there
  • CFlags should only contain needed cflags, no optimizations
    or CMAKE_CXX_FLAGS
  • external libs should go in libs.private and not libs
  • lib -> @lib@
@StellarBot

This comment has been minimized.

Copy link
Contributor

commented May 27, 2014

Can one of the admins verify this patch?

@hkaiser

This comment has been minimized.

Copy link
Member

commented May 27, 2014

How can we test that? Would it be possible to create a test which utilizes pkg_config to guarantee this will not be broken in the future?

@hkaiser hkaiser added this to the 0.9.9 milestone May 27, 2014

clean up pkg-config files
- prefix contained build dir
- rpath are Linux-specific and should not be in there
- CFlags should only contain needed cflags, no optimizations
  or CMAKE_CXX_FLAGS
- external libs should go in libs.private and not libs
- lib -> @lib@
@junghans

This comment has been minimized.

Copy link
Contributor Author

commented May 27, 2014

Also dependencies, which have pkg-config files themselves, should go into "Requires:" and not ${external_libraries_name}. For example libtcmalloc: pkg-config --libs libtcmalloc

@junghans

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2014

A test would be to build and install hpx and then build and link a trivial hpx application using a command like

c++ `pkg-config --cflags hpx_application` test.cpp `pkg-config --libs hpx_applications`

For unneeded stuff in pkg-config files, there is of course no help, other than looking at the rpmlint warnings.

@eschnett

This comment has been minimized.

Copy link
Contributor

commented May 28, 2014

... and then build and link and run a trivial hpx application. Resolving shared libraries at run time often fails.

@junghans

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2014

Running won't work with the above command, except when adding:

-Wl,-rpath,`pkg-config --variable=libdir`

But it seems to be general consensus that pkg-config is not supposed to inject any kind of unneeded flags, which includes optimization flags as well as the rpath option, into a binary.

@eschnett

This comment has been minimized.

Copy link
Contributor

commented May 28, 2014

It doesn't have to be the -rpath option. However, when HPX has been properly installed and the application has bee properly built, then there has to be a well-defined way to make the application run, and this should be tested.

@junghans

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2014

I would provide a CMake template to build applications :

#change the name here or run cmake -DNAME="XXX"
set(NAME "template_serial" CACHE STRING "Name of the hpx app")
cmake_minimum_required(VERSION 2.8.4)
project(${NAME})
enable_language(CXX)
include(CheckCXXCompilerFlag)
find_package(HPX REQUIRED)
include_directories(${HPX_INCLUDE_DIRS})
file(GLOB ${NAME}_SOURCES ${NAME}*.cc)
#add extra source files here
add_executable(${NAME} ${${NAME}_SOURCES})
target_link_libraries(${NAME} ${HPX_LIBRARIES})
install(TARGETS ${NAME} RUNTIME DESTINATION bin)

This will work multi-plattform and encode rpath as needed.

@hkaiser

This comment has been minimized.

Copy link
Member

commented Jun 3, 2014

Ok, so what is left to be done here? Can this be merged?

@junghans

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2014

We need to come up with a good test case.

@hkaiser

This comment has been minimized.

Copy link
Member

commented Sep 3, 2014

With the cmake system being overhauled now - could we please come back to this issue and see what's still missing? Christoph, would you have the time to at least verify?

@sithhell

This comment has been minimized.

Copy link
Member

commented Sep 3, 2014

This should be fixed by ecdb034. Please reopen if the problem still persists.

@sithhell sithhell closed this Sep 3, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.