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

boost: Adds with_stacktrace_noop option #7503

Closed
wants to merge 1 commit into from
Closed

boost: Adds with_stacktrace_noop option #7503

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
5 changes: 4 additions & 1 deletion recipes/boost/all/conanfile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ class BoostConan(ConanFile):
"visibility": ["global", "protected", "hidden"],
"addr2line_location": "ANY",
"with_stacktrace_backtrace": [True, False],
"with_stacktrace_noop": [True, False],
"buildid": "ANY",
"python_buildid": "ANY",
}
Expand Down Expand Up @@ -134,6 +135,7 @@ class BoostConan(ConanFile):
"visibility": "hidden",
"addr2line_location": "/usr/bin/addr2line",
"with_stacktrace_backtrace": True,
"with_stacktrace_noop": True,
"buildid": None,
"python_buildid": None,
}
Expand Down Expand Up @@ -1567,7 +1569,8 @@ def filter_transform_module_libraries(names):
self.cpp_info.components["stacktrace_backtrace"].defines.append("BOOST_STACKTRACE_USE_BACKTRACE")
self.cpp_info.components["stacktrace_backtrace"].requires.append("libbacktrace::libbacktrace")

self.cpp_info.components["stacktrace_noop"].defines.append("BOOST_STACKTRACE_USE_NOOP")
Copy link
Contributor

Choose a reason for hiding this comment

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

stacktrace_noop is a header-only library.
So the component is always available (when stacktrace is enabled)

Sure, you can add an option to specifically disable it, but is it worth it?

If you push further, please also do the following:

  • in configure, do del self.options.with_stcktrace_noop if stacktrace is disabled.
  • Also do del self.info.options.with_stacktrace_noop inpackage_id` (this can be done unconditionally)

Copy link
Author

Choose a reason for hiding this comment

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

What do you mean stacktrace_noop is a header-only library? From what I understand, the noop is an implementation detail that is activated with the BOOST_STACKTRACE_USE_NOOP define. I'm new to the conan package, so is there another way to disable the noop define? Thanks!

Copy link
Contributor

@madebr madebr Oct 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Look in the main recipe, there is only one noop match:

conan-center-index/recipes/boost/all/conanfile.py

Line 1570 in 4f4a376

self.cpp_info.components["stacktrace_noop"].defines.append("BOOST_STACKTRACE_USE_NOOP")

This component is set when stacktrace is enabled:

conan-center-index/recipes/boost/all/conanfile.py

Line 1552 in 4f4a376

if not self.options.without_stacktrace:

It does not add any libs.
self.cpp_info.components["stacktrace_noop"].libs = [...] is not present.

When writing the stacktrace tests, I found out that noop is the fallback backend of stacktrace.
If no implementation can be found, then nothing (=no-op) will be done.

The only way to disable no-op, is to make sure that another backend is available and used.

Copy link
Author

Choose a reason for hiding this comment

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

I see, then shouldn't the backtrace backend (which is True by default) disable no-op?

conan-center-index/recipes/boost/all/conanfile.py

Line 136 in 4f4a376

"with_stacktrace_backtrace": True,

Copy link
Contributor

Choose a reason for hiding this comment

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

No? no-op is always available, even when the addr2line/libbacktrace backend is built.

All three boost_stacktrace backends (noop/addr2line/libbacktrace) can be available in the same pacakge, just not used in the same target when using the cmake targets.

You can switch between the backends by carefully setting the definitions and linking to the correct libraries.

Copy link
Author

Choose a reason for hiding this comment

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

Will do, thanks!

Also, you mentioned that cmake would be deprecated as a generator. Is that for conan as a whole or just boost?

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean only for Conan.
conanbuiodinfo.cmake will be a thing of the past.
I'm taking about Conan 2.0, coming in the (near) future.

Copy link
Author

Choose a reason for hiding this comment

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

I see, thanks!

As an aside, what build system would you recommend outside of cmake these days?

Copy link
Contributor

Choose a reason for hiding this comment

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

To be honest, I use cmake for all of my private and professional c/c++ projects.
All other build systems have disadvantages that are showstoppers for me.

Though, a simple Makefile might be useful for something that has nothing to do with compiling sources.

[Meson}(https://mesonbuild.com/) is used by some OSS projects.
But I find its documentation hard to read + it is opiniated.
It has a potential though.
A meson syntax with a cmake backend would be very nice..

The grandfather of build systems for OSS is of course autotools (autoconf + automake + libtool).
But that is geared mostly towards Linux + gcc. It can work with MSVC on Windows, but it's harder.

If you really don't care about portability, you can also of course just use Visual Studio projects (.snl and .vcxproj files).

So in general, I can't really recommend anything but cmake.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you, appreciate your insights!

if self.options.with_stacktrace_noop:
self.cpp_info.components["stacktrace_noop"].defines.append("BOOST_STACKTRACE_USE_NOOP")

if self.settings.os == "Windows":
self.cpp_info.components["stacktrace_windbg"].defines.append("BOOST_STACKTRACE_USE_WINDBG")
Expand Down
9 changes: 7 additions & 2 deletions recipes/boost/all/test_package/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,13 @@ if(NOT HEADER_ONLY)
if(WITH_STACKTRACE)
find_package(Boost COMPONENTS stacktrace REQUIRED)

add_executable(stacktrace_noop_exe stacktrace.cpp)
target_compile_definitions(stacktrace_noop_exe PRIVATE TEST_STACKTRACE_IMPL=4)
add_executable(stacktrace_noop_exe stacktrace.cpp)
if(WITH_STACKTRACE_NOOP)
target_compile_definitions(stacktrace_noop_exe PRIVATE TEST_STACKTRACE_IMPL=4)
else()
# BOOST_STACKTRACE_USE_NOOP is not defined in this case, so we set it here only for this target
target_compile_definitions(stacktrace_noop_exe PRIVATE TEST_STACKTRACE_IMPL=4 PRIVATE BOOST_STACKTRACE_USE_NOOP)
endif()
Comment on lines +85 to +91
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix your formatting (space, 4chars).
Also, you always test noop. This is because noop is header-only.
So this shows why I think this noop option is non-sensical.

The only thing it does is enable/disable exposure of Boost::stacktrace_noop.

target_link_libraries(stacktrace_noop_exe PRIVATE Boost::stacktrace_noop)
Copy link
Contributor

Choose a reason for hiding this comment

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

This target will not be available when stacktrace_noop is disabled.


if(WIN32)
Expand Down