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

Fix logic in find_dylib() and find_header() #25

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

nbigaouette
Copy link

@nbigaouette nbigaouette commented Jan 20, 2019

Milksnake does not support searching for dylib/header in absolute directory.

It appends path to the in_path variable (if given): https://github.com/getsentry/milksnake/blob/ef0723e/milksnake/setuptools_ext.py#L146

This prevent setting CARGO_TARGET_DIR to an absolute path different of the rust/ subdirectory.

This PR changes the logic so that appending happens only if in_path is not an absolute path. Also, this in_path is normalized as described in #24.

Closes #24.

@jgallagher-cs
Copy link

Apologies if bumping this is poor form, but I ran into this same issue today. Could this be merged and published in a new version?

Thanks!

if in_path is not None:
path = os.path.join(path, *in_path.split('/'))
Copy link
Member

Choose a reason for hiding this comment

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

@mitsuhiko why do we do this in the first place? What speaks against os.path.join(path, in_path)?

@johntrimble
Copy link

Is something blocking this PR from getting merged?

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.

Non portable paths issue
4 participants