Bug 1062162 - Review Request: sunxi-tools - Tools to help hacking Allwinner (sunxi) based devices
Summary: Review Request: sunxi-tools - Tools to help hacking Allwinner (sunxi) based d...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Hans de Goede
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-02-06 11:30 UTC by Lubomir Rintel
Modified: 2014-02-06 16:46 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-02-06 16:46:26 UTC
Type: Bug
Embargoed:
hdegoede: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

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!


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