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 --extend-paths (alternative) #150

Merged
merged 3 commits into from
Jul 18, 2020

Conversation

itamarst
Copy link
Contributor

This is based on #140, with some changes:

  1. Improved testing.
  2. Undid one of the changes in Change the order & behavior of PYTHONPATH extension. #140. Change the order & behavior of PYTHONPATH extension. #140 made PYTHONPATH be last, but standard Python behavior is for it to be first, which was the existing shiv behavior. So this sticks to current behavior.

Fixes #138, I think.

@itamarst
Copy link
Contributor Author

itamarst commented Jul 7, 2020

@lorencarvalho I realize there's little time for maintenance in general, and pandemic has made things worse. So if there's anything company that's funding me can do to help get this going and released, please do say, e.g. if you want to set up GitHub Sponsors or Buy Me a Coffee to compensate for your time.

@lorencarvalho
Copy link
Contributor

Hi @itamarst !

I have to apologize again for neglecting your PR & issue. As I'm sure you can relate, it's been an interesting few months. I took some time this morning to grab your fork and walked through the examples in my comment from #138 and can confirm this improves the current behavior. Really appreciate your patience, and taking the time to cut this PR!

You made a comment on #140 that I want to note here,

Using set is a problem because the order isn't consistent (which is a little surprising, but seems to be the case on 3.6 at least).

Yep, we definitely need to preserve order, but I also want to make sure we deduplicate...

And having duplicate paths doesn't seem like an issue?

... normally this wouldn't be a concern, but one of shiv's primary concerns is reducing the length of sys.path for performance reasons. In fact, this is the primary motivator behind shiv's implementation, the details can be found here.

That said, I'll add a quick commit to dedupe with order retention after I merge this and then cut a release!

@lorencarvalho lorencarvalho merged commit d2212ce into linkedin:master Jul 18, 2020
@itamarst itamarst deleted the itamarst-pythonpath-fixes branch July 28, 2020 15:33
@itamarst
Copy link
Contributor Author

Oops, just noticing this now. Many thanks for merging this! And yes, totally understand, 2020 is not a fun year :)

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.

Subprocesses are not fully isolated when --extend-pythonpath is used
2 participants