Bug 1851405

Just a few quick remarks from simply looking at the provided spec file:


Summary:	Cryptographic library
Name:		bee2	
Version:	2020.06.24
Release:	1.fc32

***
The release tag MUST use a special macro for the distribution suffix, instead of hardcoding it.

> Release: 1%{?dist}
***

License:	GPLv3
Url:		http://apmi.bsu.by/resources/tools.html
Source0:	https://notabug.org/kashcheyeu/bee2/archive/master.zip
BuildRequires:	cmake, gcc

%description
Bee2 is a cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus. Additionally, Bee2 implements digital signature algorithms standardized in Russia and Ukraine.

***
The %%description of a MUST wrap lines after ~72 chars.

> %description
> Bee2 is a cryptographic library which implements cryptographic algorithm
> and protocols standardized in Belarus.  Additionally, Bee2 implements
> digital signature algorithms standardized in Russia and Ukraine.
***

%package		devel
Summary:		Headers for package bee2
Requires:		%{name} = %{version}-%{release}

***
The devel package MUST have archful requires, if the devel package is archful itself.

> Requires:		%{name}%{?_isa} = %{version}-%{release}
***

%description	devel
Headers for package bee2.

%package		static
Summary:		Static library bee2
%description	static
Static library bee2.

***
The static package MUST require the devel package.

> Requires:		%{name}-devel%{?_isa} = %{version}-%{release}
***

#------------------------------------------------------------------

%prep
%setup -q -n bee2

***
The %%prep stage SHOULD use %%autosetup, if there is no expilcit reason to prefer %%setup.

> %autosetup -n %{name} -p 1
***

%build
%cmake

***
It is highly encouraged to run CMake-based builds out-of-tree.

> %cmake -S %{_vpath_srcdir} -B %{_vpath_builddir}

The %%build stage MUST run %%make_build, if the package builds binaries.

> %make_build -C %{_vpath_builddir}
***

%install
%make_install DESTDIR=%{buildroot}

***
Specifying DESTDIR when using the %%make_install macro is NOT NEEDED.

> %make_install
***

%check
make test

***
Test SHOULD be run in parallel.

> %make_build -C %{_vpath_builddir} test
***

%files

/usr/bin/bsum

***
The path to the installed binaries MUST use macros instead hardcoding them.

> %{_bindir}/bsum
***

%{_libdir}/libbee2.so
%{_libdir}/libbee2_static.a

***
Libraries MUST be shipped in a seperate libs package for enabling multi-arch installation.

The built shared object MUST have a proper so-name, if shipped directly inside of %%{_libdir}.

Static libraries MUST be shipped in a seperate static packge, only.
***

%files static

%{_libdir}/libbee2_static.a

%files devel

/usr/share/bee2/core/b64.h
…

***
Header files for libraries MUST be installed under %%{_includedir}.
***

%changelog
* Fri Jun 26 2020 Yury Kashcheyeu <kashcheyeu> - 2020.06.24-1
- Initial RPM releas

***
Spelling error:  releas --> release
***

Another remark:  The sources used for building this package do NOT come from the canonical upstream author!  Nor have there been any releases, yet…

Thank you for your help. Done.

Spec URL: https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/fedora-32-x86_64/01514646-bee2/bee2.spec
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/01514646/bee2-2.0.5-2.fc32.src.rpm

 - You must use the Source provided in the upstream official repo: https://github.com/agievich/bee2

 - Are you sure you need the static library, it is not usually shipped.

 - Where does the version you are using is coming from? The upstream repo didn't tag any release.

 - Simplify this:

%{_includedir}/bee2/core/b64.h
%{_includedir}/bee2/core/blob.h
%{_includedir}/bee2/core/dec.h
%{_includedir}/bee2/core/der.h
%{_includedir}/bee2/core/err.h
%{_includedir}/bee2/core/hex.h
%{_includedir}/bee2/core/mem.h
%{_includedir}/bee2/core/mt.h
%{_includedir}/bee2/core/obj.h
%{_includedir}/bee2/core/oid.h
%{_includedir}/bee2/core/prng.h
%{_includedir}/bee2/core/rng.h
%{_includedir}/bee2/core/safe.h
%{_includedir}/bee2/core/stack.h
%{_includedir}/bee2/core/str.h
%{_includedir}/bee2/core/tm.h
%{_includedir}/bee2/core/u16.h
%{_includedir}/bee2/core/u32.h
%{_includedir}/bee2/core/u64.h
%{_includedir}/bee2/core/util.h
%{_includedir}/bee2/core/word.h
%{_includedir}/bee2/crypto/bake.h
%{_includedir}/bee2/crypto/bash.h
%{_includedir}/bee2/crypto/bels.h
%{_includedir}/bee2/crypto/belt.h
%{_includedir}/bee2/crypto/bign.h
%{_includedir}/bee2/crypto/botp.h
%{_includedir}/bee2/crypto/brng.h
%{_includedir}/bee2/crypto/dstu.h
%{_includedir}/bee2/crypto/g12s.h
%{_includedir}/bee2/crypto/pfok.h
%{_includedir}/bee2/defs.h
%{_includedir}/bee2/info.h
%{_includedir}/bee2/math/ec.h
%{_includedir}/bee2/math/ec2.h
%{_includedir}/bee2/math/ecp.h
%{_includedir}/bee2/math/gf2.h
%{_includedir}/bee2/math/gfp.h
%{_includedir}/bee2/math/pp.h
%{_includedir}/bee2/math/pri.h
%{_includedir}/bee2/math/qr.h
%{_includedir}/bee2/math/ww.h
%{_includedir}/bee2/math/zm.h
%{_includedir}/bee2/math/zz.h

Just use %{_includedir}/bee2/ for the whole directory.

 - You must install the license with %license in %files and you should install the %doc as well

%files
%license LICENSE
%doc AUTHORS.md README.md

 - Don't do that:

%if "%dist.%{_arch}" == ".el6.x86_64"

Just use : %if 0%{?rhel} && 0%{?rhel} < 7

You shouldn't need to specify x86_64.

 - Same: %if %{defined fedora}

Just use: %if 0%{?fedora}

 - I'd rather not mix Mageia and Suse stuff with Fedora's spec. Please use separate SPEC if you can.

Thanks. Done.

Version taken from CMakeLists.txt: https://github.com/agievich/bee2/blob/master/CMakeLists.txt

Spec URL: https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/01516023/bee2.spec
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/01516023/bee2-2.0.5-4.fc31.src.rpm

(In reply to Кощеев from comment #5)
> Thanks. Done.
> 
> Version taken from CMakeLists.txt:
> https://github.com/agievich/bee2/blob/master/CMakeLists.txt
> 
> Spec URL:
> https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/
> 01516023/bee2.spec
> SRPM URL:
> https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/
> 01516023/bee2-2.0.5-4.fc31.src.rpm

As Björn was saying there is a problem with the library, it is not versioned. Fedora requires that library are versioned: see https://docs.fedoraproject.org/en-US/packaging-guidelines/#_shared_libraries


"In cases where upstream ships unversioned .so library (so this is not needed for plugins, drivers, etc.), the packager MUST try to convince upstream to start versioning it.

If that fails due to unwilling or unresponsive upstream, the packager may start versioning downstream but this must be done with caution and ideally only in rare cases. We don’t want to create a library that could conflict with upstream if they later start providing versioned shared libraries. Under no circumstances should the unversioned library be shipped in Fedora.

For downstream versioning, the name should be composed like this:

libfoobar.so.0.n

The n should initially be a small integer (for instance, "1"). we use two digits here ("0.n") because the common practice with upstreams is to use only a single digit here. Using multiple digits helps us avoid potential future conflicts. Do not forget to add the SONAME field (see below) to the library."

You must ask upstream to version their library. There's already commented out code that specify versioning: https://github.com/agievich/bee2/blob/master/src/CMakeLists.txt#L96 If upstream do not wish to start versioning, just comment out that code in your SPEC via a patch.


 - Source must not point to master but to a specific commit if upstream has not released a version:

%global commit          2d8ccce67e5562023309244fe662ee21fd6caf79
%global shortcommit     %(c=%{commit}; echo ${c:0:7})
%global snapshotdate    20200602

Summary:	Cryptographic library
Name:		bee2	
Version:	2.0.5
Release:        4.%{snapshotdate}git%{shortcommit}%{?dist}

License:	GPLv3
Url:		http://apmi.bsu.by/resources/tools.html
Source0:	https://github.com/agievich/bee2/archive/%{commit}/%{name}-%{shortcommit}.tar.gz
BuildRequires:	cmake, gcc, unzip

[…]

%prep
%autosetup -n %{name}-%{commit} -p 1

Spec URL: https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/fedora-32-x86_64/01517353-bee2/bee2.spec
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/01517353/bee2-2.0.5-5.20200702git2d8ccce.fc32.src.rpm

 - The make_install test, if this is supposed to run test, make it run in %check


%install

%if 0%{?fedora}
%make_install -C %{_vpath_builddir}
%else
%make_install
%endif

%{__rm} -rf %{buildroot}%{_libdir}/libbee2_static.a

%check
%make_install -C %{_vpath_builddir} test
%else
%make_install test
%endif

 - Also consider EPEL8 for the condition on CMake:

%if 0%{?fedora} || 0%{?rhel} > 7


The rest looks good.

(In reply to Robert-André Mauchin 🐧 from comment #8)
> The rest looks good.

The package still MUST use %make_build BEFORE running %make_install…

In %check it MUST use %make_build instead of %make_install.

besides that the libraries MUST be shipped in a seperate libs package to resolve conflicts with multi-arch.

(In reply to Björn 'besser82' Esser from comment #9)
> (In reply to Robert-André Mauchin 🐧 from comment #8)
> > The rest looks good.
> 
> The package still MUST use %make_build BEFORE running %make_install…
> 
> In %check it MUST use %make_build instead of %make_install.

Ha yes I missed that part.

> %if 0%{?fedora} || 0%{?rhel} > 7
It doesn't work that way: https://download.copr.fedorainfracloud.org/results/kashcheyeu/bee2/epel-8-x86_64/01517970-bee2/builder-live.log

Spec URL: https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/fedora-32-x86_64/01517971-bee2/bee2.spec
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/01517971/bee2-2.0.5-6.20200702git2d8ccce.fc32.src.rpm

(In reply to Кощеев from comment #12)
> > %if 0%{?fedora} || 0%{?rhel} > 7
> It doesn't work that way:
> https://download.copr.fedorainfracloud.org/results/kashcheyeu/bee2/epel-8-
> x86_64/01517970-bee2/builder-live.log
> 
> Spec URL:
> https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/fedora-32-
> x86_64/01517971-bee2/bee2.spec
> SRPM URL:
> https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/
> 01517971/bee2-2.0.5-6.20200702git2d8ccce.fc32.src.rpm

My bad it requires at least CMake 3.13.

Still, you need to split the libraries into a libs subpackages like Björn 'besser82' Esser asked, or rename the package bee2-libs since you're not shipping any binary anymore.

Spec URL: https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/fedora-32-x86_64/01518435-bee2/bee2.spec
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/01518435/bee2-2.0.5-7.20200702git2d8ccce.fc32.src.rpm

Divided into three packages: bee2 (library), bee2-devel, and bsum (utility). Is everything okay?

> bee2 (library)

call it bee-libs
make bee2-devel depends on bee-libs

Did.

Spec URL: https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/01547296/bee2-libs.spec
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/01547296/bee2-libs-2.0.5-8.20200706git7afea8b.fc31.src.rpm

Should we call them "libbee2", "libbee2-devel" and "bsum"? There's only one library.

libbee2 is better is there is only one library. Please make the change to the SPEC name and the name in this Review request.

Spec URL: https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/01550991/libbee2.spec
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/01550991/libbee2-2.0.5-9.20200706git7afea8b.fc31.src.rpm

The package is approved.

You still need to find a sponsor: see https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group

Thank you and Björn 'besser82' Esser, you have helped me a lot.

Package never imported, resetting ticket status.
If you're still interested and still need to find a sponsor, consider open a ticket in https://pagure.io/packager-sponsors/issues

This is an automatic action taken by review-stats script.

The ticket submitter failed to clear the NEEDINFO flag in a month.
As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
we consider this ticket as DEADREVIEW and proceed to close it.