Bug 865977 - Review Request: libsigrokdecode - Basic API for running protocol decoders
Review Request: libsigrokdecode - Basic API for running protocol decoders
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Michael Schwendt
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 865979
  Show dependency treegraph
 
Reported: 2012-10-12 21:32 EDT by Alex G.
Modified: 2013-04-08 18:54 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-04-08 18:54:27 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
bugs.michael: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Alex G. 2012-10-12 21:32:58 EDT
Spec URL: http://g-tech.no-ip.org/~mrnuke/SPECS/libsigrokdecode.spec
SRPM URL: http://g-tech.no-ip.org/~mrnuke/SRPMS/libsigrokdecode-0.1.0-1.fc17.src.rpm
Description:
libsigrokdecode is a shared library written in C which provides the basic API for
running sigrok protocol decoders. The protocol decoders themselves are written
in Python.

Koji build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=4583894

Since this is part pf my first set of packages, I will also be needing a sponsor.
Comment 1 Michael Schwendt 2012-12-12 06:51:06 EST
> # Without this line, debuginfo will give the following error:
> # *** ERROR: No build ID note found
> %undefine _missing_build_ids_terminate_build

Can't reproduce.

Which build terminated for you? In koji? In Mock? A local build only? F17, F18? Also, when you felt it was necessary to undefine that macro, did you still get a working -debuginfo package? If not, this wouldn't be acceptable.  The comment in the spec file ought to answer more questions.
Comment 2 Alex G. 2012-12-12 13:12:13 EST
new Spec URL: http://g-tech.no-ip.org/~mrnuke/fedrev/libsigrokdecode-0.1.0-2/libsigrokdecode.spec
new SRPM URL: http://g-tech.no-ip.org/~mrnuke/fedrev/libsigrokdecode-0.1.0-2/libsigrokdecode-0.1.0-2.fc17.src.rpm

I got that error using a local build on F17 only. I later realized that it builds fine in mock and Koji. I have removed the undefine, since it doesn't seem to be needed.
Comment 4 Michael Schwendt 2013-03-17 16:18:59 EDT
A few questions about some changes in the updates:

>  %files
>  %defattr(-,root,root,-)
> -%doc README NEWS COPYING ChangeLog
> -%{_libdir}/*.so.*
> -%{_datadir}/*
> +%doc README NEWS COPYING
> +%{_libdir}/libsigrokdecode.so.0*
> +%{_datadir}/libsigrokdecode/*

/usr/share/libsigrokdecode/  is not included now:
https://fedoraproject.org/wiki/Packaging:UnownedDirectories


> $ rpmls -p libsigrokdecode-devel-0.1.1-1.fc18.x86_64.rpm |grep inc
> -rw-r--r--  /usr/include/sigrokdecode.h

The header includes Python.h which is a missing dependency in the -devel package.


> # Combined GPLv3+ and GPLv2+
> License:        GPLv3+

Some of the C source files are GPLv2+, others are GPLv3+, some Python based decoders are pure GPLv2+ and installed as such. License clarification would be good here, because the binary package clearly contains files, which are GPLv2+ licensed, and I think one cannot implicitly upgrade the licensing to GPLv3+.

 * https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Clarification

 * https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Mixed_Source_Licensing_Scenario

 * https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#GPL_Compatibility_Matrix


> make %{?_smp_mflags}

V=1 make %{?_smp_mflags}  and the build output would be more verbose and .e.g. show what compiler flags are used.


> %files
> %doc README NEWS COPYING

> %files devel
> %doc README

Not sure why the README file is duplicated. It doesn't add any convenience, but just creates a separate docdir for the -devel package and for just this file.
Comment 5 Alex G. 2013-03-19 23:41:03 EDT
(In reply to comment #4)
> A few questions about some changes in the updates:
> 
> >  %files
> >  %defattr(-,root,root,-)
> > -%doc README NEWS COPYING ChangeLog
> > -%{_libdir}/*.so.*
> > -%{_datadir}/*
> > +%doc README NEWS COPYING
> > +%{_libdir}/libsigrokdecode.so.0*
> > +%{_datadir}/libsigrokdecode/*
> 
> /usr/share/libsigrokdecode/  is not included now:
> https://fedoraproject.org/wiki/Packaging:UnownedDirectories
> 
Fixed. Tanks!

> > $ rpmls -p libsigrokdecode-devel-0.1.1-1.fc18.x86_64.rpm |grep inc
> > -rw-r--r--  /usr/include/sigrokdecode.h
> 
> The header includes Python.h which is a missing dependency in the -devel
> package.
>
Fixed. Tanks!

> > # Combined GPLv3+ and GPLv2+
> > License:        GPLv3+
> 
> Some of the C source files are GPLv2+, others are GPLv3+, some Python based
> decoders are pure GPLv2+ and installed as such. License clarification would
> be good here, because the binary package clearly contains files, which are
> GPLv2+ licensed, and I think one cannot implicitly upgrade the licensing to
> GPLv3+.
> 
I've contacted upstream, and they updated the README file. I've cherry-picked the commit as a patch.

> > make %{?_smp_mflags}
> 
> V=1 make %{?_smp_mflags}  and the build output would be more verbose and
> .e.g. show what compiler flags are used.
>
I'm not sure if you mean "V=1" should be permanently in the spec file, or just temporary, for testing.
compile flags:
-DHAVE_CONFIG_H -I.  -I/usr/include/python3.2mu -I/usr/include/python3.2mu -DDECODERS_DIR='"/usr/share/libsigrokdecode/decoders"'   -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4  -m64 -mtune=generic -Wall -Wextra -fvisibility=hidden -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include   -c 
link flags:
-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4  -m64 -mtune=generic -Wall -Wextra -fvisibility=hidden -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include   -version-info 0:1:0 -lpthread -ldl -lutil -lm -lpython3.2mu -Xlinker -export-dynamic -Wl,-z,relro  -o libsigrokdecode.la -rpath /usr/lib64 -lglib-2.0  

It looks like %{optflags} is respected.

> 
> > %files
> > %doc README NEWS COPYING
> 
> > %files devel
> > %doc README
> 
> Not sure why the README file is duplicated. It doesn't add any convenience,
> but just creates a separate docdir for the -devel package and for just this
> file.

Fixed. Thanks!
Comment 7 Michael Schwendt 2013-03-29 08:33:10 EDT
Good.

The README confirms now that the whole thing is a GPLv3+ project that has copied/merged some GPLv2+ files.


> I'm not sure if you mean "V=1" should be permanently in the spec file,
> or just temporary, for testing.

Keeping it in the spec file permanently makes it possibly to analyze build.log files in koji manually or with automated scripts. And if a build fails, verbose build output can be helpful, too.


> # libsigrokdecode.h includes <Python.h>
> Requires:       python3-devel

You're free to use  Requires: python3-devel%{?_isa}  here, because for non-virtual packages, these %_isa Provides are added automatically.

  $ repoquery --whatprovides 'python3-devel(x86-64)'
  python3-devel-0:3.3.0-10.fc19.x86_64

> # The pyhton files are designed to work with a python module that only exists

s/pyhton/python/


What's left? Some things you're probably aware of already:

* %buildroot is emptied automatically at beginning of %install
  https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag

* %defattr is not necessary anymore for
  https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions


APPROVED
Comment 8 Alex G. 2013-03-29 10:31:09 EDT
New Package SCM Request
=======================
Package Name: libsigrokdecode
Short Description: Basic API for running protocol decoders
Owners: mrnuke
Branches: f18 f19
InitialCC: mrnuke
Comment 9 Jon Ciesla 2013-03-29 10:49:13 EDT
Git done (by process-git-requests).
Comment 10 Fedora Update System 2013-03-29 20:33:54 EDT
libsigrokdecode-0.1.1-3.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/libsigrokdecode-0.1.1-3.fc18
Comment 11 Fedora Update System 2013-03-31 23:23:31 EDT
libsigrokdecode-0.1.1-3.fc18 has been pushed to the Fedora 18 testing repository.
Comment 12 Fedora Update System 2013-04-08 18:54:29 EDT
libsigrokdecode-0.1.1-3.fc18 has been pushed to the Fedora 18 stable repository.

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