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

Adjust per-platform link libraries#12

Merged
morrisonlevi merged 2 commits into
mainfrom
levi/link-flags
Jan 3, 2022
Merged

Adjust per-platform link libraries#12
morrisonlevi merged 2 commits into
mainfrom
levi/link-flags

Conversation

@morrisonlevi

Copy link
Copy Markdown
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.

@r1viollet r1viollet left a comment

Copy link
Copy Markdown
Contributor

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