Bug 743497

Summary: Review Request: tcplay - Utility to create/open/map TrueCrypt-compatible volumes
Product: [Fedora] Fedora Reporter: Eric Smith <spacewar>
Component: Package ReviewAssignee: Michel Alexandre Salim <michel>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: mads, michel, notting, package-review
Target Milestone: ---Flags: michel: fedora‑review+
limburgher: fedora‑cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: tcplay-0.9-0.5.20111007git97ed5f9.el6 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-11-02 02:56:21 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:

Description Eric Smith 2011-10-05 02:44:31 EDT
Spec URL: http://fedorapeople.org/~brouhaha/tcplay/tcplay.spec
SRPM URL: http://fedorapeople.org/~brouhaha/tcplay/tcplay-0.9-0.1.20111004git59c6097.fc14.src.rpm
Koji scratch build for F15: http://koji.fedoraproject.org/koji/taskinfo?taskID=3405450
Description:
The tcplay utility provides full support for creating and opening/mapping
TrueCrypt-compatible volumes.
Comment 1 Michel Alexandre Salim 2011-10-06 06:34:58 EDT
Taking this review. From a quick glance (full review forthcoming), everything looks good, but some items are no longer necessary:

- BuildRoot declaration (and clean-up in %install) only necessary if you want to target RHEL 5; you can surround them with %if %{el5} if you do target RHEL 5
- %clean section only needed in RHEL 5 & 6 (use %if %{rhel} if you want to target them)
- could you ask upstream for a standalone COPYING or LICENSE text, and put a comment linking to the request (if made on an issue tracker or mailing list) or indicating that the request has been made to the author?
Comment 2 Michel Alexandre Salim 2011-10-06 06:51:04 EDT
* TODO Review [70%]
  - [X] Names [2/2]
    - [X] Package name
    - [X] Spec name
  - [X] Package version [2/2]
	http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Package_Versioning
    - [X] Version number
	  http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Version_Tag
    - [X] Release tag
	  http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Release_Tag
	  http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Pre-Release_packages
  - [X] Meets [[http://fedoraproject.org/wiki/Packaging/Guidelines][guidelines]]
  - [ ] Source files match upstream
        Please provide your own instructions, so the tarball generation is
        reproducible, rather than using the latest github snapshot.
        And perhaps use xz rather than gz?
        e.g.
        git clone git://github.com/bwalex/tc-play
        ( cd tc-play && \
          git archive --format=tar --prefix=%{name}-%{githash}/ %{githash} \
        ) | xz - > %{name}-%{githash}.tar.xz
  - [X] [[http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries][No bundled libraries]]
  - [-] License [3/4]
    - [X] License is Fedora-approved
    - [X] No licensing conflict
    - [X] License field accurate
    - [ ] License included iff packaged by upstream
          please ask upstream for this
  - [X] rpmlint [2/2]
    - [X] on src.rpm
          tcplay.src: W: invalid-url Source0: bwalex-tc-play-59c6097.tar.gz
          1 packages and 0 specfiles checked; 0 errors, 1 warnings.
    - [X] on x86_64.rpm
          2 packages and 0 specfiles checked; 0 errors, 0 warnings.
  - [X] Language & locale [2/2]
    - [X] Spec in US English
    - [X] Spec legible
  - [X] Build [3/3]
    - [X] Koji results
          http://koji.fedoraproject.org/koji/taskinfo?taskID=3405450
    - [X] BRs complete
    - [X] Directory ownership
  - [-] Spec inspection [5/8]
    - [X] No duplicate files
    - [X] File permissions
    - [X] Filenames must be UTF-8
    - [ ] no BuildRoot ([[https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag][except if targeting RHEL5]])
    - [ ] [RHEL]  %clean section
          https://fedoraproject.org/wiki/Packaging/Guidelines#.25clean)
    - [ ] [RHEL 5] %buildroot cleaned on %install
          as noted in previous message, please indicate if you're targeting
          el5 and/or el6
    - [X] Macro usage consistent
    - [X] Documentation [1/1]
      - [X] %doc files are non-essential
Comment 3 Michel Alexandre Salim 2011-10-06 10:26:25 EDT
(In reply to comment #1)
> Taking this review. From a quick glance (full review forthcoming), everything
> looks good, but some items are no longer necessary:
> 
> - BuildRoot declaration (and clean-up in %install) only necessary if you want
> to target RHEL 5; you can surround them with %if %{el5} if you do target RHEL 5
> - %clean section only needed in RHEL 5 & 6 (use %if %{rhel} if you want to
> target them)

My bad; I mean %if 0%{?el5} and %if 0%{?rhel}
Comment 5 Eric Smith 2011-10-07 23:20:05 EDT
Updated to latest upstream, which added a LICENSE file.

Spec URL: http://fedorapeople.org/~brouhaha/tcplay/tcplay.spec
SRPM URL:
http://fedorapeople.org/~brouhaha/tcplay/tcplay-0.9-0.3.20111007git97ed5f9.fc14.src.rpm
Koji scratch build for F15: http://koji.fedoraproject.org/koji/taskinfo?taskID=3413307
Comment 6 Eric Smith 2011-10-08 15:27:05 EDT
I see that I overlooked a few of the requests and have been told that even RHEL6 doesn't need the buildroot cleaned in %install, so I've cleaned the spec up further.

Spec URL: http://fedorapeople.org/~brouhaha/tcplay/tcplay.spec
SRPM URL:
http://fedorapeople.org/~brouhaha/tcplay/tcplay-0.9-0.4.20111007git97ed5f9.fc14.src.rpm
Comment 7 Michel Alexandre Salim 2011-10-12 06:06:44 EDT
Snapshot-creating script works; source hashes match, LICENSE file is now included and the macros are cleaned up.

Review APPROVED
Comment 8 Eric Smith 2011-10-12 12:47:32 EDT
New Package SCM Request
=======================
Package Name: tcplay
Short Description: Utility to create/open/map TrueCrypt-compatible volumes
Owners: brouhaha
Branches: f14 f15 f16 el6
InitialCC:
Comment 9 Jon Ciesla 2011-10-12 13:02:10 EDT
Git done (by process-git-requests).
Comment 10 Fedora Update System 2011-10-19 04:52:30 EDT
tcplay-0.9-0.4.20111007git97ed5f9.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/tcplay-0.9-0.4.20111007git97ed5f9.fc16
Comment 11 Fedora Update System 2011-10-19 04:54:10 EDT
tcplay-0.9-0.4.20111007git97ed5f9.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/tcplay-0.9-0.4.20111007git97ed5f9.fc15
Comment 12 Fedora Update System 2011-10-19 04:54:47 EDT
tcplay-0.9-0.4.20111007git97ed5f9.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/tcplay-0.9-0.4.20111007git97ed5f9.fc14
Comment 13 Fedora Update System 2011-10-19 22:24:03 EDT
tcplay-0.9-0.4.20111007git97ed5f9.fc16 has been pushed to the Fedora 16 testing repository.
Comment 14 Fedora Update System 2011-11-02 02:56:21 EDT
tcplay-0.9-0.4.20111007git97ed5f9.fc15 has been pushed to the Fedora 15 stable repository.
Comment 15 Fedora Update System 2011-11-02 03:01:26 EDT
tcplay-0.9-0.4.20111007git97ed5f9.fc14 has been pushed to the Fedora 14 stable repository.
Comment 16 Fedora Update System 2011-11-04 21:30:24 EDT
tcplay-0.9-0.4.20111007git97ed5f9.fc16 has been pushed to the Fedora 16 stable repository.
Comment 17 Fedora Update System 2012-03-11 01:32:15 EST
tcplay-0.9-0.5.20111007git97ed5f9.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/tcplay-0.9-0.5.20111007git97ed5f9.el6
Comment 18 Fedora Update System 2012-03-28 00:55:54 EDT
tcplay-0.9-0.5.20111007git97ed5f9.el6 has been pushed to the Fedora EPEL 6 stable repository.  If problems still persist, please make note of it in this bug report.