implement --peru-file, --sync-dir, etc.

Summary:
We've been meaning to do this for a while, see
#108. Here's the full set:
PERU_FILE      --peru-file
PERU_SYNC_DIR  --sync-dir
PERU_STATE_DIR --state-dir
PERU_CACHE_DIR --cache-dir

Note that --peru-file and --sync-dir need to be set together, or not at
all; you can't just specify one of them. That's because if you were to
set the one that's normally derived from the other (which we could
imagine defining in either direction), you end up with weird
consequences for the first one that's still derived from the cwd.

Test Plan:
Several new integration tests. In the process, I found out that we had
a few cases where we passed a string instead of a tuple to test
helper methods. Added a new assertion to catch those.

Reviewers: sean

Differential Revision: https://phabricator.buildinspace.com/D210

peru/imports.py

@@ -42,7 +42,7 @@ def get_tree(runtime, scope, target_str):
 
 
 def _last_imports_path(runtime):
-    return os.path.join(runtime.peru_dir, 'lastimports')
+    return os.path.join(runtime.state_dir, 'lastimports')
 
 
 def _get_last_imports(runtime):

peru/main.py

@@ -18,7 +18,8 @@
 
 __doc__ = '''\
 Usage:
-    peru [-hqv] <command> [<args>...]
+    peru [-hqv] [--peru-file=PATH] [--sync-dir=PATH] [--state-dir=PATH]
+         [--cache-dir=PATH] <command> [<args>...]
     peru [--help|--version]
 
 Commands:
@@ -27,12 +28,19 @@
     clean     delete imports from your project
     copy      copy files directly from a module to somewhere else
     override  substitute a local directory for the contents of a module
-    help      show help for subcommands, same as --help
+    help      show help for subcommands, same as -h/--help
 
 Options:
-    -h --help     so much help
-    -q --quiet    don't print anything
-    -v --verbose  print everything
+    --cache-dir=PATH  custom location for peru's cache, normally
+                      .peru/cache
+    -h --help         so much help
+    --peru-file=PATH  explicit path to peru.yaml
+    -q --quiet        don't print anything
+    --state-dir=PATH  custom location for various peru metadata, which
+                      normally lives in .peru
+    --sync-dir=PATH   explicit path to where your imports go, normally
+                      the directory containing peru.yaml
+    -v --verbose      print everything
 '''
 
 
@@ -67,7 +75,7 @@ def decorator(f):
 ''')
 def do_sync(params):
     yield from imports.checkout(
-        params.runtime, params.scope, params.imports, params.runtime.root)
+        params.runtime, params.scope, params.imports, params.runtime.sync_dir)
 
 
 @peru_command('reup', '''\
@@ -118,7 +126,7 @@ def do_reup(params):
 ''')
 def do_clean(params):
     yield from imports.checkout(
-        params.runtime, params.scope, {}, params.runtime.root)
+        params.runtime, params.scope, {}, params.runtime.sync_dir)
 
 
 @peru_command('copy', '''\

peru/runtime.py

@@ -14,23 +14,15 @@
 
 class Runtime:
     def __init__(self, args, env):
-        peru_file_name = env.get('PERU_FILE_NAME',
-                                 parser.DEFAULT_PERU_FILE_NAME)
-        self.peru_file = find_peru_file(os.getcwd(), peru_file_name)
+        self._set_paths(args, env)
 
-        self.root = os.path.dirname(self.peru_file)
+        compat.makedirs(self.state_dir)
+        self.cache = cache.Cache(self.cache_dir)
 
-        self.peru_dir = env.get(
-            'PERU_DIR', os.path.join(self.root, '.peru'))
-        compat.makedirs(self.peru_dir)
-
-        cache_dir = env.get('PERU_CACHE', os.path.join(self.peru_dir, 'cache'))
-        self.cache = cache.Cache(cache_dir)
-
-        self._tmp_root = os.path.join(self.peru_dir, 'tmp')
+        self._tmp_root = os.path.join(self.state_dir, 'tmp')
         compat.makedirs(self._tmp_root)
 
-        self.overrides = KeyVal(os.path.join(self.peru_dir, 'overrides'),
+        self.overrides = KeyVal(os.path.join(self.state_dir, 'overrides'),
                                 self._tmp_root)
 
         self.force = args.get('--force', False)
@@ -54,6 +46,26 @@ def __init__(self, args, env):
 
         self.display = get_display(args)
 
+    def _set_paths(self, args, env):
+        getter = ArgsEnvGetter(args, env)
+        explicit_peru_file = getter.get('--peru-file', 'PERU_FILE')
+        explicit_sync_dir = getter.get('--sync-dir', 'PERU_SYNC_DIR')
+        if (explicit_peru_file is None) != (explicit_sync_dir is None):
+            raise PrintableError(
+                'If the peru file or the sync dir is set, the other must also '
+                'be set.')
+        if explicit_peru_file:
+            self.peru_file = explicit_peru_file
+            self.sync_dir = explicit_sync_dir
+        else:
+            self.peru_file = find_peru_file(
+                os.getcwd(), parser.DEFAULT_PERU_FILE_NAME)
+            self.sync_dir = os.path.dirname(self.peru_file)
+        self.state_dir = (getter.get('--state-dir', 'PERU_STATE_DIR') or
+                          os.path.join(self.sync_dir, '.peru'))
+        self.cache_dir = (getter.get('--cache-dir', 'PERU_CACHE_DIR') or
+                          os.path.join(self.state_dir, 'cache'))
+
     def tmp_dir(self):
         dir = tempfile.TemporaryDirectory(dir=self._tmp_root)
         return dir
@@ -66,7 +78,7 @@ def set_override(self, name, path):
             # to be relative (for example, so a whole workspace can be moved as
             # a group while preserving all the overrides). So reinterpret all
             # relative paths from the project root.
-            path = os.path.relpath(path, start=self.root)
+            path = os.path.relpath(path, start=self.sync_dir)
         self.overrides[name] = path
 
     def get_override(self, name):
@@ -77,12 +89,16 @@ def get_override(self, name):
             # Relative paths are stored relative to the project root.
             # Reinterpret them relative to the cwd. See the above comment in
             # set_override.
-            path = os.path.relpath(os.path.join(self.root, path))
+            path = os.path.relpath(os.path.join(self.sync_dir, path))
         return path
 
     def get_plugin_context(self):
         return plugin.PluginContext(
-            cwd=self.root,
+            # Plugin cwd is always the directory containing peru.yaml, even if
+            # the sync_dir has been explicitly set elsewhere. That's because
+            # relative paths in peru.yaml should respect the location of that
+            # file.
+            cwd=os.path.dirname(self.peru_file),
             plugin_cache_root=self.cache.plugins_root,
             parallelism_semaphore=self.fetch_semaphore,
             plugin_cache_locks=self.plugin_cache_locks,
@@ -128,3 +144,16 @@ def get_display(args):
         return display.FancyDisplay()
     else:
         return display.QuietDisplay()
+
+
+class ArgsEnvGetter:
+    def __init__(self, args, env):
+        self.args = args
+        self.env = env
+
+    def get(self, flag_name, env_name):
+        if self.args.get(flag_name):
+            return self.args.get(flag_name)
+        if self.env.get(env_name):
+            return self.env.get(env_name)
+        return None

tests/shared.py

@@ -49,6 +49,8 @@ def write_files(dir, path_contents_map):
 
 
 def read_dir(startdir, excludes=()):
+    assert isinstance(excludes, list) or isinstance(excludes, tuple), \
+            "excludes must be a list or a tuple, not " + repr(type(excludes))
     startdir = Path(startdir)
     contents = {}
     for p in startdir.glob('**/*'):
@@ -111,18 +113,18 @@ def assert_clean_tmp(peru_dir):
         assert not tmpfiles, 'cache tmp dir is not clean: ' + str(tmpfiles)
 
 
-def run_peru_command(args, test_dir, *, env_vars=None):
+def run_peru_command(args, cwd, *, env=None):
     old_cwd = os.getcwd()
     old_stdout = sys.stdout
-    os.chdir(test_dir)
+    os.chdir(cwd)
     capture_stream = io.StringIO()
     sys.stdout = capture_stream
     try:
         # Rather than invoking peru as a subprocess, just call directly into
         # the Main class. This lets us check that the right types of exceptions
         # make it up to the top, so we don't need to check specific output
         # strings.
-        peru.main.main(argv=args, env=env_vars or {}, nocatch=True)
+        peru.main.main(argv=args, env=env or {}, nocatch=True)
     finally:
         os.chdir(old_cwd)
         sys.stdout = old_stdout

tests/test_paths.py

@@ -0,0 +1,100 @@
+import os
+import textwrap
+import unittest
+
+from peru.compat import makedirs
+from peru.error import PrintableError
+
+import shared
+
+
+class PathsTest(unittest.TestCase):
+
+    def setUp(self):
+        self.test_root = shared.create_dir()
+
+        self.project_dir = os.path.join(self.test_root, 'project')
+        self.yaml = textwrap.dedent('''\
+            imports:
+                foo: ./
+            cp module foo:
+                # relative paths should always be interpreted from the dir
+                # containing peru.yaml, even if that's not the sync dir.
+                path: ../foo
+            ''')
+        shared.write_files(self.project_dir, {'peru.yaml': self.yaml})
+        self.peru_file = os.path.join(self.project_dir, 'peru.yaml')
+
+        self.foo_dir = os.path.join(self.test_root, 'foo')
+        shared.write_files(self.foo_dir, {'bar': 'baz'})
+
+        # We'll run tests from this inner subdirectory, so that we're more
+        # likely to catch places where we're using the cwd incorrectly.
+        self.cwd = os.path.join(self.project_dir, 'cwd', 'in', 'here')
+        makedirs(self.cwd)
+
+    def assert_success(self, sync_dir, state_dir, cache_dir):
+        shared.assert_contents(sync_dir, {'bar': 'baz'},
+                               excludes=['.peru', 'peru.yaml'])
+        assert os.path.isfile(os.path.join(state_dir, 'lastimports'))
+        assert os.path.isdir(os.path.join(cache_dir, 'trees'))
+
+    def test_unmodified_sync(self):
+        shared.run_peru_command(['sync'], self.cwd)
+        self.assert_success(self.project_dir,
+                            os.path.join(self.project_dir, '.peru'),
+                            os.path.join(self.project_dir, '.peru', 'cache'))
+
+    def test_peru_file_and_sync_dir_must_be_set_together(self):
+        for command, env in [(['--sync-dir=junk', 'sync'], {}),
+                             (['sync'], {'PERU_SYNC_DIR': 'junk'}),
+                             (['--peru-file=junk', 'sync'], {}),
+                             (['sync'], {'PERU_FILE': 'junk'})]:
+            with self.assertRaises(PrintableError) as e:
+                shared.run_peru_command(command, cwd=self.cwd, env=env)
+                assert 'peru file' in e.message and 'sync dir' in e.message
+
+    def test_setting_all_flags(self):
+        cwd = shared.create_dir()
+        sync_dir = shared.create_dir()
+        state_dir = shared.create_dir()
+        cache_dir = shared.create_dir()
+        shared.run_peru_command(
+            ['--peru-file', self.peru_file, '--sync-dir', sync_dir,
+             '--state-dir', state_dir, '--cache-dir', cache_dir, 'sync'],
+            cwd)
+        self.assert_success(sync_dir, state_dir, cache_dir)
+
+    def test_setting_all_env_vars(self):
+        cwd = shared.create_dir()
+        sync_dir = shared.create_dir()
+        state_dir = shared.create_dir()
+        cache_dir = shared.create_dir()
+        shared.run_peru_command(['sync'], cwd, env={
+            'PERU_FILE': self.peru_file,
+            'PERU_SYNC_DIR': sync_dir,
+            'PERU_STATE_DIR': state_dir,
+            'PERU_CACHE_DIR': cache_dir,
+        })
+        self.assert_success(sync_dir, state_dir, cache_dir)
+
+    def test_flags_override_vars(self):
+        cwd = shared.create_dir()
+        flag_sync_dir = shared.create_dir()
+        flag_state_dir = shared.create_dir()
+        flag_cache_dir = shared.create_dir()
+        env_sync_dir = shared.create_dir()
+        env_state_dir = shared.create_dir()
+        env_cache_dir = shared.create_dir()
+        shared.run_peru_command(
+            ['--peru-file', self.peru_file, '--sync-dir', flag_sync_dir,
+             '--state-dir', flag_state_dir, '--cache-dir', flag_cache_dir,
+             'sync'],
+            cwd,
+            env={
+                'PERU_FILE': self.peru_file,
+                'PERU_SYNC_DIR': env_sync_dir,
+                'PERU_STATE_DIR': env_state_dir,
+                'PERU_CACHE_DIR': env_cache_dir,
+            })
+        self.assert_success(flag_sync_dir, flag_state_dir, flag_cache_dir)

tests/test_sync.py

@@ -360,9 +360,9 @@ def test_alternate_cache(self):
                 foo: subdir
             ''', module_dir)
         cache_dir = shared.create_dir()
-        env_vars = {'PERU_CACHE': cache_dir}
+        env = {'PERU_CACHE_DIR': cache_dir}
         self.do_integration_test(['sync'], {'subdir/foo': 'bar'},
-                                 env_vars=env_vars)
+                                 env=env)
         self.assertTrue(os.path.exists(os.path.join(cache_dir, 'plugins')))
         self.assertTrue(os.path.exists(os.path.join(cache_dir, 'trees')))
         self.assertTrue(os.path.exists(os.path.join(cache_dir, 'keyval')))

tests/test_test_shared.py

@@ -27,9 +27,9 @@ def test_read_dir(self):
         read_content = shared.read_dir(test_dir)
         self.assertDictEqual(content, read_content)
         self.assertDictEqual({Path('foo'): 'a'},
-                             shared.read_dir(test_dir, excludes=('bar')))
+                             shared.read_dir(test_dir, excludes=['bar']))
         self.assertDictEqual({Path('foo'): 'a'},
-                             shared.read_dir(test_dir, excludes=('bar/baz')))
+                             shared.read_dir(test_dir, excludes=['bar/baz']))
 
     def test_assert_contents(self):
         content = {'foo': 'a', 'bar/baz': 'b'}
@@ -38,8 +38,8 @@ def test_assert_contents(self):
         shared.write_files(test_dir, {'bing': 'c'})
         with self.assertRaises(AssertionError):
             shared.assert_contents(test_dir, content)
-        shared.assert_contents(test_dir, content, excludes=('bing',))
+        shared.assert_contents(test_dir, content, excludes=['bing'])
         try:
-            shared.assert_contents(test_dir, content, excludes=('foo'))
+            shared.assert_contents(test_dir, content, excludes=['foo'])
         except AssertionError as e:
             assert e.args[0].startswith('EXPECTED FILES WERE EXCLUDED')