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

(maint) Enable builds with static cpp-pcp-client #406

Merged
merged 1 commit into from
Apr 29, 2016

Conversation

MikaelSmith
Copy link
Contributor

On Windows, Leatherman's logging/locale libraries need to be contained in
the same DLL. That can be done by either making them shared libraries, or
by creating a monolithic pxp-agent binary.

Currently our packaging makes it messy to use shared Leatherman DLLs, and
they don't bring us any advantages. Make it simple to use cpp-pcp-client
as a static library instead so we can produce a smaller package.

On Windows, Leatherman's logging/locale libraries need to be contained in
the same DLL. That can be done by either making them shared libraries, or
by creating a monolithic pxp-agent binary.

Currently our packaging makes it messy to use shared Leatherman DLLs, and
they don't bring us any advantages. Make it simple to use cpp-pcp-client
as a static library instead so we can produce a smaller package.
@mruzicka
Copy link
Contributor

LGTM

@@ -54,6 +54,12 @@ set (LIBS
${LEATHERMAN_LIBRARIES}
)

if (WIN32)
# Necessary when statically linking cpp-pcp-client on Windows.
# Shouldn't hurt when cpp-pcp-client is a DLL.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we just test what kind of cpp-pcp-client library we are linking against? Whether static or shared?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure there's a way to tell from here. /cc @branan

I could add a CMake config file to the cpp-pcp-client project that gets installed, and would replace the Findcpp-pcp-client script in this project. That would include link dependencies and whether it's static or dynamic. However there's pretty much no downside with linking an extra system library.

@mruzicka mruzicka merged commit bbf693d into puppetlabs:master Apr 29, 2016
@MikaelSmith MikaelSmith deleted the static-pcp-client branch May 2, 2016 23:26
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.

2 participants