Skip to content

4847 logger singleton 3.0 #5110

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.

Sign up for GitHub

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
jmarrec merged 6 commits into from
Mar 27, 2024
Merged

4847 logger singleton 3.0 #5110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2d0a76c
Create a new Logger singleton type
kbenne Feb 16, 2024
659d01c
Remove now duplicate and conflicting Logger template definition
kbenne Feb 16, 2024
3984b2b
Add test for Logger global instance
kbenne Feb 17, 2024
aad08ee
Make Logger::instance more global
kbenne Mar 13, 2024
69c474b
Rename LoggerSingleton to LoggerImpl
kbenne Mar 14, 2024
c162c04
Remove poor man's hack for #4847
jmarrec Mar 22, 2024
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
2 changes: 2 additions & 0 deletions ProjectMacros.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ macro(MAKE_SWIG_TARGET NAME SIMPLENAME KEY_I_FILE I_FILES PARENT_TARGET PARENT_S
set_target_properties(${swig_target} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/ruby/")
target_link_libraries(${swig_target} ${${PARENT_TARGET}_depends})
target_include_directories(${swig_target} SYSTEM PRIVATE ${RUBY_INCLUDE_DIRS})
target_compile_definitions(${swig_target} PRIVATE SHARED_OS_LIBS)
Copy link
Collaborator

@jmarrec jmarrec Mar 14, 2024

Choose a reason for hiding this comment

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

SHARED_OS_LIBS needed because?

Copy link
Contributor Author

@kbenne kbenne Mar 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The UtilitiesAPI preprocessor logic will not apply the required declspec if SHARED_OS_LIBS is not defined. That said, I think we could do away with entire SHARED_OS_LIBS definition with a bit of work. I don't think there is ever a configuration where we aren't using the openstudio.dll.

add_dependencies(${swig_target} ${PARENT_TARGET})

execute_process(COMMAND \"${CMAKE_COMMAND}\" -E copy \"\${resolved_item_var}\" \"\${CMAKE_INSTALL_PREFIX}/Ruby/openstudio/\")
Expand Down Expand Up @@ -355,6 +356,7 @@ macro(MAKE_SWIG_TARGET NAME SIMPLENAME KEY_I_FILE I_FILES PARENT_TARGET PARENT_S
${SWIG_WRAPPER}
)

target_compile_definitions(${swig_target} PRIVATE SHARED_OS_LIBS)
set_target_properties(${swig_target} PROPERTIES OUTPUT_NAME _${LOWER_NAME})
set_target_properties(${swig_target} PROPERTIES PREFIX "")
set_target_properties(${swig_target} PROPERTIES ARCHIVE_OUTPUT_DIRECTORY "${CMAKE_ARCHIVE_OUTPUT_DIRECTORY}/python/")
Expand Down
2 changes: 1 addition & 1 deletion python/engine/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ target_include_directories(pythonengine
target_link_libraries(
pythonengine
PRIVATE
openstudiolib
Copy link
Collaborator

@jmarrec jmarrec Mar 14, 2024

Choose a reason for hiding this comment

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

We need this just because of the logger? isn't it part of the minimal?

Copy link
Contributor Author

@kbenne kbenne Mar 14, 2024

Choose a reason for hiding this comment

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

Yes we need this because of the Logger and yes Logger is part of the minimal utilities target, however minimal is a different compiled unit and since we want a single global Logger we cannot have it implmented in multiple compiled units. In fact, I think we can do away with the minimal target after this.

openstudio_scriptengine
openstudio_utilities_minimal
CONAN_PKG::fmt
Python::Python
)
Expand Down
2 changes: 1 addition & 1 deletion src/airflow/Airflow.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
%module openstudioairflow
#endif

#define UTILITIES_API
%include <utilities/UtilitiesAPI.hpp>
#define AIRFLOW_API

%include <utilities/core/CommonInclude.i>
Expand Down
9 changes: 9 additions & 0 deletions src/cli/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,15 @@ endif()
###############################################################################

if(BUILD_TESTING)
# openstudio_tests is a test target for C++ based tests that extend across multiple
# compiled units such as the case for openstudio cli.
# For example, here is a good place to test issues that are concerned with
# the complexities of sharing data between the Python and Ruby script engines.
set(openstudio_tests_src
"test/Logger_GTest.cpp"
)
CREATE_TEST_TARGETS(openstudio "${openstudio_tests_src}" openstudiolib openstudio_workflow)
add_dependencies(openstudio_tests openstudio)

# Just for convenience: having to VT the seb model which is at 1.11.5 can be very time consuming, especially on debug builds
add_custom_target(update_seb_osm_in_build_dir
Expand Down
8 changes: 0 additions & 8 deletions src/cli/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,6 @@ int main(int argc, char* argv[]) {
"└{0:─^{2}}┘",
"", "The `classic` command is deprecated and will be removed in a future release", 80);
fmt::print("\n");
#if defined _WIN32
// Poor man's hack #4847
// Disable this logger, we have a duplicate in the ruby shared lib
openstudio::Logger::instance().standardOutLogger().disable();
openstudio::Logger::instance().standardErrLogger().disable();
// Avoid getting some messages during getOpenStudioModule() when we locate the DLL
openstudio::StringStreamLogSink sink;
#endif
result = openstudio::rubyCLI(rubyEngine);
} else {
CLI::App app{"openstudio"};
Expand Down
24 changes: 24 additions & 0 deletions src/cli/test/Logger_GTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#include "../../scriptengine/ScriptEngine.hpp"
#include "../../utilities/core/Logger.hpp"

#include <gtest/gtest.h>

// cppcheck-suppress syntaxError
TEST(OpenStudioTest, LoggerGlobal) {
auto isEnabled = openstudio::Logger::instance().standardOutLogger().isEnabled();
EXPECT_TRUE(isEnabled);

openstudio::ScriptEngineInstance pythonEngine("pythonengine", {});
openstudio::ScriptEngineInstance rubyEngine("rubyengine", {});

pythonEngine->exec("openstudio.Logger.instance().standardOutLogger().disable()");
isEnabled = openstudio::Logger::instance().standardOutLogger().isEnabled();
EXPECT_FALSE(isEnabled);

rubyEngine->exec("OpenStudio::Logger::instance().standardOutLogger().enable()");
isEnabled = openstudio::Logger::instance().standardOutLogger().isEnabled();
EXPECT_TRUE(isEnabled);

pythonEngine.reset();
rubyEngine.reset();
}
2 changes: 1 addition & 1 deletion src/energyplus/EnergyPlus.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#endif


#define UTILITIES_API
%include <utilities/UtilitiesAPI.hpp>
Copy link
Collaborator

@jmarrec jmarrec Mar 14, 2024

Choose a reason for hiding this comment

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

? We need to dllexport the swig stuff inside the swig lib now?

Copy link
Contributor Author

@kbenne kbenne Mar 14, 2024

Choose a reason for hiding this comment

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

The swig generated code includes Logger.hpp which declares the Logger. We either need to link the Logger implementation into the swig generated code (like we have been doing with utilities minimal), or we need to use dllimport to signal that the implentation will come from a DLL. The problem with the former is that while linking to utilities minimal satisfies the linker, we had multiple implementations of the Logger floating around therefore it was not actually global. TLDR we need to declspec the Logger API everywhere that it is used.

#define MODEL_API
#define ENERGYPLUS_API

Expand Down
2 changes: 1 addition & 1 deletion src/epjson/epJSON.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#endif


#define UTILITIES_API
%include <utilities/UtilitiesAPI.hpp>
#define EPJSON_API

// You're better off just loading the json directly in the target language, so ignore
Expand Down
2 changes: 1 addition & 1 deletion src/gbxml/gbXML.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#endif


#define UTILITIES_API
%include <utilities/UtilitiesAPI.hpp>
#define GBXML_API

%include <utilities/core/CommonInclude.i>
Expand Down
2 changes: 1 addition & 1 deletion src/gltf/Gltf.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
%module openstudiogltf
#endif

#define UTILITIES_API
%include <utilities/UtilitiesAPI.hpp>
#define MODEL_API
#define GLTF_API

Expand Down
2 changes: 1 addition & 1 deletion src/isomodel/ISOModel.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
%module openstudioisomodel
#endif

#define UTILITIES_API
%include <utilities/UtilitiesAPI.hpp>
#define ISOMODEL_API

%include <utilities/core/CommonInclude.i>
Expand Down
2 changes: 1 addition & 1 deletion src/measure/Measure.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#endif


#define UTILITIES_API
%include <utilities/UtilitiesAPI.hpp>
#define MODEL_API
#define STANDARDSINTERFACE_API
#define MEASURE_API
Expand Down
1 change: 1 addition & 0 deletions src/model/ModelGeometry.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#endif


%include <utilities/UtilitiesAPI.hpp>
%include <model/Model_Common_Include.i>
%import <model/ModelCore.i>
%import <model/ModelSimulation.i>
Expand Down
2 changes: 1 addition & 1 deletion src/model/Model_Common_Include.i
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#ifndef MODEL_COMMON_INCLUDE_I
#define MODEL_COMMON_INCLUDE_I

#define UTILITIES_API
%include <utilities/UtilitiesAPI.hpp>
#define MODEL_API

%include <utilities/core/CommonInclude.i>
Expand Down
2 changes: 1 addition & 1 deletion src/osversion/OSVersion.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#endif


#define UTILITIES_API
%include <utilities/UtilitiesAPI.hpp>
#define MODEL_API
#define OSVERSION_API

Expand Down
2 changes: 1 addition & 1 deletion src/radiance/AnnualIlluminanceMap.i
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#ifndef RADIANCE_ANNUALILLUMINANCEMAP_I
#define RADIANCE_ANNUALILLUMINANCEMAP_I

#define UTILITIES_API
%include <utilities/UtilitiesAPI.hpp>
#define RADIANCE_API

%include <utilities/core/CommonInclude.i>
Expand Down
2 changes: 1 addition & 1 deletion src/radiance/HeaderInfo.i
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#ifndef RADIANCE_HEADERINFO_I
#define RADIANCE_HEADERINFO_I

#define UTILITIES_API
%include <utilities/UtilitiesAPI.hpp>
#define RADIANCE_API

%include <utilities/core/CommonInclude.i>
Expand Down
2 changes: 1 addition & 1 deletion src/radiance/Radiance.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#endif


#define UTILITIES_API
%include <utilities/UtilitiesAPI.hpp>
#define RADIANCE_API

%include <utilities/core/CommonInclude.i>
Expand Down
2 changes: 1 addition & 1 deletion src/sdd/SDD.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#endif


#define UTILITIES_API
%include <utilities/UtilitiesAPI.hpp>
#define SDD_API

%include <utilities/core/CommonInclude.i>
Expand Down
2 changes: 1 addition & 1 deletion src/utilities/Utilities.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#endif


#define UTILITIES_API
%include <utilities/UtilitiesAPI.hpp>
#define UTILITIES_TEMPLATE_EXT

%include <utilities/core/CommonInclude.i>
Expand Down
3 changes: 1 addition & 2 deletions src/utilities/UtilitiesBCL.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@
#endif


#define UTILITIES_API
#define UTILITIES_TEMPLATE_EXT
%include <utilities/UtilitiesAPI.hpp>

%include <utilities/core/CommonInclude.i>
%import <utilities/core/CommonImport.i>
Expand Down
4 changes: 1 addition & 3 deletions src/utilities/UtilitiesCore.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@
%module openstudioutilitiescore
#endif


#define UTILITIES_API
#define UTILITIES_TEMPLATE_EXT
%include <utilities/UtilitiesAPI.hpp>

%include <utilities/core/CommonInclude.i>
%include <utilities/core/CommonImport.i> // CommonImport is included in this module
Expand Down
4 changes: 1 addition & 3 deletions src/utilities/UtilitiesData.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
#endif


#define UTILITIES_API
#define UTILITIES_TEMPLATE_EXT

%include <utilities/UtilitiesAPI.hpp>
%include <utilities/core/CommonInclude.i>
%import <utilities/core/CommonImport.i>
%import <utilities/UtilitiesTime.i>
Expand Down
4 changes: 1 addition & 3 deletions src/utilities/UtilitiesFileTypes.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@
%module openstudioutilitiesfiletypes
#endif

#define UTILITIES_API
#define UTILITIES_TEMPLATE_EXT

%include <utilities/UtilitiesAPI.hpp>
%include <utilities/core/CommonInclude.i>
%import <utilities/core/CommonImport.i>
%import <utilities/UtilitiesCore.i>
Expand Down
4 changes: 1 addition & 3 deletions src/utilities/UtilitiesGeometry.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
#endif


#define UTILITIES_API
#define UTILITIES_TEMPLATE_EXT

%include <utilities/UtilitiesAPI.hpp>
%include <utilities/core/CommonInclude.i>
%import <utilities/core/CommonImport.i>
%import <utilities/UtilitiesPlot.i>
Expand Down
4 changes: 1 addition & 3 deletions src/utilities/UtilitiesIdd.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
#endif


#define UTILITIES_API
#define UTILITIES_TEMPLATE_EXT

%include <utilities/UtilitiesAPI.hpp>
%include <utilities/core/CommonInclude.i>
%import <utilities/core/CommonImport.i>
%import <utilities/UtilitiesCore.i>
Expand Down
4 changes: 1 addition & 3 deletions src/utilities/UtilitiesIdf.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
#endif


#define UTILITIES_API
#define UTILITIES_TEMPLATE_EXT

%include <utilities/UtilitiesAPI.hpp>
%include <utilities/core/CommonInclude.i>
%import <utilities/core/CommonImport.i>
%import <utilities/UtilitiesCore.i>
Expand Down
4 changes: 1 addition & 3 deletions src/utilities/UtilitiesPlot.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
#endif


#define UTILITIES_API
#define UTILITIES_TEMPLATE_EXT

%include <utilities/UtilitiesAPI.hpp>
%include <utilities/core/CommonInclude.i>
%import <utilities/core/CommonImport.i>
%import <utilities/UtilitiesData.i>
Expand Down
4 changes: 1 addition & 3 deletions src/utilities/UtilitiesSql.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
#endif


#define UTILITIES_API
#define UTILITIES_TEMPLATE_EXT

%include <utilities/UtilitiesAPI.hpp>
%include <utilities/core/CommonInclude.i>
%import <utilities/core/CommonImport.i>
%import <utilities/UtilitiesGeometry.i>
Expand Down
4 changes: 1 addition & 3 deletions src/utilities/UtilitiesTime.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
#endif


#define UTILITIES_API
#define UTILITIES_TEMPLATE_EXT

%include <utilities/UtilitiesAPI.hpp>
%include <utilities/core/CommonInclude.i>
%import <utilities/core/CommonImport.i>
%import <utilities/UtilitiesCore.i>
Expand Down
4 changes: 1 addition & 3 deletions src/utilities/UtilitiesUnits.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
#endif


#define UTILITIES_API
#define UTILITIES_TEMPLATE_EXT

%include <utilities/UtilitiesAPI.hpp>
%include <utilities/core/CommonInclude.i>
%import <utilities/core/CommonImport.i>
%import <utilities/UtilitiesCore.i>
Expand Down
4 changes: 1 addition & 3 deletions src/utilities/UtilitiesXML.i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
#endif


#define UTILITIES_API
#define UTILITIES_TEMPLATE_EXT

%include <utilities/UtilitiesAPI.hpp>
%include <utilities/core/CommonInclude.i>
%import <utilities/core/CommonImport.i>
%import <utilities/UtilitiesCore.i>
Expand Down
2 changes: 1 addition & 1 deletion src/utilities/core/LogSink.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class UTILITIES_API LogSink
void setFormatter(const boost::log::formatter& fmter);

protected:
friend class LoggerSingleton;
friend class LoggerImpl;

// does not register in the global logger
LogSink();
Expand Down
Loading