Bug 1209971

Created attachment 1012537 [details]
fedora-review output

- 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

 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

Is there a reason to ship /usr/lib64/libusbguard.a in usbguard-devel? see https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#StaticLibraries

* 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

Fixed SRPM.

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

(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

(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.

(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)

(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.

(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.

(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.

(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.

FPC ticket: https://fedorahosted.org/fpc/ticket/523

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.

Submitted review requests for the remaining libraries. Ignore the FPC ticket.

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

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

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

Both the json and spdlog packages passed the review are now packaged in Fedora.

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:

Git done (by process-git-requests).

Thanks!