Skip to content

Fix RPM builds #578

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

Fix RPM builds #578

wants to merge 5 commits into from

Conversation

pbrezina
Copy link
Contributor

@pbrezina pbrezina commented Feb 9, 2024

SUMMARY
ISSUE TYPE
  • Bugfix Pull Request
  • Docs Pull Request
  • Feature Pull Request
ADDITIONAL INFORMATION

@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Feb 9, 2024
@pbrezina pbrezina force-pushed the rpm branch 2 times, most recently from e9af49c to 235747e Compare February 9, 2024 11:32
@packit-as-a-service Packit-as-a-Service
Copy link

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo yum install -y dnf-plugins-core on RHEL 8
  • sudo dnf install -y dnf-plugins-core on Fedora
  • dnf copr enable packit/ansible-pylibssh-578
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

@pbrezina pbrezina mentioned this pull request Feb 9, 2024
Merged
webknjaz
webknjaz reviewed Feb 13, 2024
View reviewed changes
tox.ini Outdated
SEGFAULT_SIGNALS = all
!just-pytest: PYTHONPATH = packaging/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain the motivation here? What does this affect? I want to redesign the tox setup at some point, and if I accept this change, I need to understand whether it needs to be kept later on.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's about %tox in rpm spec file. %tox sets PYTHONPATH and execute the test in current environmen instead of creating new vitual environment so the packages that are installed in buildroot are tested. However, just-pytest inherits setenv from default environment thus always overriding PYTHONPATH and therefore the test fails to import pylibsshext.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...because PYTHONPATH is packaging/ instead of site packages from buildroot.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this work?

Suggested change
!just-pytest: PYTHONPATH = packaging/
PYTHONPATH = {env:PYTHONPATH:packaging/}

Copy link
Member

@webknjaz webknjaz May 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe even

Suggested change
!just-pytest: PYTHONPATH = packaging/
PYTHONPATH = packaging/:{env:PYTHONPATH:}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing packaging/ will make the first command fail. It's prepended with - so tox will ignore its outcome. Nevertheless, it won't generate C-files with tracing information if it stops being importable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the change and updated this PR. Let's see if it works.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RPM build works.

@pbrezina
Copy link
Contributor Author

I removed commit that disabled debuginfo because I realized that it was only need for my original approach that chose off-tree tmp folder. Debuginfo is correctly collected when using in-tree tmp folder. Now testing-farm jobs fails, but it looks like an infra failure not related to this pull request.

:: [ 10:50:50 ] :: [   PASS   ] :: Command 'dnf -y copr enable packit/ansible-pylibssh-578 ' (Expected 0, got 0)
./install-and-verify.sh: line 27: rpmsort: command not found
./install-and-verify.sh: line 27: rpmsort: command not found
./install-and-verify.sh: line 27: rpmsort: command not found
:: [ 10:50:51 ] :: [  BEGIN   ] :: Running 'dnf -y install --allowerasing    '
usage: dnf install [-c [config file]] [-q] [-v] [--version]
                   [--installroot [path]] [--nodocs] [--noplugins]
                   [--enableplugin [plugin]] [--disableplugin [plugin]]
                   [--releasever RELEASEVER] [--setopt SETOPTS]
                   [--skip-broken] [-h] [--allowerasing] [-b | --nobest] [-C]
                   [-R [minutes]] [-d [debug level]] [--debugsolver]
                   [--showduplicates] [-e ERRORLEVEL] [--obsoletes]
                   [--rpmverbosity [debug level name]] [-y] [--assumeno]
                   [--enablerepo [repo]] [--disablerepo [repo] | --repo
                   [repo]] [--enable | --disable] [-x [package]]
                   [--disableexcludes [repo]] [--repofrompath [repo,path]]
                   [--noautoremove] [--nogpgcheck] [--color COLOR] [--refresh]
                   [-4] [-6] [--destdir DESTDIR] [--downloadonly]
                   [--comment COMMENT] [--bugfix] [--enhancement]
                   [--newpackage] [--security] [--advisory ADVISORY]
                   [--bz BUGZILLA] [--cve CVES]
                   [--sec-severity {Critical,Important,Moderate,Low}]
                   [--forcearch ARCH]
                   PACKAGE [PACKAGE ...]
dnf install: error: the following arguments are required: PACKAGE
:: [ 10:50:51 ] :: [   FAIL   ] :: Command 'dnf -y install --allowerasing    ' (Expected 0, got 2)
:: [ 10:50:51 ] :: [  FATAL   ] :: Failed to install al packages in the repository
:: [ 10:50:51 ] :: [   FAIL   ] :: Failed to install al packages in the repository (Assert: expected 0, got 1)

@pbrezina pbrezina force-pushed the rpm branch 3 times, most recently from 809a52a to 03b4718 Compare February 16, 2024 09:00
@pbrezina
Copy link
Contributor Author

Ok, testing-farm issue is fixed by https://gitlab.com/testing-farm/tests/-/merge_requests/35

@pbrezina pbrezina force-pushed the rpm branch 2 times, most recently from 470e729 to 44cf3fc Compare February 21, 2024 11:04
@trishnaguha
Copy link
Member

@NilashishC Please take a look.

webknjaz
webknjaz reviewed May 28, 2024
View reviewed changes
packaging/rpm/ansible-pylibssh.spec
@@ -62,6 +62,8 @@ Source24: %{pypi_source tomli 1.2.3}
BuildRequires: openssh
# sshd?
BuildRequires: openssh-server
# ssh?
BuildRequires: openssh-clients
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remind me why this is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, because 376fc52

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

376fc52#diff-9417337bb13869644ad69cf16fc956100d4729cb2e25809e4a48cc86462760dbR48

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, right… I forgot you refactored that. Thanks for reminding!

webknjaz
webknjaz reviewed May 29, 2024
View reviewed changes
pbrezina added 3 commits May 30, 2024 10:24
@pbrezina
tox: pass environment PYTHONPATH
fe05e44 
This is needed during RPM build phase where we want to use packages
from buildroot environment.
@pbrezina
spec: require openssh-clients
ad1bd3c 
ssh is now used in tests to ensure that sshd is ready to receive
connectoins.
@pbrezina
docs: add changelog fragments
85e898e
webknjaz
webknjaz reviewed May 31, 2024
View reviewed changes
docs/changelog-fragments/578.packaging.rst
@@ -0,0 +1 @@
Fixed issues that prevented RPM from building -- by :user:`pbrezina`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW this note is not very informative to the end users. It'd need to be written in a different style.

@webknjaz
Copy link
Member

I absorbed this all into devel in various forms. There are still failing RPM jobs in GHA that aren't fixed by this.

@webknjaz webknjaz closed this May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@webknjaz webknjaz webknjaz left review comments

Assignees
No one assigned
Labels
bot:chronographer:provided There is a change note present in this PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pbrezina @trishnaguha @webknjaz