Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

srpm fails when spec file is not in the project root dir and there is a custom create-archive: action #1621

Closed
martinpitt opened this issue Jun 14, 2022 · 5 comments · Fixed by #1622
Assignees
Labels
kind/bug Something isn't working.

Comments

@martinpitt
Copy link

martinpitt commented Jun 14, 2022

I've been scratching my head in cockpit-project/cockpit-podman#1016 and cockpit-project/cockpit#17450 about some packit config cleanups which we want to do. Part of it is that they stop dynamically creating a spec file in the project root directory, and move to a valid spec file in packaging/projectname.spec. We really don't want to keep the .spec in the root dir, that's just clutter -- it should go together with debian, arch, etc. packaging.

What I am seeing can be reproduced with https://github.com/martinpitt/python-dbusmock/ , which is a fairly simple and standard (wrt. packit config). With a pristine checkout, packit srpm works fine. But when I add a custom create-archive:

--- packit.yaml
+++ packit.yaml
@@ -12,6 +12,10 @@ files_to_sync:
   - src: packaging/python-dbusmock.spec
     dest: python-dbusmock.spec
 
+actions:
+  create-archive:
+    - sh -exc 'git archive HEAD | xz > python-dbusmock-$(git describe).tar.xz; ls python-dbusmock-*.tar.xz'
+
 jobs:
   - job: copr_build
     trigger: pull_request

Then packit srpm fails the same way as I saw in these (much more complex) PRs:

2022-06-14 13:53:13.818 schema.py         WARNING The 'metadata' key in jobs is deprecated and can be removed.Nest config options from 'metadata' directly under the job object.
2022-06-14 13:53:13.818 schema.py         WARNING The 'metadata' key in jobs is deprecated and can be removed.Nest config options from 'metadata' directly under the job object.
2022-06-14 13:53:13.818 schema.py         WARNING The 'metadata' key in jobs is deprecated and can be removed.Nest config options from 'metadata' directly under the job object.
2022-06-14 13:53:13.818 schema.py         WARNING The 'metadata' key in jobs is deprecated and can be removed.Nest config options from 'metadata' directly under the job object.
2022-06-14 13:53:13.827 base_git.py       INFO   Using user-defined script for ActionName.create_archive: [['sh', '-exc', 'git archive HEAD > python-dbusmock-$(git describe).tar.xz; ls python-dbusmock-*.tar.xz']]
2022-06-14 13:53:13.829 logging.py        INFO   + git archive HEAD
2022-06-14 13:53:13.829 logging.py        INFO   ++ git describe
2022-06-14 13:53:13.836 logging.py        INFO   + ls python-dbusmock-0.27.5-8-g203cfe6.tar.xz
2022-06-14 13:53:13.836 logging.py        INFO   python-dbusmock-0.27.5-8-g203cfe6.tar.xz
2022-06-14 13:53:13.837 upstream.py       INFO   Created archive:
2022-06-14 13:53:13.837 upstream.py       INFO   	parsed   path: /var/home/martin/upstream/python-dbusmock/python-dbusmock-0.27.5-8-g203cfe6.tar.xz
2022-06-14 13:53:13.837 upstream.py       INFO   	absolute path: /var/home/martin/upstream/python-dbusmock/python-dbusmock-0.27.5-8-g203cfe6.tar.xz
2022-06-14 13:53:13.837 command_handler.py INFO   Nothing to clean.
2022-06-14 13:53:13.837 utils.py          ERROR  Preparation of the repository for creation of an SRPM failed: '/var/home/martin/upstream/python-dbusmock/python-dbusmock-0.27.5-8-g203cfe6.tar.xz' is not in the subpath of '/var/home/martin/upstream/python-dbusmock/packaging' OR one path is relative and the other is absolute.

This doesn't make sense to me -- why would it require the spec file to be in the root dir? The docs explicitly say it can be somewhere else, and without create-archive that works fine.

@martinpitt
Copy link
Author

martinpitt commented Jun 14, 2022

One workaround which is arguably a bit weird is to build the tarball in packaging/ subdirectory:

--- packit.yaml
+++ packit.yaml
@@ -12,6 +12,10 @@ files_to_sync:
   - src: packaging/python-dbusmock.spec
     dest: python-dbusmock.spec
 
+actions:
+  create-archive:
+    - sh -exc 'git archive HEAD | xz > packaging/python-dbusmock-$(git describe).tar.xz; ls packaging/python-dbusmock-*.tar.xz'
+
 jobs:
   - job: copr_build
     trigger: pull_request

However, while that is easy in this reproducer, it's much harder in real projects where you call make dist -- one would have to explicitly mv it.

martinpitt added a commit to martinpitt/cockpit that referenced this issue Jun 14, 2022
Keep `Version: 0` in the spec file. This works fine with `autogen rpm`, `dnf
builddep, and Packit. It was the last macro, so now it's a real working .spec.
This avoids having to invoke this script all the time.

This simplifies our packit config a bit and stops swimming against the stream.

The only place where we want a real version is `make dist`, so that built RPMs
have a correct version. Rework the rule to modify the file in-place in distdir.

Unfortunately this uncovers another packit bug: If the .spec file is not
in the same directory as the tarball, `packit srpm` gets upset:
<packit/packit#1621>. Work around that by
copying the spec to the root directory, and pointing to it.
@TomasTomecek TomasTomecek added the kind/bug Something isn't working. label Jun 15, 2022
martinpitt added a commit to martinpitt/cockpit that referenced this issue Jun 15, 2022
Keep `Version: 0` in the spec file. This works fine with `autogen rpm`, `dnf
builddep, and Packit. It was the last macro, so now it's a real working .spec.
This avoids having to invoke this script all the time.

This simplifies our packit config a bit and stops swimming against the stream.

The only place where we want a real version is `make dist`, so that built RPMs
have a correct version. Rework the rule to modify the file in-place in distdir.

Unfortunately this uncovers another packit bug: If the .spec file is not
in the same directory as the tarball, `packit srpm` gets upset:
<packit/packit#1621>. Work around that by
copying the spec to the root directory, and pointing to it.
@TomasTomecek TomasTomecek self-assigned this Jun 15, 2022
@TomasTomecek
Copy link
Member

Found the problem @_@

it's one-liner

the function we are using for computing relative paths cannot work with subdirs; python has another that can - seriously...

TomasTomecek added a commit to TomasTomecek/packit that referenced this issue Jun 15, 2022
`Path.relative_to` can raise `ValueError` when `specfile_dir` is in a
subdir and archive path is in the project's root. This commit uses
`os.path.relpath` instead that supports having the source path in a
subdir. It's a different implementation than `.relative_to`.

Fixes: packit#1621

Signed-off-by: Tomas Tomecek <ttomecek@redhat.com>
martinpitt added a commit to cockpit-project/cockpit that referenced this issue Jun 15, 2022
Keep `Version: 0` in the spec file. This works fine with `autogen rpm`, `dnf
builddep, and Packit. It was the last macro, so now it's a real working .spec.
This avoids having to invoke this script all the time.

This simplifies our packit config a bit and stops swimming against the stream.

The only place where we want a real version is `make dist`, so that built RPMs
have a correct version. Rework the rule to modify the file in-place in distdir.

Unfortunately this uncovers another packit bug: If the .spec file is not
in the same directory as the tarball, `packit srpm` gets upset:
<packit/packit#1621>. Work around that by
copying the spec to the root directory, and pointing to it.
TomasTomecek added a commit to TomasTomecek/packit that referenced this issue Jun 16, 2022
[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.

Fixes: packit#1621

Signed-off-by: Tomas Tomecek <ttomecek@redhat.com>
Co-Authored-by: Hunor Csomortáni <csomh@redhat.com>
softwarefactory-project-zuul bot added a commit that referenced this issue Jun 17, 2022
allow having custom archives in root while specfile is in a subdirectory

TODO:


 Write new tests or update the old ones to cover new functionality.
Path.relative_to can raise ValueError when specfile_dir is in a
subdir and archive path is in the project's root. This commit uses
os.path.relpath instead that supports having the source path in a
subdir. It's a different implementation than .relative_to.


Fixes: #1621
RELEASE NOTES BEGIN
Packit now correctly handles creation of custom archives in root while a specfile is in a subdirectory.
RELEASE NOTES END

Reviewed-by: Jiri Popelka <None>
Reviewed-by: Hunor Csomortáni <csomh@redhat.com>
Reviewed-by: Tomas Tomecek <tomas@tomecek.net>
@martinpitt
Copy link
Author

Thanks! I retried it in cockpit-project/cockpit-podman#1016 and it fails differently 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?

@TomasTomecek
Copy link
Member

I'll have a look tomorrow.

@TomasTomecek
Copy link
Member

I'm unable to reproduce locally, so let's track it as a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants