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

Winreg import failure in appdirs #7495

Closed
zooba opened this issue Dec 17, 2019 · 5 comments
Closed

Winreg import failure in appdirs #7495

zooba opened this issue Dec 17, 2019 · 5 comments
Labels
auto-locked Outdated issues that have been locked by automation state: awaiting PR Feature discussed, PR is needed type: enhancement Improvements to functionality

Comments

@zooba
Copy link
Contributor

zooba commented Dec 17, 2019

Environment

  • pip version: 19.3.1
  • Python version: 3.7.5 (modified)
  • OS: Windows 10

The version of Python I am using is modified for security. Most importantly, ctypes is removed.

Description
Without ctypes available, appdirs will automatically use the _get_win_folder_from_registry function.

However, this function has not been updated for Python 3's winreg module - it tries/fails to import _winreg (the Python 2.x name).

Expected behavior

How to Reproduce

  1. Rename DLLs\_ctypes.pyd to any other name, or move it elsewhere. (Or any other mechanism you like to prevent it from being imported)
  2. Then run pip --version
  3. An error occurs.

Output

(testenv) PS C:\testenv> pip --version
  File "C:\Python37_x64\lib\runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "C:\Python37_x64\lib\runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "C:\testenv\Scripts\pip.exe\__main__.py", line 5, in <module>
  File "c:\testenv\lib\site-packages\pip\_internal\main.py", line 13, in <module>
    from pip._internal.cli.autocompletion import autocomplete
  File "c:\testenv\lib\site-packages\pip\_internal\cli\autocompletion.py", line 11, in <module>
    from pip._internal.cli.main_parser import create_main_parser
  File "c:\testenv\lib\site-packages\pip\_internal\cli\main_parser.py", line 7, in <module>
    from pip._internal.cli import cmdoptions
  File "c:\testenv\lib\site-packages\pip\_internal\cli\cmdoptions.py", line 25, in <module>
    from pip._internal.locations import USER_CACHE_DIR, get_src_prefix
  File "c:\testenv\lib\site-packages\pip\_internal\locations.py", line 28, in <module>
    USER_CACHE_DIR = appdirs.user_cache_dir("pip")
  File "c:\testenv\lib\site-packages\pip\_internal\utils\appdirs.py", line 47, in user_cache_dir
    path = os.path.normpath(_get_win_folder("CSIDL_LOCAL_APPDATA"))
  File "c:\testenv\lib\site-packages\pip\_internal\utils\appdirs.py", line 207, in _get_win_folder_from_registry
    import _winreg
ModuleNotFoundError: No module named '_winreg'
@triage-new-issues triage-new-issues bot added the S: needs triage Issues/PRs that need to be triaged label Dec 17, 2019
@zooba
Copy link
Contributor Author

zooba commented Dec 17, 2019

Current code:

def _get_win_folder_from_registry(csidl_name):
    import _winreg

    shell_folder_name = {
        "CSIDL_APPDATA": "AppData",
        "CSIDL_COMMON_APPDATA": "Common AppData",
        "CSIDL_LOCAL_APPDATA": "Local AppData",
    }[csidl_name]

    key = _winreg.OpenKey(
        _winreg.HKEY_CURRENT_USER,
        r"Software\Microsoft\Windows\CurrentVersion\Explorer\Shell Folders"
    )
    directory, _type = _winreg.QueryValueEx(key, shell_folder_name)
    return directory

Possible alternative:

def _get_win_folder_from_registry(csidl_name):
    return {
        "CSIDL_APPDATA": os.getenv("APPDATA"),
        "CSIDL_COMMON_APPDATA": os.getenv("PROGRAMDATA"),
        "CSIDL_LOCAL_APPDATA": os.getenv("LOCALAPPDATA"),
    }[csidl_name]

@zooba
Copy link
Contributor Author

zooba commented Dec 17, 2019

Alternatively, it looks like the 1.4.3 release uses slightly better handling of the imports to special-case PY3 (which personally I don't agree with - PY2 is the special case - but if you want to do the direct-vendor thing then simply updating should be okay for now).

@uranusjr
Copy link
Member

Hmm, it looks pip 19.3.1 should already be using 1.4.3; something might be off here…

https://github.com/pypa/pip/blob/19.3.1/src/pip/_vendor/appdirs.py

It might be worthwhile to submit the environment variable method to upstream though. I’m kind of surprised appdirs doesn’t do that already.

@xavfernandez
Copy link
Member

something might be off here…

Yup, cf #6040

uranusjr added a commit to uranusjr/pip that referenced this issue Dec 19, 2019
@chrahunt chrahunt added state: awaiting PR Feature discussed, PR is needed type: enhancement Improvements to functionality labels Dec 20, 2019
@triage-new-issues triage-new-issues bot removed the S: needs triage Issues/PRs that need to be triaged label Dec 20, 2019
@uranusjr
Copy link
Member

I think #7501 also resolves this because the vendored appdirs uses the correct import.

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jan 30, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Jan 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation state: awaiting PR Feature discussed, PR is needed type: enhancement Improvements to functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants