Bug 1372454

Summary: Review Request: bwrap-oci - Run OCI containers with bubblewrap
Product: [Fedora] Fedora Reporter: Giuseppe Scrivano <gscrivan>
Component: Package ReviewAssignee: Patrick Uiterwijk <puiterwijk>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: dwalsh, package-review, puiterwijk
Target Milestone: ---Flags: ignatenko: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2016-09-02 14:38:05 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:

Description Giuseppe Scrivano 2016-09-01 18:42:57 UTC
Spec URL: https://github.com/projectatomic/bwrap-oci/blob/master/bwrap-oci.spec

SRPM URL: https://copr-be.cloud.fedoraproject.org/results/gscrivano/bwrap-oci/fedora-24-x86_64/00447875-bwrap-oci/bwrap-oci-0.1.1-3.fc24.src.rpm

Description: run an OCI container through bubblewrap.

Fedora Account System Username: gscrivano

Comment 1 Igor Gnatenko 2016-09-02 07:02:45 UTC
1. Spec is not downloadable
2. SourceURL is unreachable

> %global commit0 06ed9c9054db435ae33f0c917a2d5e7d3e40e6fa
drop this, as you don't build from commit

> Source0: https://github.com/projectatomic/%{name}/archive/%{name}-%{version}.tar.gz
%global rel 3
Source0: %{url}/archive/%{name}-%{version}-%{rel}.tar.gz

> BuildRequires: git
not really

> BuildRequires: docbook-style-xsl
looks like you don't need it

> BuildRequires: libxslt
looks like you don't need it

> BuildRequires: json-glib-devel
BuildRequires: pkgconfig(json-glib-1.0)

> %autosetup -Sgit -n %{name}-%{version}
%autosetup -n %{name}-%{name}-%{version}-%{rel}

> make %{?_smp_mflags}
%make_build

> make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p -c"
%make_install INSTALL="install -p"

> find $RPM_BUILD_ROOT -name '*.la' -delete
not needed, you don't have any libtool files

> %{_mandir}/man1/*
%%{_mandir}/man1/%{name}.1*

> %{_bindir}/bwrap-oci
%{_bindir}/%{name}

* Missing BuildRequires: gcc
* Missing BuildRequires: pkgconfig(gio-unix-2.0)
* My recommendation - stop using tito ASAP, otherwise you will stick to it and always will have to do double work.

Comment 2 Igor Gnatenko 2016-09-02 07:03:30 UTC
Also no changelog.

Comment 3 Giuseppe Scrivano 2016-09-02 10:57:35 UTC
Thanks for the quick review, I have updated the spec file with your comments:

https://raw.githubusercontent.com/projectatomic/bwrap-oci/master/bwrap-oci.spec

Comment 4 Igor Gnatenko 2016-09-02 11:06:27 UTC
Things which are still not fixed.
* %{_mandir}/man1/*
* %{_bindir}/bwrap-oci
* Summary: Core execution tool for unprivileged containers

P.S. Summary is copied from bubblewrap, but it's different project. Change it to "Run OCI containers with bubblewrap"

Just fix this stuff during import.

Comment 5 Gwyn Ciesla 2016-09-02 13:54:14 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/bwrap-oci

Comment 6 Daniel Walsh 2016-09-03 09:49:36 UTC
Good job.