pyblish / pyblish-base

Pyblish base library - see https://github.com/pyblish/pyblish for details.
Other
123 stars 60 forks source link

Vendor libraries injected in sys.path are messing up environment #365

Closed buddly27 closed 4 years ago

buddly27 commented 4 years ago

Is this line absolutely necessary?

https://github.com/pyblish/pyblish-base/blob/841f40821507964389adac01683ac86f745bb5fe/pyblish/vendor/__init__.py#L5

Because of this, if you have a tool using click, it picks up your vendor library which is a bit old.. You seems to be using "vendorised" libraries from the vendor module, so why adding them to sys.path?

https://github.com/pyblish/pyblish-base/blob/8588808a0e7e6f819e618909359502c5c8327e2e/pyblish/cli.py#L30

https://github.com/pyblish/pyblish-base/blob/841f40821507964389adac01683ac86f745bb5fe/run_testsuite.py#L10

mottosso commented 4 years ago

That's a good question, thanks for pointing this out.

It was quite a long time ago, but I remember Click (or possibly another library) didn't support being vendorised. It used absolute references internally, which I "solved" by making it happy and putting it on the absolute path.

It doesn't look necessary anymore, but are you able to quickly test this? E.g. remove that line, and see if anything you normally do still works.

buddly27 commented 4 years ago

For what I can see, all vendorised libraries are being imported via the vendor module.

Unit tests are running without the sys.path.insert and the command line seems to work fine as well.

mottosso commented 4 years ago

Thanks for the thorough investigation @buddly27. Would you mind making a PR for this, such that the automated tests can kick in? I expect that if those still pass, we'll be good to go.

buddly27 commented 4 years ago

Done :)