Bug 1534052 - Review Request: golang-github-mdlayher-netlink - Package netlink provides low-level access to Linux netlink sockets
Summary: Review Request: golang-github-mdlayher-netlink - Package netlink provides low...
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Robert-André Mauchin 🐧
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1512702 1576519
TreeView+ depends on / blocked
 
Reported: 2018-01-12 22:48 UTC by Paul Gier
Modified: 2020-02-21 07:33 UTC (History)
4 users (show)

Fixed In Version: golang-github-mdlayher-netlink-0-0.1.20180511gitf8bbad5.fc29
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2020-02-21 07:33:03 UTC
Type: ---
zebob.m: fedora-review+


Attachments (Terms of Use)

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.


Note You need to log in before you can comment on or make changes to this bug.