Bug 1534052

Summary: Review Request: golang-github-mdlayher-netlink - Package netlink provides low-level access to Linux netlink sockets
Product: [Fedora] Fedora Reporter: Paul Gier <pgier>
Component: Package ReviewAssignee: Robert-André Mauchin 🐧 <zebob.m>
Status: CLOSED CURRENTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: package-review, quantum.analyst, thib, zebob.m
Target Milestone: ---Flags: zebob.m: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: golang-github-mdlayher-netlink-0-0.1.20180511gitf8bbad5.fc29 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2020-02-21 07:33:03 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1512702, 1576519    

Comment 1 Robert-André Mauchin 🐧 2018-01-14 20:09:04 UTC
 - In case of git snapshot, you need to add the commit date to the Release field:

%global commit          82077d593cd599c082317ceb5cfb22ee3fbc75ea
%global shortcommit     %(c=%{commit}; echo ${c:0:7})
%global commitdate      20170915

Name:           golang-%{provider}-%{project}-%{repo}
Version:        0
Release:        1.%{commitdate}git%{shortcommit}%{?dist}

 - Packaging the latest git commit would be nice.

 - Again, why did you comment out all BR and R?

Comment 2 Robert-André Mauchin 🐧 2018-04-25 15:59:37 UTC
If you're ok with only packaging for F28 and later, you could switch to the new Go packaging like your other package.

Comment 3 Paul Gier 2018-05-07 21:21:42 UTC
Updated spec file to follow new packaging recommendations.
New spec sources and srpm are here: https://pgier.fedorapeople.org/golang/golang-github-mdlayher-netlink/

Koji scratch build here: https://koji.fedoraproject.org/koji/taskinfo?taskID=26843056

Comment 4 Paul Gier 2018-05-09 14:12:24 UTC
New Koji scratch build to fix diff between arches: https://koji.fedoraproject.org/koji/taskinfo?taskID=26857433

Comment 5 Robert-André Mauchin 🐧 2018-05-09 15:36:05 UTC
Not ok:

# force these to be installed to avoid diffs between arches
GOSRCPATH=/usr/share/gocode/src/github.com/mdlayher/netlink
install sockopt_linux.go %{buildroot}${GOSRCPATH}/sockopt_linux.go
install sockopt_linux_386.go %{buildroot}${GOSRCPATH}/sockopt_linux_386.go
install sockopt_linux_386.s %{buildroot}${GOSRCPATH}/sockopt_linux_386.s

The package should not be noarch ih this case:

It may be tempting to make the header library package noarch, since the header files themselves are simply text. However, a library should have tests which should be run on all architectures. Also, the install process may modify the installed headers depending on the build architecture. For these reasons, header-only packages must not be marked noarch.

https://fedoraproject.org/wiki/Packaging:Guidelines#Do_not_use_noarch

Comment 6 Robert-André Mauchin 🐧 2018-05-09 15:38:55 UTC
 - Release should start at 0.1 for snapshot:

Release: 0.1%{?dist}

   Also fix it in the %changelog entry.

Comment 7 Robert-André Mauchin 🐧 2018-05-09 15:54:16 UTC
Is there any reasons to disable the checks?

Comment 8 Paul Gier 2018-05-09 19:46:11 UTC
Thanks!  I have made the requested changes, and a new build is here:
https://koji.fedoraproject.org/koji/taskinfo?taskID=26861566

I had disabled the tests because one of the was failing locally using upstream sources.  I created a patch to skip the failing test and re-enabled the checks in the specfile.

Is it correct to specify the arches using this?
ExclusiveArch: %{go_arches}

I also had to exclude the s390x arch due to failing tests.

Comment 9 Robert-André Mauchin 🐧 2018-05-09 22:25:53 UTC
(In reply to Paul Gier from comment #8)
> Thanks!  I have made the requested changes, and a new build is here:
> https://koji.fedoraproject.org/koji/taskinfo?taskID=26861566
> 
> I had disabled the tests because one of the was failing locally using
> upstream sources.  I created a patch to skip the failing test and re-enabled
> the checks in the specfile.
> 
> Is it correct to specify the arches using this?
> ExclusiveArch: %{go_arches}
> 
Not needed, %gometa already takes care of that.

> I also had to exclude the s390x arch due to failing tests.

Report the failing tests upstream. Also per the guidelines, you'll need to create a bugreport for this, see https://fedoraproject.org/wiki/Packaging:Guidelines#Architecture_Build_Failures

 - Reduce the Summary size and remove the dot at the end:

golang-github-mdlayher-netlink-devel.x86_64: W: summary-ended-with-dot C Package netlink provides low-level access to Linux netlink sockets. MIT Licensed.
golang-github-mdlayher-netlink-devel.x86_64: E: summary-too-long C Package netlink provides low-level access to Linux netlink sockets. MIT Licensed.

 - Split the description to stay below 80 characters per line:

golang-github-mdlayher-netlink-devel.x86_64: E: description-line-too-long C Package netlink provides low-level access to Linux netlink sockets. MIT Licensed.

 - Latest version packaged
 - License ok
 - Builds in Mock
 - Conforms to the Packaging Guidelines


Package approved.

Comment 10 Paul Gier 2018-05-11 13:30:07 UTC
I made the requested changed, reported the failing tests upstream, and was able to fix one of the tests so it no longer needs to be patched out.  Also re-enabled the build (but skipped tests) on s390x.
https://koji.fedoraproject.org/koji/taskinfo?taskID=26883450

Comment 11 Gwyn Ciesla 2018-05-11 13:46:48 UTC
(fedrepo-req-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/golang-github-mdlayher-netlink

Comment 12 Paul Gier 2018-05-11 14:59:55 UTC
Pagure repo has been created: https://pagure.io/releng/fedora-scm-requests/issue/6509
Build is complete: https://koji.fedoraproject.org/koji/taskinfo?taskID=26895873

Comment 13 Elliott Sales de Andrade 2020-02-21 07:33:03 UTC
Please close your review requests after you have imported and built the package.