Deduplicate paths between old and new PYTHONPATH #15648
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
Not sure how to run rerun the |
|
Hello @ericsong, I rerun the cla/google check and it is passed. Thanks! |
ckolli5
added
team-Rules-Python
|
I have a question. Is there a way to do this deduplication one level higher, that is when stub parameters are being substituted? Bazel has a lot of machinery to do this, and it might be nicer if the duplicates never get into the template. |
|
I haven't tried it out yet but with that approach, we would be swapping it out here ? I'm not sure if that would be possible since the duplicates are between that and the PYTHONPATH at runtime. We wouldn't know what old_python_path would be when the python script is being generated. |
|
@comius thoughts on the above? |
| old_python_path = os.environ.get('PYTHONPATH') | ||
| python_path = os.pathsep.join(python_path_entries) | ||
| if old_python_path: | ||
| python_path += os.pathsep + old_python_path | ||
|
|
||
| python_path = os.pathsep.join(Deduplicate(python_path.split(os.pathsep))) | ||
|
|
There was a problem hiding this comment.
Current implementation with joining, splitting, joining, is not the most readable.
Perhaps rewrite this block, by adding old_python_path to python_path_entries and then deduplicate and join with path separator.
Thanks @ericsong. Since it's coming from the environment, it in fact isn't possible to move it one layer higher. |
|
thanks for the review @comius . I made some changes per your comment. |
|
@comius anything else you'd like for me to change? |
ShreeM01
removed
the
awaiting-review
Resolves #15649 by deduplicating the PYTHONPATH after the old and new one has been joined.