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

Provide a way to disable hunter. #219

Closed
ghost opened this issue Aug 3, 2017 · 29 comments
Closed

Provide a way to disable hunter. #219

ghost opened this issue Aug 3, 2017 · 29 comments

Comments

@ghost
Copy link

ghost commented Aug 3, 2017

To ensure it's easier to integrate with any distribution it would be great to be able
to disable hunter and build against the system libraries, leaving the package handling
to the underlaying operative system, meanwhile building with it for the ones that
want upstream support.

@chfast
Copy link
Contributor

chfast commented Aug 3, 2017

This is definitely possible, but I think this is bad idea. How is going to maintain it?

@ghost
Copy link
Author

ghost commented Aug 3, 2017

Well, I was in a try to create a gentoo ebuild for ethminer and hunter got into my way.
The idea is that the package manager takes care of building against the system libraries
so that the application is not stand-alone and shipping it's own built libraries. The package
would take care of the dependencies.

@chfast
Copy link
Contributor

chfast commented Aug 3, 2017

You can disable Hunter with -DHUNTER_ENABLED=OFF. But CMake will not find dependencies because it does not know how.

You probably will end up with code like:

if(HUNTER_ENABLED)
    hunter_add_package(CURL)
    find_package(CURL CONFIG)
else()
    find_package(CURL)
endif()

@ghost
Copy link
Author

ghost commented Aug 3, 2017

I agree, it does sound that I'll have to previously patch the cmake files, would it be accepted upstream any cmake changes that turns up from the work on the ebuild? Portage can keep it side by side, but better if there is no patch at all in that side.

Thanks!

@ruslo
Copy link
Contributor

ruslo commented Aug 3, 2017

The idea is that the package manager takes care of building against the system libraries
so that the application is not stand-alone and shipping it's own built libraries

Since final result is executable and by default Hunter build static libraries there will be no "own built libraries".

@ghost
Copy link
Author

ghost commented Aug 3, 2017

Well, if you consider the size of the binary and the build time, sorry, it's still not making it.

@ruslo
Copy link
Contributor

ruslo commented Aug 3, 2017

Well, if you consider the size of the binary and the build time, sorry, it's still not making it.

Yes, it's true. I'm just saying that own build libraries is not an argument.

You probably will end up with code like

Something like this at the end:

if(HUNTER_ENABLED)
  set(curl_config "CONFIG")
else()
  set(curl_config "")
endif()

hunter_add_package(CURL)
find_package(CURL ${curl_config} REQUIRED)

if(HUNTER_ENABLED)
  target_link_libraries(... PUBLIC CURL::libcurl)
else()
  target_include_directories(... PUBLIC ${CURL_INCLUDE_DIRS})
  target_link_libraries(... PUBLIC ${CURL_LIBRARIES})
endif()

Related docs:

@ghost
Copy link
Author

ghost commented Aug 3, 2017

That seems more accurate, I'll give it a try when I get some cycles. Thanks ruslo!

@ruslo
Copy link
Contributor

ruslo commented Aug 3, 2017

I think I found a way to encapsulate it even more.

When Hunter is disabled we add extra path to CMAKE_PREFIX_PATH:

if(NOT HUNTER_ENABLED)
  list(APPEND CMAKE_PREFIX_PATH "${CMAKE_CURRENT_LIST_DIR}/cmake/Hunter/disabled-mode")
endif()

Note: this one can be added as a part of HunterGate functionality

And put fake CURLConfig.cmake there:

# cmake/Hunter/disabled-mode/CURLConfig.cmake

if(TARGET CURL::libcurl)
  return()
endif()

find_package(CURL MODULE REQUIRED)

add_library(CURL::libcurl UNKNOWN IMPORTED)

set_target_properties(
    CURL::libcurl
    PROPERTIES
    INTERFACE_INCLUDE_DIRECTORIES "${CURL_INCLUDE_DIRS}"
    IMPORTED_LOCATION "${CURL_LIBRARIES}"
)

In this case CMakeLists.txt will not change at all:

hunter_add_package(CURL)
find_package(CURL CONFIG REQUIRED)
targe_link_libraries(... PUBLIC CURL::libcurl)

When HUNTER_ENABLED=ON:

  • hunter_add_package(CURL) will download/install CURL
  • find_package(CURL CONFIG REQUIRED) will find real CURLConfig.cmake in Hunter with real CURL::libcurl

When HUNTER_ENABLED=OFF:

  • hunter_add_package(CURL) will be ignored
  • find_package(CURL CONFIG REQUIRED) will load fake CURLConfig.cmake from cmake/Hunter/disabled-mode
  • fake CURLConfig.cmake will call find_package(CURL REQUIRED) to find system libraries and create imported target CURL::libcurl

Since it may be useful for other projects I can create separate repository for cmake/Hunter/disabled-mode modules. We can add it as a submodule to ethminer. @chfast what do you think?

@chfast
Copy link
Contributor

chfast commented Aug 4, 2017

Looks awesome.
I'm not sure what from that is not done yet. Can you create a list of issues in Hunter GitHub?
Also, how would you include disabled-mode repo in Hunter releases?

@ghost
Copy link
Author

ghost commented Aug 4, 2017

ruslo can correct me but I think from your side is fair enough to add the following snippet to
your .gitmodules:

[submodule "Hunter"]
path = cmake/Hunter
url = https://github.com/ruslo/Hunter

@ruslo
Copy link
Contributor

ruslo commented Aug 4, 2017

Can you create a list of issues in Hunter GitHub?

See hunter-packages/disabled-mode#1

Also, how would you include disabled-mode repo in Hunter releases?

There is no need to add them to Hunter release because they designed to be used without Hunter. In theory I guess they can be managed as Hunter package but it's sounds too hairy: HUNTER_ENABLED is OFF but we still create HUNTER_ROOT directory, download Hunter release and download packages.

I think user have to decide how HUNTER_ENABLED=OFF is managed. Either manually by if(HUNTER_ENABLED) like shown in this comment or manually copying disabled-mode packages or adding disabled-mode submodule, etc.

ruslo can correct me but I think from your side is fair enough to add the following snippet to

No, just a tiny submodule. In general if you're not Hunter developer you never should clone this repository.

@ghost
Copy link
Author

ghost commented Aug 4, 2017

I see:
[submodule "disabled-mode"]
path = cmake/Hunter/disabled-mode
https://github.com/hunter-packages/disabled-mode

@ruslo
Copy link
Contributor

ruslo commented Aug 4, 2017

HunterGate and CURL are ready:

@EoD
Copy link
Contributor

EoD commented Aug 5, 2017

FYI
I don't want to disrupt the conversation, but I wanted to mention that I just use the brute force approach in a similar environment and it worked so far:

	sed -r -i \
		-e '/hunter_add_package\(.*\)/d' \
		-e 's/(find_package\(.*) CONFIG (.*\))/\1 \2/' \
		libethash-cl/CMakeLists.txt \
		CMakeLists.txt || die

	sed -r -i \
		-e '1s/^/set\(CMAKE_CXX_STANDARD 14\)\n\n/' \
		-e 's/include(\(HunterGate\))/function\1\nendfunction\(\)/' \
		-e '/find_package\(libjson-rpc-cpp.*\)/d' \
		-e '/find_package\(PythonInterp\)/d' \
		-e '/include\(EthCompilerSettings\)/d' \
		CMakeLists.txt || die

	sed -r -i \
		-e 's/libjson-rpc-cpp\:\:client/jsoncpp jsonrpccpp\-client jsonrpccpp\-common/' \
		ethminer/CMakeLists.txt || die

	sed -r -i \
		-e '1s/^/include_directories\(\/usr\/include\/jsoncpp\)\n\n/' \
		ethminer/CMakeLists.txt \
		libstratum/CMakeLists.txt || die

	sed -r -i \
		-e 's/(jsoncpp)_lib_static/\1 devcore/' \
		libstratum/CMakeLists.txt || die

This is from https://gpo.zugaina.org/AJAX/Ebuild/31566206/View

All in all, I would really appreciate the option to disable the hunter requirement.

@ghost
Copy link
Author

ghost commented Aug 6, 2017

Wonderful, the ebuild worked for me as well, thanks for reporting. It's not elegant so far to hack
the build with sed, but for know it's the only option, and my tries with the disabled mode still needs
some workarounds to drop CONFIG word from CMakeLists.txt, so lets keep the discussion alive
and see where we come up.

@EoD EoD mentioned this issue Aug 8, 2017
@ruslo
Copy link
Contributor

ruslo commented Aug 10, 2017

Fixed by #229

@chfast chfast closed this as completed Aug 10, 2017
@EoD
Copy link
Contributor

EoD commented Aug 11, 2017

Could we also add some documentation on how to disable it?

If you enable hunter, you are probably running into ruslo/hunter#423 as well. Is it worth opening another bug here?

@ruslo
Copy link
Contributor

ruslo commented Aug 11, 2017

Is it worth opening another bug here?

It's a known issue and actually there is a pending pull request with fix:

However it seems that author of pull request abandon it. I can finish it but I need someone to test the fix. I don't have Gentoo and not planning to install one.

@ghost
Copy link
Author

ghost commented Aug 11, 2017

I'm happy to test it for you, I'd like to see some day that we simply cmake .. -DHUNTER_ENABLED=OFF
and we get a proper build.

@EoD
Copy link
Contributor

EoD commented Aug 11, 2017

@PikkuJose I think @ruslo was talking about testing ruslo/hunter#874 and not testing a toggle to disable hunter for ethminer.

I think we should move the discussion about ruslo/hunter#874 to ruslo/hunter#874 and talk only about the toggle to disable hunter for ethminer here. Otherwise it will get confusing here.

@ruslo
Copy link
Contributor

ruslo commented Aug 11, 2017

@PikkuJose We are talking about different things I think. There is a patch that will fix Boost on Gentoo and will make possible to build ethminer with Hunter on Gentoo. That what I asking to test. HUNTER_ENABLED=OFF is build without Hunter, meaning you have to install dependencies yourself (emerge on Gentoo as far as I remember).

@ghost
Copy link
Author

ghost commented Aug 11, 2017

Yes, agreed, it was a different thing I misunderstood. Getting back to the switch, the stubs are still
still missing, so when you run the disabled mode as I said in the previous comment you just hit the
following:

CMake Error at CMakeLists.txt:113 (find_package):
Could not find a package configuration file provided by "Boost" with any of
the following names:

BoostConfig.cmake
boost-config.cmake

Add the installation prefix of "Boost" to CMAKE_PREFIX_PATH or set
"Boost_DIR" to a directory containing one of the above files. If "Boost"
provides a separate development package or SDK, be sure it has been
installed.

I'm happy to work on it when I get some cycles. But I let you know if you have more availability.

@ruslo
Copy link
Contributor

ruslo commented Aug 11, 2017

@PikkuJose check that you have latest version with my commit where "disabled-mode" introduced, also check you have submodule initialized: git submodule update --init .

@reagentoo
Copy link

reagentoo commented Aug 13, 2017

How about to add

option(HUNTER_ENABLED "Enable Hunter package manager support" ON)
if(HUNTER_ENABLED)
	include(HunterGate)
	HunterGate( ... )
endif()

to main CMakeLists.txt? I consider this approach more reliable.
And also to put hunter_add_package(...) inside if-blocks in CMakeLists.txt, libethash-cl/CMakeLists.txt.

@EoD
Copy link
Contributor

EoD commented Aug 13, 2017

I agree. Disabling hunter by adding a hunter module does not sound very resilient. So far I haven't been able to successfully (and reliably) disable hunter despite all the hints provided here.

@ruslo
Copy link
Contributor

ruslo commented Aug 14, 2017

How about to add

HUNTER_ENABLED is ON by default so adding option(HUNTER_ENABLED ... ON) is redundant. Same for if(HUNTER_ENABLED) HunterGate(...). HunterGate will only update CMAKE_PREFIX_PATH for "disabled-mode" if HUNTER_ENABLED=OFF, no downloads or anything.

Disabling hunter by adding a hunter module does not sound very resilient

You don't need to add HunterGate to disable Hunter. We already have HunterGate in code. As I already said it will do almost nothing if HUNTER_ENABLED=OFF.

@EoD
Copy link
Contributor

EoD commented Aug 14, 2017

@ruslo don't get me wrong, I like the idea of hunter. This way people without the proper background can easily build ethminer without anything else. I also don't mind that it's enabled by default.

But from a perspective of an advanced developer or even a package maintainer for a distribution (like reagentoo), it really should be as simple as possible. The more complexity a build process does have (which is introduced by hunter) the more likely it will fail at some point in the future. So having hunter as a dependency to disable hunter, is exactly why it's less resilient in the long run.

@ruslo
Copy link
Contributor

ruslo commented Aug 14, 2017

But from a perspective of an advanced developer or even a package maintainer for a distribution (like reagentoo), it really should be as simple as possible

Add -DHUNTER_ENABLED=OFF - that's all. How it can be simplier?

So having hunter as a dependency to disable hunter

I don't understand what you mean by this. Please explain it in details, I think there is huge misunderstanding about how everything is chained together.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants