[Bug 1851405] New: Review Request: bee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

Bug ID: 1851405
           Summary: Review Request: bee2 - cryptographic library which
                    implements cryptographic algorithm and protocols
                    standardized in Belarus
           Product: Fedora
           Version: rawhide
          Hardware: All
                OS: Linux
            Status: NEW
         Component: Package Review
          Severity: medium
          Assignee: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail]
  Target Milestone: ---
    Classification: Fedora

Spec URL:
https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/...
SRPM URL:
https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/...

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.

Fedora Account System Username: kashcheyeu

https://notabug.org/kashcheyeu/bee2
Bee2 package hosted in Copr:
https://copr.fedorainfracloud.org/coprs/kashcheyeu/bee2/

This is my first Fedora package, so I need a sponsor. It would be greatly
appreciated if you can sponsor me. Thanks.

[Bug 1851405] Review Request: bee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

Кощеев  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |177841 (FE-NEEDSPONSOR)

Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=177841
[Bug 177841] Tracker: Review requests from new Fedora packagers who need a
sponsor

[Bug 1851405] Review Request: bee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

Björn 'besser82' Esser  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
           Doc Type|---                         |If docs needed, set a value

--- Comment #1 from Björn 'besser82' Esser  ---
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  - 2020.06.24-1
- Initial RPM releas

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

[Bug 1851405] Review Request: bee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

--- Comment #2 from Björn 'besser82' Esser  ---
Another remark:  The sources used for building this package do NOT come from
the canonical upstream author!  Nor have there been any releases, yet…

[Bug 1851405] Review Request: bee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

--- Comment #3 from Кощеев  ---
Thank you for your help. Done.

Spec URL:
https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/fedora-32-x8...
SRPM URL:
https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/...

[Bug 1851405] Review Request: bee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

Robert-André Mauchin  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #4 from Robert-André Mauchin  ---
 - 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.

[Bug 1851405] Review Request: bee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

--- Comment #5 from Кощеев  ---
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/...
SRPM URL:
https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/...

[Bug 1851405] Review Request: bee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

--- Comment #6 from Robert-André Mauchin 🐧  ---
(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/%%7Bcommit%7D/%%7Bname%7D-%%7Bshort...
BuildRequires:  cmake, gcc, unzip

[…]

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

[Bug 1851405] Review Request: bee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

--- Comment #7 from Кощеев  ---
Spec URL:
https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/fedora-32-x8...
SRPM URL:
https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/...

[Bug 1851405] Review Request: bee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

--- Comment #8 from Robert-André Mauchin 🐧  ---

- 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.

[Bug 1851405] Review Request: bee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

--- Comment #9 from Björn 'besser82' Esser  ---
(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.

[Bug 1851405] Review Request: bee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

--- Comment #10 from Björn 'besser82' Esser  ---
besides that the libraries MUST be shipped in a seperate libs package to
resolve conflicts with multi-arch.

[Bug 1851405] Review Request: bee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

--- Comment #11 from Robert-André Mauchin 🐧  ---
(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.

[Bug 1851405] Review Request: bee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

--- Comment #12 from Кощеев  ---
> %if 0%{?fedora} || 0%{?rhel} > 7

It doesn't work that way:
https://download.copr.fedorainfracloud.org/results/kashcheyeu/bee2/epel-8-x8...

Spec URL:
https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/fedora-32-x8...
SRPM URL:
https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/...

[Bug 1851405] Review Request: bee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

--- Comment #13 from Robert-André Mauchin 🐧  ---
(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.

[Bug 1851405] Review Request: bee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

--- Comment #14 from Кощеев  ---
Spec URL:
https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/fedora-32-x8...
SRPM URL:
https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/...

[Bug 1851405] Review Request: bee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

--- Comment #15 from Кощеев  ---
Divided into three packages: bee2 (library), bee2-devel, and bsum (utility). Is
everything okay?

[Bug 1851405] Review Request: bee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

--- Comment #16 from Robert-André Mauchin 🐧  ---
> bee2 (library)

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

[Bug 1851405] Review Request: bee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

--- Comment #17 from Кощеев  ---
Did.

Spec URL:
https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/...
SRPM URL:
https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/...

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

[Bug 1851405] Review Request: bee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

--- Comment #18 from Robert-André Mauchin 🐧  ---
libbee2 is better is there is only one library. Please make the change to the
SPEC name and the name in this Review request.

[Bug 1851405] Review Request: bee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

--- Comment #19 from Кощеев  ---
Spec URL:
https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/...
SRPM URL:
https://copr-be.cloud.fedoraproject.org/results/kashcheyeu/bee2/srpm-builds/...

[Bug 1851405] Review Request: libbee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

Кощеев  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Review Request: bee2 -      |Review Request: libbee2 -
                   |cryptographic library which |cryptographic library which
                   |implements cryptographic    |implements cryptographic
                   |algorithm and protocols     |algorithm and protocols
                   |standardized in Belarus     |standardized in Belarus

[Bug 1851405] Review Request: libbee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

Robert-André Mauchin 🐧  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|[e-mail]    |[e-mail]
              Flags|                            |fedora-review+

--- Comment #20 from Robert-André Mauchin 🐧  ---
The package is approved.

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

[Bug 1851405] Review Request: libbee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

--- Comment #21 from Кощеев  ---
Thank you and Björn 'besser82' Esser, you have helped me a lot.

[Bug 1851405] Review Request: libbee2 - cryptographic library which implements cryptographic algorithm and protocols standardized in Belarus

https://bugzilla.redhat.com/show_bug.cgi?id=1851405

Mattia Verga  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|fedora-review+              |needinfo?([e-mail])
             Status|ASSIGNED                    |NEW
           Assignee|[e-mail]           |[e-mail]

--- Comment #22 from Mattia Verga  ---
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