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

Stripped and musl build options #1682

Merged
merged 25 commits into from
Sep 10, 2020
Merged

Stripped and musl build options #1682

merged 25 commits into from
Sep 10, 2020

Conversation

fntlnz
Copy link
Contributor

@fntlnz fntlnz commented Sep 2, 2020

This patch adds two new build options to the project

CMake options Meaning
-DMINIMAL_BUILD=On Build without docker, cri, mesos, marathon and kubernetes support
-DMUSL_OPTIMIZED_BUILD=On Build targeting musl libc instead of glibc. musl-libc must be provided by the target system. To produce a 100% statically linked sysdig binary, this will need to be used along with -DUSE_BUNDLED_DEPS=On

These options can be used together to accomplish a minimal build that uses musl libc.

Todo:

  • Implement the new options
  • Find a solution to replace getaddrinfo_a
    • right now we did it by doing a dumb wrapper (that will need to be changed before we merge this PR)
    • done using c-ares gethostbyname

How to compile a static binary using musl?

The easiest way is to:

docker run -v /source/sysdig:/sysdig -it alpine:3.12 sh
# in the container
apk update && apk add g++ gcc cmake cmake make  ncurses-dev git bash perl linux-headers autoconf  automake m4 libtool elfutils-dev libelf-static patch
cd /sysdig
mkdir build
cd build
cmake -DMUSL_OPTIMIZED_BUILD=True -DUSE_BUNDLED_DEPS=True ..
make -j16 sysdig

Goals

The main goal is to produce a 100% static sysdig binary that has no dependencies on the host libc and libraries.

Fixes #903

fntlnz and others added 11 commits August 31, 2020 17:47
A minimal build is a build that only produces a sysdig, scap and sinsp
artifact containing the bare minimum to let them do the syscall
instrumentation.

This means no kubernetes, container and mesos support.

Container interfaces are kept to allow a third party to hook
container support externally.

Signed-off-by: Lorenzo Fontana <[email protected]>
Minimal builds will still be able to produce and read scap files
but not compressed ones.

Signed-off-by: Lorenzo Fontana <[email protected]>
Signed-off-by: Lorenzo Fontana <[email protected]>
Signed-off-by: Lorenzo Fontana <[email protected]>
Co-Authored-By: Leonardo Grasso <[email protected]>
Signed-off-by: Lorenzo Fontana <[email protected]>
Co-Authored-By: Leonardo Grasso <[email protected]>
Signed-off-by: Lorenzo Fontana <[email protected]>
We need to patch the CRI proto while under musl because musl
does not fully respect the standard and defines stdin, stdout and stderr
as compile time constants. However, those are already defined in cri.proto and that is a reference collision.
see: https://git.musl-libc.org/cgit/musl/tree/include/stdio.h?id=0b0640219338b80cf47026d1970b5503414ed7f3#n60

Co-Authored-By: Leonardo Grasso <[email protected]>
Signed-off-by: Lorenzo Fontana <[email protected]>
Co-Authored-By: Lorenzo Fontana <[email protected]>
Signed-off-by: Leonardo Grasso <[email protected]>
leodido and others added 12 commits September 4, 2020 07:39
Co-authored-by: Lorenzo Fontana <[email protected]>
Signed-off-by: Leonardo Di Donato <[email protected]>
Co-authored-by: Lorenzo Fontana <[email protected]>
Signed-off-by: Leonardo Di Donato <[email protected]>
Co-authored-by: Lorenzo Fontana <[email protected]>
Signed-off-by: Leonardo Di Donato <[email protected]>
Co-authored-by: Lorenzo Fontana <[email protected]>
Signed-off-by: Leonardo Di Donato <[email protected]>
Co-authored-by: Lorenzo Fontana <[email protected]>
Signed-off-by: Leonardo Di Donato <[email protected]>
Co-authored-by: Lorenzo Fontana <[email protected]>
Signed-off-by: Leonardo Di Donato <[email protected]>
…o be used only when input was not an IP

Co-authored-by: Lorenzo Fontana <[email protected]>
Signed-off-by: Leonardo Di Donato <[email protected]>
Co-Authored-By: Leonardo Di Donato <[email protected]>
Signed-off-by: Lorenzo Fontana <[email protected]>
Co-authored-by: Lorenzo Fontana <[email protected]>
Signed-off-by: Leonardo Di Donato <[email protected]>
…t a minimal one

Co-authored-by: Lorenzo Fontana <[email protected]>
Signed-off-by: Leonardo Di Donato <[email protected]>
…ed only when build is not a minimal one

Co-authored-by: Lorenzo Fontana <[email protected]>
Signed-off-by: Leonardo Di Donato <[email protected]>
Co-Authored-By: Leonardo Di Donato <[email protected]>
Signed-off-by: Lorenzo Fontana <[email protected]>
leodido and others added 2 commits September 9, 2020 11:03
Co-authored-by: Leonardo Grasso <[email protected]>
Signed-off-by: Leonardo Di Donato <[email protected]>
Co-Authored-By: Leonardo Di Donato <[email protected]>
Co-Authored-By: Leonardo Grasso <[email protected]>
Signed-off-by: Lorenzo Fontana <[email protected]>
Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

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

👏

@leodido leodido merged commit 73554b9 into dev Sep 10, 2020
@rofl0r
Copy link

rofl0r commented Sep 11, 2020

that's hell of a PR just to circumvent lack of getaddrinfo_a in POSIX conformant libcs.
what i'd have prefered instead would've been

  • a configure-style check whether getaddrinfo_a is available
  • if not, use plain getaddrinfo

instead we get a "musl profile", an embedded 3rd party library and lots of cmake options...

Vaelatern added a commit to Vaelatern/void-packages that referenced this pull request Sep 11, 2020
Will be active in the next release

upstream commits: draios/sysdig#1682

73554b9c48b06612eb50494ee6fa5b779c57edc0
Vaelatern added a commit to Vaelatern/void-packages that referenced this pull request Sep 11, 2020
Will be active in the next release

upstream commits: draios/sysdig#1682

73554b9c48b06612eb50494ee6fa5b779c57edc0
Copy link

@richfelker richfelker left a comment

Choose a reason for hiding this comment

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

The changes made for build problems with musl did not fix the problem but encoded a dependency on implementation details which are not public interfaces. Comments inline. Hope this worked right - it's my first time using the GitHub review system.

endif()

if(MUSL_OPTIMIZED_BUILD)
set(SYSDIG_MUSL_FLAGS "-static -Os -D__NEED_struct_timespec -D__NEED_time_t")

Choose a reason for hiding this comment

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

These macros are not public interfaces. If they're fixing a build error for you, the underlying problem is that the file is failing to include <time.h>. All headers in bits are private implementation details and are only valid for inclusion from the other libc headers, not directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for taking the time to look at this @richfelker - I'll try to fix this. To be honest, I found those by looking at the musl source code directly, I wasn't aware of this detail so thanks!

@@ -18,6 +18,9 @@ limitations under the License.
*/

#pragma once
#ifdef MUSL_OPTIMIZED
#include <bits/alltypes.h>

Choose a reason for hiding this comment

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

This should be <time.h> and it should probably be included at the point where the types are needed rather than here.

@fntlnz fntlnz deleted the build/stripped branch September 11, 2020 14:46
@fntlnz
Copy link
Contributor Author

fntlnz commented Sep 11, 2020

@rofl0r PRs are welcome!

@leodido
Copy link
Contributor

leodido commented Sep 11, 2020

@richfelker would you please take a look at #1684 if you can? :)

@richfelker
Copy link

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

build failure on musl libc due to usage of getaddrinfo_a
5 participants