Skip to content

pdoc cli ignoring virtual environment #44

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

Closed
jlaufersweiler opened this issue Mar 22, 2019 · 10 comments · Fixed by #45
Closed

pdoc cli ignoring virtual environment #44

jlaufersweiler opened this issue Mar 22, 2019 · 10 comments · Fixed by #45
Labels
help wanted 🤷 Extra attention is needed upstream Issue affects a dependency of ours

Comments

@jlaufersweiler
Copy link
Contributor

Expected Behavior

Runing pdoc3 cli inside active venv shell session:
(venv) C:\Users\me\dev\fooproject>pdoc --html foo
Documentation should be generated based on modules active in the virtual environment.

Actual Behavior

ModuleNotFoundError and ImportError raised for any modules/packages imported by foo that are not present in the system Python directory.

Steps to Reproduce

  1. Activate virtual environment.
  2. run pdoc cli for module that imports resources that are in the virtual environment but are not in the system environment.

Additional info

  • pdoc version:0.5.3
  • platform: Windows 10
@kernc
Copy link
Member

kernc commented Mar 23, 2019

This is weird because pdoc really just imports.

https://github.com/kernc/pdoc/blob/e7868e286c260650daeee478a8b2f1e9ed4c8767/pdoc/__init__.py#L479-L481

Can you test if calling importlib.invalidate_caches() inserted just above these lines maybe helps?

What is the output of python -m site?

@jlaufersweiler
Copy link
Contributor Author

jlaufersweiler commented Mar 25, 2019

"python -m site?"
Here it is:

(venv) C:\Users\jlaufersweiler\dev\project>python -m site
sys.path = [
    'C:\\Users\\jlaufersweiler\\dev\\project',
    'C:\\Users\\jlaufersweiler\\dev\\project\\venv\\Scripts\\python36.zip',
    'C:\\Program Files\\Python36\\DLLs',
    'C:\\Program Files\\Python36\\lib',
    'C:\\Program Files\\Python36',
    'C:\\Users\\jlaufersweiler\\dev\\project\\venv',
    'C:\\Users\\jlaufersweiler\\dev\\project\\venv\\lib\\site-packages',
]
USER_BASE: 'C:\\Users\\jlaufersweiler\\AppData\\Roaming\\Python' (exists)
USER_SITE: 'C:\\Users\\jlaufersweiler\\AppData\\Roaming\\Python\\Python36\\site-packages' (exists)
ENABLE_USER_SITE: False

This correctly sees the venv layered atop the system-wide 3.6 install.
In contrast, running pdoc att the same prompt is and searching executing from my user-specific 3.7 instance beneath ~\AppData\Local...:


(venv) C:\Users\jlaufersweiler\dev\project>pdoc --html batconvert
Traceback (most recent call last):
  File "C:\Users\jlaufersweiler\AppData\Local\Programs\Python\Python37\lib\site-packages\pdoc\__init__.py", line 540, in import_module
    module.__loader__.exec_module(module)
  File "<frozen importlib._bootstrap_external>", line 728, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "C:\Users\jlaufersweiler\dev\project\batconvert\__init__.py",
(rest of exception snipped)

pdoc3 is installed in both the 3.6 and 3.7 site-packages.

@kernc
Copy link
Member

kernc commented Mar 26, 2019

Hypothesizing that pdoc executable is linked to the wrong Python (3.7), does it maybe work if you run:

python -m pdoc --html batconvert

In that case, maybe also run pip uninstall pdoc and python -m pip install pdoc and then simply pdoc might work.

I'm not familiar with the intricacies of your Windos installation. I'd appreciate your further help in debugging this. 🥇

@kernc kernc added the help wanted 🤷 Extra attention is needed label Mar 26, 2019
@jlaufersweiler
Copy link
Contributor Author

jlaufersweiler commented Mar 26, 2019

OK, I uninstalled pdoc3 from the 3.7 environment, and now invoking pdoc within the venv is looking at the system 3.6 instance upon which the venv is based. However, it's still not seeing any modules installed within the venv:

(venv) C:\Users\jlaufersweiler\dev\project>pdoc --html batconvert
Traceback (most recent call last):
  File "c:\program files\python36\lib\site-packages\pdoc\__init__.py", line 540, in import_module
    module.__loader__.exec_module(module)
  File "<frozen importlib._bootstrap_external>", line 678, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "C:\Users\jlaufersweiler\dev\project\batconvert\__init__.py", line 2, in <module>
    import requests
ModuleNotFoundError: No module named 'requests'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "C:\Program Files\Python36\Scripts\pdoc-script.py", line 11, in <module>
    load_entry_point('pdoc3==0.5.3', 'console_scripts', 'pdoc')()
  File "c:\program files\python36\lib\site-packages\pdoc\cli.py", line 338, in main
    for module in args.modules]
  File "c:\program files\python36\lib\site-packages\pdoc\cli.py", line 338, in <listcomp>
    for module in args.modules]
  File "c:\program files\python36\lib\site-packages\pdoc\__init__.py", line 542, in import_module
    raise ImportError('Error importing {!r}: {}'.format(filename, e))
ImportError: Error importing 'batconvert\\__init__.py': No module named 'requests'

I am able to successfully generate results by installing pdoc3 directly within the venv.

It seems that without pdoc3 installed within the venv, the shell looks for pdoc.exe first in %VIRTUAL_ENV%\Scripts, doesn't find it there, then works down the %PATH% until it finds it in c:\program files\python36\Scripts. (It was invoking with my user-level 3.7 install previously, as it is ahead of site-wide 3.6 in my %PATH%.)

Thus, pdoc.exe is invoking cli.py with the system python.exe from within the (venv) shell, even though %VIRTUAL_ENV% is set to C:\Users\jlaufersweiler\dev\project\venv within that shell.

It is still managing to find the module the that was passed as an argument thanks to this part of cli.py:

 def main(_args=None):
    """ Command-line entry point """
    global args
    args = _args or parser.parse_args()

    if args.close_stdin:
        sys.stdin.close()

    if args.template_dir is not None:
        if not path.isdir(args.template_dir):
            print('Error: Template dir {!r} is not a directory'.format(args.template_dir),
                  file=sys.stderr)
            sys.exit(1)
        pdoc.tpl_lookup.directories.insert(0, args.template_dir)

    # Support loading modules specified as python paths relative to cwd
    sys.path.append(os.getcwd())

That last line in the excerpt is how it's loading .\batconvert\__init__.py from C:\Users\jlaufersweiler\dev\project\ despite being in the system context rather than the venv context, since it's adding the current directory to sys.path, but it's not looking in .\venv\Lib\site-packages.

So that's what's happening and why.

Hoping for a workaround without installing pydoc3 in the venv, I uninstalled it from the venv, deactivated, set include-system-site-packages = true in ./venv/pyenv.cfg, reactivated, and tried again.

Unfortunately, while pip list inside the venv shell does now show pdoc3, pdoc --html batconvert still fails to find the modules in .\venv\Lib\site-packages. I think I see why...

Here's python -m site after the changes:

(venv) C:\Users\jlaufersweiler\eclipse-workspace\xltflask>python -m site
sys.path = [
    'C:\\Users\\jlaufersweiler\\eclipse-workspace\\xltflask',
    'C:\\Users\\jlaufersweiler\\eclipse-workspace\\xltflask\\venv\\Scripts\\python36.zip',
    'C:\\Program Files\\Python36\\DLLs',
    'C:\\Program Files\\Python36\\lib',
    'C:\\Program Files\\Python36',
    'C:\\Users\\jlaufersweiler\\eclipse-workspace\\xltflask\\venv',
    'C:\\Users\\jlaufersweiler\\eclipse-workspace\\xltflask\\venv\\lib\\site-packages',
    'C:\\Users\\jlaufersweiler\\AppData\\Roaming\\Python\\Python36\\site-packages',
    'C:\\Program Files\\Python36\\lib\\site-packages',
]
USER_BASE: 'C:\\Users\\jlaufersweiler\\AppData\\Roaming\\Python' (exists)
USER_SITE: 'C:\\Users\\jlaufersweiler\\AppData\\Roaming\\Python\\Python36\\site-packages' (exists)
ENABLE_USER_SITE: True

The venv shell can find the pdoc modules C:\Program Files\Python36\lib\site-packages now, but without pdoc.exe installed within the venv\Scripts, it can't invoke it with the venv python, since C:\Program Files\Python36\Scripts isn't in the venv's sys.path even when 'C:\Program Files\Python36\lib\site-packages' is.

PROPOSED SOLUTION: Add a routine in cli.py that checks to see if the VIRTUAL_ENV environment variable is set, and if so do something like...
sys.path.append( os.path.join( os.environ.get( 'VIRTUAL_ENV' ), 'lib', 'site-packages' ) )
just like it does the cwd.

IMPLEMENTATION QUESTION: Should this always be done, or should it be a switchable behavior? If the latter, should it default to on or off?

...

@jlaufersweiler
Copy link
Contributor Author

IMPLEMENTATION QUESTION: How should one write tests for something like this? Unit test cases for the environment variable check and sys.path modification operations would be fairly straightforward, but is that enough? Or would proper testing need to actually mock up a venv, with some modules in it that aren't in the standard library, make the example package import them, and then spawn a sub-process shell to enter the venv context and try to run the pdoc cli?

@kernc
Copy link
Member

kernc commented Mar 30, 2019

To sum up: when virtualenv exists, overlaying system site-packages where pdoc is installed, invoking pdoc doesn't "see" the virtualenv packages because pdoc console script contains a shebang pointing to the system python interpreter, so venv python interpreter is never run, and sys.path is never amended with venv paths.

I think this is a Python issue.

PEP 486 and its implementation aimed to make Python VIRTUAL_ENV-aware in some cases on Windos, but it seems to not work in our case of console scripts.

Would you consider raising this issue up on the CPython issue tracker?

@kernc kernc added the upstream Issue affects a dependency of ours label Apr 1, 2019
@jlaufersweiler
Copy link
Contributor Author

jlaufersweiler commented Apr 18, 2019

This isn't an instance of the py launcher case in that PEP, though they do touch on similar concepts, and the PR I submitted to fix this here uses a similar approach to the proposed solution for the launcher.

What this would really need to be solved at the python-wide level is another flag for venv that would parallel --system-site-packages, say "--system-scripts," that would put the system scripts directory behind the virtualenv's scripts directory in the path when activated, just like --system-site-packages does with site-packages.

That being said, venv is in the standard library. A change like that to such an important component would need to start on the dev mailing list, then go through the PEP process. While I agree that it might be worth pursuing at that level, I suggest accepting the PR in the meantime, as it could take months or even years for such a change to get into 3.future. The patch isn't invasive, and could easily be extracted in a future update should such a hypothetical change to venv come to pass.

@kernc
Copy link
Member

kernc commented Apr 19, 2019

What this would really need to be solved at the python-wide level is another flag for venv that would parallel --system-site-packages, say "--system-scripts," that would put the system scripts directory behind the virtualenv's scripts directory in the path when activated

It's what virtualenv already does by default:

$ source venv/bin/activate
$ echo $PATH
/home/user/venv/bin:/home/user/.local/bin:/usr/local/bin:/usr/bin:/bin

It just doesn't invoke the correct (current venv's) Python interpreter when the bin script shebang points to another. I think it's a very similar issue to what PEP 486 was intended to fix. 🤔

I'm not strongly inclined to merging simple workarounds without any input from the parties the issue likely most concerns. I encourage you to show the problem on CPython issue tracker (they'll know what needs or needs not be done) and please reference it here. 💮

@josqu-john
Copy link

Same Problem here no Solution in sight I guess...

@kernc
Copy link
Member

kernc commented Jun 26, 2019

I re-read PEP 486 and have changed my opinion. 😳 I guess appending virtual env path, if it exists, to sys.path is acceptable. 👍

kernc pushed a commit to jlaufersweiler/pdoc that referenced this issue Jul 2, 2019
@kernc kernc closed this as completed in #45 Jul 2, 2019
kernc pushed a commit that referenced this issue Jul 2, 2019
* FIX: pdoc cli ignoring virtual environment

fixes: #44

* REF: Simplify handling virtual environment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted 🤷 Extra attention is needed upstream Issue affects a dependency of ours
Development

Successfully merging a pull request may close this issue.

3 participants