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

Jump to bottom

Simplify how third party libraries formula work #6303

Merged
merged 1 commit into from
Mar 31, 2020
Merged

Simplify how third party libraries formula work #6303

merged 1 commit into from
Mar 31, 2020

Conversation

Smjert
Copy link
Member

@Smjert Smjert commented Mar 16, 2020

Remove a level of indirection when configuring and building formulas.
This should simplify working with them and also remove some issues
encountered when trying to build on Windows.

This should address: #6287

Depends on: #6302

@Smjert Smjert added build libraries For things referring to osquery third party libraries do not merge Do not merge PR as it's pending on some discussion or external factor. Reviewer should have context. labels Mar 16, 2020
@Smjert Smjert closed this Mar 17, 2020
@Smjert
Copy link
Member Author

Smjert commented Mar 17, 2020

Retrigger CI.

@Smjert Smjert reopened this Mar 17, 2020
@directionless
Copy link
Member

You asked if people could build this branch. Working off commit f0fa6aa, I had no trouble building. I did see a couple test failures:

git checkout remotes/trailofbits/stefano/build/simplify-formula
mkdir build-simplify
cd build-simplify/
cmake -DOSQUERY_BUILD_TESTS=ON ../
make -j8
./osquery/osqueryi
cmake --build . --target test

98% tests passed, 2 tests failed out of 80

Total Test time (real) =  77.69 sec

The following tests FAILED:
	 79 - tools_tests_testosqueryd (Failed)
	 80 - tests_integration_tables-test (Failed)

@Smjert
Copy link
Member Author

Smjert commented Mar 17, 2020 •
edited
Loading

You asked if people could build this branch. Working off commit f0fa6aa, I had no trouble building. I did see a couple test failures:

git checkout remotes/trailofbits/stefano/build/simplify-formula
mkdir build-simplify
cd build-simplify/
cmake -DOSQUERY_BUILD_TESTS=ON ../
make -j8
./osquery/osqueryi
cmake --build . --target test

98% tests passed, 2 tests failed out of 80

Total Test time (real) =  77.69 sec

The following tests FAILED:
	 79 - tools_tests_testosqueryd (Failed)
	 80 - tests_integration_tables-test (Failed)

Thanks!
Yeah those test failures are most likely unrelated and happening right now, without this PR, when run in local machines vs CI.
I have to open other issues for them.

@theopolis
Copy link
Member

Running make out of my already-configured build directory:

CMake Error at libraries/cmake/source/librpm/CMakeLists.txt:114 (add_library):
  Cannot find source file:

    /home/teddy/git/osquery-prod/libraries/cmake/source/librpm/src/lib/backend/dummydb.c

  Tried extensions .c .C .c++ .cc .cpp .cxx .cu .m .M .mm .h .hh .h++ .hm
  .hpp .hxx .in .txx
Call Stack (most recent call first):
  libraries/cmake/source/librpm/CMakeLists.txt:161 (librpmMain)


CMake Error at libraries/cmake/source/librpm/CMakeLists.txt:114 (add_library):
  No SOURCES given to target: thirdparty_librpm
Call Stack (most recent call first):
  libraries/cmake/source/librpm/CMakeLists.txt:161 (librpmMain)


CMake Generate step failed.  Build files cannot be regenerated correctly.
Makefile:12104: recipe for target 'cmake_check_build_system' failed
make: *** [cmake_check_build_system] Error 1

Fixed with:

git submodule update --init

Then everything works fine :)

@Smjert
Copy link
Member Author

Smjert commented Mar 18, 2020 •
edited
Loading

Running make out of my already-configured build directory:

CMake Error at libraries/cmake/source/librpm/CMakeLists.txt:114 (add_library):
  Cannot find source file:

    /home/teddy/git/osquery-prod/libraries/cmake/source/librpm/src/lib/backend/dummydb.c

  Tried extensions .c .C .c++ .cc .cpp .cxx .cu .m .M .mm .h .hh .h++ .hm
  .hpp .hxx .in .txx
Call Stack (most recent call first):
  libraries/cmake/source/librpm/CMakeLists.txt:161 (librpmMain)


CMake Error at libraries/cmake/source/librpm/CMakeLists.txt:114 (add_library):
  No SOURCES given to target: thirdparty_librpm
Call Stack (most recent call first):
  libraries/cmake/source/librpm/CMakeLists.txt:161 (librpmMain)


CMake Generate step failed.  Build files cannot be regenerated correctly.
Makefile:12104: recipe for target 'cmake_check_build_system' failed
make: *** [cmake_check_build_system] Error 1

Fixed with:

git submodule update --init

Then everything works fine :)

Ah yes, that is to be expected and it's actually due to the update of openssl (the other PR).
Maybe I can add a line in the commit description to either start from scratch or remember to run that command.

@Smjert Smjert mentioned this pull request Mar 27, 2020
Closed
@Smjert
Simplify how third party libraries formula work
efbd87c 
Remove a level of indirection when configuring and building formulas.
This should simplify working with them and also remove some issues
encountered when trying to build on Windows.
@Smjert Smjert force-pushed the stefano/build/simplify-formula branch from f0fa6aa to efbd87c Compare March 30, 2020 11:50
@Smjert
Copy link
Member Author

Smjert commented Mar 30, 2020

Rebased on latest master, since the PR it depends on landed.

@theopolis
Copy link
Member

@Smjert ready to merge?

@Smjert Smjert removed the do not merge Do not merge PR as it's pending on some discussion or external factor. Reviewer should have context. label Mar 31, 2020
@Smjert
Copy link
Member Author

Smjert commented Mar 31, 2020

@Smjert ready to merge?

Yeah I was waiting confirmation from this issue #6287, since everything looks good I'll merge it now, thanks!

@Smjert Smjert closed this Mar 31, 2020
@Smjert Smjert reopened this Mar 31, 2020
@Smjert Smjert merged commit c22ab5c into osquery:master Mar 31, 2020
@Smjert Smjert deleted the stefano/build/simplify-formula branch April 2, 2020 22:09
aikuchin pushed a commit to aikuchin/osquery that referenced this pull request Jul 11, 2023
@mogrein
Merge pull request osquery#9 in CLOUD/osquery from sync/osquery-4.3.0…
b7720da 
… to master

* commit '8c13dd6bd206f2909a4baea5bcfbc91d5e3f502b': (159 commits)
  release: updating changelog for 4.3.0 release (osquery#6387)
  Build hvci_status table with CMake (osquery#6378)
  Change calls to debug log to verbose (osquery#6369)
  iokit: Fix race when accessing port_ (osquery#6380)
  Check extensions are registered with osquery core (osquery#6374)
  First steps to remove the Buck build system (osquery#6361)
  Return error detaching table, only use primary database (osquery#6373)
  Copy the parent environment when launching worker
  Change process table log errors to info and fix typo (osquery#6370)
  Ensure the extension uuid is never 0 (osquery#6377)
  Remove errors when converting empty numeric rows (osquery#6371)
  Do not force a specific path to install osquery on Windows (osquery#6379)
  Fix readFile API doing blocking I/O with a non-blocking handle (osquery#6368)
  magic: Check return from magic_file (osquery#6363)
  macos: Use -1 for missing ppid in process_events (osquery#6339)
  Update OpenSSL to version 1.1.1f and fix build (osquery#6359)
  Simplify how third party libraries formula work (osquery#6303)
  Add socket_events table for socket auditing in MacOS (osquery#6028)
  Extend the fields of curl_certificate table (osquery#6176)
  add status column to deb_packages table (osquery#6341)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@theopolis theopolis theopolis approved these changes

Assignees
No one assigned
Labels
build libraries For things referring to osquery third party libraries
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Smjert @directionless @theopolis