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

easyloggingpp: 9.95.0 -> 9.96.0 #30337

Merged
merged 2 commits into from Feb 23, 2018
Merged

Conversation

acowley
Copy link
Contributor

@acowley acowley commented Oct 12, 2017

Added a pkg-config file and copied the relevant source file into the
nix store. The idea is that the user may now relatively easily include
the library’s source file in their project using common CMake features.

Motivation for this change

The easyloggingpp package used to be header-only, but long compile times motivated a change to move as much code as possible into a C++ source file that could be compiled separately. Suggested use of the library involves either:

  • Link to a static library compiled from the C++ source file
  • Include the C++ source file in your own project

The former is not preferred in nixpkgs (or several other distros), as @Mic92 rightfully pointed out in #30256

The latter has an additional advantage in that several pre-processor definitions may be used to control the logging library's behavior if one recompiles the source file.

To support including the source file in what I hope is a nixpkgs-friendly way, I create a pkg-config file for the project during installation so that including the source file in a project built by cmake involves something like the following,

pkg_check_modules(EASYLOGGINGPP REQUIRED easyloggingpp)
add_executable(main src/main.cpp ${EASYLOGGINGPP_PREFIX}/src/easylogging++.cc)

That example is included in a comment in the nix package definition.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

prefix=$out
includedir=\''${prefix}/include
Cflags: -I\''${includedir}
EOF
Copy link
Member

@Mic92 Mic92 Oct 12, 2017

Choose a reason for hiding this comment

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

I think it would be useful to provide this pkgconfig file also upstream.
Then users don't have to care which package manager they use to compile against easylogging.

@Mic92
Copy link
Member

Mic92 commented Oct 12, 2017

When I said, we prefer dynamic libraries over static libraries, I meant we base this decisions on the availability of both. Shared libraries reduced disk space, ram usage and compile time in general and we have a great package manager to take of them. Here it seems we only have either header or a static library - I would make this decision up to you.

@acowley
Copy link
Contributor Author

acowley commented Oct 12, 2017

The installation question has come up with this project before. One slightly sticky question is where the .cc file should be installed. With nix I worry about this much less since each package has its own corner of the store, but for general usage, I guess that installing into include would be the most welcome option, so I have moved the .cc file to $out/include.

I'll have to figure out the easiest way for upstream's build process to generate the .pc file so the version information gets updated.

@Mic92
Copy link
Member

Mic92 commented Oct 13, 2017

There should be plenty examples how to generate a .pc file in cmake.

@acowley acowley force-pushed the easyloggingpp-init branch 2 times, most recently from 1304d3c to 11abc66 Compare February 14, 2018 08:22
@acowley
Copy link
Contributor Author

acowley commented Feb 14, 2018

At long last upstream has merged this change! Using the library as a source file now works quite well with pkg-config.

Added a pkg-config file and copied the relevant source file into the
nix store. The idea is that the user may now relatively easily include
the library’s source file in their project using common CMake features.
@acowley acowley changed the title easyloggingpp: remove static library easyloggingpp: 9.95.0 -> 9.96.0 Feb 22, 2018
@acowley
Copy link
Contributor Author

acowley commented Feb 22, 2018

I have rebased onto a more recent nixpkgs-unstable and updated to the 9.96.0 release. ping @Mic92

@Mic92 Mic92 merged commit 4ec94f6 into NixOS:master Feb 23, 2018
@Mic92
Copy link
Member

Mic92 commented Feb 23, 2018

Thanks!

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

Successfully merging this pull request may close these issues.

None yet

3 participants