Merge pull request #1505 from pared/1486

move: handle directory as target argument

dvc/project.py

@@ -282,6 +282,8 @@ class Project(object):
         from_out = Output.loads_from(Stage(self, cwd=os.curdir),
                                      [from_path])[0]
 
+        to_path = self._expand_target_path(from_path, to_path)
+
         try:
             stage, out = next((stage, out)
                               for stage in self.stages()
@@ -294,22 +296,26 @@ class Project(object):
         if not stage.is_data_source:
             raise MoveNotDataSourceError(stage.relpath)
 
-        to_out = Output.loads_from(stage, [to_path], out.cache, out.metric)[0]
-
-        with self.state:
-            out.move(to_out)
-
         stage_name = os.path.splitext(os.path.basename(stage.path))[0]
         from_name = os.path.basename(from_out.path)
-
         if stage_name == from_name:
             os.unlink(stage.path)
 
             stage.path = os.path.join(
-                os.path.dirname(to_out.path),
+                os.path.dirname(to_path),
                 os.path.basename(to_path) + Stage.STAGE_FILE_SUFFIX
             )
 
+            stage.cwd = os.path.join(self.root_dir, os.path.dirname(to_path))
+
+        to_out = Output.loads_from(stage,
+                                   [os.path.basename(to_path)],
+                                   out.cache,
+                                   out.metric)[0]
+
+        with self.state:
+            out.move(to_out)
+
         stage.dump()
 
         self._remind_to_git_add()
@@ -1293,3 +1299,8 @@ class Project(object):
                     blue=colorama.Fore.BLUE,
                     nc=colorama.Fore.RESET)
         )
+
+    def _expand_target_path(self, from_path, to_path):
+        if os.path.isdir(to_path) and not os.path.isdir(from_path):
+            return os.path.join(to_path, os.path.basename(from_path))
+        return to_path

tests/test_move.py

@@ -6,6 +6,7 @@ from dvc.stage import Stage
 
 from tests.basic_env import TestDvc
 from tests.test_repro import TestRepro
+from tests.utils import load_stage_file
 
 
 class TestMove(TestDvc):
@@ -86,7 +87,7 @@ class TestMoveFileToDirectory(TestDvc):
         self.assertEqual(ret, 0)
         self.assertTrue(os.path.exists(foo_dvc_file))
 
-        new_foo_path = self.DATA_DIR+"/"+self.FOO
+        new_foo_path = os.path.join(self.DATA_DIR, self.FOO)
         new_foo_dvc_path = new_foo_path + Stage.STAGE_FILE_SUFFIX
         ret = main(["move", self.FOO, new_foo_path])
         self.assertEqual(ret, 0)
@@ -95,3 +96,74 @@ class TestMoveFileToDirectory(TestDvc):
         self.assertFalse(os.path.exists(foo_dvc_file))
         self.assertTrue(os.path.exists(new_foo_path))
         self.assertTrue(os.path.exists(new_foo_dvc_path))
+
+
+class TestMoveFileToDirectoryWithoutSpecifiedTargetName(TestDvc):
+    def test(self):
+        foo_stage_file_path = self.FOO + Stage.STAGE_FILE_SUFFIX
+        ret = main(['add', self.FOO])
+        self.assertEqual(ret, 0)
+        self.assertTrue(os.path.exists(foo_stage_file_path))
+
+        target_foo_path = os.path.join(self.DATA_DIR, self.FOO)
+        target_foo_stage_file_path = target_foo_path + Stage.STAGE_FILE_SUFFIX
+
+        ret = main(['move', self.FOO, self.DATA_DIR])
+        self.assertEqual(ret, 0)
+
+        self.assertFalse(os.path.exists(self.FOO))
+        self.assertFalse(os.path.exists(foo_stage_file_path))
+
+        self.assertTrue(os.path.exists(target_foo_path))
+        self.assertTrue(os.path.exists(target_foo_stage_file_path))
+
+        new_stage = load_stage_file(target_foo_stage_file_path)
+        self.assertEqual(self.FOO, new_stage['outs'][0]['path'])
+
+
+class TestMoveDirectoryShouldNotOverwriteExisting(TestDvc):
+    def test(self):
+        new_dir_name = 'data_dir2'
+        os.mkdir(new_dir_name)
+        ret = main(['add', self.DATA_DIR])
+        self.assertEqual(ret, 0)
+
+        ret = main(['move', self.DATA_DIR, new_dir_name])
+        self.assertEqual(ret, 0)
+
+        self.assertFalse(os.path.exists(self.DATA_DIR))
+        self.assertFalse(
+            os.path.exists(self.DATA_DIR + Stage.STAGE_FILE_SUFFIX))
+
+        self.assertTrue(os.path.exists(new_dir_name))
+        self.assertTrue(os.path.isfile(new_dir_name + Stage.STAGE_FILE_SUFFIX))
+
+        self.assertEqual(os.listdir(new_dir_name),
+                         [self.DATA_DIR])
+
+
+class TestMoveFileBetweenDirectories(TestDvc):
+    def test(self):
+        data_stage_file = self.DATA + Stage.STAGE_FILE_SUFFIX
+        ret = main(['add', self.DATA])
+        self.assertEqual(ret, 0)
+        self.assertTrue(os.path.exists(data_stage_file))
+
+        new_data_dir = 'data_dir2'
+        os.makedirs(new_data_dir)
+
+        ret = main(['move', self.DATA, new_data_dir])
+        self.assertEqual(ret, 0)
+
+        new_data_path = os.path.join(new_data_dir, os.path.basename(self.DATA))
+        new_data_stage_file = new_data_path + Stage.STAGE_FILE_SUFFIX
+
+        self.assertFalse(os.path.exists(self.DATA))
+        self.assertFalse(os.path.exists(data_stage_file))
+
+        self.assertTrue(os.path.exists(new_data_path))
+        self.assertTrue(os.path.exists(new_data_stage_file))
+
+        new_stage_file = load_stage_file(new_data_stage_file)
+        self.assertEqual(os.path.basename(self.DATA),
+                         new_stage_file['outs'][0]['path'])

tests/utils/__init__.py

@@ -1,3 +1,4 @@
+import yaml
 from mock import MagicMock
 
 
@@ -10,3 +11,8 @@ def spy(method_to_decorate):
 
     wrapper.mock = mock
     return wrapper
+
+
+def load_stage_file(path):
+    with open(path, 'r') as fobj:
+        return yaml.safe_load(fobj)