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 named loggers: core. #1529

Merged
merged 42 commits into from
May 1, 2017

Conversation

LGM-Doyle
Copy link
Contributor

This is part of the broken up PR #1481.

This is the core PR. It follows the preamble #1528.

Merge Message
Adds named loggers with individually controlled log levels.

This PR implements all of the functionality of the named loggers, but does not change any of the currently used loggers, or provide the OptionsWnd UI interface to change the thresholds.

@LGM-Doyle LGM-Doyle added category:feature The Issue/PR describes or implements a new game feature. component:internal The Issue/PR deals with any project component that has no explicit `component` label. labels Apr 24, 2017
@LGM-Doyle LGM-Doyle added this to the post v0.4.7 milestone Apr 24, 2017
@geoffthemedio
Copy link
Member

seems to need a rebase... can't pull and build it currently.

It was only used 3 times.
…reshold().

Previously, there was a separate function for the default logger.
The sink back end writes to the log files.
Separating the sink front end will make filtering channel logger error
thresholds individually possible.
This change will allow the Logger to check for user specified
configuration of logger not yet instantiated or instantiated in other
executables.
Add uppercase and integer parsing.
AddLoggerToOptionsDB() adds logging sources of either the default
process type or the detailed channel logger to OptionsDB.
This will allow components not at the app level, like test of UI,
universe components etc. to use the Logger without pulling in OptionsDB.
The python logging portions link to code not in the freeorioncommon
library, when linked with Clang.
…er()

The VC++ preprocessor does not correctly implement the C99 preprocessor
standard as described in these locations.
https://msdn.microsoft.com/en-us/library/hh567368.aspx
https://blogs.msdn.microsoft.com/vcblog/2017/03/07/c-standards-conformance-from-microsoft/

Specifically, it does not handle __VA_ARGS__ with commas as separate
arguments and it does not handle empty function macros correctly.

These workarounds use BOOST_PP_IS_EMPTY(__VA_ARGS__) to detect the empty
arguments and use different macros for the cases of DebugLogger() and
DebugLogger(named_logger).
Stitch the logger name together manually and avoid passing an empty
__VA_ARGS__ to the global name generator.
…able.

default_exec_logger_name may be accessed from other compilation units
during static initialization, so it is subject to the static
initialization fiasco.  Initializing it as a static function variable
guarantees that is is always initialized before access and only
initialized once.
forced_threshold may be accessed from other compilation units
during static initialization, so it is subject to the static
initialization fiasco.  Initializing it as a static function variable
guarantees that is is always initialized before access and only
initialized once.
sink_backend may be accessed from other compilation units
during static initialization, so it is subject to the static
initialization fiasco.  Initializing it as a static function variable
guarantees that is is always initialized before access and only
initialized once.
The reference to the global logger was not used.
Tracking the logger sink frontends enables initializing the sink
frontend after the sink backend has been initialized.
The default Release build type threshold is info.
The default Debug build type threshold is debug.

Based on discussions in review freeorion#1481.
…ation.

Extract SetLoggerThresholdCore from SetLoggerThreshold so that the
default logger threshold can be set before the "log" logger is created.
The default logger has no channel name and uses
LocalDefaultExecLoggerName() as its displayed name, whereas for the
regular loggers the channel and display names are the same.
The boost::scoped_lock mutex would not compile on clang or VC.

This commit switches to using a std::mutex as a class member to guard
the logger name to front end map from multi-thread access.
@geoffthemedio
Copy link
Member

I still can't pull with the standard github command line instructions. I get a bunch of merge conflicts.

}

// Signal that logger \p name has been created
using LoggerCreatedSignalType = boost::signals2::signal<void (const std::string logger)>;
Copy link
Member

Choose a reason for hiding this comment

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

could this be const std::string&

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the signal may cross threads and then the reference results in undefined behavior.

@LGM-Doyle
Copy link
Contributor Author

If you can pull #1481 and are happy with the code there, then you can merge that.

All of the code in these related PRs is a linear set of commits, so merging the last one is easier than doing each in turn.

The arrangement of the PRs is clear in any git tree visualizer, but the visualizer provided by github is abysmal.

Otherwise, could you show me the command line and the errors that you are seeing, so that I can try and fix the problem?

Thanks.

@geoffthemedio
Copy link
Member

After resetting master back a few hundred commits, re-updating, then re-pulling the pull request branch, it applied and built OK.

@geoffthemedio geoffthemedio merged commit fcea171 into freeorion:master May 1, 2017
@Vezzra Vezzra added the status:merged All relevant commits of this PR were merged into the master development branch. label May 1, 2017
@LGM-Doyle LGM-Doyle deleted the add_detailed_logging_core branch May 2, 2017 14:37
@@ -972,6 +973,7 @@
825EA75E16DB9DB10002A797 /* ModeratorAction.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ModeratorAction.cpp; sourceTree = "<group>"; };
825EA75F16DB9DB10002A797 /* ModeratorAction.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ModeratorAction.h; sourceTree = "<group>"; };
82714D761ADABE680071A329 /* libboost_log.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; path = libboost_log.a; sourceTree = "<group>"; };
82714D761ADABE680071A330 /* libboost_log_setup.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; path = libboost_log.a; sourceTree = "<group>"; };
Copy link
Member

Choose a reason for hiding this comment

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

This static library reference is broken, the path parameter points to libboost_log.a instead of libboost_log_setup.a. Don't know why building FO with this still works, but as soon as you make changes to the Xcode project, things get messed up.

@LGM-Doyle, I guess you edited the Xcode project with a text editor? Better ask someone on OSX to make such changes with Xcode, to avoid issues like this. Apparently things don't always break immediately, but at some later point, which can make fixing the broken stuff a lot harder.

Fixed in 00c389d.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry about that.

The build-bot when from unhappy to happy, so it seemed that I had done it correctly.

The joys of undocumented proprietary formats, use our tool or else.

Copy link
Member

Choose a reason for hiding this comment

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

No problem. Once @adrianbroher manages to cmakeify the build process across all platforms, we can get rid of the manually maintained MSVC and Xcode projects, which will simplify things a lot. 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:feature The Issue/PR describes or implements a new game feature. component:internal The Issue/PR deals with any project component that has no explicit `component` label. status:merged All relevant commits of this PR were merged into the master development branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants