Skip to content

Don't add -l prefix if it already exists.#59

Merged
dirk-thomas merged 1 commit intoros-visualization:kinetic-develfrom
fizyr-forks:libraries-prefix
Jan 2, 2019
Merged

Don't add -l prefix if it already exists.#59
dirk-thomas merged 1 commit intoros-visualization:kinetic-develfrom
fizyr-forks:libraries-prefix

Conversation

@hgaiser
Copy link
Copy Markdown
Contributor

@hgaiser hgaiser commented Dec 17, 2018

Similarly as discussed here, a -l shouldn't be prefixed if a library already has a -l prefix. Recently (cmake 3.13) exported its dependency on pthread by adding -lpthread to ${Boost_LIBRARIES}. Using the sip_helper.py on cmake 3.13 gives the following error:

/usr/sbin/ld: cannot find -l-lpthread

Copy link
Copy Markdown
Contributor

@brawner brawner left a comment

Choose a reason for hiding this comment

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

Super annoying issue, but just add a short comment echoing what you are saying in this PR.


def custom_platform_lib_function(self, clib, framework=0):
if os.path.isabs(clib):
if os.path.isabs(clib) or clib.startswith('-l'):
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.

Please add a comment describing why this check is necessary and libraries that already append -l

@hgaiser
Copy link
Copy Markdown
Contributor Author

hgaiser commented Jan 2, 2019

Apologies for the delay, holidays and everything. I just pushed the requested change.

@brawner brawner requested a review from dirk-thomas January 2, 2019 21:43
@dirk-thomas
Copy link
Copy Markdown
Contributor

Thanks for the patch.

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.

3 participants