Skip to content

feat: set tool path #526

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: develop2
Choose a base branch
from
Open

feat: set tool path #526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions conan_provider.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,30 @@ function(conan_install)
# success
set_property(GLOBAL PROPERTY CONAN_INSTALL_SUCCESS TRUE)
endif()

# set tool path
set(CONAN_TOOL_PATH)
string(JSON NUMBER_OF_NODES LENGTH ${conan_stdout} graph nodes)
math(EXPR NUMBER_OF_NODES "${NUMBER_OF_NODES}-1")
foreach(INDEX RANGE ${NUMBER_OF_NODES})
string(JSON CONTEXT GET ${conan_stdout} graph nodes ${INDEX} context)
if(NOT CONTEXT STREQUAL "build")
continue()
endif()

string(JSON BINDIRS GET ${conan_stdout} graph nodes ${INDEX} cpp_info root bindirs)
string(JSON BINDIRS_NODES LENGTH ${BINDIRS})
if(BINDIRS_NODES EQUAL 0)
continue()
endif()
math(EXPR BINDIRS_NODES "${BINDIRS_NODES}-1")
foreach(BINDIRS_INDEX RANGE ${BINDIRS_NODES})
string(JSON BINDIR GET ${BINDIRS} ${BINDIRS_INDEX})
list(APPEND CONAN_TOOL_PATH "${BINDIR}")
endforeach()
endforeach()
set(CONAN_TOOL_PATH ${CONAN_TOOL_PATH} CACHE STRING "Path to build tools")
message(STATUS "CMake-Conan: CONAN_TOOL_PATH=${CONAN_TOOL_PATH}")
endfunction()


Expand Down
9 changes: 9 additions & 0 deletions tests/resources/tool/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
cmake_minimum_required(VERSION 3.24)
project(MyToolApp CXX)

set(CMAKE_CXX_STANDARD 17)
find_package(hello REQUIRED)
find_program(
NINJA_EXECUTABLE ninja
PATHS ${CONAN_TOOL_PATH} REQUIRED
Copy link
Member

Choose a reason for hiding this comment

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

This sounds a bit against the intended "transparent" integration design.
Maybe it makes sense to add things directly to the path?

This might be related to a Conan feature we have thought a couple of times: generating CMake "env" files instead of shell script or .bat files with the environment definitions. Maybe worth giving it a try as a Conan built-in feature?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, it adds implicit manipulation of state. I could not think of any other way to do this. If you think it is better suited as a conan builtin, I can also create a PR there. Would this be a separate cmake-env generator?

Copy link
Member

Choose a reason for hiding this comment

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

That is a very good question, I don't know what the best interface would be, maybe an opt-in in CMakeDeps, like it shouldn't be necessary to modify all recipes for this to be active. It also depends on how the consumption story looks like.
Maybe start from a quick and dirty proof of concept as a new generator (cleaner to develop and review, and we can think later about the best UI)

Copy link
Member

Choose a reason for hiding this comment

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

Yes, of course, if you want to give it a try in Conan, go ahead. Just beware that this is pure experimentation, no guarantees that it can be moved forward, it will depend on some factors like space for future clean evolution, potential risks of being trapped in a local maxima, maintainability, etc. But if you are willing to try, that would be very welcome, of course! 🙂

NO_DEFAULT_PATH NO_CMAKE_FIND_ROOT_PATH)
5 changes: 5 additions & 0 deletions tests/resources/tool/conanfile.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[requires]
hello/0.1

[tool_requires]
ninja/1.11.1
21 changes: 21 additions & 0 deletions tests/test_smoke.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import os
import re
import platform
import shutil
import subprocess
Expand Down Expand Up @@ -134,6 +135,26 @@ def test_reconfigure_on_conanfile_changes(self, capfd, chdir_build):
assert all(expected in out for expected in expected_conan_install_outputs)


class TestTool:
@pytest.fixture(scope="class", autouse=True)
def tool_setup(self):
"Layout for tool test"
src_dir = Path(__file__).parent.parent
shutil.copytree(src_dir / 'tests' / 'resources' / 'tool', ".", dirs_exist_ok=True)
yield

@unix
def test_tool_path(self, capfd, chdir_build):
"Tools are available in CMake"
generator = "-GNinja" if platform.system() == "Windows" else ""

run(f"cmake --fresh .. -DCMAKE_PROJECT_TOP_LEVEL_INCLUDES=conan_provider.cmake -DCMAKE_BUILD_TYPE=Release {generator}")
out, _ = capfd.readouterr()
regex = re.compile(r".*--CMake-Conan:CONAN_TOOL_PATH=\/.*\/bin.*")
out_without_whitespace = "".join(out.split())
assert regex.match(out_without_whitespace)


class TestSubdir:
@pytest.fixture(scope="class", autouse=True)
def subdir_setup(self):
Expand Down