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 follow libraries which won't be copied. #120

Merged
merged 3 commits into from
Sep 17, 2021

Conversation

HexDecimal
Copy link
Collaborator

@HexDecimal HexDecimal commented Jul 30, 2021

Combines lib_flit_func and copy_flit_func behavior into one function.
This makes sure that a file which won't be copied will not have its dependencies analyzed.

Fixes #119

Combines lib_flit_func and copy_flit_func behavior into one function.
This makes sure that a file which won't be copied will not have its
dependencies analyzed.

Fixes matthew-brett#119

Since the previous behavior is assumed to be wrong the tests have been updated.
@HexDecimal
Copy link
Collaborator Author

In theory this can be back-ported to a Python 2.7 branch. Since attempting to modify system libraries is a rather critical error.

@HexDecimal HexDecimal marked this pull request as ready for review July 30, 2021 17:50
@matthew-brett
Copy link
Owner

Sorry to be slow to get to this one. Can you think of a test for this behavior?

@HexDecimal
Copy link
Collaborator Author

Sorry to be slow to get to this one. Can you think of a test for this behavior?

Not really, because this simplifies the behavior of how libraries are selected, so there's less to test than before. test_fix_plat_dylibs was modified to remove an assert because the functionality which allowed that assert to pass is what caused #119 in the first place.

Copy link
Owner

@matthew-brett matthew-brett left a comment

Choose a reason for hiding this comment

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

A suggestion - sorry to be slow.

And - for the test - how about a test where we make lib_filt_func filter out a library with a dependency, and show that neither the library, nor the dependency, gets copied?

delocate/delocating.py Show resolved Hide resolved
@HexDecimal
Copy link
Collaborator Author

And - for the test - how about a test where we make lib_filt_func filter out a library with a dependency, and show that neither the library, nor the dependency, gets copied?

I've now added this test.

@matthew-brett matthew-brett merged commit e4be05b into matthew-brett:master Sep 17, 2021
@matthew-brett
Copy link
Owner

Thanks for your patience.

@HexDecimal
Copy link
Collaborator Author

I'm still thinking about #119, that's a pretty bad error to have in one of the releases. I thought it might a good idea to make another Python 2.7 release but that might take more effort than we should be giving to Python 2.7.

@matthew-brett, I'd like you to consider yanking delocate==0.9 from PyPI. Unless you have a better reason not to.

@matthew-brett
Copy link
Owner

matthew-brett commented Sep 17, 2021 via email

@HexDecimal
Copy link
Collaborator Author

If I make a python2 branch and backport this PR will you deploy it?

@matthew-brett
Copy link
Owner

Sure - or happy for you to; can give you PyPI permission.

@matthew-brett
Copy link
Owner

I've added a coverage check in the Travis-CI.org matrix, but it would be better to have one for the Github workflows, as Travis-CI is getting a bit hard to work with (.org shutting down very soon, credit crunch on .com).

https://staging.travis-ci.org/github/matthew-brett/delocate/builds/770146

@HexDecimal
Copy link
Collaborator Author

My PyPI account is https://pypi.org/user/HexDecimal/. I'll upload the backport and then yank the problematic version afterwards if I have access.

You have some encrypted data in your TravisCI scripts, and it's generally insecure to share that information with your testing tools. I'd recommend not adding any more external tools to the TravisCI script.

@matthew-brett
Copy link
Owner

I've invited you via PyPI. I've removed the encrypted stuff in the Travis-CI file; I wasn't using it, and the relevant service is long defunct. But you're right, it's best not to extend it at this stage.

@HexDecimal
Copy link
Collaborator Author

I should probably have been a PyPI maintainer, not an owner. Giving owner permissions is a lot of trust for this type of project.

@matthew-brett
Copy link
Owner

Happy to downgrade you but a) I trust you! and b) you have become the primary author of the package over the last few months. You now know the code-base a lot better than I do.

@HexDecimal
Copy link
Collaborator Author

Fine, I'll begrudgingly accept co-ownership.

@matthew-brett
Copy link
Owner

Totally happy to degrudge down to Maintainer if you prefer...

HexDecimal pushed a commit to HexDecimal/delocate that referenced this pull request Sep 17, 2021
MRG: Don't follow libraries which won't be copied.

Combines `lib_flit_func` and `copy_flit_func` behavior into one function.
This makes sure that a file which won't be copied will not have its dependencies analyzed.

Fixes matthew-brett#119
@HexDecimal HexDecimal deleted the fix-119 branch January 1, 2022 01:42
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.

Issues with libcompression.dylib and delocate 0.9
2 participants