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:

invoke/invoke/loader.py

Lines 49 to 96 in 07b836f

	def load(self, name: Optional[str] = None) -> Tuple[ModuleType, str]:
	"""
	Load and return collection module identified by ``name``.
	This method requires a working implementation of `.find` in order to
	function.
	In addition to importing the named module, it will add the module's
	parent directory to the front of `sys.path` to provide normal Python
	import behavior (i.e. so the loaded module may load local-to-it modules
	or packages.)
	:returns:
	Two-tuple of ``(module, directory)`` where ``module`` is the
	collection-containing Python module object, and ``directory`` is
	the string path to the directory the module was found in.
	.. versionadded:: 1.0
	"""
	if name is None:
	name = self.config.tasks.collection_name
	spec = self.find(name)
	if spec and spec.loader and spec.origin:
	# Typically either tasks.py or tasks/__init__.py
	source_file = Path(spec.origin)
	# Will be 'the dir tasks.py is in', or 'tasks/', in both cases this
	# is what wants to be in sys.path for "from . import sibling"
	enclosing_dir = source_file.parent
	# Will be "the directory above the spot that 'import tasks' found",
	# namely the parent of "your task tree", i.e. "where project level
	# config files are looked for". So, same as enclosing_dir for
	# tasks.py, but one more level up for tasks/__init__.py...
	module_parent = enclosing_dir
	if spec.parent: # it's a package, so we have to go up again
	module_parent = module_parent.parent
	# Get the enclosing dir on the path
	enclosing_str = str(enclosing_dir)
	if enclosing_str not in sys.path:
	sys.path.insert(0, enclosing_str)
	# Actual import
	module = module_from_spec(spec)
	sys.modules[spec.name] = module # so 'from . import xxx' works
	spec.loader.exec_module(module)
	# Return the module and the folder it was found in
	return module, str(module_parent)
	msg = "ImportError loading {!r}, raising ImportError"
	debug(msg.format(name))
	raise ImportError

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 aa8b815 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:

invoke/invoke/loader.py

Line 85 in 07b836f

enclosing_str = str(enclosing_dir)

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)