Repository: packit/packit
Issue: spec_source_id is ignored when using Source: in spec file

When there is a multi-source spec file and `spec_source_id` is used to specify a non-default source (such as `Source1`), it is ignored, if the first source is specified as `Source:`. packit transforms the `Source:` line, not the `Source1:` one.
Workaround: use `Source0:` in the specfile, instead of `Source:`.
IMO this is a bug, `Source0:` and `Source:` should be equivalent.
Compare
https://download.copr.fedorainfracloud.org/results/packit/pcahyna-packit_demo-1/fedora-31-x86_64/01747752-beer/beer.spec (built in https://copr.fedorainfracloud.org/coprs/packit/pcahyna-packit_demo-1/, originating from https://github.com/pcahyna/packit_demo/pull/1) which shows `Source:` being replaced
with https://download.copr.fedorainfracloud.org/results/packit/pcahyna-packit_demo_2-1/fedora-31-x86_64/01751739-beer/beer.spec (built in https://copr.fedorainfracloud.org/coprs/packit/pcahyna-packit_demo_2-1/, originating from https://github.com/pcahyna/packit_demo_2/pull/1) which shows the correct behavior because `Source0:` was used.

Changed files:
tests/integration/test_upstream.py, packit/upstream.py, packit/constants.py, packit/specfile.py

diff --git a/packit/constants.py b/packit/constants.py
index 6f91a952..d2c54194 100644
--- a/packit/constants.py
+++ b/packit/constants.py
@@ -91,5 +91,3 @@ SYNCING_NOTE = (
     "This repository is maintained by packit.\\nhttps://packit.dev/\\n"
     "The file was generated using packit {packit_version}.\\n"
 )
-
-SPEC_PACKAGE_SECTION = "%package"
diff --git a/packit/specfile.py b/packit/specfile.py
index 106dbdaf..bd348723 100644
--- a/packit/specfile.py
+++ b/packit/specfile.py
@@ -24,12 +24,12 @@ import inspect
 import re
 from logging import getLogger
 from pathlib import Path
-from typing import Union, List, Tuple, Optional
+from typing import Union, List, Optional
 
 from rebasehelper.helpers.macro_helper import MacroHelper
 from rebasehelper.specfile import SpecFile, RebaseHelperError, saves
+from rebasehelper.tags import Tag, Tags
 
-from packit.constants import SPEC_PACKAGE_SECTION
 from packit.patches import PatchMetadata
 
 try:
@@ -218,30 +218,13 @@ class Specfile(SpecFile):
             f"{len(source_git_patches)}/{len(patch_list)} patches added to {self.path!r}."
         )
 
-    def get_source(self, source_name: str) -> Optional[Tuple[int, str, str]]:
+    def get_source(self, source_name: str) -> Optional[Tag]:
         """
         get specific Source from spec
 
         :param source_name: precise name of the Source, e.g. Source1, or Source
-        :return: index within the section, real name of the source, whole source line
+        :return: corresponding Source Tag
         """
-        prefix = "Source"
-        self.get_main_source()
-        regex = re.compile(r"^Source\\s*:.+$")
-        spec_section = self.spec_content.section(SPEC_PACKAGE_SECTION)
-        for idx, line in enumerate(spec_section):
-            # we are looking for Source lines
-            if line.startswith(prefix):
-                # it's a Source line!
-                if line.startswith(source_name):
-                    # it even matches the specific Source\\d+
-                    full_name = source_name
-                elif regex.match(line):
-                    # okay, let's try the other very common default
-                    # https://github.com/packit/packit/issues/536#issuecomment-534074925
-                    full_name = prefix
-                else:
-                    # nope, let's continue the search
-                    continue
-                return idx, full_name, line
-        return None
+        # sanitize the name, this will also add index if there isn't one
+        source_name, *_ = Tags._sanitize_tag(source_name, 0, 0)
+        return next(self.tags.filter(name=source_name, valid=None), None)
diff --git a/packit/upstream.py b/packit/upstream.py
index 118ddaf5..0e6789b7 100644
--- a/packit/upstream.py
+++ b/packit/upstream.py
@@ -36,7 +36,7 @@ from packit.actions import ActionName
 from packit.base_git import PackitRepositoryBase
 from packit.config import Config, SyncFilesConfig
 from packit.config.common_package_config import CommonPackageConfig
-from packit.constants import SPEC_PACKAGE_SECTION, DEFAULT_ARCHIVE_EXT, DATETIME_FORMAT
+from packit.constants import DEFAULT_ARCHIVE_EXT, DATETIME_FORMAT
 from packit.exceptions import (
     PackitException,
     PackitSRPMNotFoundException,
@@ -567,10 +567,9 @@ class Upstream(PackitRepositoryBase):
     def _fix_spec_source(self, archive):
         response = self.specfile.get_source(self.package_config.spec_source_id)
         if response:
-            idx, source_name, _ = response
-            self.specfile.spec_content.section(SPEC_PACKAGE_SECTION)[
-                idx
-            ] = f"{source_name}: {archive}"
+            self.specfile.set_raw_tag_value(
+                response.name, archive, section=response.section_index
+            )
         else:
             raise PackitException(
                 "The spec file doesn't have sources set "
diff --git a/tests/integration/test_upstream.py b/tests/integration/test_upstream.py
index bfbec781..7da68932 100644
--- a/tests/integration/test_upstream.py
+++ b/tests/integration/test_upstream.py
@@ -221,6 +221,41 @@ def test_fix_spec(upstream_instance):
     assert re.match(r"\\d\\.\\d{20}\\.\\w+\\.\\d+\\.g\\w{7}", release)
 
 
+@pytest.mark.parametrize(
+    "spec_source_id, expected_line",
+    [
+        pytest.param(
+            "Source",
+            r"Source:\\s*fixed-source-archive.tar.gz",
+            id="Source",
+        ),
+        pytest.param(
+            "Source0",
+            r"Source:\\s*fixed-source-archive.tar.gz",
+            id="Source0",
+        ),
+        pytest.param(
+            "Source100",
+            r"Source100:\\s*fixed-source-archive.tar.gz",
+            id="Source100",
+        ),
+    ],
+)
+def test__fix_spec_source(upstream_instance, spec_source_id, expected_line):
+    u, ups = upstream_instance
+
+    data = u.joinpath("beer.spec").read_text()
+    data = re.sub(r"(Source:.*)", "\\\\1\\nSource100: extra-sources.tar.gz", data)
+    with open(u.joinpath("beer.spec"), "w") as f:
+        f.write(data)
+
+    ups.package_config.spec_source_id = spec_source_id
+    ups._fix_spec_source("fixed-source-archive.tar.gz")
+    ups.specfile.write_spec_content()
+
+    assert re.search(expected_line, u.joinpath("beer.spec").read_text())
+
+
 def test_create_srpm(upstream_instance, tmp_path):
     u, ups = upstream_instance