Skip to content

Restructuring #2

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

Merged
merged 17 commits into from
Aug 14, 2025
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 35 additions & 27 deletions .clang-format
Original file line number Diff line number Diff line change
@@ -1,28 +1,29 @@
# BasedOnStyle: LLVM
AccessModifierOffset: -3
AlignAfterOpenBracket: Align
AlignConsecutiveAssignments: false
AlignConsecutiveAssignments: None
# This would be nice to have but seems to also (mis)align function parameters
AlignConsecutiveDeclarations: false
AlignEscapedNewlinesLeft: true
AlignConsecutiveDeclarations: None
AlignEscapedNewlines: Left
AlignOperands: true
AlignTrailingComments: true
AllowAllParametersOfDeclarationOnNextLine: true
AllowShortBlocksOnASingleLine: false
AllowShortBlocksOnASingleLine: Never
AllowShortCaseLabelsOnASingleLine: true
AllowShortEnumsOnASingleLine: false
AllowShortFunctionsOnASingleLine: Inline
AllowShortIfStatementsOnASingleLine: true
AllowShortLoopsOnASingleLine: true
AllowShortIfStatementsOnASingleLine: false
AllowShortLoopsOnASingleLine: false
# This option is "deprecated and is retained for backwards compatibility."
# AlwaysBreakAfterDefinitionReturnType: None
AlwaysBreakAfterReturnType: None
AlwaysBreakBeforeMultilineStrings: false
AlwaysBreakTemplateDeclarations: true
AlwaysBreakTemplateDeclarations: Yes
BinPackArguments: true
BinPackParameters: true
BraceWrapping:
AfterClass: false
AfterControlStatement: false
AfterControlStatement: Never
AfterEnum: false
AfterFunction: true
AfterNamespace: false
Expand All @@ -32,33 +33,20 @@ BraceWrapping:
BeforeCatch: false
BeforeElse: false
IndentBraces: false
BreakBeforeBinaryOperators: false
BreakBeforeBinaryOperators: None
BreakBeforeBraces: Custom
BreakBeforeTernaryOperators: true
BreakConstructorInitializersBeforeComma: false
ColumnLimit: 120
CommentPragmas: '^ IWYU pragma:'
ConstructorInitializerAllOnOneLineOrOnePerLine: false
ConstructorInitializerAllOnOneLineOrOnePerLine: true
ConstructorInitializerIndentWidth: 3
ContinuationIndentWidth: 3
Cpp11BracedListStyle: true
DerivePointerAlignment: false
DisableFormat: false
ExperimentalAutoDetectBinPacking: false
ForEachMacros: [ foreach, Q_FOREACH, BOOST_FOREACH ]
IncludeCategories:
- Regex: '^("|<)T'
Priority: 4
- Regex: '^("|<)ROOT/'
Priority: 5
- Regex: '^<.*\.h>'
Priority: 1
- Regex: '^"(llvm|llvm-c|clang|clang-c)/'
Priority: 2
- Regex: '^(<|"(gtest|isl|json)/)'
Priority: 3
- Regex: '.*'
Priority: 6
IndentCaseLabels: false
IndentWidth: 3
IndentWrappedFunctionNames: false
Expand All @@ -75,12 +63,14 @@ PenaltyBreakComment: 300
PenaltyBreakFirstLessLess: 120
PenaltyBreakString: 1000
PenaltyExcessCharacter: 1000000
PenaltyReturnTypeOnItsOwnLine: 60000
PenaltyReturnTypeOnItsOwnLine: 10
PointerAlignment: Right
ReflowComments: true
SortIncludes: false
ReflowComments: true
SortIncludes: Never
SpaceAfterCStyleCast: false
SpaceBeforeAssignmentOperators: true
# You want this : enable it if you have https://reviews.llvm.org/D32525
# SpaceBeforeColon: false
SpaceBeforeParens: ControlStatements
SpaceInEmptyParentheses: false
SpacesBeforeTrailingComments: 1
Expand All @@ -89,6 +79,24 @@ SpacesInContainerLiterals: true
SpacesInCStyleCastParentheses: false
SpacesInParentheses: false
SpacesInSquareBrackets: false
Standard: Cpp11
Standard: c++17
TabWidth: 3
UseTab: Never

# Order alphabetically and by generality the included header files.
IncludeCategories:
- Regex: '^"[^/]+\"'
Priority: 10
- Regex: '^("|<)T'
Priority: 12
- Regex: '^"ROOT/'
Priority: 15
- Regex: '^"cling/'
Priority: 20
- Regex: '^"clang/'
Priority: 30
- Regex: '^"llvm/'
Priority: 40
- Regex: '^<'
Priority: 50

76 changes: 76 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
Checks: >
-*,
bugprone-*,
clang-diagnostic-*,
clang-analyzer-*,
cppcoreguidelines-*,
llvm-*,
misc-*,
modernize-*,
performance-*,
portability-*,
readability-*,
-bugprone-narrowing-conversions,
-bugprone-easily-swappable-parameters,
-bugprone-implicit-widening-of-multiplication-result,
-bugprone-unchecked-optional-access,
-misc-const-correctness,
-misc-unused-parameters,
-misc-non-private-member-variables-in-classes,
-misc-no-recursion,
-misc-use-anonymous-namespace,
-modernize-return-braced-init-list,
-modernize-use-trailing-return-type,
-readability-braces-around-statements,
-readability-identifier-length,
-readability-magic-numbers,
-readability-named-parameter,
-readability-function-cognitive-complexity,
-readability-implicit-bool-conversion,
-cppcoreguidelines-avoid-magic-numbers,
-clang-analyzer-cplusplus.NewDeleteLeaks,

CheckOptions:
- key: readability-identifier-naming.ClassCase
value: aNy_CasE
- key: readability-identifier-naming.FunctionCase
value: aNy_CasE
- key: readability-identifier-naming.MemberCase
value: aNy_CasE
- key: readability-identifier-naming.PrivateMemberPrefix
value: 'm_'
- key: readability-identifier-naming.ProtectedMemberPrefix
value: 'm_'
- key: readability-identifier-naming.PublicMemberPrefix
value: ''
- key: readability-identifier-naming.ParameterCase
value: aNy_CasE
- key: readability-identifier-naming.UnionCase
value: CamelCase
- key: readability-identifier-naming.VariableCase
value: aNy_CasE
- key: readability-identifier-naming.IgnoreMainLikeFunctions
value: 1
- key: readability-implicit-bool-conversion.AllowPointerConditions
value: 1
- key: readability-magic-numbers.IgnorePowersOf2IntegerValues
value: 1
- key: readability-magic-numbers.IgnoredIntegerValues
value: 4;8;16;
- key: bugprone-argument-comment.CommentNullPtrs
value: 1
- key: bugprone-argument-comment.CommentBoolLiterals
value: 1
- key: bugprone-argument-comment.StrictMode
value: 1
- key: bugprone-argument-comment.CommentIntegerLiterals
value: 1
- key: bugprone-argument-comment.CommentUserDefinedLiterals
value: 1
- key: bugprone-argument-comment.CommentStringLiterals
value: 1
- key: bugprone-argument-comment.CommentFloatLiterals
value: 1
- key: bugprone-argument-comment.CommentCharacterLiterals
value: 1

6 changes: 6 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,9 @@ tmp/
*.bk
*.rootix
*.sam
build/
cmake-build-*/
Build/
BUILD/
Testing/
.vscode/
71 changes: 43 additions & 28 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,44 +1,59 @@
cmake_minimum_required(VERSION 3.16)
project(ramtools VERSION 1.0.0 LANGUAGES CXX)

project(ramtools LANGUAGES CXX)
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)

set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_POSITION_INDEPENDENT_CODE ON)

find_package(ROOT 6.26 REQUIRED
COMPONENTS Core RIO Tree ROOTNTuple)
option(RAMTOOLS_BUILD_TESTS "Build unit tests" ON)
option(RAMTOOLS_BUILD_BENCHMARKS "Build benchmarks" ON)
option(RAMTOOLS_BUILD_TOOLS "Build tools" ON)

set(RNTUPLE_LIBS ROOT::ROOTNTuple ROOT::ROOTNTupleUtil)
find_package(ROOT 6.26 REQUIRED COMPONENTS Core RIO Tree ROOTNTuple ROOTNTupleUtil)

include(${ROOT_USE_FILE})

include_directories(${CMAKE_SOURCE_DIR} ${CMAKE_SOURCE_DIR}/rntuple)

ROOT_STANDARD_LIBRARY_PACKAGE(ramrecord
SOURCES ramrecord.C
HEADERS ramrecord.h
LIBRARIES ROOT::Core ROOT::RIO ROOT::Tree
include(GNUInstallDirs)
set(CMAKE_INSTALL_LIBDIR lib)
set(CMAKE_INSTALL_INCLUDEDIR include)

set_property(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} PROPERTY INCLUDE_DIRECTORIES ${CMAKE_CURRENT_SOURCE_DIR}/inc)

Choose a reason for hiding this comment

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

That's the same as include_directories. Why do we need this to be set globally?

Copy link
Author

Choose a reason for hiding this comment

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

I am still confused about this, not able to implement removing include directories.

Choose a reason for hiding this comment

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

I will need more information about that. Compiling which library fails if you remove that line?

Copy link
Author

Choose a reason for hiding this comment

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

Made some changes, what do you think about it now?


ROOT_STANDARD_LIBRARY_PACKAGE(ramtools
HEADERS
ttree/RAMRecord.h
ttree/Utils.h
ttree/CigarOps.h
rntuple/RAMNTupleRecord.h
SOURCES
src/ttree/RAMRecord.cxx
src/rntuple/RAMNTupleRecord.cxx
LINKDEF
inc/ttree/LinkDef.h
DEPENDENCIES
ROOT::Core
ROOT::RIO
ROOT::Tree
ROOT::ROOTNTuple
ROOT::ROOTNTupleUtil
INSTALL_OPTIONS
DESTINATION ${CMAKE_INSTALL_LIBDIR}
)

set(RAMTOOLS_ROOT_LIBS ROOT::Core ROOT::RIO ROOT::Tree)
install(DIRECTORY inc/ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})

ROOT_STANDARD_LIBRARY_PACKAGE(ramntuplerecord
SOURCES rntuple/RAMntuplerecord.C
HEADERS rntuple/RAMNTupleRecord.h
LIBRARIES ROOT::Core ROOT::RIO ROOT::Tree ${RNTUPLE_LIBS}
)
if(RAMTOOLS_BUILD_TOOLS)
add_subdirectory(tools)
endif()

if(RAMTOOLS_BUILD_TESTS)
enable_testing()
add_subdirectory(test)
endif()

ROOT_EXECUTABLE(samtoramntuple
rntuple/samtoramntuple.C
LIBRARIES ramntuplerecord ${RAMTOOLS_ROOT_LIBS} ${RNTUPLE_LIBS})

include(CTest)
file(GLOB RAMTESTS tests/test_*.C)
foreach(testfile ${RAMTESTS})
get_filename_component(testname ${testfile} NAME_WE)
add_test(NAME ${testname}
COMMAND root -l -b -q ${testfile}+)
set_tests_properties(${testname} PROPERTIES WORKING_DIRECTORY ${CMAKE_SOURCE_DIR})
endforeach()
if(RAMTOOLS_BUILD_BENCHMARKS)
add_subdirectory(benchmark)
endif()

9 changes: 0 additions & 9 deletions LinkDef.h

This file was deleted.

2 changes: 2 additions & 0 deletions benchmark/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Benchmark targets can be added here in future

File renamed without changes.
File renamed without changes.
70 changes: 70 additions & 0 deletions cmake/modules/RamtoolsGoogleTest.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
set(_gtest_byproduct_binary_dir
${CMAKE_BINARY_DIR}/test/googletest-prefix/src/googletest-build)
set(_gtest_byproducts
${_gtest_byproduct_binary_dir}/lib/libgtest.a
${_gtest_byproduct_binary_dir}/lib/libgtest_main.a
${_gtest_byproduct_binary_dir}/lib/libgmock.a
${_gtest_byproduct_binary_dir}/lib/libgmock_main.a
)

if(MSVC)
set(EXTRA_GTEST_OPTS
-DCMAKE_ARCHIVE_OUTPUT_DIRECTORY_DEBUG:PATH=${_gtest_byproduct_binary_dir}/lib/
-DCMAKE_ARCHIVE_OUTPUT_DIRECTORY_MINSIZEREL:PATH=${_gtest_byproduct_binary_dir}/lib/
-DCMAKE_ARCHIVE_OUTPUT_DIRECTORY_RELEASE:PATH=${_gtest_byproduct_binary_dir}/lib/
-DCMAKE_ARCHIVE_OUTPUT_DIRECTORY_RELWITHDEBINFO:PATH=${_gtest_byproduct_binary_dir}/lib/
-Dgtest_force_shared_crt=ON)
elseif(APPLE)
set(EXTRA_GTEST_OPTS -DCMAKE_OSX_SYSROOT=${CMAKE_OSX_SYSROOT}
-DCMAKE_OSX_ARCHITECTURES=${CMAKE_OSX_ARCHITECTURES})
endif()

include(ExternalProject)
ExternalProject_Add(
googletest
GIT_REPOSITORY https://github.com/google/googletest.git
EXCLUDE_FROM_ALL 1
GIT_SHALLOW 1
GIT_TAG v1.14.0
UPDATE_COMMAND ""
CMAKE_ARGS -G ${CMAKE_GENERATOR}
-DCMAKE_BUILD_TYPE=Release
-DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER}
-DCMAKE_CXX_FLAGS=${CMAKE_CXX_FLAGS}
-DCMAKE_AR=${CMAKE_AR}
-DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}
${EXTRA_GTEST_OPTS}
BUILD_COMMAND ${CMAKE_COMMAND} --build <BINARY_DIR> --config Release
INSTALL_COMMAND ""
BUILD_BYPRODUCTS ${_gtest_byproducts}
LOG_DOWNLOAD ON
LOG_CONFIGURE ON
LOG_BUILD ON
TIMEOUT 600
)

ExternalProject_Get_Property(googletest source_dir)
set(GTEST_INCLUDE_DIR ${source_dir}/googletest/include)
set(GMOCK_INCLUDE_DIR ${source_dir}/googlemock/include)
file(MAKE_DIRECTORY ${GTEST_INCLUDE_DIR} ${GMOCK_INCLUDE_DIR})

ExternalProject_Get_Property(googletest binary_dir)
set(_G_LIBRARY_PATH ${binary_dir}/lib/)

foreach(lib gtest gtest_main gmock gmock_main)
add_library(${lib} IMPORTED STATIC GLOBAL)
set_target_properties(${lib} PROPERTIES
IMPORTED_LOCATION "${_G_LIBRARY_PATH}${CMAKE_STATIC_LIBRARY_PREFIX}${lib}${CMAKE_STATIC_LIBRARY_SUFFIX}"
INTERFACE_INCLUDE_DIRECTORIES "${GTEST_INCLUDE_DIR}"
)
add_dependencies(${lib} googletest)
endforeach()

target_include_directories(gtest INTERFACE ${GTEST_INCLUDE_DIR})
target_include_directories(gmock INTERFACE ${GMOCK_INCLUDE_DIR})

add_library(GTest::gtest ALIAS gtest)
add_library(GTest::gtest_main ALIAS gtest_main)
add_library(GTest::gmock ALIAS gmock)
add_library(GTest::gmock_main ALIAS gmock_main)

Loading