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

Bundle C++ extensions into a single executable #4335

Merged
merged 4 commits into from Jun 3, 2018

Conversation

@alessandrogario
Contributor

alessandrogario commented Apr 27, 2018

Introduction

This PR adds a new CMake function (add_osquery_extension_ex) that can be used to declare extension targets that will eventually get compiled into a single executable.

The executable name and version can be changed using the following two environment variables:

  1. OSQUERY_EXTENSION_GROUP_NAME
  2. OSQUERY_EXTENSION_GROUP_VERSION

If they are not present, the executable will be called osquery_extension_group.ext and the version will be set to 1.0.

How it works

First of all, old behavior is not modified in any way; extensions will have to be slightly modified to make use of this functionality. The two approaches can be mixed and will not conflict.

The new add_osquery_extension_ex() function aggregates all extension targets into global project properties. Once the osquery/external folder has been completely evaluated, a C++ file containing all the required includes and REGISTER_EXTERNAL directives is generated. This file is then compiled with the user-supplied implementation files in order to form a single executable.

@theopolis

Seems like a neat feature! I haven't read all of the cmake :(

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot May 1, 2018

@alessandrogario has updated the pull request. View: changes

facebook-github-bot commented May 1, 2018

@alessandrogario has updated the pull request. View: changes

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot May 1, 2018

@alessandrogario has updated the pull request.

facebook-github-bot commented May 1, 2018

@alessandrogario has updated the pull request.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot May 1, 2018

@alessandrogario has updated the pull request. View: changes

facebook-github-bot commented May 1, 2018

@alessandrogario has updated the pull request. View: changes

@alessandrogario alessandrogario changed the title from WIP: Bundle C++ extensions into a single executable to Bundle C++ extensions into a single executable May 1, 2018

@obelisk obelisk self-assigned this May 4, 2018

@obelisk obelisk self-requested a review May 4, 2018

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot May 16, 2018

@alessandrogario has updated the pull request.

facebook-github-bot commented May 16, 2018

@alessandrogario has updated the pull request.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot May 16, 2018

@alessandrogario has updated the pull request.

facebook-github-bot commented May 16, 2018

@alessandrogario has updated the pull request.

@mkareta mkareta self-requested a review May 28, 2018

@mkareta

This comment has been minimized.

Show comment
Hide comment
@mkareta

mkareta May 28, 2018

Contributor

Hi, @alessandrogario, Is it still in progress?
It's look good to me and I generally like idea of producing less binaries, so maybe we should go further and add ability to compile osqueryd and extension into one binary if they are built at the same time. What do you think?

Contributor

mkareta commented May 28, 2018

Hi, @alessandrogario, Is it still in progress?
It's look good to me and I generally like idea of producing less binaries, so maybe we should go further and add ability to compile osqueryd and extension into one binary if they are built at the same time. What do you think?

@alessandrogario

This comment has been minimized.

Show comment
Hide comment
@alessandrogario

alessandrogario May 28, 2018

Contributor

Hello @mkareta!

I feel like this is complete and ready for review (thanks for looking at this!)

I'm not really sure about the idea of merging extensions and osqueryd into a single executable, as I never really thought about this use case.

We are maintaining an extension repository so we felt the need of bundling them together to make them easier to use, but we will keep using the official packages. One thing that I really like is that the maintainers are doing an awesome job at keeping the release quality high, so my preference would still be to avoid grouping extensions and core into a single executable.

This may be really interesting for other user cases though (especially for heavy osqueryi users).

Contributor

alessandrogario commented May 28, 2018

Hello @mkareta!

I feel like this is complete and ready for review (thanks for looking at this!)

I'm not really sure about the idea of merging extensions and osqueryd into a single executable, as I never really thought about this use case.

We are maintaining an extension repository so we felt the need of bundling them together to make them easier to use, but we will keep using the official packages. One thing that I really like is that the maintainers are doing an awesome job at keeping the release quality high, so my preference would still be to avoid grouping extensions and core into a single executable.

This may be really interesting for other user cases though (especially for heavy osqueryi users).

@mkareta

mkareta approved these changes Jun 2, 2018

Alt text

@mkareta

This comment has been minimized.

Show comment
Hide comment
@mkareta

mkareta Jun 2, 2018

Contributor

ok to test

Contributor

mkareta commented Jun 2, 2018

ok to test

@mkareta mkareta merged commit 5006a02 into facebook:master Jun 3, 2018

5 checks passed

Code Audit Build finished.
Details
FreeBSD Build finished.
Details
Linux Build finished.
Details
Windows Build finished.
Details
macOS/OS X Build finished.
Details

@alessandrogario alessandrogario deleted the trailofbits:alessandro/feature/extension-bundling branch Jun 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment