Repository: macports/upt-macports

Issue:
Do not create (or clean-up) the directory structure/Portfile if something fails
When testing the templates, I ran into a few situations where while creating the package we would stumble upon an ```upt.upt.ArchiveUnavailable: No such archive could be found``` problem.

For example, 
```
py-scitools
[INFO    ] [Backend] Hello, creating the package
[INFO    ] [Backend] Creating the directory structure in /tmp/upt-pypi-packaging
[INFO    ] [Backend] Created /tmp/upt-pypi-packaging/python/py-scitools
[INFO    ] [Backend] Creating the Portfile
Traceback (most recent call last):
  File "/tmp/test-3.7/bin/upt", line 11, in <module>
    load_entry_point('upt==0.9', 'console_scripts', 'upt')()
  File "/tmp/test-3.7/lib/python3.7/site-packages/upt/upt.py", line 302, in main
    backend.create_package(upt_pkg, output=args.output)
  File "/tmp/test-3.7/lib/python3.7/site-packages/upt_macports-0.1-py3.7.egg/upt_macports/upt_macports.py", line 253, in create_package
  File "/tmp/test-3.7/lib/python3.7/site-packages/upt_macports-0.1-py3.7.egg/upt_macports/upt_macports.py", line 22, in create_package
  File "/tmp/test-3.7/lib/python3.7/site-packages/upt_macports-0.1-py3.7.egg/upt_macports/upt_macports.py", line 51, in _create_portfile
  File "/tmp/test-3.7/lib/python3.7/site-packages/upt_macports-0.1-py3.7.egg/upt_macports/upt_macports.py", line 64, in _render_makefile_template
  File "/tmp/test-3.7/lib/python3.7/site-packages/Jinja2-2.10.1-py3.7.egg/jinja2/asyncsupport.py", line 76, in render
    return original_render(self, *args, **kwargs)
  File "/tmp/test-3.7/lib/python3.7/site-packages/Jinja2-2.10.1-py3.7.egg/jinja2/environment.py", line 1008, in render
    return self.environment.handle_exception(exc_info, True)
  File "/tmp/test-3.7/lib/python3.7/site-packages/Jinja2-2.10.1-py3.7.egg/jinja2/environment.py", line 780, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/tmp/test-3.7/lib/python3.7/site-packages/Jinja2-2.10.1-py3.7.egg/jinja2/_compat.py", line 37, in reraise
    raise value.with_traceback(tb)
  File "<template>", line 1, in top-level template code
  File "<template>", line 19, in top-level template code
  File "<template>", line 8, in block "nameversion"
  File "/tmp/test-3.7/lib/python3.7/site-packages/upt_macports-0.1-py3.7.egg/upt_macports/upt_macports.py", line 144, in _python_root_name
  File "/tmp/test-3.7/lib/python3.7/site-packages/upt/upt.py", line 187, in get_archive
    raise ArchiveUnavailable()
upt.upt.ArchiveUnavailable: No such archive could be found
```

afterwards we do end up with an empty Portfile in the ```/python/py-scitools`` directory. One way around this would perhaps to first attempt to generate a Portfile in a temporary directory and only copy this over once everything finishes correctly. Or, alternatively, once we run into such Exceptions we could do some clean-up.

Discussion:
reneeotten: thanks @Korusuke: this does solve the problem I mentioned by first generating the Portfile and, only if that works, to create the output directory and write it to a file. 

I *think* it's fine to have an extra variable ```portfile``` and use that as an argument for the ```__create_portfile``` function, but there might be other ways as well. I am sure @Steap has some insight here on what is most Pythonic and/or he prefers!
Korusuke: ping @Steap I guess this PR can be merged now?
codecov[bot]: # [Codecov](https://codecov.io/gh/macports/upt-macports/pull/48?src=pr&el=h1) Report
> Merging [#48](https://codecov.io/gh/macports/upt-macports/pull/48?src=pr&el=desc) into [master](https://codecov.io/gh/macports/upt-macports/commit/f3c01cfd8ea12bf8d1ba1329849311bd6719d98c?src=pr&el=desc) will **increase** coverage by `1.14%`.
> The diff coverage is `60%`.

[![Impacted file tree graph](https://codecov.io/gh/macports/upt-macports/pull/48/graphs/tree.svg?width=650&token=8BuS1ftd2C&height=150&src=pr)](https://codecov.io/gh/macports/upt-macports/pull/48?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master      #48      +/-   ##
==========================================
+ Coverage   86.48%   87.63%   +1.14%     
==========================================
  Files           1        1              
  Lines         185      186       +1     
  Branches       10       10              
==========================================
+ Hits          160      163       +3     
+ Misses         25       23       -2
```


| [Impacted Files](https://codecov.io/gh/macports/upt-macports/pull/48?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [upt\_macports/upt\_macports.py](https://codecov.io/gh/macports/upt-macports/pull/48/diff?src=pr&el=tree#diff-dXB0X21hY3BvcnRzL3VwdF9tYWNwb3J0cy5weQ==) | `87.63% <60%> (+1.14%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/macports/upt-macports/pull/48?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/macports/upt-macports/pull/48?src=pr&el=footer). Last update [f3c01cf...733cc1c](https://codecov.io/gh/macports/upt-macports/pull/48?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Steap: > ping @Steap I guess this PR can be merged now?

Please squash the commits :) You may also want to add "Closes #42" at the end of the commit message.

Otherwise, looks good to me. @reneeotten Once Korusuke has squashed the commits, can you test this PR and merge it if it solves your issue?

Patch:
diff --git a/upt_macports/upt_macports.py b/upt_macports/upt_macports.py
index 055b455..bee3821 100755
--- a/upt_macports/upt_macports.py
+++ b/upt_macports/upt_macports.py
@@ -16,11 +16,12 @@ class MacPortsPackage(object):
     def create_package(self, upt_pkg, output):
         self.upt_pkg = upt_pkg
         self.logger.info(f'Creating MacPorts package for {self.upt_pkg.name}')
+        portfile_content = self._render_makefile_template()
         if output is None:
-            print(self._render_makefile_template())
+            print(portfile_content)
         else:
             self._create_output_directories(upt_pkg, output)
-            self._create_portfile()
+            self._create_portfile(portfile_content)
 
     def _create_output_directories(self, upt_pkg, output_dir):
         """Creates the directory layout required"""
@@ -34,12 +35,12 @@ class MacPortsPackage(object):
         except PermissionError:
             sys.exit(f'Cannot create {self.output_dir}: permission denied.')
 
-    def _create_portfile(self):
+    def _create_portfile(self, portfile_content):
         self.logger.info('Creating the Portfile')
         try:
             with open(os.path.join(self.output_dir, 'Portfile'), 'x',
                       encoding='utf-8') as f:
-                f.write(self._render_makefile_template())
+                f.write(portfile_content)
         except FileExistsError:
             sys.exit(f'Cannot create {self.output_dir}/Portfile: already exists.') # noqa
 


Test patch:
diff --git a/upt_macports/tests/test_macports_package.py b/upt_macports/tests/test_macports_package.py
index 814bac4..23ccd31 100644
--- a/upt_macports/tests/test_macports_package.py
+++ b/upt_macports/tests/test_macports_package.py
@@ -96,6 +96,17 @@ class TestDirectoryCreation(unittest.TestCase):
             self.package._create_output_directories(self.package.upt_pkg,
                                                     '/ports/')
 
+    @mock.patch.object(MacPortsPackage, '_render_makefile_template',
+                       side_effect=PermissionError)
+    @mock.patch.object(MacPortsPackage, '_create_output_directories')
+    @mock.patch.object(MacPortsPackage, '_create_portfile')
+    def test_render_makefile_error(self, portfile, outdir, render):
+        with self.assertRaises(PermissionError):
+            self.package.create_package(mock.Mock(), 'path')
+        render.assert_called()
+        outdir.assert_not_called()
+        portfile.assert_not_called()
+
 
 class TestFileCreation(unittest.TestCase):
     def setUp(self):
@@ -107,7 +118,7 @@ class TestFileCreation(unittest.TestCase):
     def test_portfile_creation(self, m_open):
         fn = 'upt_macports.upt_macports.MacPortsPackage._render_makefile_template' # noqa
         with mock.patch(fn, return_value='Portfile content'):
-            self.package._create_portfile()
+            self.package._create_portfile('Portfile content')
             m_open.assert_called_once_with('/outdir/Portfile', 'x',
                                            encoding='utf-8')
             m_open().write.assert_called_once_with('Portfile content')
@@ -115,7 +126,7 @@ class TestFileCreation(unittest.TestCase):
     @mock.patch('builtins.open', side_effect=FileExistsError)
     def test_portfile_file_exists(self, m_open):
         with self.assertRaises(SystemExit):
-            self.package._create_portfile()
+            self.package._create_portfile('Portfile content')
 
 
 class TestMacPortsPackageArchiveType(unittest.TestCase):