Use GNUInstallDirs for mapping installation directories by jonringer · Pull Request #1221 · libevent/libevent

jonringer · 2021-10-29T17:01:44Z

Was iterating on this package for Nixpkgs, and found it difficult to consume the .cmake configurations as get_filename_component(_INSTALL_PREFIX "${LIBEVENT_CMAKE_DIR}/../../.." ABSOLUTE) was not always true.

This should be a no-op for most libevent package maintainers.

azat

LGTM.

But this breaks the export test - https://github.com/libevent/libevent/runs/4050571079?check_suite_focus=true

Can you please take a look?

cmake/LibeventConfig.cmake.in

CMakeLists.txt

jonringer · 2021-11-08T14:52:11Z

The test failures were coming from the tests not being able to find LibeventConfig.cmake.

I removed the surrounding code which deletes the file and then regenerates it for the test cases. The tests were able to pass locally.

azat · 2021-11-08T20:40:59Z

CMakeLists.txt


-function(gen_package_config forinstall)
-    if(${forinstall})
-        set(CONFIG_FOR_INSTALL_TREE 1)


So, CONFIG_FOR_INSTALL_TREE can be removed from cmake/LibeventConfig.cmake.in?

I don't see why not. It's being installed to ${CMAKE_BINARY_DIR}, similar to the other .cmake files

But if we will remove CONFIG_FOR_INSTALL_TREE code, then LIBEVENT_INCLUDE_DIRS will be incorrect.

libevent/cmake/LibeventConfig.cmake.in

Lines 113 to 147 in 89505f8

if(CONFIG_FOR_INSTALL_TREE)

## Config for install tree ----------------------------------------

# Find includes

unset(_event_h CACHE)

find_path(_event_h

NAMES event2/event.h

PATHS "${_INSTALL_PREFIX}/include"

NO_DEFAULT_PATH)

if(_event_h)

set(LIBEVENT_INCLUDE_DIRS "${_event_h}")

message_if_needed(STATUS "Found libevent include directory: ${_event_h}")

else()

message_if_needed(WARNING "Your libevent library does not contain header files!")

endif()

# Find libraries

macro(find_event_lib _comp)

unset(_event_lib CACHE)

find_library(_event_lib

NAMES "event_${_comp}"

PATHS "${_INSTALL_PREFIX}/lib"

NO_DEFAULT_PATH)

if(_event_lib)

list(APPEND LIBEVENT_LIBRARIES "libevent::${_comp}")

set_case_insensitive_found(${_comp})

message_if_needed(STATUS "Found libevent component: ${_event_lib}")

else()

no_component_msg(${_comp})

endif()

endmacro()

foreach(comp ${_EVENT_COMPONENTS})

find_event_lib(${comp})

endforeach()

else()

P.S. Sorry for the delay.

tristan957 · 2022-06-30T19:16:27Z

What is the status on this PR? It would be nice to add this to the next release of libevent. I don't mind trying to take over the PR.

azat · 2022-06-30T19:18:03Z

What is the status on this PR? It would be nice to add this to the next release of libevent. I don't mind trying to take over the PR.

I think you can continue it. You can base you patches on top of this to preserve author, as long as you not completely change the patch. Thanks!

jonringer · 2022-06-30T21:46:50Z

sorry, I keep dropping things due to how many balls I'm trying to juggle. If you would like to take over, please do so!

I'll be happy to review :)

azat · 2022-07-09T17:22:29Z

@tristan957 are you going to finish this?

tristan957 · 2022-07-09T18:43:05Z

I have it on my list, but if someone else gets to it before I do even better.

- Don't destroy cmake file between test case invocations

azat · 2022-11-20T21:22:24Z

This is still a problem - #1221 (comment)

Though by some reason our test for export does not shows the problem.
This should be investigated.

azat · 2023-05-14T21:00:24Z

Merged in #1397

azat previously approved these changes Oct 29, 2021

View reviewed changes

azat self-assigned this Nov 5, 2021

azat reviewed Nov 5, 2021

View reviewed changes

cmake/LibeventConfig.cmake.in Outdated Show resolved Hide resolved

CMakeLists.txt Outdated Show resolved Hide resolved

jonringer dismissed azat’s stale review via 5d04336 November 8, 2021 14:47

jonringer force-pushed the use-gnu-install branch from 385e2f0 to ea73125 Compare November 8, 2021 14:50

azat reviewed Nov 8, 2021

View reviewed changes

azat removed their assignment Jul 9, 2022

azat mentioned this pull request Jul 10, 2022

libevent 2.2 release checklist #1094

Closed

21 tasks

azat self-assigned this Jul 10, 2022

azat force-pushed the use-gnu-install branch from ea73125 to 3bdb5e8 Compare July 10, 2022 10:52

jonringer and others added 3 commits November 13, 2022 18:55

Use GNUInstallDirs for mapping installation directories

42e14b6

Install LibeventConfig.cmake similar to other cmake files

c8be39c

- Don't destroy cmake file between test case invocations

Fix Configure_RPATH()

fce6e20

azat force-pushed the use-gnu-install branch from 3bdb5e8 to fce6e20 Compare November 13, 2022 18:01

tobim mentioned this pull request Jan 15, 2023

Use GNUInstallDirs for mapping installation directories (continued) #1397

Merged

azat closed this May 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use GNUInstallDirs for mapping installation directories#1221

Use GNUInstallDirs for mapping installation directories#1221
jonringer wants to merge 3 commits intolibevent:masterfrom
jonringer:use-gnu-install

jonringer commented Oct 29, 2021

Uh oh!

azat left a comment

Uh oh!

Uh oh!

Uh oh!

jonringer commented Nov 8, 2021

Uh oh!

azat Nov 8, 2021

Uh oh!

jonringer Nov 9, 2021

Uh oh!

azat Nov 21, 2021

Uh oh!

tristan957 commented Jun 30, 2022

Uh oh!

azat commented Jun 30, 2022

Uh oh!

jonringer commented Jun 30, 2022

Uh oh!

azat commented Jul 9, 2022

Uh oh!

tristan957 commented Jul 9, 2022

Uh oh!

azat commented Nov 20, 2022

Uh oh!

azat commented May 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

	if(CONFIG_FOR_INSTALL_TREE)
	## Config for install tree ----------------------------------------
	# Find includes
	unset(_event_h CACHE)
	find_path(_event_h
	NAMES event2/event.h
	PATHS "${_INSTALL_PREFIX}/include"
	NO_DEFAULT_PATH)
	if(_event_h)
	set(LIBEVENT_INCLUDE_DIRS "${_event_h}")
	message_if_needed(STATUS "Found libevent include directory: ${_event_h}")
	else()
	message_if_needed(WARNING "Your libevent library does not contain header files!")
	endif()

	# Find libraries
	macro(find_event_lib _comp)
	unset(_event_lib CACHE)
	find_library(_event_lib
	NAMES "event_${_comp}"
	PATHS "${_INSTALL_PREFIX}/lib"
	NO_DEFAULT_PATH)
	if(_event_lib)
	list(APPEND LIBEVENT_LIBRARIES "libevent::${_comp}")
	set_case_insensitive_found(${_comp})
	message_if_needed(STATUS "Found libevent component: ${_event_lib}")
	else()
	no_component_msg(${_comp})
	endif()
	endmacro()

	foreach(comp ${_EVENT_COMPONENTS})
	find_event_lib(${comp})
	endforeach()
	else()

Conversation

jonringer commented Oct 29, 2021

Uh oh!

azat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jonringer commented Nov 8, 2021

Uh oh!

azat Nov 8, 2021

Choose a reason for hiding this comment

Uh oh!

jonringer Nov 9, 2021

Choose a reason for hiding this comment

Uh oh!

azat Nov 21, 2021

Choose a reason for hiding this comment

Uh oh!

tristan957 commented Jun 30, 2022

Uh oh!

azat commented Jun 30, 2022

Uh oh!

jonringer commented Jun 30, 2022

Uh oh!

azat commented Jul 9, 2022

Uh oh!

tristan957 commented Jul 9, 2022

Uh oh!

azat commented Nov 20, 2022

Uh oh!

azat commented May 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants