Repository: packit-service/packit
Pull request #658 (2020-01-10 15:36:51)

build in koji from srpm
TODO:
* [x] build from upstream
* [x] build from source-git
* ~~[ ] build from dist-git~~ we don't really need this
* [x] tests

sample builds:

* https://koji.fedoraproject.org/koji/taskinfo?taskID=40372149
* https://koji.fedoraproject.org/koji/taskinfo?taskID=40372384
* sourcegit systemd https://koji.fedoraproject.org/koji/taskinfo?taskID=40610580

use like this:
```
$ PYTHONPATH=$PWD python3 -m packit.cli.packit_base build --from-upstream --scratch --nowait --koji-target=rawhide
```

softwarefactory-project-zuul[bot]: Build succeeded.

- [packit-pre-commit ](https://softwarefactory-project.io/zuul/t/local/build/00981799f24644e8af0df90e6ed9d59b) : SUCCESS in 2m 46s
- [packit-tests-rpm ](https://softwarefactory-project.io/zuul/t/local/build/349f6ffa7201452e861692361bb97258) : SUCCESS in 5m 30s
- [packit-tests-pip-deps ](https://softwarefactory-project.io/zuul/t/local/build/8c45ced6494348fab5ebbd1c8f8173f4) : SUCCESS in 5m 02s
- [packit-tests-git-master ](https://softwarefactory-project.io/zuul/t/local/build/3095cd32f6594f18b67f32ead1b6c1c1) : SUCCESS in 5m 13s
- [packit-tests-rpm-sess-rec ](https://softwarefactory-project.io/zuul/t/local/build/919f9199c19846bc9dfbea768a914e9c) : SUCCESS in 2m 11s
- [packit-tests-pip-deps-sess-rec ](https://softwarefactory-project.io/zuul/t/local/build/f5e255d3f263418e8f255f279bbea393) : SUCCESS in 2m 26s
- [packit-tests-git-master-sess-rec ](https://softwarefactory-project.io/zuul/t/local/build/7a0ff0a8d6bf49e2aeb92dccd8d24824) : SUCCESS in 2m 42s

softwarefactory-project-zuul[bot]: Build succeeded.

- [packit-pre-commit ](https://softwarefactory-project.io/zuul/t/local/build/a27f823ad3854fb9b3a6167dd91f3d8c) : SUCCESS in 1m 47s
- [packit-tests-rpm ](https://softwarefactory-project.io/zuul/t/local/build/3cfd6052023d4155bcc76443f2d55f97) : SUCCESS in 5m 03s
- [packit-tests-pip-deps ](https://softwarefactory-project.io/zuul/t/local/build/9618e42c81174fe2aad9cfd60cf4a32a) : SUCCESS in 5m 26s
- [packit-tests-git-master ](https://softwarefactory-project.io/zuul/t/local/build/8504b29960d44227889db9f94580afef) : SUCCESS in 6m 50s
- [packit-tests-rpm-sess-rec ](https://softwarefactory-project.io/zuul/t/local/build/72b19a8ec30242569f33ddde40e09a05) : SUCCESS in 2m 25s
- [packit-tests-pip-deps-sess-rec ](https://softwarefactory-project.io/zuul/t/local/build/c42c90b18433459c960af9ec7f897107) : SUCCESS in 2m 41s
- [packit-tests-git-master-sess-rec ](https://softwarefactory-project.io/zuul/t/local/build/c65df84757a04d9eb9d838821ea0350c) : SUCCESS in 2m 27s


Patch:
diff --git a/packit/api.py b/packit/api.py
index 350a9291..4be38e86 100644
--- a/packit/api.py
+++ b/packit/api.py
@@ -393,6 +393,7 @@ class PackitAPI:
         scratch: bool = False,
         nowait: bool = False,
         koji_target: Optional[str] = None,
+        from_upstream: bool = False,
     ):
         """
         Build component in Fedora infra (defaults to koji)
@@ -401,9 +402,20 @@ class PackitAPI:
         :param scratch: should the build be a scratch build?
         :param nowait: don't wait on build?
         :param koji_target: koji target to pick (see `koji list-targets`)
+        :param from_upstream: build directly from the upstream checkout?
         """
         logger.info(f"Using {dist_git_branch!r} dist-git branch")
 
+        if from_upstream:
+            srpm_path = self.create_srpm(srpm_dir=self.up.local_project.working_dir)
+            out = self.up.koji_build(
+                scratch=scratch,
+                nowait=nowait,
+                koji_target=koji_target,
+                srpm_path=srpm_path,
+            )
+            return out
+
         self.dg.create_branch(
             dist_git_branch,
             base=f"remotes/origin/{dist_git_branch}",
diff --git a/packit/base_git.py b/packit/base_git.py
index ff15c80e..7ebdaf89 100644
--- a/packit/base_git.py
+++ b/packit/base_git.py
@@ -327,12 +327,18 @@ class PackitRepositoryBase:
             last_source_position = None
             line = spec_file.readline()
             while line:
+                if line.startswith("Patch"):
+                    raise PackitException(
+                        "This specfile already contains patches, please remove them."
+                    )
                 if line.startswith("Source"):
                     last_source_position = spec_file.tell()
                 line = spec_file.readline()
 
             if not last_source_position:
-                raise Exception("Cannot found place for patches in specfile.")
+                raise PackitException(
+                    "Cannot find a place to put patches in the specfile."
+                )
 
             spec_file.seek(last_source_position)
             rest_of_the_file = spec_file.read()
diff --git a/packit/cli/build.py b/packit/cli/build.py
index 0dea4f19..c0cd9e31 100644
--- a/packit/cli/build.py
+++ b/packit/cli/build.py
@@ -29,7 +29,7 @@ from packit.cli.types import LocalProjectParameter
 from packit.cli.utils import cover_packit_exception, get_packit_api
 from packit.config import pass_config, get_context_settings
 from packit.config.aliases import get_branches
-from packit.exceptions import PackitCommandFailedError
+from packit.exceptions import PackitCommandFailedError, ensure_str
 
 logger = logging.getLogger(__file__)
 
@@ -46,6 +46,12 @@ logger = logging.getLogger(__file__)
     help="Path to dist-git repo to work in. "
     "Otherwise clone the repo in a temporary directory.",
 )
+@click.option(
+    "--from-upstream",
+    help="Build the project in koji directly from the upstream repository",
+    is_flag=True,
+    default=False,
+)
 @click.option(
     "--koji-target", help="Koji target to build inside (see `koji list-targets`)."
 )
@@ -57,12 +63,21 @@ logger = logging.getLogger(__file__)
 @pass_config
 @cover_packit_exception
 def build(
-    config, dist_git_path, dist_git_branch, scratch, nowait, path_or_url, koji_target
+    config,
+    dist_git_path,
+    dist_git_branch,
+    from_upstream,
+    scratch,
+    nowait,
+    path_or_url,
+    koji_target,
 ):
     """
     Build selected upstream project in Fedora.
 
-    Packit goes to dist-git and performs `fedpkg build` for the selected branch.
+    By default, packit checks out the respective dist-git repository and performs
+    `fedpkg build` for the selected branch. With `--from-upstream`, packit creates a SRPM
+    out of the current checkout and sends it to koji.
 
     PATH_OR_URL argument is a local path or a URL to the upstream git repository,
     it defaults to the current working directory
@@ -76,18 +91,22 @@ def build(
 
     for branch in branches_to_build:
         try:
-            api.build(
+            out = api.build(
                 dist_git_branch=branch,
                 scratch=scratch,
                 nowait=nowait,
                 koji_target=koji_target,
+                from_upstream=from_upstream,
             )
         except PackitCommandFailedError as ex:
-            logs_stdout = "\n>>> ".join(ex.stdout_output.decode().strip().split("\n"))
-            logs_stderr = "\n!!! ".join(ex.stderr_output.decode().strip().split("\n"))
+            logs_stdout = "\n>>> ".join(ex.stdout_output.strip().split("\n"))
+            logs_stderr = "\n!!! ".join(ex.stderr_output.strip().split("\n"))
             click.echo(
                 f"Build for branch '{branch}' failed. \n"
                 f">>> {logs_stdout}\n"
                 f"!!! {logs_stderr}\n",
                 err=True,
             )
+        else:
+            if out:
+                print(ensure_str(out))
diff --git a/packit/exceptions.py b/packit/exceptions.py
index d017f608..b09aa2ac 100644
--- a/packit/exceptions.py
+++ b/packit/exceptions.py
@@ -24,6 +24,14 @@ from typing import Union
 from deprecated import deprecated
 
 
+def ensure_str(inp: Union[bytes, str]) -> str:
+    """ decode bytes on input or just return the string """
+    # yes, this func doesn't have anything to do with exceptions
+    # but it needs to be placed in a leaf module
+    # utils import from exceptions hence it can't be there
+    return inp if isinstance(inp, str) else inp.decode()
+
+
 class PackitException(Exception):
     pass
 
@@ -38,16 +46,8 @@ class PackitCommandFailedError(PackitException):
         stderr_output: Union[str, bytes] = None
     ):
         super().__init__(*args)
-        self.stdout_output = (
-            stdout_output.decode()
-            if isinstance(stdout_output, bytes)
-            else stdout_output
-        )
-        self.stderr_output = (
-            stderr_output.decode()
-            if isinstance(stderr_output, bytes)
-            else stderr_output
-        )
+        self.stdout_output = ensure_str(stdout_output)
+        self.stderr_output = ensure_str(stderr_output)
 
 
 class PackitConfigException(PackitException):
diff --git a/packit/fedpkg.py b/packit/fedpkg.py
index 60cd7e31..155b4b4b 100644
--- a/packit/fedpkg.py
+++ b/packit/fedpkg.py
@@ -61,7 +61,17 @@ class FedPKG:
         scratch: bool = False,
         nowait: bool = False,
         koji_target: Optional[str] = None,
+        srpm_path: Optional[Path] = None,
     ):
+        """
+        build in koji
+
+        :param scratch: scratch (temporary) build or not?
+        :param nowait: False == wait for the build to finish
+        :param koji_target: koji target to build in (`koji list-targets`)
+        :param srpm_path: use selected SRPM for build, not dist-git repo & ref
+        :return:
+        """
         cmd = [self.fedpkg_exec, "build"]
         if scratch:
             cmd.append("--scratch")
@@ -69,6 +79,8 @@ class FedPKG:
             cmd.append("--nowait")
         if koji_target:
             cmd += ["--target", koji_target]
+        if srpm_path:
+            cmd += ["--srpm", str(srpm_path)]
         utils.run_command_remote(
             cmd=cmd,
             cwd=self.directory,
diff --git a/packit/upstream.py b/packit/upstream.py
index fe7acfc6..24793ad9 100644
--- a/packit/upstream.py
+++ b/packit/upstream.py
@@ -29,6 +29,7 @@ from typing import Optional, List, Tuple
 import git
 from packaging import version
 
+from packit import utils
 from packit.actions import ActionName
 from packit.base_git import PackitRepositoryBase
 from packit.config import Config, PackageConfig, SyncFilesConfig
@@ -692,7 +693,6 @@ class Upstream(PackitRepositoryBase):
         """
         Fetch the tarball and don't check out the upstream ref.
 
-        :param relative_to: for the purpose of Sandcastle
         :param upstream_ref: the base git ref for the source git
         :return: the source directory where we can build the SRPM
         """
@@ -723,3 +723,43 @@ class Upstream(PackitRepositoryBase):
                 upstream=upstream_ref, destination=str(self.absolute_specfile_dir)
             )
             self.add_patches_to_specfile(patches)
+
+    def koji_build(
+        self,
+        scratch: bool = False,
+        nowait: bool = False,
+        koji_target: Optional[str] = None,
+        srpm_path: Optional[Path] = None,
+    ):
+        """
+        Perform a `koji build` in the repository
+
+        :param scratch: should the build be a scratch build?
+        :param nowait: don't wait on build?
+        :param koji_target: koji target to pick (see `koji list-targets`)
+        :param srpm_path: use selected SRPM for build, not dist-git repo & ref
+        """
+        if not koji_target:
+            raise PackitException(
+                "koji target needs to be set when building directly from upstream"
+            )
+        # we can't use fedpkg b/c upstream repo is not dist-git
+        cmd = ["koji", "build"]
+        if scratch:
+            cmd.append("--scratch")
+        if nowait:
+            cmd.append("--nowait")
+        cmd += [koji_target, str(srpm_path)]
+        logger.info("Starting a koji build.")
+        if not nowait:
+            logger.info(
+                "We will be actively waiting for the build to finish, it may take some time."
+            )
+        out = utils.run_command_remote(
+            cmd,
+            cwd=self.local_project.working_dir,
+            output=True,
+            decode=True,
+            print_live=True,
+        )
+        return out
diff --git a/packit/utils.py b/packit/utils.py
index 477ca032..161ec1a4 100644
--- a/packit/utils.py
+++ b/packit/utils.py
@@ -63,18 +63,22 @@ def get_rev_list_kwargs(opt_list):
 
 
 class StreamLogger(threading.Thread):
-    def __init__(self, stream, log_level=logging.DEBUG):
+    def __init__(self, stream, log_level=logging.DEBUG, decode=False):
         super().__init__(daemon=True)
         self.stream = stream
         self.output = []
         self.log_level = log_level
+        self.decode = decode
 
     def run(self):
         for line in self.stream:
             # not doing strip here on purpose so we get real output
             # and we are saving bytes b/c the output can contain chars which can't be decoded
             self.output.append(line)
-            logger.log(self.log_level, line.rstrip(b"\n"))
+            line = line.rstrip(b"\n")
+            if self.decode:
+                line = line.decode()
+            logger.log(self.log_level, line)
 
     def get_output(self):
         return b"".join(self.output)
@@ -87,7 +91,8 @@ def run_command(
     fail: bool = True,
     output: bool = False,
     env: Optional[Dict] = None,
-    decode=True,
+    decode: bool = True,
+    print_live: bool = False,
 ):
     """
     run provided command in a new subprocess
@@ -99,6 +104,7 @@ def run_command(
     :param output: if True, return command's stdout & stderr
     :param env: set these env vars in the subprocess
     :param decode: decode stdout from utf8 to string
+    :param print_live: print output from the command realtime to INFO log
     """
     if not isinstance(cmd, list):
         cmd = shlex.split(cmd)
@@ -125,8 +131,16 @@ def run_command(
         env=cmd_env,
     )
 
-    stdout = StreamLogger(shell.stdout, log_level=logging.DEBUG)
-    stderr = StreamLogger(shell.stderr, log_level=logging.INFO)
+    stdout = StreamLogger(
+        shell.stdout,
+        log_level=logging.DEBUG if not print_live else logging.INFO,
+        decode=decode,
+    )
+    stderr = StreamLogger(
+        shell.stderr,
+        log_level=logging.DEBUG if not print_live else logging.INFO,
+        decode=decode,
+    )
 
     stdout.start()
     stderr.start()
@@ -138,11 +152,15 @@ def run_command(
         logger.error("Command %s failed", shell.args)
         logger.error("%s", error_message)
         if fail:
-            stderr_output = stderr.get_output()
-            stdout_output = stdout.get_output()
+            stderr_output = (
+                stderr.get_output().decode() if decode else stderr.get_output()
+            )
+            stdout_output = (
+                stdout.get_output().decode() if decode else stdout.get_output()
+            )
             if output:
-                logger.debug(f"Command stderr:\n{stderr_output.decode()}")
-                logger.debug(f"Command stdout:\n{stdout_output.decode()}")
+                logger.debug(f"Command stderr:\n{stderr_output}")
+                logger.debug(f"Command stdout:\n{stdout_output}")
             raise PackitCommandFailedError(
                 f"Command {shell.args!r} failed: {error_message}",
                 stdout_output=stdout_output,
@@ -169,6 +187,8 @@ def run_command_remote(
     fail=True,
     output=False,
     env: Optional[Dict] = None,
+    decode=True,
+    print_live: bool = False,
 ):
     """
     wrapper for run_command method
@@ -179,7 +199,9 @@ def run_command_remote(
         submit something to some server, and check how server reply
         call kinit of some ticket
     """
-    return run_command(cmd, error_message, cwd, fail, output, env)
+    return run_command(
+        cmd, error_message, cwd, fail, output, env, decode=decode, print_live=print_live
+    )
 
 
 class PackitFormatter(logging.Formatter):


Test patch:
diff --git a/tests/data/upstream_git_with_multiple_sources/beer.spec b/tests/data/upstream_git_with_multiple_sources/beer.spec
index 129f2e70..9799a4ee 100644
--- a/tests/data/upstream_git_with_multiple_sources/beer.spec
+++ b/tests/data/upstream_git_with_multiple_sources/beer.spec
@@ -6,8 +6,8 @@ Release:        1%{?dist}
 Summary:        A tool to make you happy
 
 License:        Beerware
-Source0:         %{upstream_name}-%{version}.tar.gz
-Source1:         https://src.fedoraproject.org/rpms/python-ogr/blob/master/f/python-ogr.spec
+Source0:        %{upstream_name}-%{version}.tar.gz
+Source1:        https://src.fedoraproject.org/rpms/python-ogr/blob/master/f/python-ogr.spec
 BuildArch:      noarch
 
 %description
diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py
index 58e448cb..f52105c7 100644
--- a/tests/integration/conftest.py
+++ b/tests/integration/conftest.py
@@ -142,8 +142,6 @@ def mock_remote_functionality(distgit: Path, upstream: Path):
     )
     flexmock(PagureUser, get_username=lambda: "packito")
 
-    mock_spec_download_remote_s(distgit)
-
     dglp = LocalProject(
         working_dir=str(distgit),
         git_url="https://packit.dev/rpms/beer",
diff --git a/tests/integration/test_build.py b/tests/integration/test_build.py
index 97ac80a1..da52997e 100644
--- a/tests/integration/test_build.py
+++ b/tests/integration/test_build.py
@@ -19,6 +19,9 @@
 # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
+from pathlib import Path
+from typing import Union, List, Optional, Dict
+
 from flexmock import flexmock
 
 from packit import utils
@@ -35,3 +38,35 @@ def test_basic_build(
         fail=True,
     ).once()
     api.build("master", scratch=True, nowait=True, koji_target="asdqwe")
+
+
+def test_build_from_upstream(
+    cwd_upstream_or_distgit, api_instance, mock_remote_functionality_upstream
+):
+    u, d, api = api_instance
+
+    def mocked_run_command(
+        cmd: Union[List[str], str],
+        error_message: str = None,
+        cwd: Union[str, Path] = None,
+        fail: bool = True,
+        output: bool = False,
+        env: Optional[Dict] = None,
+        decode=True,
+        print_live=False,
+    ):
+        assert cmd[:-1] == ["koji", "build", "--scratch", "--nowait", "hadron-collider"]
+        srpm_path = cmd[-1]
+        assert Path(srpm_path).is_file()
+        assert srpm_path.endswith(".src.rpm")
+        assert cwd == api.up.local_project.working_dir
+        return "\n\nLink to koji build: https://koji...\n"
+
+    flexmock(utils, run_command_remote=mocked_run_command)
+    api.build(
+        "master",
+        scratch=True,
+        nowait=True,
+        from_upstream=True,
+        koji_target="hadron-collider",
+    )
diff --git a/tests/integration/test_source_git.py b/tests/integration/test_source_git.py
index c57ac911..32a6ae65 100644
--- a/tests/integration/test_source_git.py
+++ b/tests/integration/test_source_git.py
@@ -39,6 +39,7 @@ def test_basic_local_update_without_patching(
 
     sourcegit, _ = sourcegit_and_remote
     distgit, _ = distgit_and_remote
+    mock_spec_download_remote_s(distgit)
 
     api_instance_source_git.sync_release("master", "0.1.0", upstream_ref="0.1.0")
 
@@ -53,6 +54,7 @@ def test_basic_local_update_empty_patch(
     """ propose-update for sourcegit test: mock remote API, use local upstream and dist-git """
 
     distgit, _ = distgit_and_remote
+    mock_spec_download_remote_s(distgit)
     api_instance_source_git.sync_release("master", "0.1.0", upstream_ref="0.1.0")
 
     assert (distgit / TARBALL_NAME).is_file()
@@ -78,6 +80,7 @@ def test_basic_local_update_patch_content(
 
     sourcegit, _ = sourcegit_and_remote
     distgit, _ = distgit_and_remote
+    mock_spec_download_remote_s(distgit)
 
     source_file = sourcegit / "big-source-file.txt"
     source_file.write_text("new changes")
@@ -150,6 +153,9 @@ def test_basic_local_update_patch_content(
         "+new changes\n" not in git_diff
     )
 
+    # ignored file should not be in the diff
+    assert "--- a/ignored_file.txt\n" not in git_diff
+
 
 def test_srpm(mock_remote_functionality_sourcegit, api_instance_source_git):
     # TODO: we need a better test case here which will mimic the systemd use case
diff --git a/tests/integration/test_update.py b/tests/integration/test_update.py
index ead3a7b2..53f1a5f3 100644
--- a/tests/integration/test_update.py
+++ b/tests/integration/test_update.py
@@ -29,6 +29,7 @@ from packit.api import PackitAPI, Config
 from packit.config import parse_loaded_config
 from packit.local_project import LocalProject
 from packit.specfile import Specfile
+from tests.integration.conftest import mock_spec_download_remote_s
 from tests.spellbook import TARBALL_NAME
 
 
@@ -59,6 +60,7 @@ def test_basic_local_update(
 ):
     """ basic propose-update test: mock remote API, use local upstream and dist-git """
     u, d, api = api_instance
+    mock_spec_download_remote_s(d)
 
     api.sync_release("master", "0.1.0")
 
@@ -77,6 +79,7 @@ def test_basic_local_update_using_distgit(
 ):
     """ basic propose-update test: mock remote API, use local upstream and dist-git """
     u, d, api = api_instance
+    mock_spec_download_remote_s(d)
 
     api.sync_release("master", "0.1.0")
 
@@ -96,6 +99,7 @@ def test_basic_local_update_direct_push(
     """ basic propose-update test: mock remote API, use local upstream and dist-git """
     u, d, api = api_instance
     _, distgit_remote = distgit_and_remote
+    mock_spec_download_remote_s(d)
 
     api.sync_release("master", "0.1.0", create_pr=False)
 
diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py
index cdb386b9..399efa99 100644
--- a/tests/unit/test_utils.py
+++ b/tests/unit/test_utils.py
@@ -1,3 +1,4 @@
+# -*- coding: utf-8 -*-
 # MIT License
 #
 # Copyright (c) 2018-2019 Red Hat, Inc.
@@ -24,7 +25,7 @@ import pytest
 import sys
 from flexmock import flexmock
 
-from packit.exceptions import PackitException
+from packit.exceptions import PackitException, ensure_str
 from packit.utils import (
     get_namespace_and_repo_name,
     git_remote_url_to_https_url,
@@ -90,3 +91,12 @@ def test_get_packit_version_not_installed():
 def test_get_packit_version():
     flexmock(Distribution).should_receive("version").and_return("0.1.0")
     assert get_packit_version() == "0.1.0"
+
+
+@pytest.mark.parametrize(
+    "inp,exp",
+    (("asd", "asd"), (b"asd", "asd"), ("🍺", "🍺"), (b"\xf0\x9f\x8d\xba", "🍺")),
+    ids=("asd", "bytes-asd", "beer-str", "beer-bytes"),
+)
+def test_ensure_str(inp, exp):
+    assert ensure_str(inp) == exp