Merge pull request #26 from Anaconda-Platform/missing-project-file-cl…

…eanup

Disallow archiving/uploading projects without a project file

anaconda_project/archiver.py

@@ -297,6 +297,18 @@ def _archive_project(project, filename):
     if failed is not None:
         return failed
 
+    if not os.path.exists(project.project_file.filename):
+        return SimpleStatus(success=False,
+                            description="Can't create an archive.",
+                            errors=[("%s does not exist." % project.project_file.basename)])
+
+    # this would most likely happen in a GUI editor, if it reloaded
+    # the project from memory but hadn't saved yet.
+    if project.project_file.has_unsaved_changes:
+        return SimpleStatus(success=False,
+                            description="Can't create an archive.",
+                            errors=[("%s has been modified but not saved." % project.project_file.basename)])
+
     errors = []
     infos = _enumerate_archive_files(project.directory_path, errors, requirements=project.requirements)
     if infos is None:

anaconda_project/commands/project_load.py

@@ -17,6 +17,7 @@ def load_project(dirname):
     project = Project(dirname)
 
     if console_utils.stdin_is_interactive():
+        had_fixable = len(project.fixable_problems) > 0
         for problem in project.fixable_problems:
             print(problem.text)
             should_fix = console_utils.console_ask_yes_or_no(problem.fix_prompt, default=False)
@@ -25,7 +26,9 @@ def load_project(dirname):
             else:
                 problem.no_fix(project)
 
-        # no-op if the fixes didn't do anything
-        project.project_file.save()
+        # both fix() and no_fix() can modify project_file, if no changes
+        # were made this is a no-op.
+        if had_fixable:
+            project.project_file.save()
 
     return project

anaconda_project/commands/test/test_bundle.py

@@ -7,24 +7,28 @@
 from __future__ import absolute_import, print_function
 
 import os
+import zipfile
 
 from anaconda_project.commands.main import _parse_args_and_run_subcommand
-from anaconda_project.internal.test.tmpfile_utils import with_directory_contents_completing_project_file
+from anaconda_project.internal.test.tmpfile_utils import (with_directory_contents,
+                                                          with_directory_contents_completing_project_file)
 from anaconda_project.project_file import DEFAULT_PROJECT_FILENAME
 
 
 def test_archive_command_on_empty_project(capsys):
     def check(dirname):
         archivefile = os.path.join(dirname, "foo.zip")
         code = _parse_args_and_run_subcommand(['anaconda-project', 'archive', '--directory', dirname, archivefile])
-        assert code == 0
+        assert code == 1
 
         out, err = capsys.readouterr()
-        assert ('  added %s\nCreated project archive %s\n' % (os.path.join(
-            os.path.basename(dirname), "anaconda-project.yml"), archivefile)) == out
-        assert '' == err
+        assert "anaconda-project.yml does not exist.\nCan't create an archive.\n" == err
+        assert '' == out
 
-    with_directory_contents_completing_project_file(dict(), check)
+        assert not os.path.exists(os.path.join(dirname, DEFAULT_PROJECT_FILENAME))
+        assert not os.path.exists(archivefile)
+
+    with_directory_contents(dict(), check)
 
 
 def test_archive_command_on_simple_project(capsys):
@@ -35,8 +39,14 @@ def check(dirname):
 
         out, err = capsys.readouterr()
         assert ('  added %s\n  added %s\nCreated project archive %s\n' % (os.path.join(
-            os.path.basename(dirname), "anaconda-project.yml"), os.path.join(
+            os.path.basename(dirname), DEFAULT_PROJECT_FILENAME), os.path.join(
                 os.path.basename(dirname), "foo.py"), archivefile)) == out
+
+        with zipfile.ZipFile(archivefile, mode='r') as zf:
+            assert [os.path.basename(x) for x in sorted(zf.namelist())] == [DEFAULT_PROJECT_FILENAME, "foo.py"]
+
+        assert os.path.exists(os.path.join(dirname, DEFAULT_PROJECT_FILENAME))
+
         assert '' == err
 
     with_directory_contents_completing_project_file({'foo.py': 'print("hello")\n'}, check)

anaconda_project/internal/test/tmpfile_utils.py

@@ -78,7 +78,7 @@ def with_directory_contents(contents, func):
 def complete_project_file_content(content):
     yaml = _load_string(content)
     if yaml is None:
-        return content
+        raise AssertionError("Broken yaml: %r" % content)
     elif 'env_specs' not in yaml:
         modified = (content + "\n" + "env_specs:\n" + "  default:\n" + "    description: default\n" + "\n")
         try:

anaconda_project/test/test_project.py

@@ -2777,3 +2777,11 @@ def check(dirname):
 
     with_directory_contents_completing_project_file(
         {DEFAULT_PROJECT_FILENAME: "env_specs:\n  foo:\n    packages: [something]\n    somejunk: True\n"}, check)
+
+
+def test_empty_file_has_problems():
+    def check(dirname):
+        project = project_no_dedicated_env(dirname)
+        assert ['anaconda-project.yml: The env_specs section is empty.'] == project.problems
+
+    with_directory_contents({DEFAULT_PROJECT_FILENAME: ""}, check)

anaconda_project/test/test_project_ops.py

@@ -2387,6 +2387,52 @@ def check(dirname):
     with_directory_contents_completing_project_file(dict(), archivetest)
 
 
+def test_archive_with_no_project_file(monkeypatch):
+    def archivetest(archive_dest_dir):
+        archivefile = os.path.join(archive_dest_dir, "foo.zip")
+
+        def check(dirname):
+            project = project_no_dedicated_env(dirname)
+            assert not os.path.exists(project.project_file.filename)
+
+            status = project_ops.archive(project, archivefile)
+
+            assert not status
+            assert not os.path.exists(archivefile)
+            assert status.status_description == "Can't create an archive."
+            assert status.errors == ["%s does not exist." % DEFAULT_PROJECT_FILENAME]
+
+        with_directory_contents(dict(), check)
+
+    with_directory_contents(dict(), archivetest)
+
+
+def test_archive_with_unsaved_project(monkeypatch):
+    def archivetest(archive_dest_dir):
+        archivefile = os.path.join(archive_dest_dir, "foo.zip")
+
+        def check(dirname):
+            project = project_no_dedicated_env(dirname)
+            assert os.path.exists(project.project_file.filename)
+            project.project_file.set_value(['name'], "hello")
+
+            status = project_ops.archive(project, archivefile)
+
+            assert not status
+            assert not os.path.exists(archivefile)
+            assert status.status_description == "Can't create an archive."
+            assert status.errors == ["%s has been modified but not saved." % DEFAULT_PROJECT_FILENAME]
+
+        with_directory_contents_completing_project_file(
+            {DEFAULT_PROJECT_FILENAME: """
+env_specs:
+  default:
+    packages: []
+"""}, check)
+
+    with_directory_contents(dict(), archivetest)
+
+
 def test_archive_zip_with_downloaded_file():
     def archivetest(archive_dest_dir):
         archivefile = os.path.join(archive_dest_dir, "foo.zip")

anaconda_project/test/test_yaml_file.py

@@ -114,6 +114,15 @@ def check_missing(dirname):
     with_directory_contents(dict(), check_missing)
 
 
+def test_read_empty_yaml_file_and_get_default_due_to_missing_section():
+    def check_missing(dirname):
+        yaml = YamlFile(os.path.join(dirname, "nope.yaml"))
+        value = yaml.get_value(["z", "b"], "default")
+        assert "default" == value
+
+    with_directory_contents({"nope.yaml": ""}, check_missing)
+
+
 def test_read_yaml_file_that_is_a_directory():
     def check_read_directory(dirname):
         filename = os.path.join(dirname, "dir.yaml")
@@ -200,6 +209,34 @@ def set_abc(dirname):
     with_directory_contents(dict(), set_abc)
 
 
+def test_read_empty_yaml_file_and_set_value():
+    def set_abc(dirname):
+        filename = os.path.join(dirname, "foo.yaml")
+        assert os.path.exists(filename)
+        yaml = YamlFile(filename)
+        value = yaml.get_value(["a", "b"])
+        assert value is None
+        yaml.set_value(["a", "b"], 42)
+        yaml.save()
+        assert os.path.exists(filename)
+
+        import codecs
+        with codecs.open(filename, 'r', 'utf-8') as file:
+            changed = file.read()
+            expected = """
+a:
+  b: 42
+""" [1:]
+
+            assert expected == changed
+
+        yaml2 = YamlFile(filename)
+        value2 = yaml2.get_value(["a", "b"])
+        assert 42 == value2
+
+    with_directory_contents({"foo.yaml": ""}, set_abc)
+
+
 def test_read_yaml_file_and_add_section():
     original_content = """
 a:

anaconda_project/yaml_file.py

@@ -53,10 +53,15 @@ def _atomic_replace(path, contents, encoding='utf-8'):
 
 
 def _load_string(contents):
-    # using RoundTripLoader incorporates safe_load
-    # (we don't load code)
-    assert issubclass(ryaml.RoundTripLoader, ryaml.constructor.SafeConstructor)
-    return ryaml.load(contents, Loader=ryaml.RoundTripLoader)
+    if contents.strip() == '':
+        # ryaml.load below returns None for an empty file, we want
+        # to return an empty dict instead.
+        return {}
+    else:
+        # using RoundTripLoader incorporates safe_load
+        # (we don't load code)
+        assert issubclass(ryaml.RoundTripLoader, ryaml.constructor.SafeConstructor)
+        return ryaml.load(contents, Loader=ryaml.RoundTripLoader)
 
 
 def _save_file(yaml, filename):
@@ -160,10 +165,15 @@ def load(self):
             self._yaml = None
 
         if self._yaml is None:
-            self._yaml = self._default_content()
-            # make it pretty
-            _block_style_all_nodes(self._yaml)
-            self._dirty = True
+            if self._corrupted:
+                # don't want to throw exceptions if people get_value()
+                # so stick an empty dict in here
+                self._yaml = dict()
+            else:
+                self._yaml = self._default_content()
+                # make it pretty
+                _block_style_all_nodes(self._yaml)
+                self._dirty = True
 
     def _default_comment(self):
         return "yaml file"
@@ -236,6 +246,11 @@ def change_count(self):
         """
         return self._change_count
 
+    @property
+    def has_unsaved_changes(self):
+        """Get whether changes are all saved."""
+        return self._dirty
+
     def use_changes_without_saving(self):
         """Apply any in-memory changes as if we'd saved, but don't actually save.