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

Don't create extra symlinks to libraries. The soversion property is enough #191

Closed
wants to merge 1 commit into from

Conversation

dstoup
Copy link
Collaborator

@dstoup dstoup commented Aug 16, 2017

The patch fixes one of the two issues mentioned in issue #178.

There is a macro getting called which makes symlinks. When called on the libraries, it creates too many levels of symlink. It should not be needed anyway since we set_target_properties for the soversion, so the sylinks exists anyway.

The question I have is, should this PR go to master or release? Since it's a bugfix, I am inclined to target release. We can discuss and I will update release-notes once I know which one to edit. Also, I did take the branch from master, so if release is desired, I will need to rebase or cherry-pick.

@kwcvrobot
Copy link
Collaborator

Mac build succeeded: https://open.cdash.org/buildSummary.php?buildid=397

@kwcvrobot
Copy link
Collaborator

Windows build succeeded: https://open.cdash.org/buildSummary.php?buildid=5022798

@kwcvrobot
Copy link
Collaborator

Linux build succeeded: https://open.cdash.org/buildSummary.php?buildid=5023012

@dstoup dstoup requested a review from mleotta August 17, 2017 21:23
Copy link
Member

@mleotta mleotta left a comment

Choose a reason for hiding this comment

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

I would put this on the release branch. Also, I would just delete the lines instead of commenting out. Can you also remove the get_target_property call? Is that BUILD_TARGET_LOCATION variable used elsewhere?

@dstoup dstoup force-pushed the libpng-symlink-error branch from 55f4608 to 56f2589 Compare August 22, 2017 14:13
@dstoup
Copy link
Collaborator Author

dstoup commented Aug 22, 2017

Closing and reopening against release

@dstoup dstoup closed this Aug 22, 2017
@kwcvrobot
Copy link
Collaborator

Mac build succeeded: https://open.cdash.org/buildSummary.php?buildid=5030060

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