[Bug 749752] Review Request: dmg2img - Uncompress the Apple compressed disk image files

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=749752

--- Comment #5 from Lubomir Rintel <lkundrak at v3.sk> 2012-01-08 18:10:24 EST ---
Thank you for your review.
Updated packages:

SPEC: http://v3.sk/~lkundrak/SPECS/dmg2img.spec
SRPM: http://v3.sk/~lkundrak/SRPMS/dmg2img-1.6.2-2.el6.src.rpm

(In reply to comment #3)
> Some issues I see:
> 
> 1. I think license should be "GPLv2+ and MIT". Since the COPYING file included
> in the upstream tarball contains GPLv2 I think we should use "GPLv2+" instead
> of "GPL+".

No. Comment is what decides and author obviously removed the version
intentionally.

> 2. As noted by Richard in comment #1, "BuildRequires: openssl-devel" is
> required to build vfdecrypt

Fixed.

> 3. Dist tag in release field should be %{?dist} instead of %{dist}

Fixed.

> 4. Unless you're packaging for EPEL 5, I recommend you:
>   Remove the BuildRoot tag
>   Use make instead of %{__make}
>   Remove "rm -rf $RPM_BUILD_ROOT" in the %install section
>   Remove the %clean section
>   Remove %defattr from the %files section

I want el5 builds to work and will probably be submitting the package for el5.
I changed the %make macro for make though.

> 5. The dmg2img-1.6.2-nostrip.patch already used in the patch and the patch I
> attached both work to produce non empty debuginfo packages. I think my version
> has a better chance of being accepted in a future upstream version but that may
> just be wishful thinking.

I believe conditional stripping is not a good idea. If upstream accepts your
patch, I'll gladly drop mine though.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.