Bug 1412128

It'd be nice to rewrap the %description's. They are very unevenly indented.

> Provides:       %{name}%{?_isa}-devel = %{version}-%{release}
> Provides:       prelude-devel = %{version}-%{release}
That's a bit unusual. Automatically generated provides are usually enough. Why is this needed?

make distcheck → %make_build distcheck. This will add -j..., which should make the whole thing faster. Also, I'm not sure if running "distcheck" is appropriate. "check" should be enough. "distcheck" is for tarball creators to make sure that all the necessary files are included in the distribution, and takes much more time because of the additional rebuild.

There seems to be some codepage corruption in changelog: Caolán McNamara → Caolán McNamara, Marcela MaÅ¡láňová → Marcela Mašláňová.

Looks great though.

fedora-review says:

- Package does not contain duplicates in %files.
  Note: warning: File listed twice:
  /usr/share/libpreludedb/classic/mysql2pgsql.sh
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#DuplicateFiles

[ ]: Spec use %global instead of %define unless justified.
     Note: %define requiring justification: %define major 7, %define
     cppmajor 2

libpreludedb.i686: W: shared-lib-calls-exit /usr/lib/libpreludedb.so.7.1.1 exit
Might be worth checking, although usually this is a false positive.

python3-preludedb.i686: E: python-bytecode-wrong-magic-value /usr/lib/python3.6/site-packages/__pycache__/preludedb.cpython-36.pyc expected 3361 (3.6), found 3379 (unknown)
python3-preludedb.i686: E: python-bytecode-wrong-magic-value /usr/lib/python3.6/site-packages/__pycache__/preludedb.cpython-36.opt-1.pyc expected 3361 (3.6), found 3379 (unknown)

I think this is a false positive in rpmlint... python3 seems to import those files just fine.


python2-preludedb needs Requires: python2-prelude,
python3-preludedb needs Requires: python3-prelude.

SPEC : https://fedorapeople.org/~totol/libpreludedb.spec
SRPM : https://fedorapeople.org/~totol/libpreludedb-3.1.0-26.fc26.src.rpm

(In reply to Zbigniew Jędrzejewski-Szmek from comment #1)
> It'd be nice to rewrap the %description's. They are very unevenly indented.
> 

Done

> > Provides:       %{name}%{?_isa}-devel = %{version}-%{release}
> > Provides:       prelude-devel = %{version}-%{release}
> That's a bit unusual. Automatically generated provides are usually enough.
> Why is this needed?

There is a mistake, it should be preludedb-devel. This was an habit, as I do in libprelude. But this is just for the lib. Do I have to remove it ?

> 
> make distcheck → %make_build distcheck. This will add -j..., which should
> make the whole thing faster. Also, I'm not sure if running "distcheck" is
> appropriate. "check" should be enough. "distcheck" is for tarball creators
> to make sure that all the necessary files are included in the distribution,
> and takes much more time because of the additional rebuild.

Done

> 
> There seems to be some codepage corruption in changelog: Caolán McNamara →
> Caolán McNamara, Marcela MaÅ¡láňová → Marcela Mašláňová.
> 

Done

> Looks great though.

(In reply to Zbigniew Jędrzejewski-Szmek from comment #2)
> fedora-review says:
> 
> - Package does not contain duplicates in %files.
>   Note: warning: File listed twice:
>   /usr/share/libpreludedb/classic/mysql2pgsql.sh
>   See: http://fedoraproject.org/wiki/Packaging/Guidelines#DuplicateFiles
> 

I do not have this on my side, can you help me on this ?

> [ ]: Spec use %global instead of %define unless justified.
>      Note: %define requiring justification: %define major 7, %define
>      cppmajor 2

Done

> 
> libpreludedb.i686: W: shared-lib-calls-exit /usr/lib/libpreludedb.so.7.1.1
> exit
> Might be worth checking, although usually this is a false positive.

As for libprelude, this is a false positive

> 
> python3-preludedb.i686: E: python-bytecode-wrong-magic-value
> /usr/lib/python3.6/site-packages/__pycache__/preludedb.cpython-36.pyc
> expected 3361 (3.6), found 3379 (unknown)
> python3-preludedb.i686: E: python-bytecode-wrong-magic-value
> /usr/lib/python3.6/site-packages/__pycache__/preludedb.cpython-36.opt-1.pyc
> expected 3361 (3.6), found 3379 (unknown)
> 
> I think this is a false positive in rpmlint... python3 seems to import those
> files just fine.

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

> 
> 
> python2-preludedb needs Requires: python2-prelude,
> python3-preludedb needs Requires: python3-prelude.

Done

(In reply to Thomas Andrejak from comment #3)
> > > Provides:       %{name}%{?_isa}-devel = %{version}-%{release}
> > > Provides:       prelude-devel = %{version}-%{release}
> > That's a bit unusual. Automatically generated provides are usually enough.
> > Why is this needed?
> 
> There is a mistake, it should be preludedb-devel. This was an habit, as I do
> in libprelude. But this is just for the lib. Do I have to remove it ?

Oh, I didn't even see that it's "prelude" not "preludedb".
But even with that correction, it still doesn't make sense:
rpm will automatically generate the following:
Provides: prelude-devel = %{version}-%{release}
Provides: prelude-devel%{?_isa} = %{version}-%{release}

and the spec file has:
Provides:       %{name}%{?_isa}-devel = %{version}-%{release}
Provides:       preludedb-devel = %{version}-%{release}

The second of those is redundant, and the first is strange because
it has %{_isa} in the wrong place. You don't _have_ to remove it,
but since it doesn't seem to serve any purpose, you _should_.
It seem to be some legacy craft, and since you're unretiring the
package, you can start with a clean slate.

> > - Package does not contain duplicates in %files.
> >   Note: warning: File listed twice:
> >   /usr/share/libpreludedb/classic/mysql2pgsql.sh
> >   See: http://fedoraproject.org/wiki/Packaging/Guidelines#DuplicateFiles
> 
> I do not have this on my side, can you help me on this ?

%files devel
%{_datadir}/%{name}
...
%attr(0755,-,-) %{_datadir}/%{name}/classic/mysql2pgsql.sh

So that file is listed twice, in the sense that it's included recursively
in the directory, and later separately.

The best option imho would be to just drop mysql2pgsql.sh, mysql2sqlite.sh
from the %files section completely. Looking into the tarball, they seem
to have proper permissions already.

Summary right now basically repeats the name. Something like "database to store IDMEF alerts" would be better.

SPEC : https://fedorapeople.org/~totol/libpreludedb.spec
SRPM : https://fedorapeople.org/~totol/libpreludedb-3.1.0-26.fc26.src.rpm

(In reply to Zbigniew Jędrzejewski-Szmek from comment #5)
> Summary right now basically repeats the name. Something like "database to
> store IDMEF alerts" would be better.

Done

(In reply to Zbigniew Jędrzejewski-Szmek from comment #4)
> (In reply to Thomas Andrejak from comment #3)
> > > > Provides:       %{name}%{?_isa}-devel = %{version}-%{release}
> > > > Provides:       prelude-devel = %{version}-%{release}
> > > That's a bit unusual. Automatically generated provides are usually enough.
> > > Why is this needed?
> > 
> > There is a mistake, it should be preludedb-devel. This was an habit, as I do
> > in libprelude. But this is just for the lib. Do I have to remove it ?
> 
> Oh, I didn't even see that it's "prelude" not "preludedb".
> But even with that correction, it still doesn't make sense:
> rpm will automatically generate the following:
> Provides: prelude-devel = %{version}-%{release}
> Provides: prelude-devel%{?_isa} = %{version}-%{release}
> 
> and the spec file has:
> Provides:       %{name}%{?_isa}-devel = %{version}-%{release}
> Provides:       preludedb-devel = %{version}-%{release}
> 
> The second of those is redundant, and the first is strange because
> it has %{_isa} in the wrong place. You don't _have_ to remove it,
> but since it doesn't seem to serve any purpose, you _should_.
> It seem to be some legacy craft, and since you're unretiring the
> package, you can start with a clean slate.

You are right, I did it

> 
> > > - Package does not contain duplicates in %files.
> > >   Note: warning: File listed twice:
> > >   /usr/share/libpreludedb/classic/mysql2pgsql.sh
> > >   See: http://fedoraproject.org/wiki/Packaging/Guidelines#DuplicateFiles
> > 
> > I do not have this on my side, can you help me on this ?
> 
> %files devel
> %{_datadir}/%{name}
> ...
> %attr(0755,-,-) %{_datadir}/%{name}/classic/mysql2pgsql.sh
> 
> So that file is listed twice, in the sense that it's included recursively
> in the directory, and later separately.
> 
> The best option imho would be to just drop mysql2pgsql.sh, mysql2sqlite.sh
> from the %files section completely. Looking into the tarball, they seem
> to have proper permissions already.

There is an issues with permissions. That was why I put it in this way the first time. I move the chmod into the install so the %files can be as it must be.



Note: I will update the libprelude package with each part of the actual review that we forgot in the libprelude review.

Hm, I still see %define, not %global. Please fix that up when importing.

> Group:          System Environment/Libraries
Not necessary [https://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections]

> Release:        26%{?dist}
Why so high? You could reset it to 1 when importing.

+ package name is OK
+ license is acceptable (GPLv2+)
+ license is specified correctly
+ builds and installs fine
+ provides/requires/buildrequires look sane
+ scriptlets are OK
+ %check is present and passes
+ python packaging guidelines are used
+ %python_provide are present

I think you could add
Suggests: preludedb-mysql
Suggests: preludedb-pgsql
Suggests: preludedb-sqlite3
in the main package.

Package is APPROVED.

SPEC : https://fedorapeople.org/~totol/libpreludedb.spec
SRPM : https://fedorapeople.org/~totol/libpreludedb-3.1.0-26.fc26.src.rpm

(In reply to Zbigniew Jędrzejewski-Szmek from comment #7)
> Hm, I still see %define, not %global. Please fix that up when importing.
> 

done

> > Group:          System Environment/Libraries
> Not necessary
> [https://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections]
> 

done

> > Release:        26%{?dist}
> Why so high? You could reset it to 1 when importing.
> 

done

> + package name is OK
> + license is acceptable (GPLv2+)
> + license is specified correctly
> + builds and installs fine
> + provides/requires/buildrequires look sane
> + scriptlets are OK
> + %check is present and passes
> + python packaging guidelines are used
> + %python_provide are present
> 
> I think you could add
> Suggests: preludedb-mysql
> Suggests: preludedb-pgsql
> Suggests: preludedb-sqlite3
> in the main package.

done

> 
> Package is APPROVED.

Thanks for the review !

libpreludedb-3.1.0-1.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2017-a0711d51ee

libpreludedb-3.1.0-1.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-a0711d51ee

libpreludedb-3.1.0-1.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-7751501934

libpreludedb-3.1.0-1.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-7751501934

libpreludedb-3.1.0-1.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-033c0d1778

libpreludedb-3.1.0-1.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-033c0d1778

libpreludedb-3.1.0-1.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.

libpreludedb-3.1.0-1.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.

libpreludedb-3.1.0-1.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.