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.

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

Add ASAN support and fix memleaks (main) #291

Merged
merged 3 commits into from
Jan 6, 2025
Merged

Conversation

alanking
Copy link
Contributor

Addresses #286
Addresses #287
Addresses #288

Cherry-pick of #289

Tests are not passing due to #290. However, there isn't much difference between this and 4-3-stable, so maybe it's okay...

diff --git a/CMakeLists.txt b/CMakeLists.txt
index b2d4a79..9197f7d 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1,6 +1,6 @@
 cmake_minimum_required(VERSION 3.5.0 FATAL_ERROR) #CPACK_DEBIAN_<COMPONENT>_PACKAGE_NAME
 
-set(IRODS_MINIMUM_VERSION "4.3.3")
+set(IRODS_MINIMUM_VERSION "4.90.0")
 find_package(IRODS "${IRODS_MINIMUM_VERSION}" REQUIRED)
 set(IRODS_PLUGIN_REVISION "0")
 set(IRODS_PLUGIN_VERSION "${IRODS_MINIMUM_VERSION}.${IRODS_PLUGIN_REVISION}")
@@ -22,7 +22,6 @@ project(irods_capability-storage_tiering
 include("${IRODS_TARGETS_PATH}")
 
 include(GNUInstallDirs)
-include(UseLibCXX)
 
 if (NOT CMAKE_CONFIGURATION_TYPES AND NOT CMAKE_BUILD_TYPE)
        set(CMAKE_BUILD_TYPE Release CACHE STRING "Choose the type of build {Debug, Release}." FORCE)
diff --git a/irods_consortium_continuous_integration_build_hook.py b/irods_consortium_continuous_integration_build_hook.py
index 73bf2bf..b94bf7d 100644
--- a/irods_consortium_continuous_integration_build_hook.py
+++ b/irods_consortium_continuous_integration_build_hook.py
@@ -12,11 +12,10 @@ def add_cmake_to_front_of_path():
 
 def install_building_dependencies(externals_directory):
     externals_list = [
-        'irods-externals-boost-libcxx1.81.0-1',
-        'irods-externals-clang-runtime13.0.1-0',
+        'irods-externals-boost1.81.0-1',
         'irods-externals-clang13.0.1-0',
         'irods-externals-cmake3.21.4-0',
-        'irods-externals-fmt-libcxx8.1.1-1',
+        'irods-externals-fmt8.1.1-1',
         'irods-externals-json3.10.4-0'
     ]
     if externals_directory == 'None' or externals_directory is None:

@trel
Copy link
Member

trel commented Dec 20, 2024

Yep - looks good.

Does it make sense to see if #290 can be addressed along with this? Then they can all go in together, passing?

Copy link
Collaborator

@korydraughn korydraughn left a comment

Choose a reason for hiding this comment

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

Code changes look good.

Deferring to @alanking on the question about #290.

@alanking
Copy link
Contributor Author

alanking commented Jan 6, 2025

Does it make sense to see if #290 can be addressed along with this? Then they can all go in together, passing?

Either way works for me. The tests are currently failing with or without this PR, so it doesn't make much difference. This is just a cherry-pick so that the changes are reflected in all the relevant branches.

@korydraughn
Copy link
Collaborator

Ok. Let's get this PR in and handle the tests in a separate PR. That reduces the number of changes for the reviewer to track.

@korydraughn
Copy link
Collaborator

Is this PR complete?

@alanking
Copy link
Contributor Author

alanking commented Jan 6, 2025

Yes, I think so. Changes look the same

Based on the --debug_build option in the build hook in the main
iRODS repository.
Requires irods-dev, irods-runtime, and irods-server packages
built with ASAN enabled. Also adds an option to the build
hook to enable ASAN from the development environment.
Copy link
Collaborator

@korydraughn korydraughn left a comment

Choose a reason for hiding this comment

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

Pound it if you're happy with it.

@alanking
Copy link
Contributor Author

alanking commented Jan 6, 2025

#'d. Mergin

@alanking alanking merged commit 4c759d6 into irods:main Jan 6, 2025
1 check failed
@alanking alanking deleted the asan.m branch January 6, 2025 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants