Repository: repobee/repobee

Issue:
Make it possible to write a local single-file plugin
In theory, there's nothing stopping us from supporting local single-file plugins, as it's super easy to load a module given its filepath. These files also wouldn't need to follow any form of convention (other than defining hook implementations). Just as an example, imagine I have written the `javac.py` plugin and it's located at `/home/slarse/javac.py`. Then, I run RepoBee like so:

```
$ repobee --plug /home/slarse/javac.py clone ...
```

Making this work is extremely easy: when loading plugins, just check if the plugin specified is a local file. It will massively lower the bar for creating plugins.

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/repobee/repobee/pull/456?src=pr&el=h1) Report
> Merging [#456](https://codecov.io/gh/repobee/repobee/pull/456?src=pr&el=desc) into [master](https://codecov.io/gh/repobee/repobee/commit/288cd97193c855458e819b50822e1eced6991844&el=desc) will **decrease** coverage by `4.11%`.
> The diff coverage is `81.48%`.

[![Impacted file tree graph](https://codecov.io/gh/repobee/repobee/pull/456/graphs/tree.svg?width=650&height=150&src=pr&token=5vOKgkphZe)](https://codecov.io/gh/repobee/repobee/pull/456?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #456      +/-   ##
==========================================
- Coverage   92.74%   88.62%   -4.12%     
==========================================
  Files          40       40              
  Lines        2177     2181       +4     
  Branches      374      376       +2     
==========================================
- Hits         2019     1933      -86     
- Misses        125      214      +89     
- Partials       33       34       +1     
```


| [Impacted Files](https://codecov.io/gh/repobee/repobee/pull/456?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/\_repobee/plugin.py](https://codecov.io/gh/repobee/repobee/pull/456/diff?src=pr&el=tree#diff-c3JjL19yZXBvYmVlL3BsdWdpbi5weQ==) | `92.81% <80.76%> (-1.27%)` | :arrow_down: |
| [src/\_repobee/main.py](https://codecov.io/gh/repobee/repobee/pull/456/diff?src=pr&el=tree#diff-c3JjL19yZXBvYmVlL21haW4ucHk=) | `91.48% <100.00%> (+3.73%)` | :arrow_up: |
| [src/\_repobee/ext/gitlab.py](https://codecov.io/gh/repobee/repobee/pull/456/diff?src=pr&el=tree#diff-c3JjL19yZXBvYmVlL2V4dC9naXRsYWIucHk=) | `63.86% <0.00%> (-28.56%)` | :arrow_down: |
| [src/\_repobee/git.py](https://codecov.io/gh/repobee/repobee/pull/456/diff?src=pr&el=tree#diff-c3JjL19yZXBvYmVlL2dpdC5weQ==) | `96.62% <0.00%> (-3.38%)` | :arrow_down: |
| [src/\_repobee/command/repos.py](https://codecov.io/gh/repobee/repobee/pull/456/diff?src=pr&el=tree#diff-c3JjL19yZXBvYmVlL2NvbW1hbmQvcmVwb3MucHk=) | `92.62% <0.00%> (-2.46%)` | :arrow_down: |
| [src/\_repobee/cli/mainparser.py](https://codecov.io/gh/repobee/repobee/pull/456/diff?src=pr&el=tree#diff-c3JjL19yZXBvYmVlL2NsaS9tYWlucGFyc2VyLnB5) | `94.64% <0.00%> (-1.79%)` | :arrow_down: |
| [src/\_repobee/ext/query.py](https://codecov.io/gh/repobee/repobee/pull/456/diff?src=pr&el=tree#diff-c3JjL19yZXBvYmVlL2V4dC9xdWVyeS5weQ==) | `48.48% <0.00%> (-1.52%)` | :arrow_down: |
| [src/\_repobee/cli/parsing.py](https://codecov.io/gh/repobee/repobee/pull/456/diff?src=pr&el=tree#diff-c3JjL19yZXBvYmVlL2NsaS9wYXJzaW5nLnB5) | `92.26% <0.00%> (-1.20%)` | :arrow_down: |
| [src/\_repobee/ext/defaults/github.py](https://codecov.io/gh/repobee/repobee/pull/456/diff?src=pr&el=tree#diff-c3JjL19yZXBvYmVlL2V4dC9kZWZhdWx0cy9naXRodWIucHk=) | `90.29% <0.00%> (-0.11%)` | :arrow_down: |
| [src/repobee\_plug/\_corehooks.py](https://codecov.io/gh/repobee/repobee/pull/456/diff?src=pr&el=tree#diff-c3JjL3JlcG9iZWVfcGx1Zy9fY29yZWhvb2tzLnB5) | `100.00% <0.00%> (ø)` | |
| ... and [5 more](https://codecov.io/gh/repobee/repobee/pull/456/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/repobee/repobee/pull/456?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/repobee/repobee/pull/456?src=pr&el=footer). Last update [288cd97...54e7915](https://codecov.io/gh/repobee/repobee/pull/456?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/src/_repobee/config.py b/src/_repobee/config.py
index 1a1b194..59aabe2 100644
--- a/src/_repobee/config.py
+++ b/src/_repobee/config.py
@@ -11,7 +11,7 @@ Contains the code required for pre-configuring user interfaces.
 import os
 import pathlib
 import configparser
-from typing import Union, List, Mapping, Optional
+from typing import Union, List, Mapping
 
 import daiquiri
 import repobee_plug as plug
@@ -137,21 +137,8 @@ def get_all_tasks() -> List[plug.Task]:
     return plug.manager.hook.setup_task() + plug.manager.hook.clone_task()
 
 
-def _fetch_token() -> Optional[str]:
-    token = os.getenv(constants.TOKEN_ENV)
-    token_from_old = os.getenv(constants.TOKEN_ENV_OLD)
-    if token_from_old:
-        LOGGER.warning(
-            "The {} environment variable has been deprecated, "
-            "use {} instead".format(
-                constants.TOKEN_ENV_OLD, constants.TOKEN_ENV
-            )
-        )
-    return token or token_from_old
-
-
 def _read_defaults(config_file: pathlib.Path) -> dict:
-    token = _fetch_token()
+    token = os.getenv(constants.TOKEN_ENV)
     if not config_file.is_file():
         return {} if not token else dict(token=token)
     defaults = dict(_read_config(config_file)[constants.DEFAULTS_SECTION_HDR])
diff --git a/src/_repobee/constants.py b/src/_repobee/constants.py
index 5a55a27..a174027 100644
--- a/src/_repobee/constants.py
+++ b/src/_repobee/constants.py
@@ -39,4 +39,3 @@ ORDERED_CONFIGURABLE_ARGS = (
 CONFIGURABLE_ARGS = set(ORDERED_CONFIGURABLE_ARGS)
 
 TOKEN_ENV = "REPOBEE_TOKEN"
-TOKEN_ENV_OLD = "REPOBEE_OAUTH"
diff --git a/src/_repobee/main.py b/src/_repobee/main.py
index 8b994a6..b51af81 100644
--- a/src/_repobee/main.py
+++ b/src/_repobee/main.py
@@ -57,7 +57,7 @@ def main(sys_args: List[str]):
                 parsed_preparser_args.plug
                 or config.get_plugin_names(config_file)
             ) or []
-            plugin.initialize_plugins(plugin_names)
+            plugin.initialize_plugins(plugin_names, allow_filepath=True)
 
         config.execute_config_hooks(config_file)
         ext_commands = plug.manager.hook.create_extension_command()
diff --git a/src/_repobee/plugin.py b/src/_repobee/plugin.py
index 2fff203..cc3f7ca 100644
--- a/src/_repobee/plugin.py
+++ b/src/_repobee/plugin.py
@@ -14,6 +14,9 @@ import tempfile
 import pkgutil
 import pathlib
 import importlib
+import hashlib
+import os
+import sys
 from types import ModuleType
 from typing import List, Optional, Iterable, Mapping, Union
 
@@ -39,7 +42,9 @@ def _external_plugin_qualname(plugin_name):
 
 
 def load_plugin_modules(
-    plugin_names: Iterable[str], allow_qualified: bool = False
+    plugin_names: Iterable[str],
+    allow_qualified: bool = False,
+    allow_filepath: bool = False,
 ) -> List[ModuleType]:
     """Load the given plugins. Plugins are loaded such that they are executed
     in the same order that they are specified in the plugin_names list.
@@ -56,18 +61,16 @@ def load_plugin_modules(
         # import nr 2
         from repobee_javac import javac
 
-    If ``allow_qualified`` is True, an additional import using the provided
-    plugin names as-is is also attempted.
-
-    .. code-block:: python
-
         # import nr 3 (only if allow_qualified)
         import javac
 
+        # import nr 4 (only if allow_filepath)
+        # Dynamically import using the name as a filepath
+
     Args:
         plugin_names: A list of plugin names.
-        allow_qualified: If True, attempts to import modules using the plugin
-            names as qualified paths.
+        allow_qualified: Allow the plugin to be specified by a qualified name.
+        allow_filepath: Allows the plugin to be specified as a filepath.
     Returns:
         a list of loaded modules.
     """
@@ -79,6 +82,7 @@ def load_plugin_modules(
             _try_load_module(_plugin_qualname(name))
             or _try_load_module(_external_plugin_qualname(name))
             or (allow_qualified and _try_load_module(name))
+            or (allow_filepath and _try_load_module_from_filepath(name))
         )
         if not plug_mod:
             msg = "failed to load plugin module " + name
@@ -87,6 +91,30 @@ def load_plugin_modules(
     return loaded_modules
 
 
+def _try_load_module_from_filepath(path: str) -> Optional[ModuleType]:
+    """Try to load a module from the specified filepath.
+
+    Adapted from code by Sebastian Rittau (https://stackoverflow.com/a/67692).
+
+    Args:
+        path: A path to a Python module.
+    Returns:
+        The module if loaded successfully, or None if there was no module at
+        the path.
+    """
+    package_name = f"_{hashlib.sha1(path.encode(sys.getdefaultencoding()))}"
+    module_name = pathlib.Path(path).stem
+    qualname = f"{package_name}.{module_name}"
+    spec = importlib.util.spec_from_file_location(qualname, path)
+    if not spec:
+        return None
+
+    mod = importlib.util.module_from_spec(spec)
+    spec.loader.exec_module(mod)
+
+    return mod
+
+
 def _try_load_module(qualname: str) -> Optional[ModuleType]:
     """Try to load a module.
 
@@ -208,7 +236,9 @@ def try_register_plugin(
 
 
 def initialize_plugins(
-    plugin_names: List[str] = None, allow_qualified: bool = False,
+    plugin_names: List[str] = None,
+    allow_qualified: bool = False,
+    allow_filepath: bool = False,
 ) -> List[Union[ModuleType, type]]:
     """Load and register plugins.
 
@@ -216,18 +246,23 @@ def initialize_plugins(
         plugin_names: An optional list of plugin names that overrides the
             configuration file's plugins.
         allow_qualified: Allows the plugin names to be qualified.
+        allow_filepath: Allows the plugin to be specified as a filepath.
     Returns:
         A list of registered modules and classes.
     Raises:
         :py:class:`_repobee.exception.PluginLoadError`
     """
+    if not allow_filepath:
+        _check_no_filepaths(plugin_names)
     if not allow_qualified:
         _check_no_qualified_names(plugin_names)
 
     registered_plugins = plug.manager.get_plugins()
     plug_modules = [
         p
-        for p in load_plugin_modules(plugin_names, allow_qualified)
+        for p in load_plugin_modules(
+            plugin_names, allow_qualified, allow_filepath
+        )
         if p not in registered_plugins
     ]
     registered = register_plugins(plug_modules)
@@ -235,8 +270,20 @@ def initialize_plugins(
     return registered
 
 
+def _is_filepath(name: str) -> bool:
+    return os.pathsep in name or os.path.exists(name)
+
+
+def _check_no_filepaths(names: List[str]):
+    filepaths = [name for name in names if _is_filepath(name)]
+    if filepaths:
+        raise exception.PluginLoadError(f"Filepaths not allowed: {filepaths}")
+
+
 def _check_no_qualified_names(names: List[str]):
-    qualified_names = [name for name in names if "." in name]
+    qualified_names = [
+        name for name in names if "." in name and not _is_filepath(name)
+    ]
     if qualified_names:
         raise exception.PluginLoadError(
             f"Qualified names not allowed: {qualified_names}"
@@ -244,9 +291,18 @@ def _check_no_qualified_names(names: List[str]):
 
 
 def resolve_plugin_version(plugin_module: ModuleType) -> Optional[str]:
-    """Return the version of the top-level package containing the plugin, or
-    None if it is not defined.
+    """Return the version of this plugin. Tries to resolve the version by
+    first checking if the plugin module itself has a ``__version__``
+    attribute, and then the top level package.
+
+    Args:
+        plugin_module: A plugin module.
+    Returns:
+        The version if found, otherwise None.
     """
+    if hasattr(plugin_module, "__version__"):
+        return plugin_module.__version__
+
     pkg_name = plugin_module.__package__.split(".")[0]
     pkg_module = _try_load_module(pkg_name)
     return (


Test patch:
diff --git a/tests/unit_tests/repobee/conftest.py b/tests/unit_tests/repobee/conftest.py
index 8d004ed..5b8d481 100644
--- a/tests/unit_tests/repobee/conftest.py
+++ b/tests/unit_tests/repobee/conftest.py
@@ -21,7 +21,6 @@ import _repobee  # noqa: E402
 
 EXPECTED_ENV_VARIABLES = [
     _repobee.constants.TOKEN_ENV,
-    _repobee.constants.TOKEN_ENV_OLD,
     "REPOBEE_NO_VERIFY_SSL",
 ]
 
diff --git a/tests/unit_tests/repobee/test_config.py b/tests/unit_tests/repobee/test_config.py
index 7466b82..7e88fa5 100644
--- a/tests/unit_tests/repobee/test_config.py
+++ b/tests/unit_tests/repobee/test_config.py
@@ -55,23 +55,6 @@ class TestGetConfiguredDefaults:
         defaults = config.get_configured_defaults(str(config_mock))
         assert defaults["token"] == constants.TOKEN
 
-    @pytest.mark.skipif(
-        "_repobee.__version__ >= '3.0.0'",
-        msg="Old token should have been removed",
-    )
-    def test_deprecated_token_env(self, config_mock, mock_getenv):
-        token = "superdupertoken"
-
-        def _env(name):
-            if name == _repobee.constants.TOKEN_ENV_OLD:
-                return token
-            return None
-
-        mock_getenv.side_effect = _env
-
-        defaults = config.get_configured_defaults(str(config_mock))
-        assert defaults["token"] == token
-
     def test_get_configured_defaults_raises_on_invalid_keys(
         self, empty_config_mock, students_file
     ):
diff --git a/tests/unit_tests/repobee/test_main.py b/tests/unit_tests/repobee/test_main.py
index 5056633..fcbabc0 100644
--- a/tests/unit_tests/repobee/test_main.py
+++ b/tests/unit_tests/repobee/test_main.py
@@ -57,7 +57,7 @@ def api_instance_mock(mocker):
 
 @pytest.fixture
 def init_plugins_mock(mocker):
-    def init_plugins(plugs=None, allow_qualified=False):
+    def init_plugins(plugs=None, allow_qualified=False, allow_filepath=False):
         list(map(module, plugs or []))
 
     return mocker.patch(
@@ -159,7 +159,7 @@ def test_plugins_args(
 
     init_plugins_mock.assert_has_calls(
         [
-            call(["javac", "pylint"]),
+            call(["javac", "pylint"], allow_filepath=True),
             call(DEFAULT_PLUGIN_NAMES, allow_qualified=True),
         ],
         any_order=True,
@@ -223,7 +223,7 @@ def test_configured_plugins_are_loaded(
 
     init_plugins_mock.assert_has_calls(
         [
-            call(["javac", "pylint"]),
+            call(["javac", "pylint"], allow_filepath=True),
             call(DEFAULT_PLUGIN_NAMES, allow_qualified=True),
         ],
         any_order=True,
@@ -245,7 +245,7 @@ def test_plugin_with_subparser_name(
 
     init_plugins_mock.assert_has_calls(
         [
-            call(["javac", "clone"]),
+            call(["javac", "clone"], allow_filepath=True),
             call(DEFAULT_PLUGIN_NAMES, allow_qualified=True),
         ],
         any_order=True,
@@ -287,7 +287,10 @@ def test_invalid_plug_options(dispatch_command_mock, init_plugins_mock):
         main.main(sys_args)
 
     init_plugins_mock.assert_has_calls(
-        [call(["javac"]), call(DEFAULT_PLUGIN_NAMES, allow_qualified=True)],
+        [
+            call(["javac"], allow_filepath=True),
+            call(DEFAULT_PLUGIN_NAMES, allow_qualified=True),
+        ],
         any_order=True,
     )
     assert not dispatch_command_mock.called
diff --git a/tests/unit_tests/repobee/test_plugin.py b/tests/unit_tests/repobee/test_plugin.py
index 3a2f2ba..9656ae2 100644
--- a/tests/unit_tests/repobee/test_plugin.py
+++ b/tests/unit_tests/repobee/test_plugin.py
@@ -6,6 +6,7 @@
         installed any other plugins, tests in here may fail unexpectedly
         without anything actually being wrong.
 """
+import shutil
 import pathlib
 import tempfile
 import types
@@ -246,3 +247,39 @@ class TestInitializePlugins:
             plugin.initialize_plugins([qualname])
 
         assert "Qualified names not allowed" in str(exc_info.value)
+
+    def test_raises_on_filepath_by_default(self, tmpdir):
+        plugin_file = pathlib.Path(str(tmpdir)) / "pylint.py"
+        shutil.copy(_repobee.ext.javac.__file__, str(plugin_file))
+
+        with pytest.raises(exception.PluginLoadError) as exc_info:
+            plugin.initialize_plugins([str(plugin_file)])
+
+        assert "Filepaths not allowed" in str(exc_info.value)
+
+    def test_initialize_from_filepath_filepath(self, tmpdir):
+        """Test initializing a plugin that's specified by a filepath."""
+        plugin_file = pathlib.Path(str(tmpdir)) / "pylint.py"
+        shutil.copy(_repobee.ext.pylint.__file__, str(plugin_file))
+
+        initialized_plugins = plugin.initialize_plugins(
+            [str(plugin_file)], allow_filepath=True
+        )
+
+        assert len(initialized_plugins) == 1
+        assert initialized_plugins[0].__file__ == str(plugin_file)
+
+    def test_raises_when_filepath_is_not_python_module(self, tmpdir):
+        not_a_python_module = pathlib.Path(str(tmpdir)) / "some_file.txt"
+        not_a_python_module.write_text(
+            "This is definitely\nnot a Python module"
+        )
+
+        with pytest.raises(exception.PluginLoadError) as exc_info:
+            plugin.initialize_plugins(
+                [str(not_a_python_module)], allow_filepath=True
+            )
+
+        assert f"failed to load plugin module {not_a_python_module}" in str(
+            exc_info.value
+        )