Skip to content

MOD-7646 cmake fetch boost if not exists#4984

Merged
DvirDukhan merged 1 commit intomasterfrom
dvirdu_boost_cmake
Sep 5, 2024
Merged

MOD-7646 cmake fetch boost if not exists#4984
DvirDukhan merged 1 commit intomasterfrom
dvirdu_boost_cmake

Conversation

@DvirDukhan
Copy link

Describe the changes in the pull request

This PR adds the ability to fetch boost headers via cmake in case BOOST_DIR wasn't given or it does not exist.
This can allow users to avoid calling install_boost.sh if they do not wish to. There is a performance penalty on fetching boost via CMake

Which issues this PR fixes
MOD-7646

Main objects this PR modified
CMakeLists.txt of geometry

Mark if applicable

  • This PR introduces API changes
  • This PR introduces serialization changes

@codecov
Copy link

codecov bot commented Sep 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.15%. Comparing base (d562549) to head (301ce91).
Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4984      +/-   ##
==========================================
- Coverage   86.16%   86.15%   -0.01%     
==========================================
  Files         192      192              
  Lines       34235    34235              
==========================================
- Hits        29497    29495       -2     
- Misses       4738     4740       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@adamiBs
Copy link

adamiBs commented Sep 3, 2024

I suggest that you'd remove from the GitHub Action the Installation of Boost to test this:

- name: Install Boost
working-directory: .install
run: ./install_boost.sh ${{ env.BOOST_VERSION }} ${{ steps.mode.outputs.mode }}

@DvirDukhan

@DvirDukhan DvirDukhan added this pull request to the merge queue Sep 4, 2024
@DvirDukhan
Copy link
Author

@adamiBs tested manually.

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 4, 2024
@DvirDukhan DvirDukhan added this pull request to the merge queue Sep 5, 2024
github-merge-queue bot pushed a commit that referenced this pull request Sep 5, 2024
cmake fetch boost if not exists
include_directories(${BOOST_DIR})
else()
message(STATUS "BOOST_DIR is not defined")
message(STATUS "BOOST_DIR is not defined or empty")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Make it even clearer?

Suggested change
message(STATUS "BOOST_DIR is not defined or empty")
message(STATUS "BOOST_DIR is not defined or is empty")

FetchContent_Declare(
Boost
GIT_REPOSITORY https://github.com/boostorg/boost.git
GIT_TAG boost-1.84.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should the actual boost version be set in the Makefile? (users could see it more easily if they wish to do it on their own)

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 5, 2024
@DvirDukhan DvirDukhan added this pull request to the merge queue Sep 5, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 5, 2024
@DvirDukhan DvirDukhan added this pull request to the merge queue Sep 5, 2024
Merged via the queue into master with commit f696cd3 Sep 5, 2024
@DvirDukhan DvirDukhan deleted the dvirdu_boost_cmake branch September 5, 2024 14:35
github-actions bot pushed a commit that referenced this pull request Sep 5, 2024
cmake fetch boost if not exists

(cherry picked from commit f696cd3)
@github-actions
Copy link

github-actions bot commented Sep 5, 2024

Successfully created backport PR for 2.8:

@github-actions
Copy link

github-actions bot commented Sep 5, 2024

Successfully created backport PR for 2.10:

github-actions bot pushed a commit that referenced this pull request Sep 5, 2024
cmake fetch boost if not exists

(cherry picked from commit f696cd3)
kei-nan pushed a commit that referenced this pull request Sep 8, 2024
cmake fetch boost if not exists
github-merge-queue bot pushed a commit that referenced this pull request Sep 8, 2024
MOD-7646 cmake fetch boost if not exists (#4984)

cmake fetch boost if not exists

(cherry picked from commit f696cd3)

Co-authored-by: DvirDukhan <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Sep 8, 2024
MOD-7646 cmake fetch boost if not exists (#4984)

cmake fetch boost if not exists

(cherry picked from commit f696cd3)

Co-authored-by: DvirDukhan <[email protected]>
kei-nan pushed a commit that referenced this pull request Sep 9, 2024
MOD-7646 cmake fetch boost if not exists (#4984)

cmake fetch boost if not exists

(cherry picked from commit f696cd3)

Co-authored-by: DvirDukhan <[email protected]>
kei-nan pushed a commit that referenced this pull request Sep 9, 2024
MOD-7646 cmake fetch boost if not exists (#4984)

cmake fetch boost if not exists

(cherry picked from commit f696cd3)

Co-authored-by: DvirDukhan <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Sep 16, 2024
* Move coord src into general module's src

* remove internal src dir in coord

* remove internal src dir in coord - now with cmake

* WIP

* single build path - wip

* WIP

* remove split

* make things work -
WIP - clean makefile, move pack

* fix pack, move coord unit tests
wip: resolve conflicts, fix tests, libuv?

* fix conflicts, split test

* remove build test step from CI

* fixes

* remove COORD=0 for coverage

* revert extract module symbols

* use foreach loop for coordinator unit tests

* Addressing Dvir's CR - simplifying test deps and removing CMAKE_FILES var that seems useless

* fix CMAKE_COORD var

* MOD-7596: Add mirroring support (#4937)

* * initial commit

* * Use RediSearch branch mirroring fork

* * Code Review - Round #1

* Change maxPending requests when changing the number of connections (#4972)

change the number of maxPending requests when changing the number of connections

* Remove RS_COORDINATOR flag and its associated code, fix bug in makefile for lite and static

* fix to allow building as LITE

* fix path in unit-tests to community

* MOD-7695: Decide Ubuntu Runner Using Variable (#4975)

* * initial commit

* * code review

* Enable MT in comunity eddition by default - [MOD-7160] (#4974)

* remove compilation flag (wip)

* finish removing compilation flag

* remove from tests

* split build for lite

* MOD-7706 fix ramp-packer version (#4980)

ramp-packer 2.5.10

* HNSW vector index - handle inplace deletion after async [MOD-7643] (#4964)

* Fix inplace deletion in tiered index in vecsim

* remove excessive thread iteration log, finalize test

* update vecsim version

* update vecsim version - fixing bug found in test vecsim with json multi

* remove print from test

* better logging

* test - print warning always

* test2 - print warning DEBUG AND CI ENV VARS

* use force for env vars

* try print instead

* print for GHA annotation

* remove light build flag, keep lite as an option fro backward compatibility

* Fix warning

* use llvm 18

* try to include in cmake cpp test dir

* try build without special includes for apple

* set apple test

* Set APPLE flag for MACOS

* debug print

* MOD-7737: Add relaxation flags to Rocky9 installations (#4989)

Add relaxing installation flags to Rocky platform

* MOD-7646 cmake fetch boost if not exists (#4984)

cmake fetch boost if not exists

* MOD-7465 MOD-7466: Add ACL support for RediSearch, phase 1 (#4957)

* Support ACL for RediSearch commands - wip

* Register RediSearch commands to redis ACL categories

* Add tests

* Small fix

* Add coordinator commands registration

* Register underscored commands to ACL as well

* Remove dangerous from INFO

* Skip test conditioned on redis version

* Fix categories according to new evaluation

* Remove comments

* Fix

* Address review

* Remove TODOs

* Replace macro with function

* Wrap with RM_TRY

* Fix unit-tests

* Touchup

* Address review

* Address review

* Address review

* Bump sanitizer and Valgrind to use 7.4

* Address review

* Touchups

* Remove TODOs

* Fix test

* Address review

* Move hack to src

* try removing visibility

* try remove visibility for coord only

* apply changes as patch after rebasing from new master

* revert visibility hidden

* revert visibility hidden

* revert

* add verbosity for debug

* add init-rm to cpp tests

* Copy init-rm in cpptests

* clean

* avoid -executable in mac

* move REDISMODULE_MAIN to module-init.c

* try removing init-rm from cpp tests

* update vecsim

* unify module.c files from coord so that "onload" is on the module not coord

* remove init-rm

* bring back sdsfree in unit test, link hiredis/sds with extern c

* remove coord_module.h, align vecsim deps

* remove sds from rmutil

* remove RMUTIL_NO_SDS totally

---------

Co-authored-by: kei-nan <[email protected]>
Co-authored-by: GuyAv46 <[email protected]>
Co-authored-by: DvirDukhan <[email protected]>
Co-authored-by: Raz Monsonego <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Jan 8, 2025
* WIP: Config parameters

* Refactor config register

* MOD-7596: Add mirroring support (#4937)

* * initial commit

* * Use RediSearch branch mirroring fork

* * Code Review - Round #1

* Change maxPending requests when changing the number of connections (#4972)

change the number of maxPending requests when changing the number of connections

* Test boolean parameters

* Fix boolean params default values

* MOD-7695: Decide Ubuntu Runner Using Variable (#4975)

* * initial commit

* * code review

* Enable MT in comunity eddition by default - [MOD-7160] (#4974)

* remove compilation flag (wip)

* finish removing compilation flag

* remove from tests

* MOD-7706 fix ramp-packer version (#4980)

ramp-packer 2.5.10

* HNSW vector index - handle inplace deletion after async [MOD-7643] (#4964)

* Fix inplace deletion in tiered index in vecsim

* remove excessive thread iteration log, finalize test

* update vecsim version

* update vecsim version - fixing bug found in test vecsim with json multi

* remove print from test

* better logging

* test - print warning always

* test2 - print warning DEBUG AND CI ENV VARS

* use force for env vars

* try print instead

* print for GHA annotation

* Add two immutable params

* Fix max-doctablesize name

* MOD-7737: Add relaxation flags to Rocky9 installations (#4989)

Add relaxing installation flags to Rocky platform

* MOD-7646 cmake fetch boost if not exists (#4984)

cmake fetch boost if not exists

* MOD-7465 MOD-7466: Add ACL support for RediSearch, phase 1 (#4957)

* Support ACL for RediSearch commands - wip

* Register RediSearch commands to redis ACL categories

* Add tests

* Small fix

* Add coordinator commands registration

* Register underscored commands to ACL as well

* Remove dangerous from INFO

* Skip test conditioned on redis version

* Fix categories according to new evaluation

* Remove comments

* Fix

* Address review

* Remove TODOs

* Replace macro with function

* Wrap with RM_TRY

* Fix unit-tests

* Touchup

* Address review

* Address review

* Address review

* Bump sanitizer and Valgrind to use 7.4

* Address review

* Touchups

* Remove TODOs

* Fix test

* Address review

* Move hack to src

* WIP: Register numeric params

* WIP: Config numeric params

* WIP: CONFIG value precedence

* Complete boolean parameters

* Test search.partial-indexed-docs

* Fix string configs

* Add index-cursor-limit config

* Fix unit tests

* Create cluster configuration params

* Fix cluster configuration params

* Test MODULE LOADEX numeric config

* WIP: Test MODULE LOADEX bool config

* Fix set_no_gc()

* Fix string config parameters

* Test MODULE LOADEX enum config

* Fix set_workers()

* Fix numeric parameter limit

* Test boolean parameters with config file

* Test with config file: numeric, enum, string

* Add max-aggregate-results and max-search-results

* tidy up

* Add more tests

* Use REDISMODULE_CONFIG_UNPREFIXED flag

* Revert "Use REDISMODULE_CONFIG_UNPREFIXED flag"

This reverts commit b1985fc.

* Use common setfn, getfn for numeric params

* Fix mem leak 1

* Fix mem leak 2 - string config

* Disable immutable limits tests

* Use RLTest >= 0.7.14

* Check version before loading config

* Add testConfigAPILoadTimeNumericParams()

* Fix unit-tests

* Refactor config API function parameters for consistency

* Use privdata in numeric config params

* Refactor boolean config registration and clean up unused macros

* Refactor configuration setter functions to improve consistency

* Refactor coord config functions

* Refactor config functions for improved clarity and consistency

* search-gc-scan-size is Load-Time immutable

* Fixes from code review

* Remove parameter modified validation

* Fix search-ext-load and search-friso-ini tests

* Remove global config vars

* Remove str validation from get_string_config()

* Update default configuration to initialize extLoad and frisoIni as empty strings

* MOD-7613: Add search-oss-global-password (#5237)

1. Configure the module parameter search-oss-global-password which is equivalent to the previous OSS_GLOBAL_PASSWORD module parameter.
2. Configure the module parameter search-oss-acl-username which is equivalent to the previous OSS_ACL_USERNAME

---------

Co-authored-by: kei-nan <[email protected]>
Co-authored-by: GuyAv46 <[email protected]>
Co-authored-by: DvirDukhan <[email protected]>
Co-authored-by: alonre24 <[email protected]>
Co-authored-by: Raz Monsonego <[email protected]>
Co-authored-by: oshadmi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants