https://bugzilla.redhat.com/show_bug.cgi?id=1725840 --- Comment #3 from Jakub Jelen <jjelen@xxxxxxxxxx> --- Thank you for the review. Comments inline. (In reply to Jerry James from comment #2) > [...] > > ===== Issues ==== > > 1. The project does advertise its license as LGPLv2+. However, there are > files > with other licenses. > > MIT: > src/simclist.c > src/simclist.h > src/strlcpy.c > > BSD: > src/misc.h > src/openct: the entire directory > src/parser.h > src/strlcpycat.h > src/tokenparser.l If I understand the licensing right, the BSD license is permissive and derivative/combined work can be licensed under different FOSS license. The same, I think apply for the MIT license. If that is wrong understanding, I can indeed list also the other two, or what would be your proposal in this way? > 2. For ease of verifying that the correct compiler flags are in use, please > either pass --disable-silent-rules to %configure, or add V=1 to the make > invocation. Incidentally, the %make_build macro is equivalent to the make > invocation in the spec file. It's a convenient shorthand that I > recommend. I used the %make_build macro. Thanks. > 3. The package does contain bundled libraries. They must either be unbundled > or the correct Provides added to the spec file, as described here: > https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling > > The bundled libraries I see are: > SimCList: http://mij.oltrelinux.com/devel/simclist/ > OpenCT: https://github.com/OpenSC/openct/wiki SimCList is simple copylib. I do not see that packaged in the Fedora and I do not think it make sense to do that since it is single-small file. I will specify it as a bundled. The OpenCT directory here is to my understanding not a whole bundled library, but just parts of it with significant modification to match the requirements of this driver. Out of curiosity, I tried to compare the proto-t1.c file from upstream and there is hardly anything kept in place: $ diff /tmp/proto-t1.c acsccid-1.1.6/src/openct/proto-t1.c | wc -l 766 I am not sure what is the best way to handle this, but I would certainly not call that a bundled library, rather derivative work. > 4. Add the -p flag to the install invocations in %install to preserve > timestamps. Fixed. Thanks > 5. Fix the script-without-shebang error produced by rpmlint; see below. > *Both* invocations of install in %install should pass -m 644. Fixed. > 6. The .so has undefined symbols: > > $ ldd -r > /usr/lib64/pcsc/drivers/ifd-acsccid.bundle/Contents/Linux/libasccid.so > linux-vdso.so.1 (0x00007ffc3257b000) > libusb-1.0.so.0 => /lib64/libusb-1.0.so.0 (0x00007f121ef66000) > libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f121ef45000) > libc.so.6 => /lib64/libc.so.6 (0x00007f121ed7e000) > libudev.so.1 => /lib64/libudev.so.1 (0x00007f121ed51000) > /lib64/ld-linux-x86-64.so.2 (0x00007f121efae000) > libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f121ed37000) > undefined symbol: log_xxd (./libacsccid.so) > undefined symbol: log_msg (./libacsccid.so) > > Those symbols are defined in src/debug.c, which is apparently not linked > into the final .so. The src/Makefile.am links them to the final so only if we build the library without pcsc. These symbols are defined in /usr/include/PCSC/debuglog.h (from /usr/lib64/libpcsclite.so), which loads this library and therefore provides these symbols if I understand the logic here. The updated spec file and srpm: Spec URL: https://jjelen.fedorapeople.org/pcsc-lite-acsccid.spec SRPM URL: https://jjelen.fedorapeople.org/pcsc-lite-acsccid-1.1.6-1.fc31.src.rpm -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
