Bug 1579045 - Review Request: virt-bootstrap - System container rootfs creation tool
Summary: Review Request: virt-bootstrap - System container rootfs creation tool
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Lubomir Rintel
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2018-05-16 21:11 UTC by Fabiano Fidêncio
Modified: 2018-05-30 14:08 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2018-05-30 14:08:58 UTC
Type: Bug
Embargoed:
lkundrak: fedora-review+


Attachments (Terms of Use)

Comment 1 Lubomir Rintel 2018-05-17 07:55:29 UTC
This is done rather well overall. Some details need need to be addressed before this can be imported, see below.

* Package named correctly
* Packaging the most recent upstream release
* License tag is correct
* The license is good for Fedora
* The SPEC file is clean and legible
* Macros are used consistently
* Package builds fine in mock -- BRs correct
* Filelist sane
* Requires/provides look good
* Follows modern Python 3 packaging standards

0.) This is an architecture independent package

You should set "BuildArch: noarch". Then remove this:

> %global debug_package %{nil}

I guess you added that because the rpmbuild complained of empty debuginfo?

1.) Buildroot tar is obsolete:

> BuildRoot: %{_tmppath}/%{name}-%{version}-build

2.) In general, when adding patches, comment on their upstreaming status

> Patch0001: 0001-docker-source-Avoid-skopeo-copy-in-cache.patch
> Patch0002: 0002-docker-source-Get-list-of-layers-without-raw.patch
> Patch0003: 0003-docker-source-Support-blobs-without-.tar-ext.patch
> Patch0004: 0004-safe_untar-Check-for-permissions-to-set-attribs.patch

If they are upstream backports, just say so. If you're involved with upstream development perhaps you can get new version released?

3.) Drop the %defattr tag

> %defattr(-,root,root)

It's not needed by any of the RPM versions that are relevant these days.

Comment 2 Lubomir Rintel 2018-05-17 08:14:49 UTC
(In reply to Lubomir Rintel from comment #1)
> * Package builds fine in mock -- BRs correct

Spoke too soon. Please add "BuildRequires: /usr/bin/git" for the %autosetup with git to work.

> 1.) Buildroot tar is obsolete:

tag

Also there's one more thing to fix, rpmlint seems somewhat unhappy:

> virt-bootstrap.noarch: E: wrong-script-interpreter /usr/lib/python3.6/site-packages/virtBootstrap/virt_bootstrap.py /usr/bin/env python
> virt-bootstrap.noarch: E: non-executable-script /usr/lib/python3.6/site-packages/virtBootstrap/virt_bootstrap.py 644 /usr/bin/env python

If a shebang is needed, then it should be /usr/bin/python3, not /usr/bin/env python nor /usr/bin/python. Here it is not needed and should be just dropped -- the file is in /usr/lib and doesn't have an executable bit.

Comment 4 Lubomir Rintel 2018-05-17 16:10:56 UTC
This looks much better, thanks. There's one thing you need to fix before importing the package -- avoid expanding macros in the changelog:

-- Drop "%defattr" tag as it's obsolete
-- Add "BuildRequires: /usr/bin/git" (due to %autosetup -S git)
+- Drop "%%defattr" tag as it's obsolete
+- Add "BuildRequires: /usr/bin/git" (due to %%autosetup -S git)

No need to block the review for this, the package is now APPROVED

Comment 5 Gwyn Ciesla 2018-05-21 16:17:07 UTC
(fedrepo-req-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/virt-bootstrap

Comment 6 Fedora Update System 2018-05-21 17:47:31 UTC
virt-bootstrap-1.0.0-2.fc28 has been submitted as an update to Fedora 28. https://bodhi.fedoraproject.org/updates/FEDORA-2018-e35c1ec2b6

Comment 7 Fedora Update System 2018-05-22 19:38:03 UTC
virt-bootstrap-1.0.0-2.fc28 has been pushed to the Fedora 28 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-e35c1ec2b6

Comment 8 Fedora Update System 2018-05-30 14:08:58 UTC
virt-bootstrap-1.0.0-2.fc28 has been pushed to the Fedora 28 stable repository. If problems still persist, please make note of it in this bug report.


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