Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.

Bug 1062162

Summary: Review Request: sunxi-tools - Tools to help hacking Allwinner (sunxi) based devices
Product: [Fedora] Fedora Reporter: Lubomir Rintel <lkundrak>
Component: Package ReviewAssignee: Hans de Goede <hdegoede>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: hdegoede, package-review, yaneti
Target Milestone: ---Flags: hdegoede: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-02-06 16:46:26 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Description Lubomir Rintel 2014-02-06 11:30:44 UTC
SPEC: http://v3.sk/~lkundrak/SPECS/sunxi-tools.spec
SRPM: http://v3.sk/~lkundrak/SRPMS/sunxi-tools-1.1-1.20140131git271130b.fc20.src.rpm

Description:

This package contains various tools to help hacking Allwinner (aka sunxi) based
devices and possibly it's successors.

Comment 1 Yanko Kaneti 2014-02-06 11:37:42 UTC
Perhaps you should use
https://fedoraproject.org/wiki/Packaging:SourceURL#Github

Comment 2 Hans de Goede 2014-02-06 15:09:51 UTC
Looks good at a first glance, I agree with Yanko that it would be good to use the SourceURL stuff for github from the guidelines. If you can do a version with that fixed I'll do a full review.

Comment 3 Lubomir Rintel 2014-02-06 15:34:32 UTC
It's not possible to get github generate sane file names. Currently they look like this:

https://github.com/linux-sunxi/sunxi-tools/archive/271130b31cea62700e8a48971ec98b17f56fd4db.tar.gz
https://codeload.github.com/linux-sunxi/sunxi-tools/tar.gz/271130b31cea62700e8a48971ec98b17f56fd4db

I believe that's worse than what's currently in the spec file and I'd prefer not to change it until GitHub fixes their part (I've opened a ticket to them a couple of months ago).

Comment 4 Yanko Kaneti 2014-02-06 15:51:18 UTC
(In reply to Lubomir Rintel from comment #3)
> It's not possible to get github generate sane file names. Currently they
> look like this:

Thats not true really, could please check the guidelines again.
The source lines becomes:
Source0:        https://github.com/linux-sunxi/sunxi-tools/archive/%{commit}/%{name}-%{version}-%{shortcommit}.tar.gz

and the filename is: 
sunxi-tools-1.1-271130b.tar.gz

Comment 5 Hans de Goede 2014-02-06 15:55:24 UTC
Good:

- rpmlint checks return:
sunxi-tools.src: W: invalid-url Source0: sunxi-tools.tar.gz
sunxi-tools.x86_64: W: no-manual-page-for-binary fex2bin
sunxi-tools.x86_64: W: no-manual-page-for-binary pio
sunxi-tools.x86_64: W: no-manual-page-for-binary fel
sunxi-tools.x86_64: W: no-manual-page-for-binary bootinfo
sunxi-tools.x86_64: W: no-manual-page-for-binary usb-boot
sunxi-tools.x86_64: W: no-manual-page-for-binary nand-part
sunxi-tools.x86_64: W: no-manual-page-for-binary fel-gpio
3 packages and 0 specfiles checked; 0 errors, 8 warnings.
 These can all be ignored
- package meets naming guidelines
- package meets packaging guidelines
- license (GPLv2+) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file

Everything looks good, approved.

Comment 6 Lubomir Rintel 2014-02-06 15:57:52 UTC
(In reply to Yanko Kaneti from comment #4)
> (In reply to Lubomir Rintel from comment #3)
> > It's not possible to get github generate sane file names. Currently they
> > look like this:
> 
> Thats not true really, could please check the guidelines again.
> The source lines becomes:
> Source0:       
> https://github.com/linux-sunxi/sunxi-tools/archive/%{commit}/%{name}-
> %{version}-%{shortcommit}.tar.gz
> 
> and the filename is: 
> sunxi-tools-1.1-271130b.tar.gz

Yanko, are you shure that works? I'd really like to use, but I can't figure out the actual URL that would work and not give me 404 error.

New Package SCM Request
=======================
Package Name: sunxi-tools
Short Description: Tools to help hacking Allwinner (sunxi) based devices
Owners: lkundrak
Branches: f19 f20 el6 epel7

Comment 7 Yanko Kaneti 2014-02-06 16:02:53 UTC
It works. Just following the guidelines.

%global commit 271130b31cea62700e8a48971ec98b17f56fd4db
%global shortcommit %(c=%{commit}; echo ${c:0:7})

Source0: https://github.com/linux-sunxi/sunxi-tools/archive/%{commit}/%{name}-%{version}-%{shortcommit}.tar.gz

Comment 8 Yanko Kaneti 2014-02-06 16:04:35 UTC
On a different note, I thinks its quite bad taste to ship executables in %{_bindir} called "bootinfo" and "usb-boot", despite those being very special limited purpose tools.

Comment 9 Gwyn Ciesla 2014-02-06 16:06:41 UTC
Git done (by process-git-requests).

Comment 10 Hans de Goede 2014-02-06 16:07:53 UTC
(In reply to Yanko Kaneti from comment #8)
> On a different note, I thinks its quite bad taste to ship executables in
> %{_bindir} called "bootinfo" and "usb-boot", despite those being very
> special limited purpose tools.

Ah, good point, Lubomir, can you please rename those to sunxi-foo ?

Thanks,

Hans

Comment 11 Lubomir Rintel 2014-02-06 16:46:26 UTC
Imported and built (with both issues addressed: source url and binary names).

Thank you for the review and Git!