Fix so trinity respects --trinity-root-dir setting (#1268)

* Fix so trinity respects --trinity-root-dir setting

* cleanup path utils

* docstring cleanup

Author: pipermerriam

tests/trinity/conftest.py

@@ -39,7 +39,7 @@ def xdg_trinity_root(monkeypatch, tmpdir):
 
     assert not is_under_path(os.path.expandvars('$HOME'), get_xdg_trinity_root())
 
-    return str(dir_path)
+    return Path(str(dir_path))
 
 
 @pytest.fixture(scope='session')

tests/trinity/core/chains-utils/test_chain_config_object.py

@@ -1,3 +1,5 @@
+from pathlib import Path
+
 import pytest
 
 from eth_utils import (
@@ -7,6 +9,7 @@
 from eth_keys import keys
 
 from trinity.utils.chains import (
+    get_data_dir_for_network_id,
     get_local_data_dir,
     get_nodekey_path,
 )
@@ -15,30 +18,42 @@
     DATABASE_DIR_NAME,
 )
 from trinity.utils.filesystem import (
-    is_same_path,
+    is_under_path,
 )
 
 
-def test_chain_config_computed_properties():
-    data_dir = get_local_data_dir('muffin')
-    chain_config = ChainConfig(network_id=1234, max_peers=1, data_dir=data_dir)
+def test_chain_config_computed_properties(xdg_trinity_root):
+    data_dir = get_local_data_dir('muffin', xdg_trinity_root)
+    chain_config = ChainConfig(network_id=1234, data_dir=data_dir)
 
     assert chain_config.network_id == 1234
     assert chain_config.data_dir == data_dir
     assert chain_config.database_dir == data_dir / DATABASE_DIR_NAME / "full"
     assert chain_config.nodekey_path == get_nodekey_path(data_dir)
 
 
+def test_chain_config_computed_properties_custom_xdg(tmpdir, xdg_trinity_root):
+    alt_xdg_root = tmpdir.mkdir('trinity-custom')
+    assert not is_under_path(alt_xdg_root, xdg_trinity_root)
+
+    data_dir = get_data_dir_for_network_id(1, alt_xdg_root)
+    chain_config = ChainConfig(trinity_root_dir=alt_xdg_root, network_id=1)
+
+    assert chain_config.network_id == 1
+    assert chain_config.data_dir == data_dir
+    assert chain_config.database_dir == data_dir / DATABASE_DIR_NAME / "full"
+    assert chain_config.nodekey_path == get_nodekey_path(data_dir)
+
+
 def test_chain_config_explicit_properties():
     chain_config = ChainConfig(
         network_id=1,
-        max_peers=1,
         data_dir='./data-dir',
         nodekey_path='./nodekey'
     )
 
-    assert is_same_path(chain_config.data_dir, './data-dir')
-    assert is_same_path(chain_config.nodekey_path, './nodekey')
+    assert chain_config.data_dir == Path('./data-dir').resolve()
+    assert chain_config.nodekey_path == Path('./nodekey').resolve()
 
 
 NODEKEY = '0xd18445cc77139cd8e09110e99c9384f0601bd2dfa5b230cda917df7e56b69949'
@@ -61,7 +76,6 @@ def nodekey_path(tmpdir, nodekey_bytes):
 def test_chain_config_nodekey_loading(nodekey_bytes, nodekey_path):
     chain_config = ChainConfig(
         network_id=1,
-        max_peers=1,
         nodekey_path=nodekey_path,
     )
 
@@ -72,7 +86,6 @@ def test_chain_config_nodekey_loading(nodekey_bytes, nodekey_path):
 def test_chain_config_explictely_provided_nodekey(nodekey_bytes, as_bytes):
     chain_config = ChainConfig(
         network_id=1,
-        max_peers=1,
         nodekey=nodekey_bytes if as_bytes else keys.PrivateKey(nodekey_bytes),
     )
 

tests/trinity/core/filesystem-utils/test_is_same_path.py

@@ -26,26 +26,3 @@
 def test_is_under_path(base_path, path, expected):
     actual = is_under_path(base_path, path)
     assert actual is expected
-
-
-@pytest.mark.parametrize(
-    'base_path,path,expected',
-    (
-        # Same Path
-        ('foo', 'foo', True),
-        ('foo', 'foo/bar/..', True),
-        # up a directory (or two)
-        ('foo', '..', False),
-        ('foo', 'foo/bar/../../', False),
-        # relative and abs
-        ('foo', '/foo/bar', False),
-        ('foo', '/foo', False),
-        # actually nested
-        ('foo', 'foo/bar.sol', True),
-        ('foo', 'foo/bar', True),
-        ('foo', 'foo/bar/../../foo/baz', True),
-    ),
-)
-def test_is_under_path_not_strict(base_path, path, expected):
-    actual = is_under_path(base_path, path, strict=False)
-    assert actual is expected

tests/trinity/core/filesystem-utils/test_is_under_path.py

@@ -42,13 +42,14 @@
     AsyncHeaderDB,
     AsyncHeaderDBProxy,
 )
+from trinity.utils.filesystem import (
+    is_under_path,
+)
 from trinity.utils.mp import (
     async_method,
     sync_method,
 )
-from trinity.utils.xdg import (
-    is_under_xdg_trinity_root,
-)
+
 
 from .header import (
     AsyncHeaderChain,
@@ -97,7 +98,11 @@ def is_database_initialized(chaindb: AsyncChainDB) -> bool:
 
 
 def initialize_data_dir(chain_config: ChainConfig) -> None:
-    if not chain_config.data_dir.exists() and is_under_xdg_trinity_root(chain_config.data_dir):
+    should_create_data_dir = (
+        not chain_config.data_dir.exists() and
+        is_under_path(chain_config.trinity_root_dir, chain_config.data_dir)
+    )
+    if should_create_data_dir:
         chain_config.data_dir.mkdir(parents=True, exist_ok=True)
     elif not chain_config.data_dir.exists():
         # we don't lazily create the base dir for non-default base directories.
@@ -109,9 +114,11 @@ def initialize_data_dir(chain_config: ChainConfig) -> None:
         )
 
     # Logfile
-    if (not chain_config.logdir_path.exists() and
-            is_under_xdg_trinity_root(chain_config.logdir_path)):
-
+    should_create_logdir = (
+        not chain_config.logdir_path.exists() and
+        is_under_path(chain_config.trinity_root_dir, chain_config.logdir_path)
+    )
+    if should_create_logdir:
         chain_config.logdir_path.mkdir(parents=True, exist_ok=True)
         chain_config.logfile_path.touch()
     elif not chain_config.logdir_path.exists():

trinity/chains/__init__.py

@@ -39,6 +39,10 @@
 from trinity.utils.filesystem import (
     PidFile,
 )
+from trinity.utils.xdg import (
+    get_xdg_trinity_root,
+)
+
 
 if TYPE_CHECKING:
     # avoid circular import
@@ -48,6 +52,7 @@
 
 
 class ChainConfig:
+    _trinity_root_dir: Path = None
     _data_dir: Path = None
     _nodekey_path: Path = None
     _logfile_path: Path = None
@@ -61,7 +66,8 @@ class ChainConfig:
 
     def __init__(self,
                  network_id: int,
-                 max_peers: int,
+                 max_peers: int=25,
+                 trinity_root_dir: str=None,
                  data_dir: str=None,
                  nodekey_path: str=None,
                  logfile_path: str=None,
@@ -75,6 +81,9 @@ def __init__(self,
         self.sync_mode = sync_mode
         self.port = port
 
+        if trinity_root_dir is not None:
+            self.trinity_root_dir = trinity_root_dir
+
         if not preferred_nodes and network_id in DEFAULT_PREFERRED_NODES:
             self.preferred_nodes = DEFAULT_PREFERRED_NODES[self.network_id]
         else:
@@ -92,32 +101,28 @@ def __init__(self,
         else:
             self.bootstrap_nodes = bootstrap_nodes
 
-        # validation
-        if nodekey is not None and nodekey_path is not None:
-            raise ValueError("It is invalid to provide both a `nodekey` and a `nodekey_path`")
-
-        # set values
         if data_dir is not None:
             self.data_dir = data_dir
-        else:
-            self.data_dir = get_data_dir_for_network_id(self.network_id)
 
-        if nodekey_path is not None:
+        if nodekey is not None and nodekey_path is not None:
+            raise ValueError("It is invalid to provide both a `nodekey` and a `nodekey_path`")
+        elif nodekey_path is not None:
             self.nodekey_path = nodekey_path
         elif nodekey is not None:
             self.nodekey = nodekey
 
         if logfile_path is not None:
             self.logfile_path = logfile_path
-        else:
-            self.logfile_path = get_logfile_path(self.data_dir)
 
     @property
     def logfile_path(self) -> Path:
         """
         Return the path to the log file.
         """
-        return self._logfile_path
+        if self._logfile_path is not None:
+            return self._logfile_path
+        else:
+            return get_logfile_path(self.data_dir)
 
     @logfile_path.setter
     def logfile_path(self, value: Path) -> None:
@@ -130,21 +135,49 @@ def logdir_path(self) -> Path:
         """
         return self.logfile_path.parent
 
+    @property
+    def trinity_root_dir(self) -> Path:
+        """
+        The trinity_root_dir is the base directory that all trinity data is
+        stored under.
+
+        The default ``data_dir`` path will be resolved relative to this
+        directory.
+        """
+        if self._trinity_root_dir is not None:
+            return self._trinity_root_dir
+        else:
+            return get_xdg_trinity_root()
+
+    @trinity_root_dir.setter
+    def trinity_root_dir(self, value: str) -> None:
+        self._trinity_root_dir = Path(value).resolve()
+
     @property
     def data_dir(self) -> Path:
         """
         The data_dir is the base directory that all chain specific information
-        for a given chain is stored.  All other chain directories are by
-        default relative to this directory.
+        for a given chain is stored.
+
+        All defaults for chain directories are resolved relative to this
+        directory.
         """
-        return self._data_dir
+        if self._data_dir is not None:
+            return self._data_dir
+        else:
+            return get_data_dir_for_network_id(self.network_id, self.trinity_root_dir)
 
     @data_dir.setter
     def data_dir(self, value: str) -> None:
         self._data_dir = Path(value).resolve()
 
     @property
     def database_dir(self) -> Path:
+        """
+        Path where the chain database will be stored.
+
+        This is resolved relative to the ``data_dir``
+        """
         if self.sync_mode == SYNC_FULL:
             return self.data_dir / DATABASE_DIR_NAME / "full"
         elif self.sync_mode == SYNC_LIGHT:
@@ -154,14 +187,23 @@ def database_dir(self) -> Path:
 
     @property
     def database_ipc_path(self) -> Path:
+        """
+        Path for the database IPC socket connection.
+        """
         return get_database_socket_path(self.data_dir)
 
     @property
     def jsonrpc_ipc_path(self) -> Path:
+        """
+        Path for the JSON-RPC server IPC socket.
+        """
         return get_jsonrpc_socket_path(self.data_dir)
 
     @property
     def nodekey_path(self) -> Path:
+        """
+        Path where the nodekey is stored
+        """
         if self._nodekey_path is None:
             if self._nodekey is not None:
                 return None
@@ -203,11 +245,18 @@ def nodekey(self, value: Union[bytes, PrivateKey]) -> None:
 
     @classmethod
     def from_parser_args(cls, parser_args: argparse.Namespace) -> 'ChainConfig':
+        """
+        Helper function for initializing from the namespace object produced by
+        an ``argparse.ArgumentParser``
+        """
         constructor_kwargs = construct_chain_config_params(parser_args)
         return cls(**constructor_kwargs)
 
     @property
     def node_class(self) -> Type['Node']:
+        """
+        The ``Node`` class that trinity will use.
+        """
         from trinity.nodes.mainnet import (
             MainnetFullNode,
             MainnetLightNode,