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 crashpad [Draft] [Help Needed] #5419

Closed
wants to merge 14 commits into from

Conversation

ericriff
Copy link
Contributor

@ericriff ericriff commented May 3, 2021

Specify library name and version: crashpad/1.0

This is a draft PR to get some help on a recipe for google crashpad. It is a missing requirement of sentry-native.
This is heavily based on https://github.com/bincrafters/community/blob/main/recipes/crashpad.

I also have a draft PR for Sentry.io's fork of crashpad, which has CMake Support. #5432
Packaging the fork is way easier, but it also has its limitations.

The challenges I'm currently facing are:

  1. Getting the sources.
    The official method relies on depot-tools using gclient or sync. This pulls the sources and also the submodules needed to build. If I understand correctly this is a conan antipattern since the submodules should be different packages, right? Even if we move forward with this approach, can the commit hash be listed in conandata.yml (instead of the sources link)?
    There is a github mirror that would allow us to do a more traditional conan source pull, listing a github link, a sha556 and then using tools.get(). The problem with this is that it doesn't pull the submodules.

  2. This library uses google's custom ninja files generator called gn. There is work being done right now to package it.

  3. Passing profile configs to the build system.
    The bincrafters recipe had this snippet which picks up env variables like CXX, CC, CXXFLAGS and translate them to what the build system understand. This only works if you have this configured in the [env] section of the profile, otherwise they get completely ignored.

    def _set_env_arg(self, args, envvar, gnvar):
        val = os.getenv(envvar)
        if val:
            args += [ "%s=\\\"%s\\\"" % (gnvar, val) ]
 ...
 
    self._set_env_arg(args, "CC",       "custom_cc")
    self._set_env_arg(args, "CXX",      "custom_cxx")
    self._set_env_arg(args, "CFLAGS",   "extra_cflags_c")
    self._set_env_arg(args, "CFLAGS",   "extra_cflags_objc")
    self._set_env_arg(args, "CXXFLAGS", "extra_cflags_cc")
    self._set_env_arg(args, "CXXFLAGS", "extra_cflags_objcc")
    self._set_env_arg(args, "LDFLAGS",  "extra_ldflags")

For example I build with a profile that sets compiler=gcc, compiler.version=8, compiler.libcxx=libstdc++11, but during build clang gets used instead.

What would be the best way to parse all this configurations?

  1. No 32 bits arm support on mini_crhomium
  2. Consuming dependencies as conan packages is not straight forward.

As discused here #5417 (comment) there is a fork of this library which has CMake support. It would be an option to ditch this all together and use those sources for the conan package.

I know there are a lot of smaller problems on this recipe (like the version not being the latest or not starting with cci.), but I would rather focus on the main issues first so please don't comment on those now so we can keep it cleaner :)

  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

@conan-center-bot

This comment has been minimized.

@madebr
Copy link
Contributor

madebr commented May 4, 2021

I think you can build it without the need for gclient or sync.
It needs python2 though.

  1. git clone gn + build gn: https://gn.googlesource.com/gn/ (conan needs a recipe that packages this executable as a tool)
  2. git clone crashpad
  3. git clone mini_chromium into the third_party\mini_chromium subdirectory of crashpad
  4. run gn gen out/Default in the crashpad folder

This fails on my Windows box because it needs python2.
My error is:

  File "C:\Users\maarten\source\repos\crashpad\third_party\mini_chromium\mini_chromium\build\win_helper.py", line 145
    print line
          ^
SyntaxError: Missing parentheses in call to 'print'. Did you mean print(line)?
ERROR at //third_party/mini_chromium/mini_chromium/build/config/BUILD.gn:568:20: Script returned non-zero exit code.
  toolchain_data = exec_script(helper_path,
                   ^----------
Current dir: C:/Users/maarten/source/repos/crashpad/out/Default/
Command: C:/Python39/python.exe C:/Users/maarten/source/repos/crashpad/third_party/mini_chromium/mini_chromium/build/win_helper.py get-visual-studio-data C:/Users/maarten/source/repos/crashpad/out/Default C:/Users/maarten/source/repos/crashpad/third_party/mini_chromium/mini_chromium/build/config/<autodetect>
Returned 1.
See //build/test.gni:43:7: which caused the file to be included.
      executable(target_name) {

@ericriff
Copy link
Contributor Author

ericriff commented May 4, 2021

gn is part of depot_tools which is already on Conan so no big deal there.
The problem with getting the sources with git clone is that you can't use conandata.yaml and I think the hooks makes in mandatory. Would it be OK if I use the github mirror for the official sources and then git clone the submodules I need? I think it would be minichromium and lss (single header).
In my opinion those should be conan packages, but I don't think conan can model what this build system needs (the sources and not the artifacts).

@madebr
Copy link
Contributor

madebr commented May 4, 2021

It looks like it is possible to download certain commits as tarballs.
See https://chromium.googlesource.com/crashpad/crashpad/+archive/b8d3be9b78071f98ad82ff65aa3df7761c05bfc8.tar.gz
Because crashpad needs mini_chromium checked out in a subdirectory, I think you need to also download its tarball in the third_party\mini_chromium subdirectory.

@ericriff
Copy link
Contributor Author

ericriff commented May 4, 2021

I'll look into it.
It is similar to what I was planning to do but downloading the tarball from the github mirror (not the fork).

@jgsogo jgsogo added the help wanted Need some help to be finish label May 4, 2021
@ericriff
Copy link
Contributor Author

ericriff commented May 4, 2021

Downloading the sources as a taball doesn't work since gn will not work unless you're on a git checkout. I've tried your link and also the one from github.

----Running------
> gn gen out/Conan --args="is_debug=false target_cpu=\"x64\" custom_conan_compiler_args_file=\"@/home/eric/.conan/data/crashpad/cci.20210429/asd/asd/build/b80d46004713aa37d6a90b42e2a326a056a237b5/conanbuildinfo.args\" custom_cxx_is_gcc=true"
-----------------
fatal: not a git repository (or any of the parent directories): .git
fatal: not a git repository (or any of the parent directories): .git
gn.py: Could not find checkout in any parent of the current path.
This must be run inside a checkout.

The same happens if you run gn on a random folder. It wants to be on a git checkout

$ gn --version      
fatal: not a git repository (or any of the parent directories): .git
fatal: not a git repository (or any of the parent directories): .git
gn.py: Could not find checkout in any parent of the current path.
This must be run inside a checkout.

🤔

@SSE4
Copy link
Contributor

SSE4 commented May 4, 2021

this seems to be a (shell script/python) wrapper around gn executable. as building gn from sources results in valid executable which doesn't require checkout.

@ericriff
Copy link
Contributor Author

ericriff commented May 4, 2021

Looking into gn sources it seems like it would also be a pain to build, it doesn't use a standard build system.
It uses a python script that generates ninja files.
I guess using sentry's fork of crashpad will be way easier.

@madebr
Copy link
Contributor

madebr commented May 4, 2021

It looks like the google git tarballs are created dynamically and are not reproducible (their hashes always differ).
See e.g. https://gn.googlesource.com/gn/+archive/6771ce569fb4803dad7a427aa2e2c23e960b917e.tar.gz

I don't find an up-to-date github fork for gn.

@madebr
Copy link
Contributor

madebr commented May 4, 2021

I'm looking into building gn.
build/gn.py has the following options:

  --platform=PLATFORM   target platform (linux/darwin/mingw/msys/msvc/aix/fuch
                        sia/freebsd/netbsd/openbsd/haiku/solaris)
  --host=HOST           host platform (linux/darwin/mingw/msys/msvc/aix/fuchsi
                        a/freebsd/netbsd/openbsd/haiku/solaris)

What does target platform mean in this context?

Does a gn build only support one platform?
e.g. when I build gn to run on Windows (=host).
Is that gn binary only able to build for one platform (=target)?

@ericriff
Copy link
Contributor Author

ericriff commented May 4, 2021

I have no idea, I have no experience with gn at all. I just need crashpad as a backend for sentry-native.
I have a draft of crashpad using sentry's own fork, which is what they use internally. It seems to work as-is with very little work involved.
It is worth considering using it since building it and handling its dependencies (openSSL and zlib) is straightforward on CMake.

EDIT: See draft PR for getsentry/crashpad here #5432

@madebr
Copy link
Contributor

madebr commented May 5, 2021

I've created an initial gn recipe at #5433

@conan-center-bot

This comment has been minimized.

@conan-center-bot
Copy link
Collaborator

Failure in build 3 (eb45f87410e8398045210b6f1d48b80869cd4db9):

  • crashpad/cci.20210429@:
    CI failed to create some packages (All logs)

    Logs for packageID f8bda7f0751e4bc3beaa6c3b2eb02d455291c8a2:
    [settings]
    arch=x86_64
    arch_build=x86_64
    build_type=Release
    compiler=apple-clang
    compiler.libcxx=libc++
    compiler.version=10.0
    os=Macos
    os_build=Macos
    
    ********************************************************************************
    conan install crashpad/cci.20210429@ --profile=/Users/jenkins/w/BuildSingleReference/11107/3703b487-cbf6-4164-9d2c-02c3a849cbb2/profile.txt --build=crashpad
    ********************************************************************************
    Configuration:
    [settings]
    arch=x86_64
    arch_build=x86_64
    build_type=Release
    compiler=apple-clang
    compiler.libcxx=libc++
    compiler.version=10.0
    os=Macos
    os_build=Macos
    [options]
    [build_requires]
    [env]
    
    crashpad/cci.20210429: Forced build from source
    gn/cci.20210429: Not found in local cache, looking in remotes...
    gn/cci.20210429: Trying with 'conan-upstream'...
    gn/cci.20210429: Trying with 'conan-center'...
    ERROR: Failed requirement 'gn/cci.20210429' from 'crashpad/cci.20210429'
    ERROR: Unable to find 'gn/cci.20210429' in remotes
    

Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability.

@madebr
Copy link
Contributor

madebr commented May 6, 2021

I've pushed a WIP crashpad recipe at https://github.com/madebr/conan-center-index/tree/sentry_wip
It is able to build the client and minidump library (tested with gcc on Linux).
TODO is:

  • shared/static
  • packaging (the recipe only builds, does not install)
  • is static libcurl possible?
  • the recipe uses embedded mini_chromium and linux-syscall-support, not packages provided by conan. It would be nice to be able to completely switch to conan packages.

@ericriff
Copy link
Contributor Author

ericriff commented May 6, 2021

I've pushed a WIP crashpad recipe at https://github.com/madebr/conan-center-index/tree/sentry_wip
It is able to build the client and minidump library (tested with gcc on Linux).
TODO is:

  • shared/static
  • packaging (the recipe only builds, does not install)
  • is static libcurl possible?
  • the recipe uses embedded mini_chromium and linux-syscall-support, not packages provided by conan. It would be nice to be able to completely switch to conan packages.

I've been working on the CMake fork, the draft is on the description if this PR
It is way easier to build with conan.
I think that the package layout will be up to us since crashpad doesn't install.

@madebr
Copy link
Contributor

madebr commented May 6, 2021

The crashpad recipe at https://github.com/madebr/conan-center-index/tree/sentry_wip is now fully working on Linux (gcc-static).
I don't think I can make shared builds working.
I've re-used the test case of your sentry-crashpad recipe.

@ericriff
Copy link
Contributor Author

ericriff commented May 6, 2021

I'll do some testing and let you know how it goes.
To keep in mind:

  1. The CMake version explicitly depends on zlib and openSSL on Linux and Android. Does the official sources avoid this somehow?
  2. I've encounter issues building if glibc < 2.27. Apparently crashpad generates a compat library that the bincrafters recipe links only if we're using an older glibc, which makes it work. I don't see any mention of this on your recipe (I improved glibc support in my recipe, but it still not perfect)
  3. Have you tried crosscompiling?
  4. Sentry's fork hardcodes every library to STATIC, maybe there is a reason for it.

@ghost
Copy link

ghost commented May 10, 2021

I detected other pull requests that are modifying crashpad/all recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

@ericriff ericriff closed this May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Failed help wanted Need some help to be finish
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants