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

pylint removes first item from sys.path when running from runpy. #7231

Closed
karthiknadig opened this issue Jul 26, 2022 · 4 comments · Fixed by #7277
Closed

pylint removes first item from sys.path when running from runpy. #7231

karthiknadig opened this issue Jul 26, 2022 · 4 comments · Fixed by #7277
Labels
Bug 🪲 Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Milestone

Comments

@karthiknadig
Copy link
Contributor

Bug description

This is the line where the first item from sys.path is removed.
https://github.com/PyCQA/pylint/blob/ce7cccf96454fb6e286e4a8f38919733a0f28f44/pylint/__init__.py#L99

I think there should be a check to ensure that the first item is "", "." or os.getcwd() before removing.

Configuration

No response

Command used

Run programmatically to repro this, using this code:

import sys
import runpy

sys.path.insert(0, "something")

runpy.run_module('pylint', run_name="__main__", alter_sys=True)

Pylint output

When using pylint extension which bundles the libraries, the extension add them to sys.path depending on user settings. Pylint removes the first entry from sys path causing it to fail to load.

Expected behavior

Check if "", "." or os.getcwd() before removing the first item from sys.path

Pylint version

pylint 2.14.5

OS / Environment

No response

Additional dependencies

No response

@karthiknadig karthiknadig added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Jul 26, 2022
@Pierre-Sassoulas Pierre-Sassoulas added Bug 🪲 Needs specification 🔐 Accepted as a potential improvement, and needs to specify edge cases, message names, etc. Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Jul 26, 2022
@Pierre-Sassoulas
Copy link
Member

This is a touchy part of the code (very hard to test). It probably make sense to do what you suggest but I don't understand this part of the code very well so I think some investigation/specification is required.

@jacobtylerwalls
Copy link
Member

I think it makes sense to take this suggestion as it makes the implementation agree with the docstring. @karthiknadig would you like to prepare a PR?

@jacobtylerwalls jacobtylerwalls added Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning Needs specification 🔐 Accepted as a potential improvement, and needs to specify edge cases, message names, etc. labels Aug 8, 2022
@karthiknadig
Copy link
Contributor Author

Will do :)

@karthiknadig
Copy link
Contributor Author

@jacobtylerwalls PR created #7231

@jacobtylerwalls jacobtylerwalls added this to the 2.15.0 milestone Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🪲 Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants