Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
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: ---
Embargoed:
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
View All

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 19 Daniel Kopeček 2015-04-20 14:42:43 UTC 
Spec URL: https://fedorapeople.org/~dkopecek/usbguard/usbguard.spec
SRPM URL: 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.