Bug 1209971 - Review Request: usbguard - A tool for implementing USB device usage policy
Summary: Review Request: usbguard - A tool for implementing USB device usage policy
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Petr Lautrbach
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 1210753 1210754
Blocks: usbguard-applet-qt
TreeView+ depends on / blocked
 
Reported: 2015-04-08 14:54 UTC by Daniel Kopeček
Modified: 2015-04-30 15:33 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-04-30 15:33:35 UTC
Type: ---
plautrba: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)
fedora-review output (9.53 KB, text/plain)
2015-04-09 08:01 UTC, Petr Lautrbach
no flags Details

Description Daniel Kopeček 2015-04-08 14:54:12 UTC
Spec URL: https://fedorapeople.org/~dkopecek/usbguard/usbguard.spec
SRPM URL: https://fedorapeople.org/~dkopecek/usbguard/usbguard-0.2-2.fc20.src.rpm
Description: A tool for implementing USB device usage policy
Fedora Account System Username: mildew

Comment 1 Petr Lautrbach 2015-04-09 08:01:15 UTC
Created attachment 1012537 [details]
fedora-review output

Comment 2 Petr Lautrbach 2015-04-09 08:08:33 UTC
- usbguard.x86_64: E: zero-length /etc/usbguard/rules.conf

It could be useful to have at least some commented example or reference to the documentation

- usbguard.x86_64: E: executable-marked-as-config-file /etc/usbguard/usbguard-daemon.conf

$ ls -lZ /etc/usbguard/usbguard-daemon.conf
-rwxr-xr-x. 1 root root system_u:object_r:etc_t:s0 946 Apr  8 16:34 /etc/usbguard/usbguard-daemon.conf


- usbguard.x86_64: E: script-without-shebang /etc/usbguard/usbguard-daemon.conf
- usbguard.x86_64: E: script-without-shebang /usr/lib/systemd/system/usbguard.service

$ ls -lZ /etc/usbguard/usbguard-daemon.conf /usr/lib/systemd/system/usbguard.service
-rwxr-xr-x. 1 root root system_u:object_r:etc_t:s0               946 Apr  8 16:34 /etc/usbguard/usbguard-daemon.conf
-rwxr-xr-x. 1 root root system_u:object_r:systemd_unit_file_t:s0 230 Apr  3 18:05 /usr/lib/systemd/system/usbguard.service

Comment 3 Petr Lautrbach 2015-04-09 08:12:59 UTC
 54 %ifarch sparc64
 55 #sparc64 need big PIE
 56 export CXXFLAGS="$RPM_OPT_FLAGS -fPIE"
 57 export CFLAGS=$CXXFLAGS
 58 export LDFLAGS="-pie -Wl,-z,relro -Wl,-z,now"
 59 %else
 60 export CXXFLAGS="$RPM_OPT_FLAGS -fpie"
 61 export CFLAGS=$CXXFLAGS
 62 export LDFLAGS="-pie -Wl,-z,relro -Wl,-z,now"
 63 %endif

I would rather use:

%global _hardened_build 1

see https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#PIE

Comment 4 Petr Lautrbach 2015-04-09 08:25:17 UTC
Is there a reason to ship /usr/lib64/libusbguard.a in usbguard-devel? see https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#StaticLibraries

Comment 5 Ralf Corsepius 2015-04-09 09:06:28 UTC
* Building is non-verbose
This renders checking buildflags from build.logs impossible.
Please append --disable-silent-rules to %configure.

* Presumably bundled libraries:
ThirdParty/json
ThirdParty/spdlog
ThirdParty/cppformat

* Licensing is entirely unclear to me.
There doesn't seem to be any detached "license" file nor a README which states a clear license. Most source files seem to lack proper copyright/license clauses.

* The devel package likely should R: libstdc++-devel

Comment 6 Daniel Kopeček 2015-04-09 15:31:13 UTC
Fixed SRPM.

SRPM URL: https://fedorapeople.org/~dkopecek/usbguard/usbguard-0.3-1.fc20.src.rpm

Comment 7 Daniel Kopeček 2015-04-09 15:32:18 UTC
(In reply to Petr Lautrbach from comment #2)
> - usbguard.x86_64: E: zero-length /etc/usbguard/rules.conf
> 
> It could be useful to have at least some commented example or reference to
> the documentation
> 
> - usbguard.x86_64: E: executable-marked-as-config-file
> /etc/usbguard/usbguard-daemon.conf

Fixed.

> $ ls -lZ /etc/usbguard/usbguard-daemon.conf
> -rwxr-xr-x. 1 root root system_u:object_r:etc_t:s0 946 Apr  8 16:34
> /etc/usbguard/usbguard-daemon.conf
>
> - usbguard.x86_64: E: script-without-shebang
> /etc/usbguard/usbguard-daemon.conf
> - usbguard.x86_64: E: script-without-shebang
> /usr/lib/systemd/system/usbguard.service

Fixed.
 
> $ ls -lZ /etc/usbguard/usbguard-daemon.conf
> /usr/lib/systemd/system/usbguard.service
> -rwxr-xr-x. 1 root root system_u:object_r:etc_t:s0               946 Apr  8
> 16:34 /etc/usbguard/usbguard-daemon.conf
> -rwxr-xr-x. 1 root root system_u:object_r:systemd_unit_file_t:s0 230 Apr  3
> 18:05 /usr/lib/systemd/system/usbguard.service

Comment 8 Daniel Kopeček 2015-04-09 15:32:53 UTC
(In reply to Petr Lautrbach from comment #3)
>  54 %ifarch sparc64
>  55 #sparc64 need big PIE
>  56 export CXXFLAGS="$RPM_OPT_FLAGS -fPIE"
>  57 export CFLAGS=$CXXFLAGS
>  58 export LDFLAGS="-pie -Wl,-z,relro -Wl,-z,now"
>  59 %else
>  60 export CXXFLAGS="$RPM_OPT_FLAGS -fpie"
>  61 export CFLAGS=$CXXFLAGS
>  62 export LDFLAGS="-pie -Wl,-z,relro -Wl,-z,now"
>  63 %endif
> 
> I would rather use:
> 
> %global _hardened_build 1
> 
> see
> https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/
> Guidelines#PIE

Fixed by using the _hardened_build variable.

Comment 9 Daniel Kopeček 2015-04-09 15:33:26 UTC
(In reply to Petr Lautrbach from comment #4)
> Is there a reason to ship /usr/lib64/libusbguard.a in usbguard-devel? see
> https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/
> Guidelines#StaticLibraries

Fixed (by removing from the binary rpm)

Comment 10 Daniel Kopeček 2015-04-09 15:36:11 UTC
(In reply to Ralf Corsepius from comment #5)
> * Building is non-verbose
> This renders checking buildflags from build.logs impossible.
> Please append --disable-silent-rules to %configure.

Fixed.

> * Presumably bundled libraries:
> ThirdParty/json
> ThirdParty/spdlog
> ThirdParty/cppformat

Can't fix. These are small header-only dependencies and the project requires them. There are no packages in Fedora for these projects.

> * Licensing is entirely unclear to me.
> There doesn't seem to be any detached "license" file nor a README which
> states a clear license. Most source files seem to lack proper
> copyright/license clauses.
> 
> * The devel package likely should R: libstdc++-devel

Added.

Comment 11 Ralf Corsepius 2015-04-09 16:06:52 UTC
(In reply to Daniel Kopeček from comment #10)
> (In reply to Ralf Corsepius from comment #5)

> > * Presumably bundled libraries:
> > ThirdParty/json
> > ThirdParty/spdlog
> > ThirdParty/cppformat
> 
> Can't fix. These are small header-only dependencies and the project requires
> them. There are no packages in Fedora for these projects.
This doesn't invalidate my considerations.

IMO, they are bundled libraries - This is a MUSTFIX.

Either you need to file tickets to FPC to apply for bundling exceptions or you need to package these as separate packages.

> > * Licensing is entirely unclear to me.
> > There doesn't seem to be any detached "license" file nor a README which
> > states a clear license. Most source files seem to lack proper
> > copyright/license clauses.
What I about this? It's a MUSTFIX.

Comment 12 Daniel Kopeček 2015-04-09 16:19:45 UTC
(In reply to Ralf Corsepius from comment #11)
> (In reply to Daniel Kopeček from comment #10)
> > (In reply to Ralf Corsepius from comment #5)
> > > * Licensing is entirely unclear to me.
> > > There doesn't seem to be any detached "license" file nor a README which
> > > states a clear license. Most source files seem to lack proper
> > > copyright/license clauses.
> What I about this? It's a MUSTFIX.

I've added a LICENSE file and license headers to the source files.

Comment 13 Daniel Kopeček 2015-04-09 16:20:34 UTC
(In reply to Ralf Corsepius from comment #11)
> (In reply to Daniel Kopeček from comment #10)
> > (In reply to Ralf Corsepius from comment #5)
> 
> > > * Presumably bundled libraries:
> > > ThirdParty/json
> > > ThirdParty/spdlog
> > > ThirdParty/cppformat
> > 
> > Can't fix. These are small header-only dependencies and the project requires
> > them. There are no packages in Fedora for these projects.
> This doesn't invalidate my considerations.
> 
> IMO, they are bundled libraries - This is a MUSTFIX.
> 
> Either you need to file tickets to FPC to apply for bundling exceptions or
> you need to package these as separate packages.

I'll file for an exception then. They can be classified as copylibs, I think.

Comment 14 Daniel Kopeček 2015-04-09 16:36:12 UTC
FPC ticket: https://fedorahosted.org/fpc/ticket/523

Comment 15 Daniel Kopeček 2015-04-10 10:05:22 UTC
Removed the cppformat library from the source tree

New SRPM is at: https://fedorapeople.org/~dkopecek/usbguard/usbguard-0.3p1-1.fc20.src.rpm

spec file url is the same.

Comment 16 Daniel Kopeček 2015-04-10 13:52:51 UTC
Submitted review requests for the remaining libraries. Ignore the FPC ticket.

Comment 17 Daniel Kopeček 2015-04-11 14:12:13 UTC
Added dependencies on json-static and spdlog-static. Bundled libraries are removed in %prep.

New SRPM is at: https://fedorapeople.org/~dkopecek/usbguard/usbguard-0.3p2-1.fc20.src.rpm

Comment 18 Daniel Kopeček 2015-04-14 11:20:58 UTC
Released usbguard-0.3p3 because of removal of the .pc files from json and spdlog packages.

New SRPM is at: https://fedorapeople.org/~dkopecek/usbguard/usbguard-0.3p3-1.fc20.src.rpm

Comment 20 Daniel Kopeček 2015-04-29 16:22:54 UTC
Both the json and spdlog packages passed the review are now packaged in Fedora.

Comment 21 Daniel Kopeček 2015-04-30 12:34:36 UTC
New Package SCM Request
=======================
Package Name: usbguard
Short Description: A tool for implementing USB device usage policy
Upstream URL: https://dkopecek.github.io/usbguard/
Owners: mildew
Branches: f20 f21 f22 epel7
InitialCC:

Comment 22 Gwyn Ciesla 2015-04-30 15:07:40 UTC
Git done (by process-git-requests).

Comment 23 Daniel Kopeček 2015-04-30 15:33:35 UTC
Thanks!


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