In the 'wrapper' subdirectory, there are bindings for various languages. Your package doesn't contain any of those. For example I would be interested in the python wrapper, and I'm pretty sure other people would like to see other wrappers.

Please consider including some if not all of those.

I will wait for Spot's legal OK before spending time with adding the bindings to the packages.

Ok. Seems I didn't read the 'Notes' section carefully enough, sorry.

I am unaware of any valid patents which would prevent this package from being included in Fedora. Lifting FE-Legal.

Updated Spec URL: http://fedora.danny.cz/reviews/libdmtx.spec
Updated SRPM URL: http://fedora.danny.cz/reviews/libdmtx-0.7.2-2.fc14.src.rpm

ChangeLog:
- added subpackages with php/python/ruby bindings

I'm still looking over old review tickets.  This one builds fine; rpmlint says:

  libdmtx.x86_64: W: shared-lib-calls-exit
   /usr/lib64/libdmtx.so.0.0.0 exit.5
Libraries shouldn't do this, but there's not much you can do about it.

  php-libdmtx.x86_64: W: private-shared-object-provides
   /usr/lib64/php/modules/dmtx.so dmtx.so()(64bit)
  python-libdmtx.x86_64: W: private-shared-object-provides
   /usr/lib64/python2.7/site-packages/_pydmtx.so _pydmtx.so()(64bit)
This is one of the packages where you cannot use the filtering macros since it installs both public libraries and binary executables.  So there's not much you can do about rpmlint's complaints.

Am I wrong or is wrapper/php/dmtx_write.c not GPLv2+ instead of LGPLv2+?  It appears that this code is linked intothe php dmtx.so module, which would make that code GPLv2+, wouldn't it?  And if so, you'll have to include COPYING somewhere, not just COPYING.LESSER.

What does the test suite actually do?  I see that it gets built, but I don't see where the built tests actually get run.  I'm not even sure if the can be run automatically.


* source files match upstream.  sha256sum:
  73af17374cf45478fdcb4184c2321d3b923291780430dec4908d9ea686143646
   libdmtx-0.7.2.tar.bz2
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
X license field matches the actual license.
* license is open source-compatible.
X license text not included in package.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* package builds in mock (f14, x86_64).
* package installs properly.
* debuginfo package looks complete.
* rpmlint has acceptable complaints.
* final provides and requires are sane:
  libdmtx-0.7.2-2.fc14.x86_64.rpm
   libdmtx.so.0()(64bit)  
   libdmtx = 0.7.2-2.fc14
   libdmtx(x86-64) = 0.7.2-2.fc14
  =
   /sbin/ldconfig  
   libdmtx.so.0()(64bit)  

  libdmtx-devel-0.7.2-2.fc14.x86_64.rpm
   pkgconfig(libdmtx) = 0.7.2
   libdmtx-devel = 0.7.2-2.fc14
   libdmtx-devel(x86-64) = 0.7.2-2.fc14
  =
   /usr/bin/pkg-config  
   libdmtx = 0.7.2-2.fc14
   libdmtx.so.0()(64bit)  

  libdmtx-utils-0.7.2-2.fc14.x86_64.rpm
   libdmtx-utils = 0.7.2-2.fc14
   libdmtx-utils(x86-64) = 0.7.2-2.fc14
  =
   libMagickCore.so.4()(64bit)  
   libMagickWand.so.4()(64bit)  
   libdmtx = 0.7.2-2.fc14
   libdmtx.so.0()(64bit)  
   libgomp.so.1()(64bit)  

  php-libdmtx-0.7.2-2.fc14.x86_64.rpm
   dmtx.so()(64bit)  
   php-libdmtx = 0.7.2-2.fc14
   php-libdmtx(x86-64) = 0.7.2-2.fc14
  =
   libdmtx = 0.7.2-2.fc14
   libdmtx.so.0()(64bit)  
   php-common  

  python-libdmtx-0.7.2-2.fc14.x86_64.rpm
   _pydmtx.so()(64bit)  
   python-libdmtx = 0.7.2-2.fc14
   python-libdmtx(x86-64) = 0.7.2-2.fc14
  =
   libdmtx = 0.7.2-2.fc14
   libdmtx.so.0()(64bit)  
   libpython2.7.so.1.0()(64bit)  
   python(abi) = 2.7

  ruby-libdmtx-0.7.2-2.fc14.x86_64.rpm
   Rdmtx.so()(64bit)  
   ruby(libdmtx) = 0.7.2
   ruby-libdmtx = 0.7.2-2.fc14
   ruby-libdmtx(x86-64) = 0.7.2-2.fc14
  =
   libdmtx = 0.7.2-2.fc14
   libdmtx.so.0()(64bit)  
   libruby.so.1.8()(64bit)  

? %check is present, but tests are not run.  Can they be run?
* shared libraries present; ldconfig called properly.  Unversioned .so link
   is in the -devel subpackage.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no generically named files
* scriptlets are OK (ldconfig).
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* headers are in the -devel package.
* pkgconfig files are in the -devel package.
* no static libraries.
* no libtool .la files.

(In reply to comment #6)

Thanks for looking into this ticket. I'm the lead developer for libdmtx and would like to help on these issues if possible. This package has been developed primarily on Fedora since day one, so it would give me great satisfaction for it to find a home in the repositories. Please see my comments below:

> libdmtx.x86_64: W: shared-lib-calls-exit
>   /usr/lib64/libdmtx.so.0.0.0 exit.5
> Libraries shouldn't do this, but there's not much you can do about it.

Whoops - looks like I missed that one. I've added an item to my TODO list to clean up any remaining calls to exit().

> This is one of the packages where you cannot use the filtering macros since
> it installs both public libraries and binary executables.  So there's not
> much you can do about rpmlint's complaints.

Would it greatly simplify things if I split the package into a library-only package and a separate utilities/wrappers/other package? I have been tempted to do this for maintenance reasons anyway, but would appreciate any input from someone on the packager side of the fence.

> Am I wrong or is wrapper/php/dmtx_write.c not GPLv2+ instead of LGPLv2+?  It
> appears that this code is linked intothe php dmtx.so module, which would make
> that code GPLv2+, wouldn't it?  And if so, you'll have to include COPYING
> somewhere, not just COPYING.LESSER.

Everything was intended to be LGPLv2, so if you are seeing otherwise then it's probably due to a mistake somewhere. What are you seeing that suggests GPLv2+?

> What does the test suite actually do?  I see that it gets built, but I don't
> see where the built tests actually get run.  I'm not even sure if the can be
> run automatically.

There are a few different testing components representing a mixture of uses:

 * compare_test - Regression testing script (invoked manually)
 * multi_test - Visualization and experimental program for interactive testing
 * rotate_test - Visualization and experimental program for interactive testing 
 * simple_test - Basic round-trip test of library internals
 * unit_test - Intended to someday hold unit tests for automated testing

But to answer your question, nothing is set up to execute when you issue a 'make test' during the main build.

Going through your open issues list:

X license field matches the actual license.
X license text not included in package.
  [Mike] Please let me know what I can do to resolve this issue. I may still have the original email where that contributor agreed to LGPL if that helps.

? %check is present, but tests are not run.  Can they be run?
  [Mike] Is there an automated testing requirement for inclusion in Fedora? If so, please point me to a resource where I can get up to speed.

Thanks again for looking into this old ticket. I'd like to package libdmtx in a way that makes life easy for packagers, but unfortunately I'm not very familiar with that world.

Mike

(In reply to comment #7)
> (In reply to comment #6)
> > Am I wrong or is wrapper/php/dmtx_write.c not GPLv2+ instead of LGPLv2+?  It
> > appears that this code is linked intothe php dmtx.so module, which would make
> > that code GPLv2+, wouldn't it?  And if so, you'll have to include COPYING
> > somewhere, not just COPYING.LESSER.
> 
> Everything was intended to be LGPLv2, so if you are seeing otherwise then it's
> probably due to a mistake somewhere. What are you seeing that suggests GPLv2+?

it's the license header in the wrapper/php/dmtx_write.c source file, it contains

...
 * libdmtx-php is free software; you can redistribute it and/or modify
 * it under the terms of the GNU General Public License as published by
 * the Free Software Foundation; either version 2 of the License, or
 * (at your option) any later version.
...

and it makes the php module GPLv2+ licensed (+ means the "or any later version")

(In reply to comment #7)

> Would it greatly simplify things if I split the package into a library-only
> package and a separate utilities/wrappers/other package?

Not really.  This is just an rpmlint complaint; what it's complaining about is an issue but it isn't particularly problematic.  In order to truly fix it, the language bindings would need to be build separately, but I honestly don't tink it's worth it.  The restriction on using dependency filters are that they can't be used on any package that includes visible libraries (in /usr/lib(64)) or binary executables.  This means that as long as the package produces the utilities in /usr/bin, we can't use or filtering.

Dan answered the bit about the license.

(In reply to comment #8 and comment #9)

> it's the license header in the wrapper/php/dmtx_write.c source file, it
> contains

Yes, it's obvious now that you pointed it out. :)

I went back and found the author's 2008 email stating that we can distribute it under LGPL, but I suppose that doesn't help since it's already "out there" with the unintentional GPL text in the header.

I will replace the header with LGPL content in my TRUNK after re-confirming with the author, however I wasn't planning to create a new release for at least a few more months. Is it possible for you to add the COPYING file until it's officially fixed upstream?

Thanks for finding this (and other) issues.

Updated Spec URL: http://fedora.danny.cz/reviews/libdmtx.spec
Updated SRPM URL: http://fedora.danny.cz/reviews/libdmtx-0.7.2-3.fc14.src.rpm

ChangeLog:
- updated license for the php subpackage
- run few tests - building and running them can't hurt

Looks good to me; thanks.

APPROVED

Jason, thanks for the review.

New Package SCM Request
=======================
Package Name: libdmtx
Short Description: Library for working with Data Matrix 2D barcodes
Owners: sharkcz
Branches: el5 el6 f13 f14

Git done (by process-git-requests).

imported and built