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 filter .pth files in python plugin #822

Merged
merged 1 commit into from Sep 23, 2016
Merged

Don't filter .pth files in python plugin #822

merged 1 commit into from Sep 23, 2016

Conversation

ghost
Copy link

@ghost ghost commented Sep 22, 2016

The .pth files are needed by some projects to help with importing. We
currently filter them out, breaking imports for those projects.

LP: #1626668

The .pth files are needed by some projects to help with importing. We
currently filter them out, breaking imports for those projects.

LP: #1626668
@ghost
Copy link
Author

ghost commented Sep 22, 2016

Heh. Coverage falling again. what a suprise

@sergiusens
Copy link
Collaborator

it is so little though

@come-maiz
Copy link
Contributor

Hey @SamYaple, thanks again for your code! Do you have a simple project that fails because of this?

@ghost
Copy link
Author

ghost commented Sep 22, 2016

@ElOpio it failed for me with dogpile.cache, it will for you too. There are others as well that this type of import would break.

I commented on it in this patch #765

Im trying to write a quick simple project that will break like this and publish it to pypi so we can write a test against it for the future

@come-maiz
Copy link
Contributor

@SamYaple yes, that would be awesome. If it's a dummy simple project, we can put it in our integration suite. If it's more complex, we can put it in our demos. ^_^

@sergiusens
Copy link
Collaborator

El 22/09/16 a las 15:18, Leo Arias escribió:

@SamYaple https://github.com/SamYaple yes, that would be awesome. If
it's a dummy simple project, we can put it in our integration suite.
If it's more complex, we can put it in our demos. ^_^

The reason I want it our demos is to be able to run it from the snap.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#822 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABF_5QHaDPmn9NDfHgc5WIZgZsL6j2R-ks5qssZmgaJpZM4KEKln.

@sergiusens
Copy link
Collaborator

ok to test

@ghost
Copy link
Author

ghost commented Sep 23, 2016

Lets go ahead and merge this. I have filed a tech debt bug to add an appropriate regression test for this.

https://bugs.launchpad.net/snapcraft/+bug/1626839

I dont want to hold up this fix for that test though.

@sergiusens
Copy link
Collaborator

I tried to this http://paste.ubuntu.com/23220251/ on an unpatched snapcraft and was able to do this http://paste.ubuntu.com/23220246/ (I am not sure how to expose the current .pth bug)

@ghost
Copy link
Author

ghost commented Sep 23, 2016

http://paste.ubuntu.com/23220481/

For dogpile.cache, it doesn't have the issue on python3 and version 0.6.x do not have the silly .pth usage.

I'd rather not be using dogpile.cache specifically for testing, but it is the easiest one to demonstrate at teh moment. I am working on a simple module to reproduce this.

The issue does occur on python3 as well, but that will be easier to show in a sample python project.

@sergiusens
Copy link
Collaborator

@SamYaple I am not questioning it doesn't happen, I was just not seeing it :-)

@ghost
Copy link
Author

ghost commented Sep 23, 2016

Well I can't reproduce it on python3 yet. I would like to have an example of that for reference sake. But that is what the other bug will track. I feel like this is a regression that could end up happening again.

@sergiusens sergiusens merged commit 66d6f78 into canonical:master Sep 23, 2016
@ghost ghost deleted the python_pth_restore branch September 23, 2016 15:26
kalikiana pushed a commit to kalikiana/snapcraft that referenced this pull request Apr 6, 2017
The .pth files are needed by some projects to help with importing. We
currently filter them out, breaking imports for those projects.

LP: #1626668
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.

None yet

3 participants