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.

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

Jump to bottom

remove conan_message #11625

Merged
merged 2 commits into from Jul 13, 2022
Merged

remove conan_message #11625

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a5d12b3
remove conan_message
memsharded Jul 12, 2022
3913fe9
fixing tests
memsharded Jul 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions conan/tools/cmake/cmake.py
View file
Open in desktop
@@ -1,11 +1,9 @@
import os
import platform

from conan.tools.build import build_jobs
from conan.tools.cmake.presets import load_cmake_presets, get_configure_preset
from conan.tools.cmake.utils import is_multi_configuration
from conan.tools.files import chdir, mkdir
from conan.tools.files.files import load_toolchain_args
from conan.tools.microsoft.msbuild import msbuild_verbosity_cmd_line_arg
from conans.client.tools.oss import args_to_string
from conans.errors import ConanException
Expand Down
6 changes: 3 additions & 3 deletions conan/tools/cmake/cmakedeps/templates/config.py
View file
Open in desktop
Expand Up @@ -66,7 +66,7 @@ def template(self):

# Only the first installed configuration is included to avoid the collision
foreach(_BUILD_MODULE {{ '${' + pkg_name + '_BUILD_MODULES_PATHS' + config_suffix + '}' }} )
conan_message(STATUS "Conan: Including build module from '${_BUILD_MODULE}'")
message(STATUS "Conan: Including build module from '${_BUILD_MODULE}'")
include({{ '${_BUILD_MODULE}' }})
endforeach()

Expand All @@ -76,9 +76,9 @@ def template(self):
if({{ file_name }}_FIND_COMPONENTS)
foreach(_FIND_COMPONENT {{ '${'+file_name+'_FIND_COMPONENTS}' }})
if (TARGET ${_FIND_COMPONENT})
conan_message(STATUS "Conan: Component '${_FIND_COMPONENT}' found in package '{{ pkg_name }}'")
message(STATUS "Conan: Component '${_FIND_COMPONENT}' found in package '{{ pkg_name }}'")
else()
conan_message(FATAL_ERROR "Conan: Component '${_FIND_COMPONENT}' NOT found in package '{{ pkg_name }}'")
message(FATAL_ERROR "Conan: Component '${_FIND_COMPONENT}' NOT found in package '{{ pkg_name }}'")
endif()
endforeach()
endif()
Expand Down
18 changes: 6 additions & 12 deletions conan/tools/cmake/cmakedeps/templates/macros.py
View file
Open in desktop
Expand Up @@ -26,22 +26,16 @@ def context(self):
@property
def template(self):
return textwrap.dedent("""
function(conan_message MESSAGE_TYPE MESSAGE_CONTENT)
if(NOT CONAN_CMAKE_SILENT_OUTPUT)
message(${MESSAGE_TYPE} "${MESSAGE_CONTENT}")
endif()
endfunction()

macro(conan_find_apple_frameworks FRAMEWORKS_FOUND FRAMEWORKS FRAMEWORKS_DIRS)
if(APPLE)
foreach(_FRAMEWORK ${FRAMEWORKS})
# https://cmake.org/pipermail/cmake-developers/2017-August/030199.html
find_library(CONAN_FRAMEWORK_${_FRAMEWORK}_FOUND NAMES ${_FRAMEWORK} PATHS ${FRAMEWORKS_DIRS} CMAKE_FIND_ROOT_PATH_BOTH)
if(CONAN_FRAMEWORK_${_FRAMEWORK}_FOUND)
list(APPEND ${FRAMEWORKS_FOUND} ${CONAN_FRAMEWORK_${_FRAMEWORK}_FOUND})
conan_message(DEBUG "Framework found! ${FRAMEWORKS_FOUND}")
message(VERBOSE "Framework found! ${FRAMEWORKS_FOUND}")
else()
conan_message(FATAL_ERROR "Framework library ${_FRAMEWORK} not found in paths: ${FRAMEWORKS_DIRS}")
message(FATAL_ERROR "Framework library ${_FRAMEWORK} not found in paths: ${FRAMEWORKS_DIRS}")
endif()
endforeach()
endif()
Expand All @@ -56,7 +50,7 @@ def template(self):
find_library(CONAN_FOUND_LIBRARY NAMES ${_LIBRARY_NAME} PATHS ${package_libdir}
NO_DEFAULT_PATH NO_CMAKE_FIND_ROOT_PATH)
if(CONAN_FOUND_LIBRARY)
conan_message(DEBUG "Library ${_LIBRARY_NAME} found ${CONAN_FOUND_LIBRARY}")
message(VERBOSE "Conan: Library ${_LIBRARY_NAME} found ${CONAN_FOUND_LIBRARY}")
list(APPEND _out_libraries ${CONAN_FOUND_LIBRARY})

# Create a micro-target for each lib/a found
Expand All @@ -69,12 +63,12 @@ def template(self):
set_target_properties(${_LIB_NAME} PROPERTIES IMPORTED_LOCATION ${CONAN_FOUND_LIBRARY})
list(APPEND _CONAN_ACTUAL_TARGETS ${_LIB_NAME})
else()
conan_message(STATUS "Skipping already existing target: ${_LIB_NAME}")
message(VERBOSE "Conan: Skipping already existing target: ${_LIB_NAME}")
endif()
list(APPEND _out_libraries_target ${_LIB_NAME})
conan_message(DEBUG "Found: ${CONAN_FOUND_LIBRARY}")
message(VERBOSE "Conan: Found: ${CONAN_FOUND_LIBRARY}")
else()
conan_message(FATAL_ERROR "Library '${_LIBRARY_NAME}' not found in package. If '${_LIBRARY_NAME}' is a system library, declare it with 'cpp_info.system_libs' property")
message(FATAL_ERROR "Library '${_LIBRARY_NAME}' not found in package. If '${_LIBRARY_NAME}' is a system library, declare it with 'cpp_info.system_libs' property")
endif()
unset(CONAN_FOUND_LIBRARY CACHE)
endforeach()
Expand Down
4 changes: 2 additions & 2 deletions conan/tools/cmake/cmakedeps/templates/targets.py
View file
Open in desktop
Expand Up @@ -66,13 +66,13 @@ def template(self):
foreach(_COMPONENT {{ '${' + pkg_name + '_COMPONENT_NAMES' + '}' }} )
if(NOT TARGET ${_COMPONENT})
add_library(${_COMPONENT} INTERFACE IMPORTED)
conan_message(STATUS "Conan: Component target declared '${_COMPONENT}'")
message(STATUS "Conan: Component target declared '${_COMPONENT}'")
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you be opposed to changing this and line 75 to VERBOSE instead of STATUS?

I have a project where several submodules import conan packages (the super project is only setting up CMake paths to all the conan packages, not importing them) and these messages get printed several times now. I was previously silencing them by setting CONAN_CMAKE_SILENT_OUTPUT.

Copy link
Member Author

Choose a reason for hiding this comment

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

The thing is that this information is generally super helpful for many users, that will get the targets listed by default. Because not listing them makes challenging to know what would be the available target names, and changing the default verbosity of CMake for this is not that straightforward (or users won't really run in verbose, because they don't know that Conan will show the targets if they do that). So I am concerned about the experience for many Conan users, specially learners.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was worried you might say that :)

How about checking <PackageName>_FIND_QUIETLY within the generated code and if it's set either disable the message completely or change it to VERBOSE? That would give users control over the printing through the find_package call. I've never tried using this variable before, so unsure if it will actually work, but I could give it a shot and upload a PR.

I found related bug/feature requests too while Googling this, #10857 and #9959

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the current implementation do not implement QUIET handling, and it would make sense that it is listened. If you want to try it, these would be the guidelines:

  • CMakeDeps only. It is not worth the effort of modifying the other legacy ones, Conan 2.0 is next month
  • Don't overengineer with too much abstraction. A bit of repetition is not that bad if makes things cleaner.

endif()
endforeach()

if(NOT TARGET {{ root_target_name }})
add_library({{ root_target_name }} INTERFACE IMPORTED)
conan_message(STATUS "Conan: Target declared '{{ root_target_name }}'")
message(STATUS "Conan: Target declared '{{ root_target_name }}'")
endif()

{%- for alias, target in cmake_target_aliases.items() %}
Expand Down