loader.py: Wrong directory inserted into sys.path for modules

[chrwang]

Background

In loader.py, invoke uses this load function to load the tasks collection. This function attempts to detect whether tasks is a module or a package:

https://github.com/pyinvoke/invoke/blob/07b836f2663bb073a7bcef3d6c454e1dc6b867ae/invoke/loader.py#L49-L96

It first sets enclosing_dir to be the parent directory of whatever file the import resolved to (this is tasks.py for modules, and __init__.py within the package directory for packages). Then, it checks if the discovered ModuleSpec has a non-empty entry for .parent, which would indicate that the imported tasks is a package. If it is a package, it sets module_parent to be one level above enclosing_dir. If it is not a package, module_parent is set to be enclosing_dir.

Bug

This is all correct, however the path that gets inserted into the path is always enclosing_dir, not module_parent. If we have:

foo
| -- pyinvoke
     | -- tasks.py

and then resolve the ModuleSpec for tasks, we correctly insert /foo/pyinvoke into the path. However, if we have:

foo
| -- pyinvoke
     | -- tasks
          | -- __init__.py

we end up inserting /foo/pyinvoke/tasks into the path, which is wrong, the path inserted should still be /foo/pyinvoke.

Note: #944 is similar but predates the refactor to load that happened in 2.1.3. In https://github.com/pyinvoke/invoke/commit/aa8b815d7a7a43c8ac814c6deb9c4555b8ac6b9e logic was added to fix the search path for config files, but that did not extend to search path for packages.

Note 2: This is all fine and dandy for most use cases where everything lives in tasks or tasks.py. This only broke because we have the following structure:

foo
| -- pyinvoke
    | -- tasks
        | -- __init__.py
        | -- foo_task.py
    | -- barutils
        | -- __init__.py
        | -- bar.py

From within foo_task.py we have from barutils.bar import bartender, which won't work when the inserted path is /foo/pyinvoke/tasks/ since barutils isn't in tasks.

Solution?

I'd like to propose that in:

https://github.com/pyinvoke/invoke/blob/07b836f2663bb073a7bcef3d6c454e1dc6b867ae/invoke/loader.py#L85

we replace enclosing_dir with module_parent.

@bitprophet if this fix seems reasonable I can put in a PR and add some tests for this.

[rtb-zla-karma]

I think I have the same Issue.

I've compared sys.path between venvs with invoke==1.7.1 and invoke==2.2.0. When invoke is run from project directory (I will use examples above) /foo, in the former version sys.path gives ['/foo/pyinvoke', ...] but for latter version it returns ['/foo/pyinvoke/tasks', ...] which is the reason why I can't import from module on the same level as /foo/barutils.

Moreover all commands in invoke -l changed from barutils.command-x to just command-x.

Currently I have no other solution for this broken version than "hacking" sys.path by replacing manually the first path which still leaves all commands changed to version without barutils.. Hack is to add this to foo_task.py:

from pathlib import Path
import sys
sys.path[0] = str(Path(__file__).resolve().parent.parent)