Repository: packit/packit
Issue: SRPM cannot be created in Copr when packaging files are in a subdir

TODO:
* [ ] For SRPM builds in Copr, replace archive symlinks with actual files (for example, as another step after running `packit prepare-sources`
* [ ] make sure sandcastle SRPM builds still work

Thanks! I retried it in https://github.com/cockpit-project/cockpit-podman/pull/1016 and it [fails differently](https://copr-be.cloud.fedoraproject.org/results/packit/cockpit-project-cockpit-podman-1016/srpm-builds/04550455/builder-live.log.gz) now:

    ```
    2022-06-21 10:28:25.695 upstream.py       INFO   Created archive:
    2022-06-21 10:28:25.695 upstream.py       INFO   	parsed   path: /tmp/tmpueaiwen2/cockpit-podman-49.1.10.gd95b5ed.tar.xz
    2022-06-21 10:28:25.696 upstream.py       INFO   	absolute path: /tmp/tmpueaiwen2/cockpit-podman-49.1.10.gd95b5ed.tar.xz
    2022-06-21 10:28:25.696 upstream.py       INFO   Linking to the specfile directory: /tmp/tmpueaiwen2/packaging/cockpit-podman-49.1.10.gd95b5ed.tar.xz -> ../cockpit-podman-49.1.10.gd95b5ed.tar.xz (given path to archive: /tmp/tmpueaiwen2/cockpit-podman-49.1.10.gd95b5ed.tar.xz)
    [...]
    
    cmd: ['rpmbuild', '-bs', '--define', '_sourcedir /var/lib/copr-rpmbuild/workspace/workdir-nfw3ragk/srcdir', '--define', '_rpmdir /var/lib/copr-rpmbuild/workspace/workdir-nfw3ragk/srcdir', '--define', '_builddir /var/lib/copr-rpmbuild/workspace/workdir-nfw3ragk/srcdir', '--define', '_specdir /var/lib/copr-rpmbuild/workspace/workdir-nfw3ragk/srcdir', '--define', '_srcrpmdir /var/lib/copr-rpmbuild/results', '/var/lib/copr-rpmbuild/workspace/workdir-nfw3ragk/srcdir/cockpit-podman.spec']
    cwd: .
    rc: 1
    stdout: 
    stderr: error: File /var/lib/copr-rpmbuild/workspace/workdir-nfw3ragk/srcdir/cockpit-podman-49.1.10.gd95b5ed.tar.xz: No such file or directory
    ```
    
    New issue?

_Originally posted by @martinpitt in https://github.com/packit/packit/issues/1621#issuecomment-1161563439_

---

I tried to reproduce locally but all seems to work well:
```
...
2022-06-22 14:11:39.395 logging.py        INFO   tar --xz -cf cockpit-podman-49.1.10.gd95b5ed.tar.xz --transform 's,^,cockpit-podman/,' \\
2022-06-22 14:11:39.395 logging.py        INFO          --transform 's,cockpit-podman.spec,packaging/cockpit-podman.spec,' \\
2022-06-22 14:11:39.395 logging.py        INFO          --exclude '*.in' --exclude test/reference --exclude node_modules --exclude packaging/cockpit-podman.spec \\
2022-06-22 14:11:39.395 logging.py        INFO          $(git ls-files) pkg/lib test/common tools/git-utils.sh tools/make-bots tools/node-modules  package-lock.json cockpit-podman.spec packaging/arch/PKGBUILD packaging/debian/changelog
dist/
2022-06-22 14:11:40.061 logging.py        INFO   rm cockpit-podman.spec
2022-06-22 14:11:40.062 logging.py        INFO   cockpit-podman-49.1.10.gd95b5ed.tar.xz
2022-06-22 14:11:40.063 upstream.py       INFO   Created archive:
2022-06-22 14:11:40.063 upstream.py       INFO          parsed   path: /home/tt/g/cockpit-project/cockpit-podman/cockpit-podman-49.1.10.gd95b5ed.tar.xz
2022-06-22 14:11:40.063 upstream.py       INFO          absolute path: /home/tt/g/cockpit-project/cockpit-podman/cockpit-podman-49.1.10.gd95b5ed.tar.xz
2022-06-22 14:11:40.063 upstream.py       INFO   Linking to the specfile directory: /home/tt/g/cockpit-project/cockpit-podman/packaging/cockpit-podman-49.1.10.gd95b5ed.tar.xz -> ../cockpit-podman-49.1.10.gd95b5ed.tar.xz (given path to a
rchive: /home/tt/g/cockpit-project/cockpit-podman/cockpit-podman-49.1.10.gd95b5ed.tar.xz)
2022-06-22 14:11:40.595 upstream.py       INFO   Version in upstream registries is '49.1'.
2022-06-22 14:11:40.596 upstream.py       INFO   Version in spec file is '0'.
2022-06-22 14:11:40.596 upstream.py       WARNING Version '0' in spec file is outdated.
2022-06-22 14:11:40.596 upstream.py       INFO   Picking version '49.1' from upstream registry.
2022-06-22 12:11:40.993 api.py            INFO   Directory with sources: /home/tt/g/cockpit-project/cockpit-podman/prepare_sources_result
```

Archives:
```
$ ll prepare_sources_result
total 8.0K
drwxr-xr-x. 1 tt tt   38 jún 22 14:11 arch
lrwxrwxrwx. 1 tt tt   41 jún 22 14:11 cockpit-podman-49.1.10.gd95b5ed.tar.xz -> ../cockpit-podman-49.1.10.gd95b5ed.tar.xz
-rw-r--r--. 1 tt tt 2.4K jún 22 14:11 cockpit-podman.spec
drwxr-xr-x. 1 tt tt  106 jún 22 14:11 debian

$ ll packaging
total 8.0K
drwxr-xr-x. 1 tt tt   38 jún 22 14:11 arch
lrwxrwxrwx. 1 tt tt   41 jún 22 14:11 cockpit-podman-49.1.10.gd95b5ed.tar.xz -> ../cockpit-podman-49.1.10.gd95b5ed.tar.xz
-rw-r--r--. 1 tt tt 2.4K jún 22 14:11 cockpit-podman.spec
drwxr-xr-x. 1 tt tt  106 jún 22 14:11 debian

$ ll packaging/cockpit-podman-49.1.10.gd95b5ed.tar.xz
lrwxrwxrwx. 1 tt tt 41 jún 22 14:11 packaging/cockpit-podman-49.1.10.gd95b5ed.tar.xz -> ../cockpit-podman-49.1.10.gd95b5ed.tar.xz
```

Changed files:
tests/unit/test_utils.py, packit/utils/source_script.py, packit/cli/prepare_sources.py, packit/api.py, tests/integration/test_upstream.py, packit/upstream.py

diff --git a/packit/api.py b/packit/api.py
index e499f8ac..af2547d8 100644
--- a/packit/api.py
+++ b/packit/api.py
@@ -1219,6 +1219,7 @@ The first dist-git commit to be synced is '{short_hash}'.
         bump_version: bool = True,
         release_suffix: Optional[str] = None,
         result_dir: Union[Path, str] = None,
+        create_symlinks: Optional[bool] = True,
     ) -> None:
         """
         Prepare sources for an SRPM build.
@@ -1228,6 +1229,9 @@ The first dist-git commit to be synced is '{short_hash}'.
             release_suffix: specifies local release suffix. `None` represents default suffix.
             bump_version: specifies whether version should be changed in the spec-file.
             result_dir: directory where the specfile directory content should be copied
+            create_symlinks: whether symlinks should be created instead of copying the files
+                (currently when the archive is created outside the specfile dir, or in the future
+                 if we will create symlinks in some other places)
         """
         self.up.run_action(actions=ActionName.post_upstream_clone)
 
@@ -1236,6 +1240,7 @@ The first dist-git commit to be synced is '{short_hash}'.
                 upstream_ref=upstream_ref,
                 bump_version=bump_version,
                 release_suffix=release_suffix,
+                create_symlinks=create_symlinks,
             )
         except Exception as ex:
             raise PackitSRPMException(
diff --git a/packit/cli/prepare_sources.py b/packit/cli/prepare_sources.py
index d7015887..afe99b90 100644
--- a/packit/cli/prepare_sources.py
+++ b/packit/cli/prepare_sources.py
@@ -90,6 +90,13 @@ def load_job_config(job_config):
     type=click.STRING,
     help="Specifies target branch which PR should be merged into.",
 )
+@click.option(
+    "--create-symlinks/--no-create-symlinks",
+    is_flag=True,
+    default=True,
+    help="Specifies whether Packit should create symlinks or copy the "
+    "files (e.g. archive outside specfile dir).",
+)
 @click.argument(
     "path_or_url",
     type=LocalProjectParameter(
@@ -115,6 +122,7 @@ def prepare_sources(
     pr_id,
     merge_pr,
     target_branch,
+    create_symlinks,
 ):
     """
     Prepare sources for a new SRPM build using content of the upstream repository.
@@ -142,4 +150,5 @@ def prepare_sources(
         bump_version=bump,
         release_suffix=release_suffix,
         result_dir=result_dir,
+        create_symlinks=create_symlinks,
     )
diff --git a/packit/upstream.py b/packit/upstream.py
index 17f34e4a..cf265174 100644
--- a/packit/upstream.py
+++ b/packit/upstream.py
@@ -280,8 +280,10 @@ class Upstream(PackitRepositoryBase):
 
         return version
 
-    def create_archive(self, version: str = None) -> str:
-        return Archive(self, version).create()
+    def create_archive(
+        self, version: str = None, create_symlink: Optional[bool] = True
+    ) -> str:
+        return Archive(self, version).create(create_symlink=create_symlink)
 
     def get_last_tag(self, before: str = None) -> Optional[str]:
         """
@@ -509,6 +511,7 @@ class Upstream(PackitRepositoryBase):
         upstream_ref: Optional[str] = None,
         bump_version: bool = True,
         release_suffix: Optional[str] = None,
+        create_symlinks: Optional[bool] = True,
     ):
         """
         1. determine version
@@ -520,9 +523,13 @@ class Upstream(PackitRepositoryBase):
             upstream_ref: the base git ref for the source git
             bump_version: increase version in spec file
             release_suffix: suffix %release part of NVR with this
+            create_symlinks: whether symlinks should be created instead of copying the files
+                (e.g. when the archive is created outside the specfile dir)
         """
         SRPMBuilder(upstream=self, ref=upstream_ref).prepare(
-            bump_version=bump_version, release_suffix=release_suffix
+            bump_version=bump_version,
+            release_suffix=release_suffix,
+            create_symlinks=create_symlinks,
         )
 
     def create_patches_and_update_specfile(self, upstream_ref) -> None:
@@ -968,12 +975,18 @@ class SRPMBuilder:
             )
         self.upstream.specfile.reload()  # the specfile could have been changed by the action
 
-    def prepare(self, bump_version: bool, release_suffix: Optional[str] = None):
+    def prepare(
+        self,
+        bump_version: bool,
+        release_suffix: Optional[str] = None,
+        create_symlinks: Optional[bool] = True,
+    ):
         if self.upstream_ref:
             self._prepare_upstream_using_source_git(bump_version, release_suffix)
         else:
             created_archive = self.upstream.create_archive(
-                version=self.current_git_describe_version
+                version=self.current_git_describe_version,
+                create_symlink=create_symlinks,
             )
             self._fix_specfile_to_use_local_archive(
                 archive=created_archive,
@@ -1017,13 +1030,17 @@ class Archive:
             self._version = self.upstream.get_current_version()
         return self._version
 
-    def create(self) -> str:
+    def create(self, create_symlink: Optional[bool] = True) -> str:
         """
         Create archive from the content of the upstream repository, only committed
         changes are present in the archive.
 
         Uses `git archive` by default, unless `create_archive` action is defined.
 
+        Args:
+            create_symlink: whether symlink to archive should be created when the
+                created archive is outside the specfile dir or the archive should be copied
+
         Returns:
             Name of the archive.
         """
@@ -1051,7 +1068,7 @@ class Archive:
                     "The create-archive action did not output a path to the generated archive. "
                     "Please make sure that you have valid path in the single line of the output."
                 )
-            self._add_link_to_archive_from_specdir_if_needed(archive_path)
+            self._handle_archive_outside_specdir_if_needed(archive_path, create_symlink)
             return archive_path.name
 
         return self._create_archive_using_default_way(dir_name, env)
@@ -1098,7 +1115,7 @@ class Archive:
             outputs: Outputs produced by creating the archive.
 
         Returns:
-            Path to the archive if found, `None` otherwise.
+            Absolute path to the archive if found, `None` otherwise.
         """
         for output in reversed(outputs):
             for line in reversed(output.splitlines()):
@@ -1107,31 +1124,44 @@ class Archive:
                     return archive_path
         return None
 
-    def _add_link_to_archive_from_specdir_if_needed(self, archive_path: Path) -> None:
+    def _handle_archive_outside_specdir_if_needed(
+        self, archive_path: Path, create_symlink: Optional[bool] = True
+    ) -> None:
         """
         Create a relative symlink to the archive from in the specfile directory
-        if necessary.
+        or copy the archive to the specfile directory if necessary.
 
         Args:
-            archive_path: Relative path to the archive from the specfile dir.
+            archive_path: Absolute path to the archive from the specfile dir.
+            create_symlink: Whether a symlink should be created.
         """
         absolute_specfile_dir = self.upstream.absolute_specfile_dir
 
         if archive_path.parent.absolute() != absolute_specfile_dir:
             archive_in_spec_dir = absolute_specfile_dir / archive_path.name
 
-            # [PurePath.relative_to()](https://docs.python.org/3/library/pathlib.html#pathlib.PurePath.relative_to)
-            # requires self to be the subpath of the argument, but
-            # [os.path.relpath()](https://docs.python.org/3/library/os.path.html#os.path.relpath)
-            # does not.
-            relative_archive_path = os.path.relpath(archive_path, absolute_specfile_dir)
+            if create_symlink:
+                # [PurePath.relative_to()](https://docs.python.org/3/library/pathlib.html#pathlib.PurePath.relative_to)
+                # requires self to be the subpath of the argument, but
+                # [os.path.relpath()](https://docs.python.org/3/library/os.path.html#os.path.relpath)
+                # does not.
+                relative_archive_path = os.path.relpath(
+                    archive_path, absolute_specfile_dir
+                )
 
-            logger.info(
-                "Linking to the specfile directory:"
-                f" {archive_in_spec_dir} -> {relative_archive_path}"
-                f" (given path to archive: {archive_path})"
-            )
-            archive_in_spec_dir.symlink_to(relative_archive_path)
+                logger.info(
+                    "Linking to the specfile directory:"
+                    f" {archive_in_spec_dir} -> {relative_archive_path}"
+                    f" (given path to archive: {archive_path})"
+                )
+                archive_in_spec_dir.symlink_to(relative_archive_path)
+
+            else:
+                logger.info(
+                    "Copying the archive to the specfile directory: "
+                    f"{archive_path} -> {archive_in_spec_dir}"
+                )
+                shutil.copy2(archive_path, archive_in_spec_dir)
 
     def _create_archive_using_default_way(
         self, dir_name: str, env: Dict[str, str]
diff --git a/packit/utils/source_script.py b/packit/utils/source_script.py
index db09a12b..3aab5ffa 100644
--- a/packit/utils/source_script.py
+++ b/packit/utils/source_script.py
@@ -27,5 +27,8 @@ def create_source_script(
     if release_suffix:
         options += ["--release-suffix", f"'{release_suffix}'"]
 
+    # do not create symlinks in Copr environment
+    options += ["--no-create-symlinks"]
+
     options += [url]
     return COPR_SOURCE_SCRIPT.format(options=" ".join(options))
diff --git a/tests/integration/test_upstream.py b/tests/integration/test_upstream.py
index 4992b2b2..72df4b9b 100644
--- a/tests/integration/test_upstream.py
+++ b/tests/integration/test_upstream.py
@@ -236,8 +236,11 @@ def test_create_archive_spec_subdir(upstream_instance, with_create_archive_actio
 
     ups.create_archive()
 
-    assert len(list(packaging_dir.glob("*.tar.gz"))) == 1
+    archives_in_packaging_dir = list(packaging_dir.glob("*.tar.gz"))
+    assert len(archives_in_packaging_dir) == 1
+
     if with_create_archive_action:
+        assert archives_in_packaging_dir[0].is_symlink()
         assert len(list(ups.local_project.working_dir.glob("*.tar.gz"))) == 1
 
 
@@ -404,3 +407,35 @@ def test_get_archive_root_dir(template, expected_output, upstream_instance):
     archive_root_dir = Archive(ups, ups.get_version()).get_archive_root_dir(archive)
 
     assert archive_root_dir == expected_output
+
+
+def test_create_archive_not_create_symlink(upstream_instance):
+    """test archive creation when an archive is in root, but spec is in a subdir
+    and create_symlink=False"""
+    u, ups = upstream_instance
+
+    archive_path = u.joinpath("beer.tar.gz")
+    ups.package_config.actions[ActionName.create_archive] = [
+        [
+            "git",
+            "archive",
+            "--output",
+            str(archive_path),
+            "--prefix",
+            f"{u.name}/",
+            "HEAD",
+        ],
+        ["echo", str(archive_path)],
+    ]
+
+    packaging_dir = u / "packaging"
+    packaging_dir.mkdir()
+    spec_path = packaging_dir / "some.spec"
+    spec_path.write_text("asd")
+    ups._specfile_path = spec_path
+
+    ups.create_archive(create_symlink=False)
+
+    archives = list(packaging_dir.glob("*.tar.gz"))
+    assert len(archives) == 1
+    assert not archives[0].is_symlink()
diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py
index 414e32d1..6cf729a6 100644
--- a/tests/unit/test_utils.py
+++ b/tests/unit/test_utils.py
@@ -391,7 +391,8 @@ def test_get_message_from_metadata(metadata, header, result):
             None,
             None,
             "https://github.com/packit/ogr",
-            'packit -d prepare-sources --result-dir "$resultdir" https://github.com/packit/ogr',
+            'packit -d prepare-sources --result-dir "$resultdir" --no-create-symlinks '
+            "https://github.com/packit/ogr",
         ),
         (
             "123",
@@ -400,7 +401,7 @@ def test_get_message_from_metadata(metadata, header, result):
             None,
             None,
             "https://github.com/packit/ogr",
-            'packit -d prepare-sources --result-dir "$resultdir" --ref 123 '
+            'packit -d prepare-sources --result-dir "$resultdir" --ref 123 --no-create-symlinks '
             "https://github.com/packit/ogr",
         ),
         (
@@ -411,7 +412,7 @@ def test_get_message_from_metadata(metadata, header, result):
             None,
             "https://github.com/packit/ogr",
             'packit -d prepare-sources --result-dir "$resultdir" --pr-id 1 '
-            "--no-merge-pr https://github.com/packit/ogr",
+            "--no-merge-pr --no-create-symlinks https://github.com/packit/ogr",
         ),
         (
             None,
@@ -421,7 +422,7 @@ def test_get_message_from_metadata(metadata, header, result):
             None,
             "https://github.com/packit/ogr",
             'packit -d prepare-sources --result-dir "$resultdir" --pr-id 1 '
-            "--merge-pr --target-branch main https://github.com/packit/ogr",
+            "--merge-pr --target-branch main --no-create-symlinks https://github.com/packit/ogr",
         ),
         (
             None,
@@ -431,7 +432,8 @@ def test_get_message_from_metadata(metadata, header, result):
             0,
             "https://github.com/packit/ogr",
             'packit -d prepare-sources --result-dir "$resultdir" --pr-id 1 '
-            "--merge-pr --target-branch main --job-config-index 0 https://github.com/packit/ogr",
+            "--merge-pr --target-branch main --job-config-index 0 "
+            "--no-create-symlinks https://github.com/packit/ogr",
         ),
     ],
 )