[Bug 1563680] Package Review: libocxl - library for OpenCAPI accelerator

[bugzilla]

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

Hanns-Joachim Uhl  changed:

   What|Removed |Added

 Blocks||1524656



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl - library for OpenCAPI accelerator

[bugzilla]

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

Fedora Update System  changed:

   What|Removed |Added

 Status|ON_QA   |CLOSED
 Resolution|--- |ERRATA
Last Closed||2018-04-12 16:24:06



--- Comment #20 from Fedora Update System  ---
libocxl-1.0.0-0.1.fc28 has been pushed to the Fedora 28 stable repository. If
problems still persist, please make note of it in this bug report.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl - library for OpenCAPI accelerator

[bugzilla]

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

Fedora Update System  changed:

   What|Removed |Added

 Status|MODIFIED|ON_QA



--- Comment #19 from Fedora Update System  ---
libocxl-1.0.0-0.1.fc28 has been pushed to the Fedora 28 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-2018-80a761dd88

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl - library for OpenCAPI accelerator

[bugzilla]

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

Fedora Update System  changed:

   What|Removed |Added

 Status|ASSIGNED|MODIFIED



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl - library for OpenCAPI accelerator

[bugzilla]

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



--- Comment #18 from Fedora Update System  ---
libocxl-1.0.0-0.1.fc28 has been submitted as an update to Fedora 28.
https://bodhi.fedoraproject.org/updates/FEDORA-2018-80a761dd88

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl - library for OpenCAPI accelerator

[bugzilla]

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



--- Comment #17 from Gwyn Ciesla  ---
(fedrepo-req-admin):  The Pagure repository was created at
https://src.fedoraproject.org/rpms/libocxl

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl - library for OpenCAPI accelerator

[bugzilla]

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

Michel Normand  changed:

   What|Removed |Added

Summary|Package Review: libocxl |Package Review: libocxl -
   |library for OpenCAPI|library for OpenCAPI
   |accelerator |accelerator



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl library for OpenCAPI accelerator

[bugzilla]

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



--- Comment #16 from Michel Normand  ---
scratch build on koji:
https://koji.fedoraproject.org/koji/taskinfo?taskID=26291946

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl library for OpenCAPI accelerator

[bugzilla]

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

Dan Horák  changed:

   What|Removed |Added

  Flags||fedora-review+



--- Comment #15 from Dan Horák  ---
OK, the package looks good, APPROVED.

one nitpick - the Summary for docs should be "HTML doc files for ..."
(s/doxygen/HTML/) or even only "Documentation files for %{name}". Please fix
before building.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl library for OpenCAPI accelerator

[bugzilla]

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



--- Comment #14 from Michel Normand  ---
OK I moved man in *devel rpm and kept html in *docs rpm

I did not change the Source0 to ease compare with previous URL line and allow
direct access from either vi or emacs editors when in spec.
URL: https://github.com/OpenCAPI/libocxl
Source0: https://github.com/OpenCAPI/libocxl/archive/%{version}-beta2.tar.gz

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl library for OpenCAPI accelerator

[bugzilla]

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



--- Comment #13 from Dan Horák  ---
If the content of the man pages and html file is the same, then I would merge
the man pages into the devel subpackage (where usually basic API documentation
goes, I mean headers + man pages should be installed together) and removed the
docs subpackage. On the other hand the html version viewed in a browser is
nice, so it would make sense to keep it ...

You can use
Source0:
https://github.com/OpenCAPI/%{name}/archive/%{version}-beta2/%{name}-%{version}-beta2.tar.gz
to have better named source archive file.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl library for OpenCAPI accelerator

[bugzilla]

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



--- Comment #12 from Michel Normand  ---
(In reply to Michel Normand from comment #11)
> I updated the spec file for your previous comments
> except the man pages and html ones both generated by doxygen, 
> so kept them  both in the noarch package.

Dan is it OK or should I really split man and html in differerent rpms ?

spec and srpm updated from new beta2 upstream release (no more patches in spec)
spec: https://michelmno.fedorapeople.org/libocxl/libocxl.spec
srpm:
https://michelmno.fedorapeople.org/libocxl/SRPMS/libocxl-1.0.0-0.1.fc29.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl library for OpenCAPI accelerator

[bugzilla]

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



--- Comment #11 from Michel Normand  ---
I updated the spec file for your previous comments
except the man pages and html ones both generated by doxygen, 
so kept them  both in the noarch package.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl library for OpenCAPI accelerator

[bugzilla]

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



--- Comment #10 from Dan Horák  ---
- the Release tag value should be 0.1 as the 1.0.0 version is in beta (see
https://fedoraproject.org/wiki/Packaging:Versioning#Prerelease_versions)
- I would put the man pages to -devel, but kept the html in the separate -docs,
this is the usual split, but aren't the HTML docs duplicate of the man pages
content-wise (just different format)?
- you can use %{name}-%{version} in the Source0 URL
- I would split the BuildRequires into 2 lines, for more complex packages it's
then easier to track changes in BR

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl library for OpenCAPI accelerator

[bugzilla]

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



--- Comment #9 from Michel Normand  ---
done

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl library for OpenCAPI accelerator

[bugzilla]

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



--- Comment #8 from Dan Horák  ---
If we won't plan to include libocxl to F-27 (and I think it's not necessary,
F-28+ should be good), then you can remove the ldconfig scriptlets.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl library for OpenCAPI accelerator

[bugzilla]

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



--- Comment #7 from Michel Normand  ---
spec and srpm updated from Robert-Andre's review
spec: https://michelmno.fedorapeople.org/libocxl/libocxl.spec
srpm:
https://michelmno.fedorapeople.org/libocxl/SRPMS/libocxl-1.0.0-1.fc29.src.rpm

Dan, I already added the suggested %ldconfig_scriptlets, should I remove it ?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl library for OpenCAPI accelerator

[bugzilla]

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



--- Comment #6 from Dan Horák  ---
Just add "BuildArch: noarch" into the "%package docs" section.

I agree with Robert-Andre's review, but you should be able to drop the
"ldconfig" stuff completely as standard stuff is handled automagically.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl library for OpenCAPI accelerator

[bugzilla]

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



--- Comment #5 from Michel Normand  ---
(In reply to Michel Normand from comment #4)
> 
> Thank your for all comments;
> Question: how to define in spec the *-docs subpackage as noarch package ?
> while other packages are arch specific (with ExclusiveArch)

answering myself: using BuildArch in related Package definition.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl library for OpenCAPI accelerator

[bugzilla]

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



--- Comment #4 from Michel Normand  ---
(In reply to Robert-André Mauchin from comment #3)
> [CUT] ...
> Actually since it seems to be documentation, I believe it should be
> installed in %{_pkgdocdir} (i.e /usr/share/doc/libocxl ). Since you're
> already patching the Makefile, you could probably change the install
> directory of the docs.
> 
> 
>  - The docs should be split in a separate noarch -docs subpackage:
> 
> [!]: Large data in /usr/share should live in a noarch subpackage if package
>  is arched.
>  Note: Arch-ed rpms have a total of 1228800 bytes in /usr/share
> 

Thank your for all comments;
Question: how to define in spec the *-docs subpackage as noarch package ? while
other packages are arch specific (with ExclusiveArch)

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl library for OpenCAPI accelerator

[bugzilla]

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



--- Comment #3 from Robert-André Mauchin  ---
 - Please add a comment for each patch explaining what they do

 - Use the new %ldconfig_scriptlets macro instead of:

%post -p /sbin/ldconfig

%postun -p /sbin/ldconfig

   See
https://fedoraproject.org/wiki/Changes/Removing_ldconfig_scriptlets#Upgrade.2Fcompatibility_impact

 - Build error:

BUILDSTDERR: /var/tmp/rpm-tmp.PWgKId: line 41: cd: libocxl-1.0: No such file or
directory

   Fix it by passing the correct directory to %setup:

%setup -q -n %{name}-%{version}-RELEASE

 - You could replace:

%setup -q -n %{name}-%{version}-RELEASE
%patch1 -p1
%patch2 -p1
%patch3 -p1

   with: 

%autosetup -p1 -n %{name}-%{version}-RELEASE

 - Patch error:

+ /usr/bin/cat /builddir/build/SOURCES/irq_trace_ppc64.patch
+ /usr/bin/patch -p1 -s --fuzz=0 --no-backup-if-mismatch
Reversed (or previously applied) patch detected!  Assume -R? [n] 
Apply anyway? [n] 
1 out of 1 hunk ignored -- saving rejects to file src/irq.c.rej

Patch irq_trace_ppc64.patch is already applied in the 1.0 Release.

 - Own /usr/share/libocxl by removing the * in %files:

%{_datarootdir}/libocxl

[!]: Package must own all directories that it creates.
 Note: Directories without known owners: /usr/share/libocxl

Actually since it seems to be documentation, I believe it should be installed
in %{_pkgdocdir} (i.e /usr/share/doc/libocxl ). Since you're already patching
the Makefile, you could probably change the install directory of the docs.


 - The docs should be split in a separate noarch -docs subpackage:

[!]: Large data in /usr/share should live in a noarch subpackage if package
 is arched.
 Note: Arch-ed rpms have a total of 1228800 bytes in /usr/share

 - The Makefile doesn't keep timestamps while installing files. To fix this,
replace the occurrences of "install" with $(INSTALL), i.e.:

$(INSTALL) -m 0755 obj/$(LIBNAME) $(libdir)/
cp -d obj/libocxl.so obj/$(LIBSONAME) $(libdir)/
$(INSTALL) -m 0644 src/include/libocxl.h  $(includedir)/
$(INSTALL) -m 0644 -D docs/man/man3/* $(mandir)/man3
$(INSTALL) -m 0644 -D docs/html/*.* $(datadir)/libocxl
$(INSTALL) -m 0644 -D docs/html/search/* $(datadir)/libocxl/search

The $(INSTALL) variable is set up by the %make_install macro, replacing it with
install -p, which keeps timestamps.



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]: Header files in -devel subpackage, if present.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.
[x]: Development (unversioned) .so files in -devel subpackage, if present.

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.
 Note: Checking patched sources after %prep for licenses. Licenses
 found: "Apache (v2.0)", "Unknown or generated", "*No copyright* Apache
 (v2.0)". 22 files have unknown license. Detailed output of
 licensecheck in /home/bob/packaging/review/libocxl/review-
 libocxl/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[x]: Package requires other packages for directories it uses.
 Note: No known owner of /usr/share/libocxl
[!]: Package must own all directories that it creates.
 Note: Directories without known owners: /usr/share/libocxl
[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.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: 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.
[-]: Large documentation must go in a -doc subpackage. Large could be size
 (~1MB) or number of files.
 Note: Documentation size is 10240 bytes in 1 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and 

[Bug 1563680] Package Review: libocxl library for OpenCAPI accelerator

[bugzilla]

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

Hanns-Joachim Uhl  changed:

   What|Removed |Added

 Blocks||1523862



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl library for OpenCAPI accelerator

[bugzilla]

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

Hanns-Joachim Uhl  changed:

   What|Removed |Added

 CC||bugpr...@us.ibm.com,
   ||hannsj_...@de.ibm.com
External Bug ID||IBM Linux Technology Center
   ||166425
 OS|Unspecified |Linux



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl library for OpenCAPI accelerator

[bugzilla]

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

Dan Horák  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
   Assignee|nob...@fedoraproject.org|d...@danny.cz
  Flags|needinfo?(d...@danny.cz) |



--- Comment #2 from Dan Horák  ---
yup, taking

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1563680] Package Review: libocxl library for OpenCAPI accelerator

[bugzilla]

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

Michel Normand  changed:

   What|Removed |Added

 CC||d...@danny.cz
  Flags||needinfo?(d...@danny.cz)



--- Comment #1 from Michel Normand  ---
Hello Dan, could you take this bug for review ?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org