Skip to content

[refactor]: cmake library installation#228

Merged
1a1a11a merged 6 commits intodevelopfrom
1a1a11a/cmake_lib
Jun 25, 2025
Merged

[refactor]: cmake library installation#228
1a1a11a merged 6 commits intodevelopfrom
1a1a11a/cmake_lib

Conversation

@1a1a11a
Copy link
Copy Markdown
Owner

@1a1a11a 1a1a11a commented Jun 25, 2025

I am in the process of re-enabling the plugin system and noticed that the current library installation does not work. So this PR tries to fix the library installation.
Previously, we compiled each folder as a static library and installed all of them; however, we do not have a combined library. In this PR, each subfolder is compiled into an object, and we will install one library for libCacheSim. Currently we install both static and shared library.

1a1a11a added 4 commits June 25, 2025 01:45
…rity

- Set CMAKE_POSITION_INDEPENDENT_CODE to ON for better compatibility.
- Transitioned from global include directories to target-specific includes for better encapsulation.
- Changed library definitions from INTERFACE to OBJECT for modular libraries to avoid transitive dependencies.
- Updated target linking to use the unified PROJECT_NAME for consistency across executables.
- Enhanced installation targets for unified libraries and improved include directory management.
- Cleaned up unnecessary global includes and streamlined dependency management across various components.
@1a1a11a 1a1a11a requested a review from haochengxia as a code owner June 25, 2025 02:25
@1a1a11a 1a1a11a requested a review from Copilot June 25, 2025 02:25

This comment was marked as outdated.

@haochengxia
Copy link
Copy Markdown
Collaborator

Should we use an option to determine building the shared lib or not?

@1a1a11a
Copy link
Copy Markdown
Owner Author

1a1a11a commented Jun 25, 2025

Should we use an option to determine building the shared lib or not?

Sounds good!

@1a1a11a
Copy link
Copy Markdown
Owner Author

1a1a11a commented Jun 25, 2025

updated

@haochengxia haochengxia requested a review from Copilot June 25, 2025 03:08
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the CMake build to produce a unified libCacheSim static (and optional shared) library by compiling submodules as OBJECT libraries and updating the library installation.

  • Converted each module (cache, traceReader, profiler, etc.) into OBJECT libraries and removed individual installations.
  • Added a combined static libCacheSim target (with optional shared variant) including proper target_include_directories, linking, and installation rules.
  • Updated the request_t struct to nest key_size/val_size inside a kv sub-struct and adjusted relevant trace reader code.

Reviewed Changes

Copilot reviewed 15 out of 23 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/CMakeLists.txt Switched test executables to link against ${PROJECT_NAME} and added manual include paths.
libCacheSim/utils/CMakeLists.txt Converted utils_lib to an OBJECT library.
libCacheSim/traceReader/*.h Updated request_t field references from req->key_size to req->kv.key_size.
libCacheSim/traceReader/CMakeLists.txt Converted traceReader_lib to an OBJECT library.
libCacheSim/traceAnalyzer/reuse.h Removed vtime_granularity_ member and its constructor parameter.
libCacheSim/traceAnalyzer/CMakeLists.txt Converted traceAnalyzer_lib to an OBJECT library and adjusted linking.
libCacheSim/profiler/CMakeLists.txt Converted profiler_lib to an OBJECT library.
libCacheSim/mrcProfiler/CMakeLists.txt Converted mrcProfiler_lib to an OBJECT library.
libCacheSim/include/libCacheSim/request.h Wrapped bitfields key_size/val_size in a kv struct.
libCacheSim/dataStructure/CMakeLists.txt Converted dataStructure_lib to an OBJECT library.
libCacheSim/cache/CMakeLists.txt Converted cache_lib_c/cache_lib_cpp to OBJECT libraries.
CMakeLists.txt Enabled CMAKE_POSITION_INDEPENDENT_CODE, added BUILD_SHARED_LIBS, centralized dependencies into ${PROJECT_NAME} static/shared library, and updated installation rules.
Comments suppressed due to low confidence (3)

libCacheSim/include/libCacheSim/request.h:49

  • The request_t struct now nests key_size and val_size inside kv, which breaks existing code references (e.g., in print_request). Ensure all accesses (including in print_request and any other modules) are updated to use req->kv.key_size and consider providing helper accessors if needed for clarity.
  } kv;

test/CMakeLists.txt:28

  • [nitpick] Since the ${PROJECT_NAME} target exports INTERFACE include directories, you can remove or reduce these manual target_include_directories entries and rely on the imported interface includes. This centralizes include paths and reduces duplication.
        ${CMAKE_SOURCE_DIR}/libCacheSim  # Add source dir to find utils/include/mymath.h

CMakeLists.txt:283

  • The install() call for the static libCacheSim target includes a LIBRARY DESTINATION, which only applies to shared libraries. You can drop the LIBRARY DESTINATION for this static-only install and keep only ARCHIVE DESTINATION to avoid confusion.
    LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}

@1a1a11a 1a1a11a merged commit b8c3a8b into develop Jun 25, 2025
9 checks passed
@1a1a11a 1a1a11a deleted the 1a1a11a/cmake_lib branch June 25, 2025 03:19
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.

3 participants