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 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 join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant