Bug 1161483

Summary: Review Request: o3dgc - an open 3D graphics compression library
Product: [Fedora] Fedora Reporter: David Tardon <dtardon>
Component: Package ReviewAssignee: Christopher Meng <i>
Status: CLOSED WONTFIX QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: alex.ploumistos, dtardon, i, package-review, samuel-rhbugs
Target Milestone: ---Flags: i: fedora-review?
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2017-10-30 07:39:46 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1161487    

Description David Tardon 2014-11-07 08:38:00 UTC
Spec URL: https://dtardon.fedorapeople.org/rpm/o3dgc.spec
SRPM URL: https://dtardon.fedorapeople.org/rpm/o3dgc-0-1.fc21.src.rpm
Description:
o3dgc is a library providing an efficient implementation of patent free MPEG tools for 3D graphics compression.

Fedora Account System Username: dtardon

Comment 2 Alexander Ploumistos 2015-04-11 10:17:56 UTC
Hello,

This is an unofficial review.

I don't know if anyone more qualified than me will take issue with the source (re)packaging, which gives rpmlint something to complain about, but the rationale seems obvious.

The patch is also pretty much self-explanatory, perhaps someone would prefer a more explicit comment someplace.

There does seem to be an issue with the licenses though:

BSD (2 clause)
--------------
o3dgc/src/o3dgc_common_lib/inc/o3dgcArithmeticCodec.h
o3dgc/src/o3dgc_common_lib/src/o3dgcArithmeticCodec.cpp

MIT/X11 (BSD like)
------------------
o3dgc/js/o3dgc.js
o3dgc/src/o3dgc_common_lib/inc/o3dgcAdjacencyInfo.h
o3dgc/src/o3dgc_common_lib/inc/o3dgcBinaryStream.h
o3dgc/src/o3dgc_common_lib/inc/o3dgcCommon.h
o3dgc/src/o3dgc_common_lib/inc/o3dgcDVEncodeParams.h
o3dgc/src/o3dgc_common_lib/inc/o3dgcDynamicVector.h
o3dgc/src/o3dgc_common_lib/inc/o3dgcFIFO.h
o3dgc/src/o3dgc_common_lib/inc/o3dgcIndexedFaceSet.h
o3dgc/src/o3dgc_common_lib/inc/o3dgcSC3DMCEncodeParams.h
o3dgc/src/o3dgc_common_lib/inc/o3dgcTimer.h
o3dgc/src/o3dgc_common_lib/inc/o3dgcTriangleFans.h
o3dgc/src/o3dgc_common_lib/inc/o3dgcVector.h
o3dgc/src/o3dgc_common_lib/src/o3dgcTools.cpp
o3dgc/src/o3dgc_common_lib/src/o3dgcTriangleFans.cpp
o3dgc/src/o3dgc_decode_lib/inc/o3dgcDynamicVectorDecoder.h
o3dgc/src/o3dgc_decode_lib/inc/o3dgcSC3DMCDecoder.h
o3dgc/src/o3dgc_decode_lib/inc/o3dgcTriangleListDecoder.h
o3dgc/src/o3dgc_decode_lib/src/o3dgcDynamicVectorDecoder.cpp
o3dgc/src/o3dgc_encode_lib/inc/o3dgcDynamicVectorEncoder.h
o3dgc/src/o3dgc_encode_lib/inc/o3dgcSC3DMCEncoder.h
o3dgc/src/o3dgc_encode_lib/inc/o3dgcTriangleListEncoder.h
o3dgc/src/o3dgc_encode_lib/src/o3dgcDynamicVectorEncoder.cpp
o3dgc/src/test/src/main.cpp

Unknown or generated
--------------------
o3dgc/install/run.py
o3dgc/js/index.php
o3dgc/js/three.min.js


You need to specify all licenses in the spec file and it would be nice to sort files in the %files section by their license. See
 
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/LicensingGuidelines#Multiple_Licensing_Scenarios

I am away from my main system and I don't have koji setup here, so I could only test locally for x86_64. Have you tried building the package for other architectures?

Comment 4 David Tardon 2015-04-11 12:32:35 UTC
(In reply to Alexander Ploumistos from comment #2)
> I am away from my main system and I don't have koji setup here, so I could
> only test locally for x86_64. Have you tried building the package for other
> architectures?

http://copr.fedoraproject.org/coprs/dtardon/pending-review/build/83444/

Comment 5 Alexander Ploumistos 2015-04-11 23:49:42 UTC
The latest link to the spec file still points to the one from 0-2, not the one in your source rpm and the scripts will whine about that.

I did the exact same thing a few days ago, that's why I checked :)

Comment 6 David Tardon 2015-04-12 07:47:53 UTC
(In reply to Alexander Ploumistos from comment #5)
> The latest link to the spec file still points to the one from 0-2, not the
> one in your source rpm and the scripts will whine about that.
> 
> I did the exact same thing a few days ago, that's why I checked :)

No, you did not. Otherwise you would see that the spec had been updated.

Comment 7 Alexander Ploumistos 2015-04-12 08:19:30 UTC
Sorry, browser cache...

Comment 8 David Tardon 2017-10-30 07:39:46 UTC
I don't need this anymore.