Bug 2120661 - Review Request: dnf5 - package management library
Summary: Review Request: dnf5 - package management library
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Miroslav Suchý
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2022-08-23 13:40 UTC by Nicola Sella
Modified: 2023-10-08 08:14 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2023-10-08 08:14:07 UTC
Type: ---
Embargoed:
msuchy: fedora-review+


Attachments (Terms of Use)

Description Nicola Sella 2022-08-23 13:40:20 UTC
Spec URL: https://raw.githubusercontent.com/rpm-software-management/dnf5/dnf5-release-fedora/dnf5.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/rpmsoftwaremanagement/dnf5-src-fedora/fedora-rawhide-x86_64/04760250-dnf5/dnf5-5.0.0-0~pre.fc38.src.rpm
Description: DNF5 is a command-line package manager that automates the process of installing, upgrading, configuring, and removing computer programs in a consistent manner. It supports RPM packages, modulemd modules, and comps groups & environments.
Fedora Account System Username: rpmsoftwaremanagement

Edit: due to ho we perform nightly builds, there was a mismatch between the dnf5.spec and specfile inside srpm (which was from nightliy repo). Now the links are fixed.

Comment 1 Fabio Valentini 2022-08-23 13:50:59 UTC
The linked .spec file doesn't appear to match up with the linked SRPM file. Please make sure they correspond to each other.

Comment 2 Nicola Sella 2022-08-23 14:06:25 UTC
Hi, here's an updated link.
SRPM URL: https://download.copr.fedorainfracloud.org/results/rpmsoftwaremanagement/dnf5-src-fedora/fedora-rawhide-x86_64/04760250-dnf5/dnf5-5.0.0-0~pre.fc38.src.rpm

A note:
the .spec file was slightly different due to our automation used to build nightly. Since it fetches the .spec file directly upstream I didn't think about the possibility of it being different. Sorry about it, should be good now.

Comment 3 Miroslav Suchý 2022-09-16 13:31:14 UTC
Taking.
I do some preliminary checks and give Nicola comments in a video call.

Comment 5 Miroslav Suchý 2022-09-21 12:28:40 UTC
[ ]: Package requires other packages for directories it uses.
     Note: No known owner of /usr/lib64/libdnf5/plugins, /usr/lib64/libdnf5
[ ]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/share/dbus-1/system-
     services, /usr/share/polkit-1/actions, /usr/lib64/libdnf5/plugins,
     /usr/share/dbus-1/interfaces, /usr/share/polkit-1, /etc/dbus-1,
     /usr/lib64/libdnf5, /usr/share/dbus-1, /etc/dbus-1/system.d

You either should own these directories or require some package that owns them.

- Package must not depend on deprecated() packages.
  Note: openssl1.1-devel is deprecated, you must not depend on it.
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/deprecating-packages/

- Package does not contain duplicates in %files.                                                                                                                                      
  Note: warning: File listed twice: /usr/lib/python3.11/site-
  packages/libdnf_plugins/README
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/#_duplicate_files

This is caused by

%{python3_sitelib}/libdnf_plugins/
%doc %{python3_sitelib}/libdnf_plugins/README  

because REDAME is the only file in this directory, just put %dir there. I.e.:

%dir %{python3_sitelib}/libdnf_plugins/
%doc %{python3_sitelib}/libdnf_plugins/README 


Several Summary fields ends with dot. Summary should not end with dot.
Summary should start with capital character. E.g., dnf5-plugin start with lowercase.


/etc/dnf/dnf5-aliases.d/README should be marked as %doc

I am worried about rpmlint error
  dnf5-debuginfo.x86_64: E: missing-PT_GNU_STACK-section /usr/lib/debug/.dwz/dnf5-5.0.0-0~pre.fc38.x86_64
but I have no idea what it means.

This as well
dnf5daemon-server.x86_64: E: dbus-policy-allow-without-destination <allow send_interface="org.rpm.dnf.v0"/> /etc/dbus-1/system.d/org.rpm.dnf.v0.conf
dnf5daemon-server.x86_64: E: dbus-policy-allow-without-destination <allow send_interface="org.rpm.dnf.v0"/> /etc/dbus-1/system.d/org.rpm.dnf.v0.conf

Unversioned so-files
--------------------
perl5-libdnf5: /usr/lib64/perl5/vendor_perl/auto/libdnf5/base/base.so
perl5-libdnf5: /usr/lib64/perl5/vendor_perl/auto/libdnf5/common/common.so
perl5-libdnf5: /usr/lib64/perl5/vendor_perl/auto/libdnf5/comps/comps.so
perl5-libdnf5: /usr/lib64/perl5/vendor_perl/auto/libdnf5/conf/conf.so
perl5-libdnf5: /usr/lib64/perl5/vendor_perl/auto/libdnf5/logger/logger.so
perl5-libdnf5: /usr/lib64/perl5/vendor_perl/auto/libdnf5/plugin/plugin.so
perl5-libdnf5: /usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so
perl5-libdnf5: /usr/lib64/perl5/vendor_perl/auto/libdnf5/rpm/rpm.so
perl5-libdnf5: /usr/lib64/perl5/vendor_perl/auto/libdnf5/transaction/transaction.so
perl5-libdnf5-cli: /usr/lib64/perl5/vendor_perl/auto/libdnf5_cli/progressbar/progressbar.so
python3-libdnf5: /usr/lib64/python3.11/site-packages/libdnf5/_base.so
python3-libdnf5: /usr/lib64/python3.11/site-packages/libdnf5/_common.so
python3-libdnf5: /usr/lib64/python3.11/site-packages/libdnf5/_comps.so
python3-libdnf5: /usr/lib64/python3.11/site-packages/libdnf5/_conf.so
python3-libdnf5: /usr/lib64/python3.11/site-packages/libdnf5/_logger.so
python3-libdnf5: /usr/lib64/python3.11/site-packages/libdnf5/_plugin.so
python3-libdnf5: /usr/lib64/python3.11/site-packages/libdnf5/_repo.so
python3-libdnf5: /usr/lib64/python3.11/site-packages/libdnf5/_rpm.so
python3-libdnf5: /usr/lib64/python3.11/site-packages/libdnf5/_transaction.so
python3-libdnf5-cli: /usr/lib64/python3.11/site-packages/libdnf5_cli/_progressbar.so
ruby-libdnf5: /usr/lib64/ruby/vendor_ruby/libdnf5/base.so
ruby-libdnf5: /usr/lib64/ruby/vendor_ruby/libdnf5/common.so
ruby-libdnf5: /usr/lib64/ruby/vendor_ruby/libdnf5/comps.so
ruby-libdnf5: /usr/lib64/ruby/vendor_ruby/libdnf5/conf.so
ruby-libdnf5: /usr/lib64/ruby/vendor_ruby/libdnf5/logger.so
ruby-libdnf5: /usr/lib64/ruby/vendor_ruby/libdnf5/plugin.so
ruby-libdnf5: /usr/lib64/ruby/vendor_ruby/libdnf5/repo.so
ruby-libdnf5: /usr/lib64/ruby/vendor_ruby/libdnf5/rpm.so
ruby-libdnf5: /usr/lib64/ruby/vendor_ruby/libdnf5/transaction.so
ruby-libdnf5-cli: /usr/lib64/ruby/vendor_ruby/libdnf5_cli/progressbar.so
libdnf5-plugin-actions: /usr/lib64/libdnf5/plugins/actions.so
python3-libdnf5-python-plugins-loader: /usr/lib64/libdnf5/plugins/python_plugins_loader.so                                                                                            
dnf5-plugins: /usr/lib64/dnf5/plugins/builddep_cmd_plugin.so
dnf5-plugins: /usr/lib64/dnf5/plugins/changelog_cmd_plugin.so

See https://stackoverflow.com/questions/7553184/how-to-do-versioning-of-shared-library
This is something you should really do.
This will also likely fix bunch or rpmlint errors
perl5-libdnf5.x86_64: W: undefined-non-weak-symbol /usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so ....

Comment 6 Petr Pisar 2022-10-03 12:58:10 UTC
(In reply to Miroslav Suchý from comment #5)
> - Package must not depend on deprecated() packages.
>   Note: openssl1.1-devel is deprecated, you must not depend on it.
>   See: https://docs.fedoraproject.org/en-US/packaging-
>   guidelines/deprecating-packages/
> 
This package does not depend on openssl-1.1-devel:

# rpm -qa | grep openssl
openssl-libs-3.0.5-5.fc38.x86_64
openssl-3.0.5-5.fc38.x86_64
openssl-devel-3.0.5-5.fc38.x86_64
# dnf --repoid=f38-build builddep ~test/rpmbuild/SPECS/dnf5.spec
[...]
=======================================================================================================
Installing:
 bash-completion                noarch      1:2.11-8.fc37                         f38-build      291 k
 cppunit-devel                  x86_64      1.15.1-7.fc37                         f38-build       57 k
 doxygen                        x86_64      2:1.9.5-2.fc38                        f38-build      4.7 M
 fmt-devel                      x86_64      9.1.0-1.fc38                          f38-build      123 k
 gpgme-devel                    x86_64      1.17.0-4.fc37                         f38-build      165 k
 libcomps-devel                 x86_64      0.1.18-4.fc37                         f38-build       28 k
 librepo-devel                  x86_64      1.14.4-1.fc38                         f38-build       34 k
 libsmartcols-devel             x86_64      2.38.1-2.fc38                         f38-build       14 k
 python3-breathe                noarch      4.34.0-3.fc37                         f38-build      241 k
 python3-sphinx_rtd_theme       noarch      1.0.0-8.fc37                          f38-build       69 k
 ruby-devel                     x86_64      3.1.2-169.fc38                        f38-build      419 k
 rubygem-test-unit              noarch      3.5.3-202.fc37                        f38-build       79 k
 sdbus-cpp-devel                x86_64      1.2.0-1.fc38                          f38-build       32 k
 sqlite-devel                   x86_64      3.39.3-2.fc38                         f38-build      143 k
 toml11-devel                   x86_64      3.7.1-2.fc37                          f38-build       83 k
 zchunk-devel                   x86_64      1.2.3-1.fc38                          f38-build       15 k
Installing dependencies:
 clang-libs                     x86_64      15.0.0-3.fc38                         f38-build       21 M
 clang-resource-filesystem      x86_64      15.0.0-3.fc38                         f38-build       13 k
 cppunit                        x86_64      1.15.1-7.fc37                         f38-build      138 k
 fmt                            x86_64      9.1.0-1.fc38                          f38-build      118 k
 fontawesome-fonts              noarch      1:4.7.0-14.fc37                       f38-build      204 k
 google-roboto-slab-fonts       noarch      1.100263-0.18.20150923git.fc37        f38-build      239 k
 lato-fonts                     noarch      2.015-14.fc37                         f38-build      3.1 M
 libassuan-devel                x86_64      2.5.5-5.fc37                          f38-build       62 k
 ruby                           x86_64      3.1.2-169.fc38                        f38-build       41 k
 ruby-default-gems              noarch      3.1.2-169.fc38                        f38-build       31 k
 ruby-libs                      x86_64      3.1.2-169.fc38                        f38-build      3.2 M
 rubygem-io-console             x86_64      0.5.11-169.fc38                       f38-build       26 k
 rubygem-power_assert           noarch      2.0.1-201.fc37                        f38-build       19 k
 rubygem-psych                  x86_64      4.0.3-169.fc38                        f38-build       51 k
 rubygems                       noarch      3.3.22-201.fc38                       f38-build      311 k
 rubypick                       noarch      1.1.1-17.fc37                         f38-build      9.9 k
 sdbus-cpp                      x86_64      1.2.0-1.fc38                          f38-build       98 k
 systemd-devel                  x86_64      251.5-607.fc38                        f38-build      459 k

Transaction Summary
=======================================================================================================

I guess a tool you used for this review does take into account that "pkgconfig(libcrypto)", which is indeed build-required, is provided by multiple OpenSSL packages:

# dnf --quiet --enablerepo=f38-build repoquery --whatprovides 'pkgconfig(libcrypto)'
openssl-devel-1:3.0.5-5.fc38.i686
openssl-devel-1:3.0.5-5.fc38.x86_64
openssl1.1-devel-1:1.1.1q-2.fc37.i686
openssl1.1-devel-1:1.1.1q-2.fc37.x86_64


> perl5-libdnf5: /usr/lib64/perl5/vendor_perl/auto/libdnf5/base/base.so
> perl5-libdnf5: /usr/lib64/perl5/vendor_perl/auto/libdnf5/common/common.so
> perl5-libdnf5: /usr/lib64/perl5/vendor_perl/auto/libdnf5/comps/comps.so
> perl5-libdnf5: /usr/lib64/perl5/vendor_perl/auto/libdnf5/conf/conf.so
> perl5-libdnf5: /usr/lib64/perl5/vendor_perl/auto/libdnf5/logger/logger.so
> perl5-libdnf5: /usr/lib64/perl5/vendor_perl/auto/libdnf5/plugin/plugin.so
> perl5-libdnf5: /usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so
> perl5-libdnf5: /usr/lib64/perl5/vendor_perl/auto/libdnf5/rpm/rpm.so
> perl5-libdnf5:
> /usr/lib64/perl5/vendor_perl/auto/libdnf5/transaction/transaction.so
> perl5-libdnf5-cli:
> /usr/lib64/perl5/vendor_perl/auto/libdnf5_cli/progressbar/progressbar.so
> python3-libdnf5: /usr/lib64/python3.11/site-packages/libdnf5/_base.so
> python3-libdnf5: /usr/lib64/python3.11/site-packages/libdnf5/_common.so
> python3-libdnf5: /usr/lib64/python3.11/site-packages/libdnf5/_comps.so
> python3-libdnf5: /usr/lib64/python3.11/site-packages/libdnf5/_conf.so
> python3-libdnf5: /usr/lib64/python3.11/site-packages/libdnf5/_logger.so
> python3-libdnf5: /usr/lib64/python3.11/site-packages/libdnf5/_plugin.so
> python3-libdnf5: /usr/lib64/python3.11/site-packages/libdnf5/_repo.so
> python3-libdnf5: /usr/lib64/python3.11/site-packages/libdnf5/_rpm.so
> python3-libdnf5: /usr/lib64/python3.11/site-packages/libdnf5/_transaction.so
> python3-libdnf5-cli:
> /usr/lib64/python3.11/site-packages/libdnf5_cli/_progressbar.so
> ruby-libdnf5: /usr/lib64/ruby/vendor_ruby/libdnf5/base.so
> ruby-libdnf5: /usr/lib64/ruby/vendor_ruby/libdnf5/common.so
> ruby-libdnf5: /usr/lib64/ruby/vendor_ruby/libdnf5/comps.so
> ruby-libdnf5: /usr/lib64/ruby/vendor_ruby/libdnf5/conf.so
> ruby-libdnf5: /usr/lib64/ruby/vendor_ruby/libdnf5/logger.so
> ruby-libdnf5: /usr/lib64/ruby/vendor_ruby/libdnf5/plugin.so
> ruby-libdnf5: /usr/lib64/ruby/vendor_ruby/libdnf5/repo.so
> ruby-libdnf5: /usr/lib64/ruby/vendor_ruby/libdnf5/rpm.so
> ruby-libdnf5: /usr/lib64/ruby/vendor_ruby/libdnf5/transaction.so
> ruby-libdnf5-cli: /usr/lib64/ruby/vendor_ruby/libdnf5_cli/progressbar.so

These files are fine. All of them are plugins. Their respective language interpreters indeed expect file names without a soname version. Compare to other Perl, Pyhon, and Ruby packages.

> libdnf5-plugin-actions: /usr/lib64/libdnf5/plugins/actions.so
> python3-libdnf5-python-plugins-loader:
> /usr/lib64/libdnf5/plugins/python_plugins_loader.so                         
> 
> dnf5-plugins: /usr/lib64/dnf5/plugins/builddep_cmd_plugin.so
> dnf5-plugins: /usr/lib64/dnf5/plugins/changelog_cmd_plugin.so
> 
None of these libraries need to be versions. Only libraries which are in a standard dynamic linker path (see /etc/ld.so.conf) need to be versioned.


> perl5-libdnf5.x86_64: W: undefined-non-weak-symbol
> /usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so ....

We need to see which exact symbol is missing.

Maybe the object was not linked to libperl.so (missing -lperl from LDFLAGS). Though it would be only a cosmetic fix because the only code which loads Perl extensions is libperl.so. Hence always implicitly loaded.

Comment 7 Petr Pisar 2022-10-03 14:06:41 UTC
(In reply to Petr Pisar from comment #6)
> (In reply to Miroslav Suchý from comment #5)
> > perl5-libdnf5.x86_64: W: undefined-non-weak-symbol
> > /usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so ....
> 
> We need to see which exact symbol is missing.
> 

# ldd -d -r /usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1
[...]
        libcrypt.so.2 => /lib64/libcrypt.so.2 (0x00007f8729d3e000)
undefined symbol: PL_thr_key    (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_sv_2bool_flags   (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_more_sv  (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_newRV    (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_call_method      (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_markstack_grow   (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_sv_2nv_flags     (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_croak_nocontext  (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_sv_setsv_flags   (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_sv_bless (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_free_tmps        (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_gv_init_pvn      (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_sv_2pv_flags     (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_get_sv   (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_sv_newmortal     (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_sv_setref_pv     (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_mg_get   (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_sv_2uv_flags     (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_newSVsv_flags    (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_pop_scope        (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_newSVuv  (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_newSViv  (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_sv_isobject      (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_hv_common_key_len        (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_sv_free2 (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_newSV    (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_sv_magic (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_newSVpvn (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_sv_setiv (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_sv_setpvn        (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_newRV_noinc      (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_gv_add_by_type   (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_savetmps (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_stack_grow       (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_sv_setpvf_nocontext      (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_newXS    (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_sv_setpv (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_av_make  (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_hv_common        (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_av_len   (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_sv_2mortal       (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_more_bodies      (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_newSVnv  (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_sv_2iv_flags     (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_av_fetch (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_gv_stashpv       (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_mg_find  (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_push_scope       (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_sv_derived_from  (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)
undefined symbol: Perl_sv_backoff       (/usr/lib64/perl5/vendor_perl/auto/libdnf5/repo/repo.so.1)

It's missing -lperl:

$ scanelf -n  redhat-linux-build/bindings/perl5/auto/libdnf5/repo/repo.so
 TYPE   NEEDED FILE 
ET_DYN libdnf5.so.1,libstdc++.so.6,libfmt.so.9,libgcc_s.so.1,libc.so.6 redhat-linux-build/bindings/perl5/auto/libdnf5/repo/repo.so 

From a build log:

/usr/bin/g++ -fPIC -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64  -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -Wl,--enable-new-dtags -Wl,-z,relro -Wl,--as-needed -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -Wl,--build-id=sha1 -Wl,-z,relro -Wl,--as-needed  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -Wl,--build-id=sha1 -specs=/usr/lib/rpm/redhat/redhat-package-notes -shared -Wl,-soname,repo.so.1 -o ../auto/libdnf5/repo/repo.so.1 CMakeFiles/perl5_repo.dir/CMakeFiles/perl5_repo.dir/repoPERL_wrap.cxx.o  -Wl,-rpath,/home/test/rpmbuild/BUILD/dnf5-5.0.0/redhat-linux-build/libdnf: ../../../libdnf/libdnf5.so.1 -ldl -lstdc++fs -lstdc++ -lfmt -ljson-c -lmodulemd -lgobject-2.0 -lglib-2.0 -lsolv -lsolvext -lrpm -lrpmio -lpopt -lxml2 -lgpgme -lrepo -lglib-2.0 -lsolv -lsolvext -lrpm -lrpmio -lpopt -lxml2 -lgpgme -lrepo -lsqlite3 

(Now something actually sets a dummy soname to "repo.so.1" and it breaks the Perl loader: perl -e 'require libdnf5::repo': Can't locate loadable object for module libdnf5::common. It searches for libdnf5/common.so.)

Upstream Perl does not link extensions to libperl. Fedora as well as some other distributions patch Perl build systems to do it. Swig probably was missed. (The problem is that upstream Perl does not have a configuration option to save -lperl linker flag to.) I'd guess it's not a big deal.

Comment 8 Petr Pisar 2022-10-03 14:10:12 UTC
From Perl packaging point of view the package names are wrong. perl5-libdnf5 should be called perl-libdnf5. I.e. without the digit 5 after perl.

Comment 9 Petr Pisar 2022-10-03 14:14:57 UTC
And Perl packages are missing an explicit dependency on perl(:MODULE_COMPAT_...) <https://docs.fedoraproject.org/en-US/packaging-guidelines/Perl/#_versioned_module_compat_requires>.
Also the /usr/share/licenses/perl5-libdnf5/lgpl-2.1.txt is not necessary because the same file is already packaged in libdnf5 which is required by the Perl packages.

Comment 11 Miroslav Suchý 2022-10-06 17:10:29 UTC
[ ]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/share/dbus-1,
     /etc/dbus-1, /usr/share/dbus-1/system-services, /usr/lib/.build-id/58,
     /etc/dbus-1/system.d

While /usr/lib/.build-id/58 is false negative, the other seems valid issues to me. All these are owned by dbus-common, which is transitively owned by dbus you require, but I think you should require directly dbus-common to be sure.

But beside this I see no other issues. 

@ppisar do you have any additional comments? This is a big package, I may miss something.

Comment 12 Nicola Sella 2022-10-06 17:16:11 UTC
> [ ]: Package must own all directories that it creates.
>     Note: Directories without known owners: /usr/share/dbus-1,
>     /etc/dbus-1, /usr/share/dbus-1/system-services, /usr/lib/.build-id/58,
>     /etc/dbus-1/system.d
>
> While /usr/lib/.build-id/58 is false negative, the other seems valid issues to me. All these are owned by dbus-common, which is transitively owned by dbus you require, but I think you should require directly dbus-common to be sure.

Directory with no ownership should be fixed. Your quoted request is addressed by this line [1] part of this commit [2]

[1] https://github.com/rpm-software-management/dnf5/blob/main/dnf5.spec#L133
[2] https://github.com/rpm-software-management/dnf5/commit/4d9b0c1d76b3c7b326271d331a26bb305016eb99

Comment 13 Petr Pisar 2022-10-07 11:18:57 UTC
Indeed the package large. Here are my comments for a rpmlint output:

dnf5.x86_64: W: non-conffile-in-etc /etc/dnf/dnf5-aliases.d/README
This is Ok.

dnf5daemon-server.x86_64: W: non-conffile-in-etc /etc/dbus-1/system.d/org.rpm.dnf.v0.conf
FIX: This configures access permissions to the D-Bus service. I'm not familiar with D-Bus, but I believe the file should be marked with `%config(noreplace)'. It's clearly a configuration file which user may want to modify (e.g. add a permission to other users). And e.g. avahi package does the same.

dnf5.x86_64: W: no-manual-page-for-binary dnf5
dnf5daemon-client.x86_64: W: no-manual-page-for-binary dnf5daemon-client
dnf5daemon-server.x86_64: W: no-manual-page-for-binary dnf5daemon-server
These are regressions in rpmlint (bug #2132936). Ok.

dnf5-plugins.x86_64: W: no-documentation
Ok.

perl-libdnf5.x86_64: W: no-documentation
perl-libdnf5-cli.x86_64: W: no-documentation
python3-libdnf5.x86_64: W: no-documentation
python3-libdnf5-cli.x86_64: W: no-documentation
ruby-libdnf5.x86_64: W: no-documentation
ruby-libdnf5-cli.x86_64: W: no-documentation
Ok.
TODO: Inform upstream that some documentation would be great. Upstream should consider adding a dummy documentation which point a reader to a C API documentation.

libdnf5.x86_64: E: missing-hash-section /usr/lib64/libdnf5.so.1
libdnf5-cli.x86_64: E: missing-hash-section /usr/lib64/libdnf-cli.so.1
This is a new test in rpmlint or a new feature of Fedora toolchain (removal of SystV hash section. There is still a GNU hash section). Ok.

dnf5-plugins.x86_64: W: description-shorter-than-summary
TODO: The description could name packaged plugins. Once independent plugins appear, users will ask what's inside dnf5-plugins.


And here are my comments for the packaged files and dependencies:

$ rpm -q -lv -p ../RPMS/x86_64/dnf5-5.0.0-0~pre.fc38.x86_64.rpm 
drwxr-xr-x    2 root     root                        0 Oct  7 10:14 /etc/dnf/dnf5-aliases.d
-rw-r--r--    1 root     root                      166 Oct  4 10:22 /etc/dnf/dnf5-aliases.d/README
-rwxr-xr-x    1 root     root                   960616 Oct  7 10:15 /usr/bin/dnf5
drwxr-xr-x    2 root     root                        0 Oct  7 10:15 /usr/lib/.build-id
drwxr-xr-x    2 root     root                        0 Oct  7 10:15 /usr/lib/.build-id/8e
lrwxrwxrwx    1 root     root                       24 Oct  7 10:15 /usr/lib/.build-id/8e/9303befb3457d311377cc4f2f8febd69256c19 -> ../../../../usr/bin/dnf5
drwxr-xr-x    2 root     root                        0 Oct  7 10:14 /usr/lib/dnf5
drwxr-xr-x    2 root     root                        0 Oct  7 10:14 /usr/lib/dnf5/aliases.d
-rw-r--r--    1 root     root                     4315 Oct  4 10:22 /usr/lib/dnf5/aliases.d/compatibility.conf
drwxr-xr-x    2 root     root                        0 Oct  7 10:14 /usr/lib64/dnf5
drwxr-xr-x    2 root     root                        0 Oct  7 10:14 /usr/lib64/dnf5/plugins
-rw-r--r--    1 root     root                      131 Oct  4 10:22 /usr/lib64/dnf5/plugins/README
drwxr-xr-x    2 root     root                        0 Oct  7 10:14 /usr/share/bash-completion
drwxr-xr-x    2 root     root                        0 Oct  7 10:14 /usr/share/bash-completion/completions
-rw-r--r--    1 root     root                      265 Oct  4 10:22 /usr/share/bash-completion/completions/dnf5
drwxr-xr-x    2 root     root                        0 Oct  7 10:15 /usr/share/licenses/dnf5
-rw-r--r--    1 root     root                      964 Oct  4 10:22 /usr/share/licenses/dnf5/COPYING.md
-rw-r--r--    1 root     root                    18092 Oct  4 10:22 /usr/share/licenses/dnf5/gpl-2.0.txt
-rw-r--r--    1 root     root                      448 Oct  7 10:14 /usr/share/man/man8/dnf5.8.gz
TODO: Section (8) of dnf5 manual page and location (/usr/bin) of the executable do not match. I recommend moving dnf5 manual to section #1 as the tool can be used by a regular user, e.g. for "dnf repoquery".

$ rpm -q -lv -p ../RPMS/x86_64/libdnf5-plugin-actions-5.0.0-0~pre.fc38.x86_64.rpm 
drwxr-xr-x    2 root     root                        0 Oct  7 10:15 /usr/lib/.build-id
drwxr-xr-x    2 root     root                        0 Oct  7 10:15 /usr/lib/.build-id/0c
lrwxrwxrwx    1 root     root                       48 Oct  7 10:15 /usr/lib/.build-id/0c/9d6e06e1ebda323152dff4e89f4c07967aca49 -> ../../../../usr/lib64/libdnf5/plugins/actions.so
drwxr-xr-x    2 root     root                        0 Oct  7 10:14 /usr/lib64/libdnf5/plugins
-rwxr-xr-x    1 root     root                   122112 Oct  7 10:15 /usr/lib64/libdnf5/plugins/actions.so
TODO: /usr/lib64/libdnf5/plugins is already owned by dnf5 package. While it's not against guidelines, I recommend you to choose only one package. Why it's not libfn5 package? Compare to 
dnf5-plugins which does not own the directory.

$ rpm -q -lv -p ../RPMS/x86_64/dnf5daemon-client-5.0.0-0~pre.fc38.x86_64.rpm 
-rwxr-xr-x    1 root     root                   244304 Oct  7 10:15 /usr/bin/dnf5daemon-client
drwxr-xr-x    2 root     root                        0 Oct  7 10:15 /usr/lib/.build-id
drwxr-xr-x    2 root     root                        0 Oct  7 10:15 /usr/lib/.build-id/12
lrwxrwxrwx    1 root     root                       37 Oct  7 10:15 /usr/lib/.build-id/12/0c5e65bba93042bd48df606cb1a9f366d531c0 -> ../../../../usr/bin/dnf5daemon-client
drwxr-xr-x    2 root     root                        0 Oct  7 10:15 /usr/share/licenses/dnf5daemon-client
-rw-r--r--    1 root     root                      964 Oct  4 10:22 /usr/share/licenses/dnf5daemon-client/COPYING.md
-rw-r--r--    1 root     root                    18092 Oct  4 10:22 /usr/share/licenses/dnf5daemon-client/gpl-2.0.txt
-rw-r--r--    1 root     root                      469 Oct  7 10:14 /usr/share/man/man8/dnf5daemon-client.8.gz
TODO: If provided dnf5daemon-client can be used by a regular user, I recommend moving the manual page to section 1. Otherwise the executable should be moved to /usr/sbin.

$ rpm -q -lv -p ../RPMS/x86_64/dnf5daemon-server-5.0.0-0~pre.fc38.x86_64.rpm
-rw-r--r--    1 root     root                      328 Oct  4 10:22 /etc/dbus-1/system.d/org.rpm.dnf.v0.conf
-rwxr-xr-x    1 root     root                   378688 Oct  7 10:15 /usr/bin/dnf5daemon-server
drwxr-xr-x    2 root     root                        0 Oct  7 10:15 /usr/lib/.build-id
drwxr-xr-x    2 root     root                        0 Oct  7 10:15 /usr/lib/.build-id/91
lrwxrwxrwx    1 root     root                       37 Oct  7 10:15 /usr/lib/.build-id/91/d947cdee3d3dbf3359169273a2440a9af6dac7 -> ../../../../usr/bin/dnf5daemon-server
-rw-r--r--    1 root     root                      141 Oct  4 10:22 /usr/lib/systemd/system/dnf5daemon-server.service
-rw-r--r--    1 root     root                     3727 Oct  4 10:22 /usr/share/dbus-1/interfaces/org.rpm.dnf.v0.Goal.xml
-rw-r--r--    1 root     root                     2541 Oct  4 10:22 /usr/share/dbus-1/interfaces/org.rpm.dnf.v0.SessionManager.xml
-rw-r--r--    1 root     root                     4059 Oct  4 10:22 /usr/share/dbus-1/interfaces/org.rpm.dnf.v0.rpm.Repo.xml
-rw-r--r--    1 root     root                    10503 Oct  4 10:22 /usr/share/dbus-1/interfaces/org.rpm.dnf.v0.rpm.Rpm.xml
-rw-r--r--    1 root     root                      119 Oct  4 10:22 /usr/share/dbus-1/system-services/org.rpm.dnf.v0.service
drwxr-xr-x    2 root     root                        0 Oct  7 10:15 /usr/share/licenses/dnf5daemon-server
-rw-r--r--    1 root     root                      964 Oct  4 10:22 /usr/share/licenses/dnf5daemon-server/COPYING.md
-rw-r--r--    1 root     root                    18092 Oct  4 10:22 /usr/share/licenses/dnf5daemon-server/gpl-2.0.txt
-rw-r--r--    1 root     root                     3427 Oct  7 10:14 /usr/share/man/man8/dnf5daemon-dbus-api.8.gz
-rw-r--r--    1 root     root                      469 Oct  7 10:14 /usr/share/man/man8/dnf5daemon-server.8.gz
-rw-r--r--    1 root     root                     1509 Oct  4 10:22 /usr/share/polkit-1/actions/org.rpm.dnf.v0.policy
FIX: Move dnf5daemon-server executable to /usr/sbin.

$ rpm -q --requires -p ../RPMS/x86_64/dnf5-5.0.0-0~pre.fc38.x86_64.rpm | sort -f | uniq -c
      1 dnf-data
      1 libc.so.6()(64bit)
      1 libc.so.6(GLIBC_2.14)(64bit)
      1 libc.so.6(GLIBC_2.2.5)(64bit)
      1 libc.so.6(GLIBC_2.3.4)(64bit)
      1 libc.so.6(GLIBC_2.32)(64bit)
      1 libc.so.6(GLIBC_2.34)(64bit)
      1 libc.so.6(GLIBC_2.4)(64bit)
      1 libdnf-cli.so.1()(64bit)
      1 libdnf5(x86-64) = 5.0.0-0~pre.fc38
      1 libdnf5.so.1()(64bit)
      1 libfmt.so.9()(64bit)
      1 libgcc_s.so.1()(64bit)
      1 libgcc_s.so.1(GCC_3.0)(64bit)
      1 libgcc_s.so.1(GCC_3.3.1)(64bit)
      1 libsmartcols.so.1()(64bit)
      1 libsmartcols.so.1(SMARTCOLS_2.25)(64bit)
      1 libsmartcols.so.1(SMARTCOLS_2.29)(64bit)
      1 libstdc++.so.6()(64bit)
      1 libstdc++.so.6(CXXABI_1.3)(64bit)
      1 libstdc++.so.6(CXXABI_1.3.9)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.11)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.14)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.15)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.18)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.19)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.20)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.21)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.26)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.29)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.30)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.9)(64bit)
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsZstd) <= 5.4.18-1
      1 rpmlib(TildeInVersions) <= 4.10.0-1
      1 rtld(GNU_HASH)
      3 systemd
The dependency on systemd is strange. I believe dnf5 binary package should be free of systemd. I have no idea why the dependency was generated here.

$ rpm -q --requires -p ../RPMS/x86_64/dnf5daemon-client-5.0.0-0~pre.fc38.x86_64.rpm | sort -f | uniq -c
      1 dnf5daemon-server
      1 libc.so.6()(64bit)
      1 libc.so.6(GLIBC_2.14)(64bit)
      1 libc.so.6(GLIBC_2.2.5)(64bit)
      1 libc.so.6(GLIBC_2.34)(64bit)
      1 libc.so.6(GLIBC_2.4)(64bit)
      1 libdnf-cli.so.1()(64bit)
      1 libdnf5(x86-64) = 5.0.0-0~pre.fc38
      1 libdnf5-cli(x86-64) = 5.0.0-0~pre.fc38
      1 libdnf5.so.1()(64bit)
      1 libfmt.so.9()(64bit)
      1 libgcc_s.so.1()(64bit)
      1 libgcc_s.so.1(GCC_3.0)(64bit)
      1 libgcc_s.so.1(GCC_3.3.1)(64bit)
      1 libsdbus-c++.so.1()(64bit)
      1 libsmartcols.so.1()(64bit)
      1 libsmartcols.so.1(SMARTCOLS_2.25)(64bit)
      1 libsmartcols.so.1(SMARTCOLS_2.29)(64bit)
      1 libstdc++.so.6()(64bit)
      1 libstdc++.so.6(CXXABI_1.3)(64bit)
      1 libstdc++.so.6(CXXABI_1.3.9)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.11)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.20)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.21)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.22)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.26)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.29)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.30)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.9)(64bit)
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsZstd) <= 5.4.18-1
      1 rpmlib(TildeInVersions) <= 4.10.0-1
      1 rtld(GNU_HASH)
TODO: Can the client talk to dnf5daemon-server over network? If it can, it should not depend on dnf5daemon-server (multi-host scenario). Otherwise, I recommend constraining the dependency with exactly the same version-release.

$ rpm -q --requires -p ../RPMS/x86_64/dnf5daemon-server-5.0.0-0~pre.fc38.x86_64.rpm | sort -f | uniq -c
      3 /bin/sh
      1 dbus
      1 dnf-data
      1 libc.so.6()(64bit)
      1 libc.so.6(GLIBC_2.14)(64bit)
      1 libc.so.6(GLIBC_2.2.5)(64bit)
      1 libc.so.6(GLIBC_2.27)(64bit)
      1 libc.so.6(GLIBC_2.3)(64bit)
      1 libc.so.6(GLIBC_2.32)(64bit)
      1 libc.so.6(GLIBC_2.34)(64bit)
      1 libc.so.6(GLIBC_2.4)(64bit)
      1 libdnf5(x86-64) = 5.0.0-0~pre.fc38
      1 libdnf5-cli(x86-64) = 5.0.0-0~pre.fc38
      1 libdnf5.so.1()(64bit)
      1 libfmt.so.9()(64bit)
      1 libgcc_s.so.1()(64bit)
      1 libgcc_s.so.1(GCC_3.0)(64bit)
      1 libgcc_s.so.1(GCC_3.3.1)(64bit)
      1 libsdbus-c++.so.1()(64bit)
      1 libstdc++.so.6()(64bit)
      1 libstdc++.so.6(CXXABI_1.3)(64bit)
      1 libstdc++.so.6(CXXABI_1.3.9)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.11)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.18)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.19)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.20)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.21)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.22)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.26)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.29)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.30)(64bit)
      1 libstdc++.so.6(GLIBCXX_3.4.9)(64bit)
      1 polkit
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsZstd) <= 5.4.18-1
      1 rpmlib(TildeInVersions) <= 4.10.0-1
      1 rtld(GNU_HASH)
TODO: If the 'dbus' dependency is for /etc/dbus-1/system.d directory, it should be 'dbus-common'. Fedora has (or had) multiple implementations of D-Bus.

$ rpm -q --provides -p ../RPMS/x86_64/libdnf5-devel-5.0.0-0~pre.fc38.x86_64.rpm | sort -f | uniq -c
      1 libdnf5-devel = 5.0.0-0~pre.fc38
      1 libdnf5-devel(x86-64) = 5.0.0-0~pre.fc38
      1 pkgconfig(libdnf)
TODO: 'pkgconfig(libdnf)' should be versioned. Report upstream that the pkg-config file is missing a version field.

$ rpm -q --provides -p ../RPMS/x86_64/libdnf5-cli-devel-5.0.0-0~pre.fc38.x86_64.rpm | sort -f | uniq -c
      1 libdnf5-cli-devel = 5.0.0-0~pre.fc38
      1 libdnf5-cli-devel(x86-64) = 5.0.0-0~pre.fc38
      1 pkgconfig(libdnf-cli)
TODO: 'pkgconfig(libdnf-cli)' should be versioned. Report upstream that the pkg-config file is missing a version field.

Otherwise, I cannot see any other problems.

Comment 14 Vít Ondruch 2022-10-07 14:30:15 UTC
I wish the package was named just `dnf` and the old DNF was renamed to `dnf4` instead.

Comment 15 Nicola Sella 2022-10-10 13:04:17 UTC
> FIX: This configures access permissions to the D-Bus service. I'm not familiar with D-Bus, but I believe the file should be marked with `%config(noreplace)'. It's clearly a configuration file which user may want to modify (e.g. add a permission to other users). And e.g. avahi package does the same.

Fixed

> FIX: Move dnf5daemon-server executable to /usr/sbin.

Fixed

Thank you for the feedback, I added the TODOs to future changes list while the FIXes were addressed now to land in Fedora. I believe that everything discussed as blocking has now been fixed and included.

SPEC: https://download.copr.fedorainfracloud.org/results/rpmsoftwaremanagement/dnf5-src-fedora/fedora-rawhide-x86_64/04898744-dnf5/dnf5.spec
SRPM: https://download.copr.fedorainfracloud.org/results/rpmsoftwaremanagement/dnf5-src-fedora/fedora-rawhide-x86_64/04898744-dnf5/dnf5-5.0.0-0~pre.fc38.src.rpm

msuchy what's your opinion?

Comment 16 Nicola Sella 2022-10-11 14:00:51 UTC
> TODO: Can the client talk to dnf5daemon-server over network? If it can, it should not depend on dnf5daemon-server (multi-host scenario). Otherwise, I recommend constraining the dependency with exactly the same version-release.

Dnf5daemon-client is incapable to talk to the server through network at the moment. The communication is handled through dbus.

Comment 17 Nicola Sella 2022-10-11 14:43:41 UTC
Update:

> The dependency on systemd is strange. I believe dnf5 binary package should be free of systemd. I have no idea why the dependency was generated here.

removed %{?systemd_requires}

> TODO: The description could name packaged plugins. Once independent plugins appear, users will ask what's inside dnf5-plugins.

Updated with examples

SPEC: https://download.copr.fedorainfracloud.org/results/rpmsoftwaremanagement/dnf5-src-fedora/fedora-rawhide-x86_64/04900756-dnf5/dnf5.spec
SRPM: https://download.copr.fedorainfracloud.org/results/rpmsoftwaremanagement/dnf5-src-fedora/fedora-rawhide-x86_64/04900756-dnf5/dnf5-5.0.0-0~pre.fc38.src.rpm

Comment 18 Miroslav Suchý 2022-10-11 14:54:43 UTC
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files in private %_libdir subdirectory (see
     attachment). Verify they are not in ld path.
[x]: If your application is a C or C++ application you must list a
     BuildRequires against gcc, gcc-c++ or clang.
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig not called in %post and %postun for Fedora 28 and later.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.
Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
[x]: License file installed when any subpackage combination is installed.
[-]: Package must own all directories that it creates.
     Note: Directories without known owners: /etc/dbus-1,
     /etc/dbus-1/system.d, /usr/share/dbus-1, /usr/share/dbus-1/system-
     services
     This is IMHO false positive.
[x]: Package does not own files or directories owned by other packages.
     Note: Dirs in package are owned also by: /usr/share/bash-
     completion....
     /usr/include/libdnf/conf(libdnf-devel),
     /usr/include/libdnf/plugin(libdnf-devel),
     /usr/include/libdnf/utils(libdnf-devel)
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[x]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
     Note: this is complicated. There is still ongoing discussion how the replacement should be done. I will not block review on this.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[x]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: Package requires other packages for directories it uses.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: %config files are marked noreplace or the reason is justified.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: No %config files under /usr.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: systemd_post is invoked in %post, systemd_preun in %preun, and
     systemd_postun in %postun for Systemd service files.
     Note: Systemd service file(s) in dnf5daemon-server
[x]: File names are valid UTF-8.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

Python:
[x]: Python eggs must not download any dependencies during the build
     process.
[-]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Packages MUST NOT have dependencies (either build-time or runtime) on
     packages named with the unversioned python- prefix unless no properly
     versioned package exists. Dependencies on Python packages instead MUST
     use names beginning with python2- or python3- as appropriate.
[x]: Python packages must not contain %{pythonX_site(lib|arch)}/* in %files
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[-]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in libdnf5
     , libdnf5-cli-devel , perl-libdnf5-cli , python3-libdnf5-cli , ruby-
     libdnf5-cli
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Scriptlets must be sane, if used.
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[?]: Package should compile and build into binary rpms on all supported
     architectures.
[x]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: The placement of pkgconfig(.pc) files are correct.
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on debuginfo package(s).
     Note: There are rpmlint messages (see attachment).
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
[x]: Spec file according to URL is the same as in SRPM.


APPROVED

Disclaimer: the upstream package is still under development and the package is huge. If I missed something during this review, then please open BZ and Nicola (who is part of the upstream) will fix it.

Comment 19 Gwyn Ciesla 2022-10-12 14:00:44 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/dnf5

Comment 20 Package Review 2023-10-08 08:14:07 UTC
Package is now in repositories, closing review.


Note You need to log in before you can comment on or make changes to this bug.