Repository: regro/rever (language: python)

Issue:
 Check for $TAG_TEMPLATE var on building GitHub source url



Discussion / hints:
This needs news
Thanks @igortg! Please just add a news entry as per https://regro.github.io/rever-docs/devguide.html#changelog
Hey guys,

Is there anything missing on this PR so it could be merged? Any problems with the implementation, please let me know
Whoops sorry! In the future, please ping the issue/PR once you think it is ready again, so that we get an email :wink: 

Fix patch:
diff --git a/rever/activities/pypi.xsh b/rever/activities/pypi.xsh
index e61e356..d6d9475 100644
--- a/rever/activities/pypi.xsh
+++ b/rever/activities/pypi.xsh
@@ -3,6 +3,7 @@ import os
 import re
 import sys
 import getpass
+import itertools
 from configparser import ConfigParser, ExtendedInterpolation
 
 from lazyasd import lazyobject
@@ -106,22 +107,61 @@ class PyPI(Activity):
         else:
             return rc
 
+    def _add_dist_dir_argument(self, build_commands, dist_dir):
+        r"""
+        Return ``build_commands`` augmented with ``--dist-dir`` arguments as needed.
+        """
+        # Split build_commands into individual subcommands and their arguments.
+        subcommands = []
+        for command in build_commands:
+          if command.startswith("-") and subcommands:
+            # Append this argument to the preceding subcommand.
+            subcommands[-1].append(command)
+          else:
+            # This command starts a new subcommand.
+            subcommands.append([command])
+
+        def needs_dist_dir_argument(subcommand):
+            # Return whether this command needs an explicit --dist-dir
+            # argument, i.e., whether it's some "dist" command that has no
+            # --dist-dir argument yet.
+            if not "dist" in subcommand[0]:
+                return False
+            for argument in subcommand[1:]:
+                if argument == "-d":
+                    return False
+                if argument.startswith("-d="):
+                    return False
+                if argument.startswith("--dist-dir"):
+                    return False
+
+            return True
+
+        for subcommand in subcommands:
+          if needs_dist_dir_argument(subcommand):
+            subcommand.extend(["--dist-dir", dist_dir])
+
+        return list(itertools.chain(*subcommands))
+
     def _func(self, rc='$HOME/.pypirc', build_commands=('sdist',),
               upload=True, name=None, sign=None):
+        if not build_commands:
+          raise ValueError("PyPI build commands must not be empty.")
+
         rc = self._ensure_rc(rc)
+
         # isolate distributions for rever
         $dist_dir = os.path.join($REVER_DIR, "dist")
         if os.path.exists($dist_dir):
             rmtree($dist_dir, force=True)
-        # build distributions
-        dist_in_cmds = any(["dist" in c for c in build_commands])
-        dist_dir_in_cmds = any([c.startswith("-d=") or c == "-d" or c.startswith("--dist-dir")
-                                for c in build_commands])
-        if dist_in_cmds and not dist_dir_in_cmds:
-            build_commands = list(build_commands) + ["--dist-dir", $dist_dir]
+
+        # build ditribution
+        build_commands = self._add_dist_dir_argument(build_commands, $dist_dir)
+
         p = ![$PYTHON setup.py @(build_commands)]
         if p.rtn != 0:
             raise RuntimeError("Failed to build Python distributions!")
+
         # upload, as needed
         if upload:
             upload_args = ["upload"]


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index d41933a..e247751 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -12,7 +12,7 @@
 
 @pytest.fixture
 def gitrepo(request):
-    """A test fixutre that creates and destroys a git repo in a temporary
+    """A test fixture that creates and destroys a git repo in a temporary
     directory.
     This will yield the path to the repo.
     """
diff --git a/tests/test_pypi_activity.py b/tests/test_pypi_activity.py
index 8b05a0f..5593784 100644
--- a/tests/test_pypi_activity.py
+++ b/tests/test_pypi_activity.py
@@ -110,3 +110,14 @@ def test_pypi_activity(gitrepo):
                                   universal_newlines=True)
     out = out.strip()
     assert '42.1.1' == out
+
+def test_add_dist_dir_argument():
+    from rever.activities.pypi import PyPI
+
+    activity = PyPI()
+
+    assert activity._add_dist_dir_argument([], "/tmp/dist") == []
+    assert activity._add_dist_dir_argument(["sdist"], "/tmp/dist") == ["sdist", "--dist-dir", "/tmp/dist"]
+    assert activity._add_dist_dir_argument(["sdist", "--dist-dir=/tmp/explicit/dist/dir"], "/tmp/dist") == ["sdist", "--dist-dir=/tmp/explicit/dist/dir"]
+    assert activity._add_dist_dir_argument(["sdist", "bdist_wheel"], "/tmp/dist") == ["sdist", "--dist-dir", "/tmp/dist", "bdist_wheel", "--dist-dir", "/tmp/dist"]
+    assert activity._add_dist_dir_argument(["build"], "/tmp/dist") == ["build"]