Skip to content
This repository was archived by the owner on Jul 6, 2025. It is now read-only.

Add shared library support and fix library install path on Linux #815

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

musicinmybrain
Copy link

@musicinmybrain musicinmybrain commented Aug 27, 2022

This is a

  • Breaking change
  • New feature
  • Bugfix

I have

This PR uses the major version number as the SOVERSION, which presumes that no ABI-breaking changes will be made without bumping the major version.

I see two test failures, neither of which was introduced by this PR:

[ RUN      ] CommandLineArgsTest.LoggingFlagsArg
/home/ben/src/forks/easyloggingpp/test/command-line-args-test.h:50: Failure
Value of: Loggers::hasFlag(LoggingFlag::NewLineForContainer)
  Actual: true
Expected: false
/home/ben/src/forks/easyloggingpp/test/command-line-args-test.h:51: Failure
Value of: Loggers::hasFlag(LoggingFlag::LogDetailedCrashReason)
  Actual: true
Expected: false
[  FAILED  ] CommandLineArgsTest.LoggingFlagsArg (0 ms)
[ RUN      ] HelpersTest.ConvertTemplateToStdString
/home/ben/src/forks/easyloggingpp/test/helpers-test.h:15: Failure
Expected equality of these values:
  "[1, 2, 3, 4]"
  strVecInt
    Which is: "[1\n    2\n    3\n    4]"
With diff:
@@ -1,1 +1,4 @@
-[1, 2, 3, 4]
+[1
+    2
+    3
+    4]

[  FAILED  ] HelpersTest.ConvertTemplateToStdString (0 ms)

This fixes installation paths on Linux. For example, the static library
is installed to /usr/lib64/ when appropriate.
Use the major version number as the SOVERSION.
@musicinmybrain
Copy link
Author

Upon further investigation, given the number of configuration preprocessor macros affecting the compilation of the .cc file and intended to be set by the library user, it may not be practical to package this as a system-wide shared (or static) library for general use.

I think I will try to follow other distributions’ lead in packaging this as a header-only library, where the .cc file is “also a header” to be installed in, e.g., /usr/include/.

I will leave the PR here in case it is useful anyway…

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant