taking for review

Michel, the URLs don't work for me, the spec returns me back to bugzilla, the srpm returns 404.

(In reply to Dan Horák from comment #2)
> Michel, the URLs don't work for me, the spec returns me back to bugzilla,
> the srpm returns 404.


Dan, sorry for the inconvenience
I moved the spec and source rpm in fedora server:

Spec URL: https://michelmno.fedorapeople.org/libcxl.spec
SRPM URL: https://michelmno.fedorapeople.org/libcxl-1.3-4.2.src.rpm
Description: Coherent accelerator interface user-space library
Fedora Account System Username: michelmno

This is my first request for package creation, I need a sponsor.

related scratch build on ppc koji: (not updated since initial description)
http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=3089837

Thanks, I see it's using SUSE spec file, I would recommend to start from scratch or rather use "rpmdev-newspec libcxl.spec" to create a Fedora compliant template. Maintaining a cross distro spec adds a lot of code that makes the spec file hardly readable. First item to fix would be the naming, Fedora doesn't set the major version number into the package name.

Please see https://fedoraproject.org/wiki/Packaging:Guidelines for the standards Fedora packages need to follow.

(In reply to Dan Horák from comment #4)
> Thanks, I see it's using SUSE spec file, I would recommend to start from
> scratch or rather use "rpmdev-newspec libcxl.spec" to create a Fedora
> compliant template. Maintaining a cross distro spec adds a lot of code that
> makes the spec file hardly readable. First item to fix would be the naming,
> Fedora doesn't set the major version number into the package name.
> 
> Please see https://fedoraproject.org/wiki/Packaging:Guidelines for the
> standards Fedora packages need to follow.

spec file modified as suggested
 
Spec URL: https://michelmno.fedorapeople.org/libcxl.spec
SRPM URL: https://michelmno.fedorapeople.org/libcxl-1.3-1.fc23.src.rpm
Description: Coherent accelerator interface user-space library
Fedora Account System Username: michelmno

related scratch build on ppc koji:
http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=3107206

Few comments on latest spec file:
* With recent changes in guideline, C application should include compiler used for compilation source. In this case add, 
    BuildRequires: gcc
https://fedoraproject.org/wiki/Packaging:C_and_C%2B%2B#BuildRequires_and_Requires
* %install section contains "ldconfig -n $RPM_BUILD_ROOT%{_libdir}", not sure why it is mentioned here? ld cache will already be updated through %post and %postrun section
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=PackagingGuidelines#Shared_Libraries
* In %file and %file devel section, specifying %defattr(-,root,root) is not required now
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=PackagingGuidelines#File_Permissions
* LICENSE file is getting installed in both main and devel package. Since, devel package has dependency on main package, removing %license LICENSE from %files devel section will be fine.
* Is there any significance of including static library (libcxl.a) in devel package? If yes, static libraries must be packaged in a -static sub-package https://fedoraproject.org/wiki/Packaging:Guidelines?rd=PackagingGuidelines#Packaging_Static_Libraries_2

(In reply to Sinny Kumari from comment #6)
I accepted all you comments and updated the spec.
* adding BuildRequires: gcc
* removed ldconfig ... in %install
* remove %defattr in %file devel
* remove LICENSE  in %file devel
* remove static lib

Spec URL: https://michelmno.fedorapeople.org/libcxl.spec
SRPM URL: https://michelmno.fedorapeople.org/libcxl-1.3-1.fc23.src.rpm

related scratch build on ppc koji:
http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=3115696

(In reply to Michel Normand from comment #7)
> Spec URL: https://michelmno.fedorapeople.org/libcxl.spec
> SRPM URL: https://michelmno.fedorapeople.org/libcxl-1.3-1.fc23.src.rpm
> 
> related scratch build on ppc koji:
> http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=3115696

Looks good to me.

Small note - After modifying SPEC file, it's good to add a new Changelog entry as well. Refer https://fedoraproject.org/wiki/Packaging:Guidelines#Multiple_Changelog_Entries_per_Release for more information.

(In reply to Sinny Kumari from comment #8)
> [CUT] ...
> Small note - After modifying SPEC file, it's good to add a new Changelog
> entry as well. Refer
> https://fedoraproject.org/wiki/Packaging:
> Guidelines#Multiple_Changelog_Entries_per_Release for more information.

Thanks for the review.
I voluntarily did not modified the Changelog because not yet in fedora db.

(In reply to Sinny Kumari from comment #8)
> (In reply to Michel Normand from comment #7)
> > Spec URL: https://michelmno.fedorapeople.org/libcxl.spec
> > SRPM URL: https://michelmno.fedorapeople.org/libcxl-1.3-1.fc23.src.rpm
> > 
> > related scratch build on ppc koji:
> > http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=3115696
> 
> Looks good to me.
> 
.
Hello Red Hat / Sinny or Dan,
... which are the next steps to malke this package available in Fedora ..?
Please advise ...
Thanks for your support.

formal review is here, see the notes explaining OK* and BAD statuses below:

OK      source files match upstream:
            46189245b46fa3a53b9f983d6c30130661ad928f  v1.3.tar.gz
OK      package meets naming and versioning guidelines.
BAD     specfile is properly named, is cleanly written and uses macros consistently.
OK      dist tag is present.
OK      license field matches the actual license.
OK      license is open source-compatible (ASL 2.0). License text included in package.
OK      latest version is being packaged.
OK      BuildRequires are proper.
BAD     compiler flags are appropriate.
OK      package builds in mock (Rawhide/ppc64,ppc64le).
OK      debuginfo package looks complete.
BAD     rpmlint is silent.
OK      final provides and requires look sane.
N/A     %check is present and all tests pass.
BAD     no shared libraries are added to the regular linker search paths.
OK      owns the directories it creates.
OK      doesn't own any directories it shouldn't.
OK      no duplicates in %files.
OK      file permissions are appropriate.
OK      correct scriptlets present.
OK      code, not content.
OK      documentation is small, so no -docs subpackage is necessary.
OK      %docs are not necessary for the proper functioning of the package.
OK      headers in devel subpackage.
OK      no pkgconfig files.
OK      no libtool .la droppings.
OK      not a GUI app.

- do not explicitly compress the man pages, it's done automagically by rpmbuild
- you should skip the "rm -rf" in %install, rpmbuild does that automatically
- use a verbose build so we can confirm what compiler flags are used, the distro-wide flags from $RPM_OPT_FLAGS/%{optflags} must be used (https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags)
- rpmlint complains a bit
libcxl.ppc64le: W: spelling-error %description -l en_US Userland -> User land, User-land, Slanderous
libcxl.ppc64le: W: spelling-error %description -l en_US powerpc -> PowerPC
libcxl.ppc64le: W: spelling-error %description -l en_US cxl -> cl, cal, col
libcxl.ppc64le: W: spelling-error %description -l en_US txt -> text, ext, tit
libcxl.ppc64le: W: spelling-error %description -l en_US userland -> user land, user-land, slanderous
libcxl.ppc64le: E: no-ldconfig-symlink /usr/lib64/libcxl.so
3 packages and 0 specfiles checked; 1 errors, 5 warnings.
- libcxl.so should be a symlink to the actual library file that contains version info (eg. soname version) in the name, with the symlink in devel subpackage

Thank you Dan for the review.
I modified my spec file to address the reported issues, except for "spelling error" I am not able to verify. (see comment below)

(In reply to Dan Horák from comment #11)
> formal review is here, see the notes explaining OK* and BAD statuses below:
> 
> {CUT] ...
> 
> - do not explicitly compress the man pages, it's done automagically by
> rpmbuild
> - you should skip the "rm -rf" in %install, rpmbuild does that automatically
> - use a verbose build so we can confirm what compiler flags are used, the
> distro-wide flags from $RPM_OPT_FLAGS/%{optflags} must be used
> (https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags)
> - rpmlint complains a bit
> libcxl.ppc64le: W: spelling-error %description -l en_US Userland -> User
> land, User-land, Slanderous
> libcxl.ppc64le: W: spelling-error %description -l en_US powerpc -> PowerPC
> libcxl.ppc64le: W: spelling-error %description -l en_US cxl -> cl, cal, col
> libcxl.ppc64le: W: spelling-error %description -l en_US txt -> text, ext, tit
> libcxl.ppc64le: W: spelling-error %description -l en_US userland -> user
> land, user-land, slanderous

from where are coming the "spelling-error" check ?
If I run myself the rpmlint with no specific option on generated rpm file I do not have such warning.
Do you have a specific configuration file ?

> libcxl.ppc64le: E: no-ldconfig-symlink /usr/lib64/libcxl.so
> 3 packages and 0 specfiles checked; 1 errors, 5 warnings.
> - libcxl.so should be a symlink to the actual library file that contains
> version info (eg. soname version) in the name, with the symlink in devel
> subpackage

(In reply to Michel Normand from comment #12)
> Thank you Dan for the review.
> I modified my spec file to address the reported issues, except for "spelling
> error" I am not able to verify. (see comment below)

thanks, please upload the updated spec and srpm when finished
 
> (In reply to Dan Horák from comment #11)
> > formal review is here, see the notes explaining OK* and BAD statuses below:
> > 
> > {CUT] ...
> > 
> > - do not explicitly compress the man pages, it's done automagically by
> > rpmbuild
> > - you should skip the "rm -rf" in %install, rpmbuild does that automatically
> > - use a verbose build so we can confirm what compiler flags are used, the
> > distro-wide flags from $RPM_OPT_FLAGS/%{optflags} must be used
> > (https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags)
> > - rpmlint complains a bit
> > libcxl.ppc64le: W: spelling-error %description -l en_US Userland -> User
> > land, User-land, Slanderous
> > libcxl.ppc64le: W: spelling-error %description -l en_US powerpc -> PowerPC
> > libcxl.ppc64le: W: spelling-error %description -l en_US cxl -> cl, cal, col
> > libcxl.ppc64le: W: spelling-error %description -l en_US txt -> text, ext, tit
> > libcxl.ppc64le: W: spelling-error %description -l en_US userland -> user
> > land, user-land, slanderous
> 
> from where are coming the "spelling-error" check ?
> If I run myself the rpmlint with no specific option on generated rpm file I
> do not have such warning.
> Do you have a specific configuration file ?

hm, nothing special AFAIK, I'm running it on F-22, but the spelling errors are often harmless. Only "userland" seems to be valid complaint here (and still with question mark :-)).
 
> > libcxl.ppc64le: E: no-ldconfig-symlink /usr/lib64/libcxl.so
> > 3 packages and 0 specfiles checked; 1 errors, 5 warnings.
> > - libcxl.so should be a symlink to the actual library file that contains
> > version info (eg. soname version) in the name, with the symlink in devel
> > subpackage

(In reply to comment #13)
I made changes for all reported issues (last one reducing Description text)

Spec URL: https://michelmno.fedorapeople.org/libcxl.spec
SRPM URL: https://michelmno.fedorapeople.org/libcxl-1.3-1.fc24.src.rpm

related scratch build on ppc koji:
http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=3192570

additional comments:
- please update release for each iteration, it hard to track the changes made if it's kept the same and spec and srpms are overridden
- you can drop also the "build" dir for man pages, it's a no-op at the end
mkdir -p build/man3
cp man3/*.3 build/man3
- I would use only VERS_LIB=%{version} otherwise it appends also ".1.fc24" which is not correct IMO
- the libcxl.so symlink need to be created manually or the makefile be updated to create it, then "%files" should have
%{_libdir}/libcxl.so.*
and "%files devel" then
%{_libdir}/libcxl.so

(In reply to Dan Horák from comment #15)
> additional comments:
> - please update release for each iteration, it hard to track the changes
> made if it's kept the same and spec and srpms are overridden

I will do for next

> - you can drop also the "build" dir for man pages, it's a no-op at the end
> mkdir -p build/man3
> cp man3/*.3 build/man3

There are other files in  man3/ I do not want to package
this is the reason why i did a copy before to install build/man3/

> - I would use only VERS_LIB=%{version} otherwise it appends also ".1.fc24"
> which is not correct IMO

I will do

> - the libcxl.so symlink need to be created manually or the makefile be
> updated to create it, then "%files" should have
> %{_libdir}/libcxl.so.*
> and "%files devel" then
> %{_libdir}/libcxl.so

I am lost :( for me the symbolic link is created by ldconfig at install time
from the soname information present in the shipped so file
===
$ls -ltr /usr/lib64/libcxl.so*
-rwxr-xr-x. 1 root root 69216 mars   3 16:59 /usr/lib64/libcxl.so.1.3
lrwxrwxrwx. 1 root root    13 mars   3 17:00 /usr/lib64/libcxl.so.1 -> libcxl.so.1.3
===
$rpm -ql libcxl
/usr/lib64/libcxl.so.1.3
/usr/share/doc/libcxl
/usr/share/doc/libcxl/README.md
/usr/share/licenses/libcxl
/usr/share/licenses/libcxl/LICENSE
===

(In reply to Dan Horák from comment #15)
> additional comments:
> - please update release for each iteration, it hard to track the changes
> made if it's kept the same and spec and srpms are overridden

I did it => release 2 for rpm below

> - you can drop also the "build" dir for man pages, it's a no-op at the end
> mkdir -p build/man3
> cp man3/*.3 build/man3

not done as detailed in comment #16

> - I would use only VERS_LIB=%{version} otherwise it appends also ".1.fc24"
> which is not correct IMO

I did it

> - the libcxl.so symlink need to be created manually or the makefile be
> updated to create it, then "%files" should have
> %{_libdir}/libcxl.so.*
> and "%files devel" then
> %{_libdir}/libcxl.so

not done as per question in comment #16


Spec URL: https://michelmno.fedorapeople.org/libcxl.spec
SRPM URL: https://michelmno.fedorapeople.org/libcxl-1.3-2.fc24.src.rpm

Dan, any update ?

(In reply to Michel Normand from comment #16)
> (In reply to Dan Horák from comment #15)
> > additional comments:
> > - please update release for each iteration, it hard to track the changes
> > made if it's kept the same and spec and srpms are overridden
> 
> I will do for next
> 
> > - you can drop also the "build" dir for man pages, it's a no-op at the end
> > mkdir -p build/man3
> > cp man3/*.3 build/man3
> 
> There are other files in  man3/ I do not want to package
> this is the reason why i did a copy before to install build/man3/
> 
> > - I would use only VERS_LIB=%{version} otherwise it appends also ".1.fc24"
> > which is not correct IMO
> 
> I will do
> 
> > - the libcxl.so symlink need to be created manually or the makefile be
> > updated to create it, then "%files" should have
> > %{_libdir}/libcxl.so.*
> > and "%files devel" then
> > %{_libdir}/libcxl.so
> 
> I am lost :( for me the symbolic link is created by ldconfig at install time
> from the soname information present in the shipped so file
> ===
> $ls -ltr /usr/lib64/libcxl.so*
> -rwxr-xr-x. 1 root root 69216 mars   3 16:59 /usr/lib64/libcxl.so.1.3
> lrwxrwxrwx. 1 root root    13 mars   3 17:00 /usr/lib64/libcxl.so.1 ->
> libcxl.so.1.3

there are usually 3 files for a library
- first the versioned real file - /usr/lib64/libcxl.so.1.3
- second the symlink with soname (can be created by ldconfig) - /usr/lib64/libcxl.so.1
- and last the development symlink libfoo.so that is used by a linker when linking an app with the foo library
The first 2 files go to the main package (via %{_libdir}/libfoo.so.*), the last one to the devel subpackage (via %{_libdir}/libfoo.so).

(In reply to Dan Horák from comment #19)
> (In reply to Michel Normand from comment #16)
> [CUT] ...
> 
> there are usually 3 files for a library
> - first the versioned real file - /usr/lib64/libcxl.so.1.3
> - second the symlink with soname (can be created by ldconfig) -
> /usr/lib64/libcxl.so.1
> - and last the development symlink libfoo.so that is used by a linker when
> linking an app with the foo library
> The first 2 files go to the main package (via %{_libdir}/libfoo.so.*), the
> last one to the devel subpackage (via %{_libdir}/libfoo.so).

spec updated as requested:

Spec URL: https://michelmno.fedorapeople.org/libcxl.spec.rel3
SRPM URL: https://michelmno.fedorapeople.org/libcxl-1.3-3.fc24.src.rpm

related scratchbuild
http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=3215313

Two minor nitpicks - use an empty line to separate the entries in %changelog and use "cp -p" when copying the man pages to the man3 dir in %build. Please do changes before committing the initial version to git.

The libcxl package is APPROVED.

Awaiting package request creation in fedora db, before to commit above.
https://admin.fedoraproject.org/pkgdb/packager/michelmno/requests

Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/libcxl

package source available now in master and scratchbuild OK
http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=3226265

I see it's even a regular build so we can close the bug. Please do also a rawhide (f25) build from the master branch.