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.

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

Add breakpad recipe #5639

Merged
merged 8 commits into from
Jun 4, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
4 changes: 4 additions & 0 deletions recipes/breakpad/all/conandata.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
sources:
"cci.20210521":
url: "https://github.com/google/breakpad/archive/f7428bc.tar.gz"
sha256: "55a688a49ffc476d94d92c3fd73f9264c974c25af8d6371c3901bd3451081e47"
119 changes: 119 additions & 0 deletions recipes/breakpad/all/conanfile.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
from conans import ConanFile, AutoToolsBuildEnvironment, tools
import os
import textwrap

required_conan_version = ">=1.33.0"

class BreakpadConan( ConanFile ):
name = "breakpad"
description = "A set of client and server components which implement a crash-reporting system"
topics = ["crash", "report", "breakpad"]
license = "BSD-3-Clause"
url = "https://github.com/conan-io/conan-center-index"
homepage = "https://chromium.googlesource.com/breakpad/breakpad/"
settings = "os", "compiler", "build_type", "arch"
provides = "breakpad"
options = {
"fPIC": [True, False]
}
default_options = {
"fPIC": True
}
_env_build = None

@property
def _source_subfolder(self):
return "source_subfolder"

def config_options(self):
if self.settings.os == "Windows":
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it build on Windows?

I tried to build it with gyp, but I couldn't get it working.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No idea :)

del self.options.fPIC

def requirements(self):
if self.settings.os == "Linux":
self.requires("linux-syscall-support/cci.20200813")

def _patch_sources(self):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll make this a .patch once the PR is ready.

# Use Conan's lss instead of the submodule
# 1. Remove from include dirs
# 2. Remove from list of headers to install
# 3. Patch all #include statements
tools.replace_in_file(os.path.join(self._source_subfolder, "Makefile.in"),
"$(includegbc_HEADERS) $(includelss_HEADERS) ",
"$(includegbc_HEADERS) "
)
tools.replace_in_file(os.path.join(self._source_subfolder, "Makefile.in"),
"install-includelssHEADERS install-includepHEADERS ",
"iinstall-includepHEADERS "
)
files_to_patch = [
"src/tools/linux/md2core/minidump-2-core.cc",
"src/processor/testdata/linux_test_app.cc",
"src/common/memory_allocator.h",
"src/common/linux/memory_mapped_file.cc",
"src/common/linux/file_id.cc",
"src/common/linux/safe_readlink.cc",
"src/client/minidump_file_writer.cc",
"src/client/linux/handler/exception_handler.cc",
"src/client/linux/handler/exception_handler_unittest.cc",
"src/client/linux/log/log.cc",
"src/client/linux/crash_generation/crash_generation_client.cc",
"src/client/linux/minidump_writer/linux_dumper.cc",
"src/client/linux/minidump_writer/linux_dumper_unittest_helper.cc",
"src/client/linux/minidump_writer/proc_cpuinfo_reader.h",
"src/client/linux/minidump_writer/minidump_writer.cc",
"src/client/linux/minidump_writer/linux_ptrace_dumper.cc",
"src/client/linux/minidump_writer/cpu_set.h",
"src/client/linux/minidump_writer/directory_reader.h",
"src/client/linux/minidump_writer/line_reader.h"
]

for file in files_to_patch:
tools.replace_in_file(
os.path.join(self._source_subfolder, file),
"#include \"third_party/lss/linux_syscall_support.h\"",
"#include <linux_syscall_support.h>"
)

# Let Conan handle fPIC
Copy link
Contributor Author

@ericriff ericriff May 26, 2021

Choose a reason for hiding this comment

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

Is this something we want? Or should we assume that it is hardcoded "for a reason" and don't mess with it?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is hardcoded so users don't have to change anything to use it in a shared library.

tools.replace_in_file(
os.path.join(self._source_subfolder, "Makefile.in"),
textwrap.dedent("""\
@LINUX_HOST_TRUE@am__append_2 = -fPIC
@LINUX_HOST_TRUE@am__append_3 = -fPIC"""),
""
)

def _configure_autotools(self):
if not self._env_build:
self._env_build = AutoToolsBuildEnvironment(self)
self._env_build.configure(configure_dir=self._source_subfolder)
return self._env_build

def source(self):
tools.get(**self.conan_data["sources"][self.version], destination=self._source_subfolder, strip_root=True)

def build(self):
if self.settings.os == "Linux":
self._patch_sources()
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's okay to always patch?

Suggested change
if self.settings.os == "Linux":
self._patch_sources()
self._patch_sources()

Copy link
Contributor Author

@ericriff ericriff May 26, 2021

Choose a reason for hiding this comment

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

All the changes are Linux specific, but since the paths to the files are hardcored, E.g. src/client/linux/minidump_writer/line_reader.h I don't know if Windows will take them (doesn't it use \ instead of / for the folder separation?)
Besides I was planning on make this a real patch file once it the PR is stable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed.
I don't think the autotools build system was written with support for Windows built-in.
I think we should raise a ConanInvalidConfiguration for win.

I don't have a clue what to do with Macos. Throwing an exception would be fine for me too.


env_build = self._configure_autotools()
env_build.make()

def package(self):
self.copy("LICENSE", src=self._source_subfolder, dst="licenses")
env_build = self._configure_autotools()
env_build.install()
tools.rmdir(os.path.join(self.package_folder, "share"))
tools.rmdir(os.path.join(self.package_folder, "lib", "pkgconfig"))

def package_info( self ):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some help is needed here. sentry-native consumes this package using pkgConfig, but I'm not familiar with that system, so I don't know how to model it on Conan.
https://github.com/getsentry/sentry-native/blob/master/CMakeLists.txt#L439

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll fork this recipe and open a pr with some fixes.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be interesting to create a sentry-breakpad recipe (such as I'm doing for sentry-crashpad), to see what components are needs to interface it with sentry-native.

self.cpp_info.libs = ["breakpad", "breakpad_client"]
self.cpp_info.includedirs = [os.path.join("include", "breakpad")]

if self.settings.os == "Linux":
self.cpp_info.system_libs.append("pthread")

bindir = os.path.join(self.package_folder, "bin")
self.output.info("Appending PATH environment variable: {}".format(bindir))
self.env_info.PATH.append(bindir)
Comment on lines +75 to +77
Copy link
Contributor

Choose a reason for hiding this comment

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

This

9 changes: 9 additions & 0 deletions recipes/breakpad/all/test_package/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
cmake_minimum_required(VERSION 3.1)
project(test_package CXX)

include(${CMAKE_BINARY_DIR}/conanbuildinfo.cmake)
conan_basic_setup()

add_executable(${PROJECT_NAME} test_package.cpp)
set_property(TARGET ${PROJECT_NAME} PROPERTY CXX_STANDARD 11)
target_link_libraries(${PROJECT_NAME} ${CONAN_LIBS})
16 changes: 16 additions & 0 deletions recipes/breakpad/all/test_package/conanfile.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
from conans import ConanFile, CMake, tools
import os

class BreakpadTestConan(ConanFile):
settings = "os", "compiler", "build_type", "arch"
generators = "cmake"

def build(self):
cmake = CMake(self)
cmake.configure()
cmake.build()

def test(self):
if not tools.cross_building(self.settings):
bin_path = os.path.join("bin", "test_package")
Copy link
Contributor

@madebr madebr May 26, 2021

Choose a reason for hiding this comment

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

This one does not need to use the binaries in the bin package folder? (as the crashpad test package does?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think everything in bin are tools.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup. I was asking whether breakpad has a handler binary, such as crashpad.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nop. Breakpad is easier to use since you just link the library to your app. No need to ship an extra handler.

Copy link
Contributor

Choose a reason for hiding this comment

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

does not match this

Copy link
Contributor

Choose a reason for hiding this comment

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

@prince-chrismc
breakpad ships utility executables to examine stacktraces post-crash, but no crashpad_handler-like executable that must be executed in parallel with the victim-executable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahhhhhh thank you for explaining

self.run(bin_path, run_environment=True)
35 changes: 35 additions & 0 deletions recipes/breakpad/all/test_package/test_package.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#include <iostream>
#include "client/linux/handler/exception_handler.h"

using namespace google_breakpad;

namespace {

bool callback(const MinidumpDescriptor &descriptor,
void *context,
bool succeeded) {
// if succeeded is true, descriptor.path() contains a path
// to the minidump file. Context is the context passed to
// the exception handler's constructor.
return succeeded;
}

}

int main(int argc, char *argv[]) {

std::cout << "Breakpad test_package" << std::endl;

MinidumpDescriptor descriptor("path/to/cache");
ExceptionHandler eh(
descriptor,
/* filter */ nullptr,
callback,
/* context */ nullptr,
/* install handler */ true,
/* server FD */ -1
);

// run your program here
return 0;
}
3 changes: 3 additions & 0 deletions recipes/breakpad/config.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
versions:
"cci.20210521":
folder: all