Skip to content
This repository has been archived by the owner on Jun 28, 2022. It is now read-only.

Adjust per-platform link libraries #12

Merged
merged 2 commits into from
Jan 3, 2022
Merged

Adjust per-platform link libraries #12

merged 2 commits into from
Jan 3, 2022

Conversation

morrisonlevi
Copy link
Collaborator

What does this PR do?

Adjusts the per-platform link libraries included in the pkg-config and
cmake helpers.

Motivation

While investigating a sigsegv due to strange linking, I realized that
the link libraries that Rust suggests are per platform.

Additional Notes

This is brittle. Any change in these flags should be investigated by a
human.

How to test the change?

Existing build scripts should still work, although there may now be more
runtime dependencies than before. It's unclear to me how many of these
libraries actually need to be linked at runtime, but I don't have a
good programmatic way of figuring out how to pare it down.

While investigating a sigsegv due to strange linking, I realized that
the link libraries that Rust suggests are per platform. This commit
adjusts the pkg-config and cmake helpers in a per-platform way.

Note that this is brittle. Any change in these flags should be
investigated by a human.
This is because if it is present, even if -static-libgcc is provided,
then it will be present in the final library runtime dependencies.
This is undesirable at least on Alpine Linux, where it may not even be
present due to the platform's minimalism.
Copy link
Contributor

@r1viollet r1viollet left a comment

Choose a reason for hiding this comment

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

Good thing we are now secured against link time changes. Thanks

@morrisonlevi morrisonlevi merged commit daab60b into main Jan 3, 2022
@morrisonlevi morrisonlevi deleted the levi/link-flags branch January 3, 2022 21:45
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.

2 participants