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

Avoid adding extension again when looking for library from versioned name #19663

Merged
merged 1 commit into from
Jan 31, 2017

Conversation

nalimilan
Copy link
Member

@nalimilan nalimilan commented Dec 20, 2016

Improve detection of extension by first skipping trailing numbers and dots.
Previously, looking for a library via its versioned name (including extension)
would try with the extension added twice if loading using the raw name failed.
This would mask any errors from that step (e.g. due to missing symbols),
printing instead the "file not found" error from the second attempt with the
extension added.

@nalimilan nalimilan changed the title Avoid adding extension again when looking for library from versioned … Avoid adding extension again when looking for library from versioned name Dec 20, 2016
…name

Improve detection of extension by first skipping trailing numbers and dots.
Previously, looking for a library via its versioned name (including extension)
would try with the extension added twice if loading using the raw name failed.
This would mask any errors from that step (e.g. due to missing symbols),
printing instead the "file not found" error from the second attempt with the
extension added.
@nalimilan
Copy link
Member Author

Made the test less strict to avoid relying on OS-dependent messages.

@tkelman
Copy link
Contributor

tkelman commented Dec 20, 2016

does this cause any issues on mac? don't the numbers come before the .dylib there?

@nalimilan
Copy link
Member Author

I confess I'm not familiar with how this works on Mac, but this shouldn't create any problems: this change only allows detecting the extension when it's followed by a mix of digits and dots. If the version number appears before the extension, then it was already detected correctly, and nothing is changed in that regard.

(At some point, we probably should add a generic API to require a specific version, so that Julia takes care of generating the library name in a system-independent fashion.)

@nalimilan
Copy link
Member Author

Though I could make this conditional on the extension being .so. Thoughts?

@nalimilan
Copy link
Member Author

@tkelman More comments?

@tkelman
Copy link
Contributor

tkelman commented Jan 27, 2017

Hoping this won't break anything? It's been long enough since this last ran on CI that we should make sure it still passes on Travis. Won't bother with appveyor since the queue is really long there right now, but travis isn't bad.

@tkelman
Copy link
Contributor

tkelman commented Jan 27, 2017

Looks like on linux the tests passed but the build complained due to old cmake from having a stale .travis.yml. On Mac the stale .travis.yml prevented the build from going through. Good enough to merge I think.

@nalimilan nalimilan merged commit 88ef041 into master Jan 31, 2017
@nalimilan nalimilan deleted the nl/dlopen branch January 31, 2017 12:25
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