Skip to content

Add option to CMake for building static libraries#12890

Open
Bindu-Bhabu wants to merge 2 commits intofacebook:mainfrom
Bindu-Bhabu:patch-1
Open

Add option to CMake for building static libraries#12890
Bindu-Bhabu wants to merge 2 commits intofacebook:mainfrom
Bindu-Bhabu:patch-1

Conversation

@Bindu-Bhabu
Copy link
Copy Markdown

ROCKSDB creates a STATIC library target reference by default.
Modify the cmake so that the STATIC library is also an option just like
creating a SHARED library but kept OFF by default.

@Bindu-Bhabu Bindu-Bhabu marked this pull request as ready for review July 26, 2024 09:51
@rhubner
Copy link
Copy Markdown
Contributor

rhubner commented Jul 26, 2024

Hello @Bindu-Bhabu,

thank you for you PR.

I tested your PR locally and it break our Java builds. I need to add extra parameter -D ROCKSDB_BUILD_STATIC=ON. I considering that previously we always build static library, I think it will be better do use same defaults :

option(ROCKSDB_BUILD_STATIC "Build static versions of the RocksDB libraries" ON)

Radek

@Bindu-Bhabu
Copy link
Copy Markdown
Author

@rhubner , default is updated, Kindly check

ROCKSDB creates a STATIC library target reference by default.
Modify the cmake so that the STATIC library is also an option just like creating a SHARED library but kept OFF by default.
Update Static library option default to ON.
@rhubner
Copy link
Copy Markdown
Contributor

rhubner commented Jul 31, 2024

LGTM ✅

kraj pushed a commit to YoeDistro/meta-openembedded that referenced this pull request Aug 9, 2024
Modify the CMakeLists.txt to add an Option for
STATIC target import, as available for shared library.

Link: facebook/rocksdb#12890

Configure static library default to switched off
as shared libraries are sufficient in most cases.

Signed-off-by: Bhabu Bindu <bindu.bhabu@kpit.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
kraj pushed a commit to YoeDistro/meta-openembedded that referenced this pull request Aug 9, 2024
Modify the CMakeLists.txt to add an Option for
STATIC target import, as available for shared library.

Link: facebook/rocksdb#12890

Configure static library default to switched off
as shared libraries are sufficient in most cases.

Signed-off-by: Bhabu Bindu <bindu.bhabu@kpit.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
kraj pushed a commit to YoeDistro/meta-openembedded that referenced this pull request Aug 9, 2024
Modify the CMakeLists.txt to add an Option for
STATIC target import, as available for shared library.

Link: facebook/rocksdb#12890

Configure static library default to switched off
as shared libraries are sufficient in most cases.

Signed-off-by: Bhabu Bindu <bindu.bhabu@kpit.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
kraj pushed a commit to YoeDistro/meta-openembedded that referenced this pull request Aug 9, 2024
Modify the CMakeLists.txt to add an Option for
STATIC target import, as available for shared library.

Link: facebook/rocksdb#12890

Configure static library default to switched off
as shared libraries are sufficient in most cases.

Signed-off-by: Bhabu Bindu <bindu.bhabu@kpit.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
kraj pushed a commit to YoeDistro/meta-openembedded that referenced this pull request Aug 10, 2024
Modify the CMakeLists.txt to add an Option for
STATIC target import, as available for shared library.

Link: facebook/rocksdb#12890

Configure static library default to switched off
as shared libraries are sufficient in most cases.

Signed-off-by: Bhabu Bindu <bindu.bhabu@kpit.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
jpuhlman pushed a commit to MontaVista-OpenSourceTechnology/meta-openembedded that referenced this pull request Oct 13, 2024
Source: meta-openembedded
MR: 166941
Type: Integration
Disposition: Merged from meta-openembedded
ChangeID: 72018ca
Description:

Modify the CMakeLists.txt to add an Option for
STATIC target import, as available for shared library.

Link: facebook/rocksdb#12890

Configure static library default to switched off
as shared libraries are sufficient in most cases.

Signed-off-by: Bhabu Bindu <bindu.bhabu@kpit.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
(cherry picked from commit 233079a)
Signed-off-by: Armin Kuster <akuster808@gmail.com>
Signed-off-by: Jeremy A. Puhlman <jpuhlman@mvista.com>
@Bindu-Bhabu
Copy link
Copy Markdown
Author

Hello @rhubner ,
May I know why this is not yet integrated in the upstream main branch?

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.

3 participants