-
Notifications
You must be signed in to change notification settings - Fork 654
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
Allow use without activation of virtualenv #1860
Conversation
2f6dc7e
to
7340dd4
Compare
f5757ad
to
a847c79
Compare
939feea
to
3eba0a3
Compare
c7411b6
to
7bd4ace
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just saw #1866 and I like it better because of a more accurate approach and no import-time patches.
31617ae
to
e49a690
Compare
@webknjaz I had a talk with @cidrblock and I will attempt to remove the code running at import from linter before doing this change. If it works we may be able to ditch that anti-pattern. |
I am very aware about the other change which is altering a huge amount of code, likely 10x more than this one and that is a big deal. I do not want to modify all subprocess calls to inject full paths in order to address this issue. That may come back to us very soon and it might conflict with other use cases and we could easily introduce bugs when calling other processes and forgetting to add the magic sanitization call to them. At least the PATH adulteration at start is clear and ensures that whatever we call will be using the same rules. I also sorted the import issues and avoided run of code as import. |
Allow users to call ansible-lint with a full path even when it is installed inside an inactive virtualenv.
This change also includes a fail-safe test that prevents future regressions.
Fixes: #1507
Closes: #1866 (duplicate)