bndr / pipreqs

pipreqs - Generate pip requirements.txt file based on imports of any project. Looking for maintainers to move this project forward.
Apache License 2.0
6.38k stars 388 forks source link

os.walk() is overkill and may bring wrong packages #403

Open solstag opened 1 year ago

solstag commented 1 year ago

Ni! Using os.walk() in get_locally_installed_packages() will break in case a parent directory of multiple python versions slips by accident into sys.path. For example, if sys.path contains "/usr/lib/", the walk will traverse all packages-like descendant directories, no matter if they're is in sys.path or not.

A solution would be to check whether something like root[ len(path.rstrip(os.sep) + 1):].count(os.sep) == 1 to ensure we only look inside child directories, where python would actually look.

jrwds commented 1 year ago

I've written a replacement that I use locally. If others are interested, I can create a pull request for it. This assumes there is only one virtual environment to be found in sys.path, which is the case for me but I am unsure if that's true always, since sys.path can be edited. You will also have to ctrl-f for 'exports' and remove those lines of code as well, I am not sure if exports is really necessary anymore.

This is related to #374 as well.

def get_locally_installed_packages(encoding=None):
    packages = []

   #for each possible path,
    for path in sys.path:

        #if site-packages is in the path name
        if "site-packages" in path:

            #for each file in the site-packages directory,
            for file in os.listdir(path):
                #if it contains dist-info in the directory name, it will contain the version info
                if "dist-info" in file:

                    #parsing out the package name and version from 
                    package_name = file.split(os.sep)[-1].split("-")
                    package = package_name[0]
                    version = package_name[1]
                    version = version.split(os.sep)[-1].split(".dist")
                    version = version[0]

                    #appending the package and version
                    packages.append({
                        'name': package,
                        'version': version
                    })

    return packages 
solstag commented 1 year ago

Thanks! But the correct solution is for os.walk() not to go deeper than one level.

Beware that your modifications concern an older version of pipreqs (which notably has a bug fixed by for path in reversed(sys.path)), and that in general there are no guarantees that paths will have "site-packages" in their name.

Cheers