This is my first package and I need an sponsor.

I declined you membership for cvsextras in the accounts system until you've
found a sponsor

Ok. And how long does it take to find one??

I'm willing todo a review on this, but I cannot sponsor you, finding a sponsor
with a fully reviewed package should be easier though.

Also I have some experience with ctapi but I don't have the cardreader this lib
is for so I'll have to believe you on your blue eyes that this thing actually
works, or you can send me a reader if you have a spare one :)

Does this sound ok to you?

Yes. I have this reader and testet it with Fedora 4 and 5. So this package will
work:) I use the ctapi to write apps that use chipcards. I have build this
package  because this reader is very often use in germany.

Note that this isn't a formal review.  Here's a couple of quick points:

1. Drop the packager, we don't use this in FE.
2. Don't hardcode the dist tag, use %{?dist}.
3. Spec file name shouldn't include the version.
4. Use full URL for the source.
5. Use rpmlint on your binaries, and correct any errors.

Before you can be sponsored, you've got to demonstate an understanding of the
packaging policies of Fedora Extras.  I would suggest reading the wiki fully,
since most of the errors are addressed there.

Sorry, no time for a formal review right now, but I wonder what is the .soname
of the library this package installs, how does this mix with the ctapi driver
for towitoko card readers? How can an application determine which one it will
get, should the application determine this or should there be a layer in between
with its own configuration.

I know lots of questions for someone who is just packaging the driver (lib) but
I wonder how this will interact with other cardreader drivers, and I would
rather get this right in one go if possible.

Thanks I have fixed this 5 erros and use rpmlint.
The URL of the new files is:
Spec file URL:
http://prdownloads.sourceforge.net/qsmartcard/ctapi-cyberjack.spec?download
SRPM file URL:
http://prdownloads.sourceforge.net/qsmartcard/ctapi-cyberjack-2.0.8-2.src.rpm?download

But one error of rpmlint is not true.
rpmlint print:
E: ctapi-cyberjack library-not-linked-against-libc
/usr/lib/readers/libctapi-cyberjack.so.2.0.8
but the lib is linked against libc
ldd /usr/lib/readers/libctapi-cyberjack.so.2.0.8
linux-gate.so.1 =>  (0x008c8000)
        /lib/libNoVersion.so.1 (0x003f9000)
        libusb-0.1.so.4 => /usr/lib/libusb-0.1.so.4 (0x00769000)
        libc.so.6 => /lib/libc.so.6 (0x00d45000)
        /lib/ld-linux.so.2 (0x008c9000)

(In reply to comment #7)
> Sorry, no time for a formal review right now, but I wonder what is the .soname
> of the library this package installs, how does this mix with the ctapi driver
> for towitoko card readers? How can an application determine which one it will
> get, should the application determine this or should there be a layer in between
This is an general problem with the ctapi driver's on linux. That all drivers
have  an lib called ctap.so. So I think that is needed, that all persons witch
make packages witch contains ctapi driver must talk to provide an solution for
this. My idear is that the lib's will go to /usr/lib/readers and the files are
called ctapi-name.so.version where name will be the name of the reader for witch
the lib is. For the application this will be no problem since it load the lib
via dlopen(). And need an file as arument.
So when here on the list other persons witch build ctapi driver then we must
talk about it.

I thought I'd take a look what triggers the allegedly wrong rpmlint error, but
could not because the build fails on x86_64:

ld -x --shared -lusb -o libctapi-cyberjack.so ctn_list.o cjctapi_beep.o
cjctapi_switch.o  ecom/libctapi-ecom.a ppa/libctapi-ppa.a
ld: ctn_list.o: relocation R_X86_64_32 against `a local symbol' can not be used
when making a shared object; recompile with -fPIC
ctn_list.o: could not read symbols: Bad value
make[1]: *** [libso] Error 1

I don't have an EMT64(alias x86_64) CPU so I can't test it on it.
But in the Makefile the -fPIC set in the CFLAGS.

Ok I have found the problem. I will write an patch for the makefile.

Ok the problem shut now be fixed.
Try the new files
Spec file URL:
http://prdownloads.sourceforge.net/qsmartcard/ctapi-cyberjack.spec?download
SRPM file URL:
http://prdownloads.sourceforge.net/qsmartcard/ctapi-cyberjack-2.0.8-3.src.rpm?download

The problem was in tze old spec file there was some testcode for the new 2.0.10
version. But this version have bugs so I use 2.0.8

I'll look at the rpmlint error later, but at least
/usr/lib64/readers/libctapi-cyberjack.so.2.0.8 is missing executable permissions.

The pcsc subpackage should have "Provides: pcsc-ifd-handler".  pcsc-lite
requires at least one such package installed.

Additionally, I'd suggest using "pcsc-lite-cyberjack" as the PCSC package name
for consistency with openct and the package naming guidelines for plugin-like
packages.

Also, the preferred install location for pcsc-lite drivers can be retrieved with
"pkg-config libpcsclite --variable=usbdropdir", I'd suggest using adding a build
dependency on pcsc-lite-devel and using that for the pcsc stuff.

only read for /usr/lib64/readers/libctapi-cyberjack.so.2.0.8 is ok because the
file will only be readed via dlopen() and not exec. The PC/SC part are not
testet at this time because I have no apps for real testing. I have only try
pcscd -f and this will work.
It say:
pcscdaemon.c:251 main: pcscd set to foreground
readerfactory.c:1391 RFInitializeReader: Attempting startup of REINER SCT
cyberJack pinpad/e-com USB 0 0.
readerfactory.c:1133 RFBindFunctions: Loading IFD Handler 2.0
Ok I can rename ctapi-cyberjack-PC-SC to ctapi-cyberjack-ifd-handler when this
is better.

Frank,

Please:
-use "pkg-config libpcsclite --variable=usbdropdir" to get the location for
 dropping the PCSC-lite driver
-add executable permission to all .so files .so files should always be 755

More even more improtant:
-don't install the lib under the to generic name libctapi.so, this is asking for
 conflicts with other packages. Instead name it something like
 libctapi-cyberjack.so.0 (watch the versioning!) and put the ctapi lib part 
 in plain %{_libdir} not the PSCS-lite readers dir, since its generic.
-don't install ctapi.h (where is ctbcs.h?) in /usr/include, but put it in
 /usr/include/ctapi-cyberjack.
-create a pkgconfig-file which can be used to get the correct CFLAGS and
 LDFLAGS to link against the cyberjack ctapi.

Alternativly come up with some other scheme to allow multiple libctapi's to
co-exist, I think you / we should be able to come up with something better, just
taking the libctapi.so and ctapi.h names in the global namespace is unacceptable
since for example the towitoko drivers provide the same.

Once this is done I would mind sponsering you and doing a full review of this
package, but this problem has to be tackled first.

So I am working on it.
But with the PCSC-lite driver directory it fails, because 
pkg-config libpcsclite --variable=usbdropdir returns only empty.
So the build will fail:(
And the makefile try to install the driver at
install -m 755 pcsc/libcyberjack_ifd.so
'/var/tmp/ctapi-cyberjack-2.0.8-4%{dist}-root-frankpkg-config libpcsclite
--variable=usbdropdir/libcyberjack_ifd.so.2.0.8'
In the spec file I have written:
%define readers_dir "pkg-config libpcsclite --variable=usbdropdir"

(In reply to comment #17)
> But with the PCSC-lite driver directory it fails, because 
> pkg-config libpcsclite --variable=usbdropdir returns only empty.

You'll need pcsc-lite-devel >= 1.3.0 (which is available in >= FC5) for that,
earlier versions don't have the usbdropdir variable.

$ pkg-config libpcsclite --variable=usbdropdir
/usr/lib64/pcsc/drivers

> In the spec file I have written:
> %define readers_dir "pkg-config libpcsclite --variable=usbdropdir"

That can't work, try %(pkg-config ...) instead of the quotes.

ok. And how build I this with FC4??

The simplest solution would be to blindly define readers_dir as
%{_libdir}/pcsc/drivers in the FC4 specfile.  That would work for FC5+ currently
too, but there are no guarantees; querying it from pkg-config is the official
upstream way.

So I have build an new spec file.
I think I have do all needed changes.
I have alos fix the readers config file.
And the spec file can now be used for FC4 and FC5.
here the URL:
http://prdownloads.sourceforge.net/qsmartcard/ctapi-cyberjack.spec?download

Still needs a bit of work, I'll do a full review shortly.

Well thats a nice start:
[hans@shalem ~]$ rpmbuild -ba /usr/src/redhat/SPECS/ctapi-cyberjack.spec 
error: parse error in expression
error: /usr/src/redhat/SPECS/ctapi-cyberjack.spec:15: parseExpressionBoolean
returns -1
error: Package has no %description: ctapi-cyberjack

This is caused by the following lines in the spec:
%if %{dist} == "FC4"
        %define readers_dir    %{_libdir}/readers
%else
        %define readers_dir %(pkg-config libpcsclite --variable=usbdropdir)
%endif

I was already planning on adding a "MUST Fix" item for those, but not this soon
in the review. This is not the Fedora Extras way of doing things to avoid %if
filled specfiles we have one specfile per release in our CVS. Reviews should
always happen against the devel release, but in most cases (this one included)
the latest stable is fine too.

So for this review please target FC-5 and FC-5 only then once your package is
approved and imported (into the devel branch where all imports happen) you can
request CVS branches for FC-5 and FC-4 and then change the specfile for FC-4 .

It is ok to add a comment to the specfile submitted for review about nescesarry
changes for older releases. So you could change the above lines to:
# define this to %{_libdir}/readers when building on FC-4
%define readers_dir %(pkg-config libpcsclite --variable=usbdropdir)

Since it looks like I won't be able todo a full review right now after all
because of lack of time and because there is alot to fix in a first glance, here
is a quick and probably incomplete list of "MUST Fix" items:

MUST Fix
========

* Don't use release conditional %if constructs, instead target FC-5 with the
  spec submitted for review (see above).
* Don't use German in the specfile not even in comments
* Remove the unused "%define _realrelease    1"
* Replace "4%{dist}" with "4%{?dist}" watch the ?
* Have you tried replacing "make" with "make %{?_smp_mflags}" ?
  Ifso please add a comment that make breaks with %{?_smp_mflags} if not
  please add %{?_smp_mflags}.
* When linking libctapi-cyberjack.so it doesn't get passed an soname flag, the 
  ld -x --shared -lusb -o libctapi-cyberjack.so ctn_list.o cjctapi_beep.o ...
  command should get passed "-soname libctapi-cyberjack.so.0"
* When installing libctapi-cyberjack.so you install it as
  libctapi-cyberjack.so.%{version} this is not correct you should install it as
  libctapi-cyberjack.so.0 (matching the -soname above).
  When the ABI of the library changes (which it will probably never will) you
  change both the -soname and the install command to libctapi-cyberjack.so.1,
  when the ABI changes again to .2 etc.
* Don't use a .so.%{version} install for the ifd, this is a plugin which always
  gets dlopen-ed and as such should be installed as a plain .so, thus you also
  do not need to add -soname to the link command for the ifd only for the ctapi 
  lib.
* libcyberjack_ifd.so gets staticly linked against libctapi-cyberjack, please 
  make this dynamic (currently the link command includes:
  "../ctapi/libctapi-cyberjack.a" replace this with "-L../ctapi 
  -llibctapi-cyberjack").
* drop the "Requires:       %{name} = %{version}" for PC-SC subpackage,
  currently it is staticly linked and the dyn link which should be there will
  lead to an autodependency generated by rpm.
* Rename the "PC-SC" subpakcage to pcsc to match the way pcsc is written in
  other packages names (pcsc-lite, pcsc-tools, pcsc-perl)
* Don't create the docdir and install the docs there just name the docs
  as they are releative to the builddir afyter %doc, rpmbuild will copy
  them for you and create the docdir itself.

And some personal preferences of mine, not obligatory in anyway:
* Don't put a "," between BuildRequires whitespace alone is enough.
* Don't use %{buildroot} because %{buildroot}%{_datadir} is hard to read,
  instead use $RPM_BUILD_ROOT, $RPM_BUILD_ROOT%{_datadir} is IMHO much easier
  to read.


Phew, long list. Good luck fixing all these, most are trivial to fix though and
don't get scared away by this you picked a hard package to start with and as
package more software your reviews will go more and more smoothly. Please post a
new version addressing all the above MUST Fix items, then I'll take a second
stab at doing a Full review and hopefully approving your first package. Feel
free to ask questions about the above if there is anything you don't understand
or disagree with.

So I have try to fix it but I have found an big bug in rpmbuild:(
to support FC4 and FC5 I have written this:
#use this for FC4
#%define readers_dir XXXX
#use this for FC5
#%define readers_dir YYYY
but rpmbuild ignore the # so when remove the # for FC4
the variable readers_dir ist not XXXX it is YYYY

Known problem / "feature" Its not really a bug but it is anoying the part of
rpmbuild doing % macro expansions doesn;t know about comments. If you want tuse
use a % in a comment write %%

So the next try I hope I have fixed all
Here the URL:
https://sourceforge.net/project/showfiles.php?group_id=155466&package_id=186639&release_id=413834

Erm:
[hans@shalem ~]$ rpmbuild -ba /usr/src/redhat/SPECS/ctapi-cyberjack.spec
error: File /usr/src/redhat/SOURCES/ctapi-cyberjack_MakefileCtAPI.patch: No such
file or directory

Thats why usually wy prefer SRPM's for reviews.

OK here the source rpm it contains the code for FC5 and the remarks for FC4.
http://prdownloads.sourceforge.net/qsmartcard/ctapi-cyberjack-2.0.8-6.src.rpm?download

Sorry, no more time (today) for a full review. I did build it and ran it through
rpmlint:
W: ctapi-cyberjack strange-permission ctapi-cyberjack-2.0.8.tar.bz2 0744
W: ctapi-cyberjack strange-permission ctapi-cyberjack.spec 0600
You should fix these, easy.

E: ctapi-cyberjack library-not-linked-against-libc
/usr/lib64/libctapi-cyberjack.so.0
Hmm, bad error! Probably something wrong with the link command I can look into
this one for you if you want, just let me know.

W: ctapi-cyberjack devel-file-in-non-devel-package /usr/lib64/libctapi-cyberjack.so
That should be fixed, easy.

E: ctapi-cyberjack library-without-ldconfig-postin
/usr/lib64/libctapi-cyberjack.so.0
E: ctapi-cyberjack library-without-ldconfig-postun
/usr/lib64/libctapi-cyberjack.so.0
2 more which you should fix.

W: ctapi-cyberjack-devel no-documentation
No problem

E: ctapi-cyberjack-pcsc library-not-linked-against-libc /libcyberjack_ifd.so
Again probably the strange ld invocation, let me know if you need help

W: ctapi-cyberjack-pcsc conffile-without-noreplace-flag
/etc/reader.conf.d/cyberjack.conf
Amke tbis %config(noreplace)

W: ctapi-cyberjack-pcsc no-documentation
No problem

E: ctapi-cyberjack-pcsc executable-marked-as-config-file
/etc/reader.conf.d/cyberjack.conf
E: ctapi-cyberjack-pcsc script-without-shellbang /etc/reader.conf.d/cyberjack.conf
The second one is becayse of the first one, change the mode of this to 644.

I think a new version fixing this before the fullreview is a good idea, don't
hesitate if you need help with the not linked to libc errors, that should be
easy to fix for me.

So I am working again:)
And what is ctapi-cyberjack devel-file-in-non-devel-package
/usr/lib64/libctapi-cyberjack.so??
It is an simble lib and no static oder header file.
So why it is called an devel file??

But I can not find the pronlem for the 
"E: ctapi-cyberjack library-not-linked-against-libc"
ldd say's this is linked again libc:(
ldd /usr/lib/libctapi-cyberjack.so
        linux-gate.so.1 =>  (0x00d37000)
        /lib/libNoVersion.so.1 (0x007b6000)
        libusb-0.1.so.4 => /usr/lib/libusb-0.1.so.4 (0x0091e000)
        libc.so.6 => /lib/libc.so.6 (0x007c2000)
        /lib/ld-linux.so.2 (0x00d38000)

so all is fixed. only the problem with the link to the lib exists:(
W: ctapi-cyberjack devel-file-in-non-devel-package /usr/lib64/libctapi-cyberjack.so
I have try to remove the file, but then the apps that use the lib will not run:(
Because they call libctapi-cyberjack.so via dl_load()

Great that you've almost all rpmlint errors and warnings fixed. About the last one:

Hmm,

With normal dyn linked libs the lniker will search for the -soname so xxx.so.0
and the plain .so symlink is only used during compile to find the real .so.x
file. What apps are using dlopen, the included ones? Couldn't you fix them to
use dlopen .so.0 ?

The .so symlink really belongs in the -devel package. We could make an exception
but thats rather ugly.  For example there apps that ldopen libGL.so, but
libGL.so still is in the -devel package and the apps are supposed to open
libGL.so.1 The whole idea behind versioned .so is that an open will open the
versioned lib so that if an ABI change happens ther app will keep working as
long as the old version is not uninstalled.

I have done some little error.
The lib for the CT-API dos not put in the ld cache, because all apps the use
CT-API call must use the lib via dl_open().
The the error about ctapi-cyberjack library-without-ldconfig-postin is in this
case not an error, because of the structure of the CT-API system.

(In reply to comment #33)
> The lib for the CT-API dos not put in the ld cache, because all apps the use
> CT-API call must use the lib via dl_open().

Hmm, is this written / documented somewhere? Since there are multiple possible
providers for the ctapi (libopenctapi.so , libtowitoko.so,
libctapi-cyberjack.so) this doesn't sound completly unlogical, but I wonder if
its documented somewhere. Also how is a user supposed to find out what ctapi
providers there are with all these non consistent names?

The CT-API system is complete diferent from PC/SC
When an app will use an reader via CT-API then it load's first the 3 function's
CT_init CT_data and CT_close via dl_load.
So for every reader you need an extra lib.
On Windows this files called ctXX32.dll wehre XX is the name of the reader.
So on Linux the files must be called libctapi-XX.so where XX is the name of the
reader. And as an result of this the package for the towitoko reader must
changed, because the wrong name. The complete description of the CT-API system
is done in german because it was primaly develop for the german helth system.
But niw in germany nearliy all application that need access to SmartCards use
this system, because it is mutch simpler than PC/SC. There exits also an english
description of the CT-API but can not tell if all correct in this description.
When you will have an look to this vist http://www.teletrust.de

here the next try
http://prdownloads.sourceforge.net/qsmartcard/ctapi-cyberjack-2.0.8-7.src.rpm?download

I see,

In that case the libctapi-xx.so shouldn't be versioned again, but then we have
the problem of the ifd handler linking to it, or we could once again make the
ifd-handler staticly linked to the ctapi part, but that would mean having
essentially the same code on your HD twice. Still I believe this is the best,
which effectivly means that you can undo all the makefile changes as upstream's
makefile then does seem todo the right thing after all, sorry about all this.

This might actually all workout nicely since with the ifd handler staitcly
linked it will not auto require and has no reason to manually require the main
package allowing for it to be installed seperatly.

Did you already drop the versioning in your next try (and link the ifd handler
staticly against ctapi-xxx?) ifso let me know and I'll do a full review as /
when time permits. If not, please post yet another version.

Also I can acutally read (and speak a bit) German, but your German is no doubt
better, could you copy the relevants parts avout the libctapi-xxxx.so being the
standard convention from the standard to a txt file and attach that?

Then I can review the standard and file a bug against openct. I personally
believe that since these .so files are unversioned and dlopened they should be
put under /usr/lib/ctapi/ instead of in plain /usr/lib does the standard say
anything about this?

Sorry about all this ping - pong you've picked a hard one to start with and I
want to get this right, currently the towitoko drivers aren't packaged and if
we're going to set a precedent on  how to handle this I would like to set a good
precedent.

The standart says noting about the the place of the libs.
But /usr/lib/ctapi will be an solution. Normaly the ctapi libs will live under
/usr/lib. The apps will normale only call dlopen(XX). XX=name of the lib.
When put the files to /usr/lib/ctapi then there we have the next problem.
Here the part of the doc about the dload function of the C lib about the search
path:
- (ELF only) If the executable file for the calling program contains a DT_RPATH
tag, and does not contain a DT_RUNPATH tag, then the directories listed in the
DT_RPATH tag are searched.
- If the environment variable LD_LIBRARY_PATH is defined to  contain  a 
colon-separated list of directories, then these are searched.  (As a security
measure this variable is ignored for set-UID and set-GID programs.)
- (ELF only) If the executable file for the calling program contains a 
DT_RUNPATH  tag, then the directories listed in that tag are searched.
-  The  cache file /etc/ld.so.cache (maintained by ldconfig(8)) is checked to
see whether  it contains an entry for filename.
- The directories /lib and /usr/lib are searched (in that order).
When the files are in /usr/lib/ctapi the the apps will not find it without set
an global LD_LIBRARY_PATH. Or we simple add the files to /usr/lib/ctapi and put
an extra config file to /etc/ld.so.conf.d But this have the problem the each
ctapi driver muste create an extra dir.
example:
/usr/lib/ctapi/ReinerSCT/libctapi-cyberjack.so
/usr/lib/ctapi/Kobil/libctapi-XXXX.so
/usr/lib/ctapi/ZZ/libctapi-XXX.so
ZZ= maufacture of the device
XXX= produce name
I think before I can work forwart we must found an global solution for CT-API in
Fedora. 
Here the link to the offical CT-API doc:
In german:
http://www.teletrust.de/index.php?id=303
In english:
http://www.teletrust.de/index.php?id=548
In my package the are 2 sample apps that use this API.

The import part is part 3 of the docu

(In reply to comment #37)
> I think before I can work forwart we must found an global solution for CT-API in
> Fedora. 

I agree, unfortunatly I don't have much (any?) time for that this week I'll try
to take a look this weekend.

I've taken a look at part 3 of the docu at:
http://www.teletrust.de/index.php?id=548

It seems that the only relevant part of this pdf for our discussion is in
Appendix A under "identification of CTAPI filenames with a wki" what the
document says here is basicly that dynamicloaded libs implementing the ctapi
should have a name in the following format: ctxxx[yyy] where xxx is a 3 letter
abbreviation of the manufacturer (CTM ID) (the abbreviations are supposed to be
asigned by a national goverment instance)

So that helps a bit, what it says is that the way to install ctapi
implementations for different cardreaders is indeed to use dlopen and name the
implementation .so files different. The 3 letter thingie clearly is because of
the 8 char length limitation dos had, and since there is no list of assigned
abreviations to be found I say we standardize on the name:
libctapi-<manufacturer>.so

But since these libraries are intented to be dlopen-ed and dlopen-ed only, and
as such are unversioned (.so instead of .so.x) I don't believe the belong
directly under %{_libdir} but that they shoud instead be put under say
%{_libdir}/ctapi .

Your story about DT_RPATH and LD_LIBARY_PATH above is about how things work for
regular linked binari%{_libdir}/ctapies (iow not using dlopen). Notice that we
don't need to create subdirs under  for each ctapi-lib instead we could just put
%{_libdir}/ctapi in a file under /etc/ld.so.conf.d . But that would beat the
entire purpose of putting these files in a seperate dir: not poluting the global
library soname namespace with these "plugins" .

Another solution would be to teach the dlopen-ing applications about the new
locations. I've moved ctapi-cyberjack.so* to %{_libdir}/ctapi on my system and
then tried to run cjgeldkarte:
[hans@shalem ~]$ cjgeldkarte
Error loading CT-API library.
[hans@shalem ~]$ cjgeldkarte -l ctapi/libctapi-cyberjack.so
Error loading CT-API library.
[hans@shalem ~]$ cjgeldkarte -l /usr/lib64/ctapi/libctapi-cyberjack.so
Error doing CT_init. (Return code:-1)

As you can see cjgeldkarte can no longer find its default libctapi-cyberjack.so
and passing ctapi/libctapi-cyberjack.so doesn't help either, we need to pass a
full path. Thats unfortunate because I find the full path ugly and it differs
from one arch to the other (usr/lib vs /usr/lib/64) now we can patch the correct
path into the binary during during built but thats not pretty.

The last option is to set DT_RPATH in the elf headers of the binaries using the
plugins to %{_libdir}/ctapi. This is quite easy, just change the "make" line in
the specfile to:
export LD_RUN_PATH=%{_libdir}/ctapi
make 

I must say I prefer this option, because it keeps the unversioned ctapi
libraries out of the regular soname namespace.

I'll open a bug against openct which currently also installs an unversioned .so
file in %{_libdir} for the ctapi.

Hopefully with the input of the openct maintainer we can choose one of the 3
choises which I see we have:
1) drop unversioned .so 's only intended for dlopen in %{_libdir}
2) put them in %{_libdir}/ctapi and code full path's to them (and users
   who want to use a different then the defualtlib also must specify the full 
   path).
3) put them in %{_libdir}/ctapi and set rpath to %{_libdir}/ctapi for binaries
   using the ctapi

1 and 3 are realistic options in my vision 2 is not.

As promised I've opened a bug against openct on this to get some sorta
resolution, its bug 190903 .

I agree with your vision wrt. 1) and 2), but I think 3) represents a too FE
centric world view which will cause some pain for 3rd party non-FE apps that
rightfully (per the spec) assume that they can just dlopen() the module.

1) is not too nice, agreed, but a fourth alternative would be to ship let's say
a ctapi-common package which drops a /etc/ld.so.conf.d/ctapi.conf snippet which
adds %{_libdir}/ctapi to the linker's load path, as well as include a README in
the package that describes the install location and naming conventions for FE
CT-API modules.

There is also a slight issue wrt. the libctapi-<implementation>.so scheme
because in some cases, for example openct and (I guess) towitoko, it would
differ from upstream module names.  I think we should either stick with upstream
naming or create compatibility symlinks.

Forgot to note that when using the fourth approach, ctapi-common would own
%{_libdir}/ctapi and all packages that install CT-API modules would install them
into that dir and have a dependency on ctapi-common.

The 4th alternative sounds like the best one to me. In this case I don't think
we need to standardise the .so filenames, since they are already under
%{_libdir}/ctapi, making clear what they are and giving apps a unique way to
enumerate all ctapi implementations available (all .so files under %{_libdir}/ctapi)

So I say lets go with the 4th approach:
-ctapi implementing libs go under %{_libdir}/ctapi
-%{_libdir}/ctapi is owned by ctapi-common
-ctapi-common drops a (64 and 32bit?) file under /etc/ld.so.conf.d
-ctapi implementing libs must depend on ctapi-common(.arch?)

Who wants to create the ctapi-common package?

We should also think about a ctapi-devel package containing a unified ctapi.h
ctbcs.h and maybe manpages (from the towitoko ctapi upstream)

I think number 4 is the best solution. But where build the "ctapi-common"
package?? It must be an solution where non OpenSource software also work, for
example somethink like moneyplex. Ok when an final work about the solutions was
find I will countinus to work on my package.

an unified ctapi.h is an second problem, because some manufacture add specal
function the the api. But we can do make can ctapi-common-devel package, witch
contains an general ctapi.h witch only has the the 3 needed
funtion's(ct_init,ct_data,ct_close) so it can be used with all readers.

Bug 190911 contains a first cut of the ctapi-common package, feel free to review
and/or take ownership.

One more thing we might want to consider is to add something like "Provides:
ctapi-module" (versioned using the CT-API spec if appliable?) to all packages
shipping CT-API modules so that other packages that require some module being
available can use that virtual dependency instead of having to leave it out or
to depend on a specific implementation.

I've just approved the ctapi-common package (bug 190911). I guess Ville will
import and build this soon.

So we've "solved" the where to put the ctapi library issue. Please modify your
package to install the library under %{_libdir}/ctapi and add:
Requires: %{_libdir}/ctapi

Also please remove the versioning (.0) from ctapi-cyberjack.so, but you should
still pass the -soname flag to the linker, just without the .0, .so files should
always should have an soname set.

I'm not sure what todo with regards to soname versioning for the pcsc
ifd-handler .so file. Ville do you know if these should be versioned or not?

Once we have the versioning of the pcsc ifd-handler .so file figured out please
create yet another version of your package. Sorry for the rough ride for your
first package. As I already said you picked a hard one. Well concider this a
good introduction to the review process and the general community process.

I've seen both versioned and unversioned ifdhandler *.so's.  But that's of
cosmetic nature; the drivers are loaded based on filenames anyway, either though
"bundles" where Info.plist contains the filename of the driver, or full path to
it in /etc/reader.conf.  Both ccid and openct contain examples of both usages,
and ccid has versioned, openct unversioned *.so.

ok I make the next build on some hours.

So the next try please
http://prdownloads.sourceforge.net/qsmartcard/ctapi-cyberjack-2.0.8-8FC5.src.rpm?download

But now the next problem:(
the lib is called libctapi-cyberjack.so.0 in the libcache.
And so the apps can't find it because the look for libctapi-cyberjack.so

Did you change the -soname option you added to the Makefile to remove the .0 ?
Also while we are at it, since upstream doesn't version the ifdhandler I think
neither should we, but as for ctapi, we should still at (an unversioned) -soname
option to the linkerflags.

So now the lib will be find again:)
Here the package
http://prdownloads.sourceforge.net/qsmartcard/ctapi-cyberjack-2.0.8-9FC5.src.rpm?download

MUST:
=====
* rpmlint output is:
W: ctapi-cyberjack one-line-command-in-%postun /sbin/ldconfig
This one needs fixing, replace:
%postun
/sbin/ldconfig
with:
%postun -p /sbin/ldconfig
The same goes for %post, but rpmlint doesn't detect that one?
W: ctapi-cyberjack-devel no-documentation
W: ctapi-cyberjack-pcsc no-documentation
These may be ignored
* Package and spec file named appropriately
* Packaged according to packaging guidelines
* License (GPL) ok, license file included
* spec file is legible and in Am. English.
* Source matches upstream
* Compiles and builds on devel-x86_64 & devel-i386
* BR: ok
* No locales
* Shared libraries, ldconfig is run
* Not relocatable
* Package owns / or requires all dirs
* No duplicate files & Permissions ok
* %clean & macro usage OK
* Contains code only
* %doc does not affect runtime, and isn't large enough to warrent a sub package
* -devel package for the .h file(s)
* no gui -> no .desktop file required


MUST fix:
=========
* Replace:
   %postun
   /sbin/ldconfig
  with:
   %postun -p /sbin/ldconfig
  The same goes for %post.
* The pcsc ifd gets installed as  /libcyberjack_ifd.so which is ofcourse not
  correct.

Should fix:
===========
* You could replace sed -e '....' aap > aap.tmp; mv aap.tmp aap with:
  sed -i '....' aap
* %package pcsc is missing the following Requires:
   Requires(post): /usr/sbin/update-reader.conf
   Requires(postun): /usr/sbin/update-reader.conf
 Although this does get provides by the Requires: pcsc-lite, why this
 Requires? Wouldn't the above 2 be better?

Concider yourself sponsored, feel free to create an account in the account
system and sign the CLA I'll sponsor you once thats done.

I'm awaiting the next and hopefully final revision of this package.

What so you men with "The pcsc ifd gets installed as  /laibcyberjck_ifd.so which
is ofcourse not" The file will not be direct under root.
on FC5 the file will be put in "pkg-config libpcsclite
--variable=usbdropdir"/laibcyberjck_ifd.so and on FC in
{_libdir}/readers/laibcyberjck_ifd.so

I an account but some one has suspend it:(

It really got installed as /libcyberjack_ifd.so on my system because I didn't
have pcsc-lite-devel. rpmbuild should have detected this but your spec file
misses a: "BuildRequires: pcsc-lite-devel" Add that to your MUST Fix list.

I'll see what I can do about your account.

This with the BuildRequires is fixed in the new one.
In about 2-4 hours the next one is comming:)

Who suspended your account? Then I know who to contact to unsuspend it.

It turns out I've managed to renable you account and sponosr you with any help
so you should have CVS access now. But do _not_ import this package yet it first
needs to be approved, please post a version with all the Must Fix items fixed
and I'll see if that one (finally) is it.

yes of course but I must wait until my system I ready to start the next try

So now I have modify %postun -p /sbin/ldconfig but this give now antoher error
of rpmlind. The new one is here:
http://prdownloads.sourceforge.net/qsmartcard/ctapi-cyberjack-2.0.8-10FC5.src.rpm?download

I cannot get that latest srpm because of sf.net mirror issues, I will try again
later. In the mean time whats the rpmlint message you get now?

after change from 
%postun
/sbin/ldconfig 
to 
%postun -p /sbin/ldconfig 
rpmlint says:
E: ctapi-cyberjack non-empty-%post /sbin/ldconfig

That's because what you intended as a comment above %postun -p /sbin/ldconfig
("#unrerister [sic]...") is not actually a comment but it will be passed to
/sbin/ldconfig as the %post script contents.  Some versions of ldconfig choke on
that.  Fix: remove the "comments", and remember that you can't use comments that
way ;)

By the way, you don't need to call /usr/sbin/update-reader.conf, the pcscd init
script does that automatically on startup.

Ok is fixed:)
here the new try:
http://prdownloads.sourceforge.net/qsmartcard/ctapi-cyberjack-2.0.8-11FC5.src.rpm?download

Sorry Frank,

But there are still some issues. My fault completly:
-I forgot to mention in my review that you should also add an -soname option
 to the ifd handler "ld" command in the Makefile. This is a should fix though
 and doesn't block approval

-The %post and %postun scripts for pcsc however do need some minor changes and
 that I do concider a blocker:
 -The comments should be below the %post resp  %postun not above. As it is now
  the comment above %post will become part of %install (and do nothing there)
  and the comment above %postun will become part of %post (try rpm -q --scripts)
  instead of %postun.
 -A bigger problem is that each command in a %post script should end with "|| :"
  Because rpm will concider a package as not installed (or not removed!) if the
  script fails and bash uses the exit of the last command as script exit.
  So change: "/sbin/service pcscd condrestart" to
  "/sbin/service pcscd condrestart || :"
 -You now have the Requires(post[un]): /usr/sbin/update-reader.conf as I 
  adviced, but you no longer use those. I guess that your initial solution of
  just requiring pcsc-lite was better. Sorry about this, I though the explicit
  /usr/sbin/update-reader.conf Requires would be better thinking that maybe one
  day we will have more then one pcsc implementation, but that seems highly 
  unlikely. So just move back to "Requires: pcsc-lite" for the -pcsc subpackage
  (_Sorry_).

With these few easy fixes we really should be there! I also notices some nasty
64 bit related warnings, but I've checked the relevant part of the sources and
they seem harmless. Looking forward to -12 and to approving it!

So here try #12
http://prdownloads.sourceforge.net/qsmartcard/ctapi-cyberjack-2.0.8-12FC5.src.rpm?download

Should fix:
-The ctapi-cyberjack_MakefilePCSC.patch sets soname to
 libcyberjack_ifd.so.0 this should be libcyberjack_ifd.so
 This is not a blocker because the .so file gets opened with a full
 path, and thus not through ld.so.cache. You do really need to fix this
 though.
-The comments for: "/sbin/service pcscd condrestart||:" may be above
 "/sbin/service pcscd condrestart||:" as long as they are below the
 "%postXXX"


You've fixed all blockers, so your package is approved!

p.s.

This was first discussed by mail because of bugzilla outage, I've received -13
by mail which fixed both Shoulds (excellent!). Al ready to import -13 and
request a build! Don't forget to add this package to owners.list .

So but I have an litle problem to acces the CVS system because the filename of
my public SSH key is not called id_rsa.pub it has onother name. So what paramter
must I add to "cvs co common" to say the ssh use fedora_rsa.pub and not id_rsa.pub??

(In reply to comment #72)
Strictly speaking, you should be using the fedora_rsa file, not fedora_rsa.pub
(which would be server-side).  But, to actually answer the question, put this in
your ~/.ssh/config (which needs to be chmod'd 600):

Host cvs.fedora.redhat.com
  IdentityFile ~/.ssh/fedora_rsa

You don't need to add anything to the cvs command (assuming you have CVSROOT set
to :ext:your_user.redhat.com:/cvs/extras, anyway).

Thank's now it works.
Must the file called ctapi-cyberjack-2.0.8-13.src.rpm or ctapi-cyberjack.src.rpm?

Thank's now it works.
Must the file called ctapi-cyberjack-2.0.8-13FC5.src.rpm or
ctapi-cyberjack-FC5.src.rpm simple ctapi-cyberjack.src.rpm ?

AFAIK the SRPM filename doesn't matter I just use the filename as generated by
rpmbuild -bs

> Thank's now it works.
> Must the file called ctapi-cyberjack-2.0.8-13FC5.src.rpm or
> ctapi-cyberjack-FC5.src.rpm simple ctapi-cyberjack.src.rpm ?

I don't mean to offend, but I am surprised that by the end of this lengthy
process you are asking basic questions like this.  Please review the packaging
guidelines and package naming guidelines carefully, as understanding them are
very important to Fedora cvsextras membership.

http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/NamingGuidelines

(In reply to comment #77)
> > Thank's now it works.
> > Must the file called ctapi-cyberjack-2.0.8-13FC5.src.rpm or
> > ctapi-cyberjack-FC5.src.rpm simple ctapi-cyberjack.src.rpm ?
> 
> I don't mean to offend, but I am surprised that by the end of this lengthy
> process you are asking basic questions like this.  Please review the packaging
> guidelines and package naming guidelines carefully, as understanding them are
> very important to Fedora cvsextras membership.
> 
> http://fedoraproject.org/wiki/Packaging/Guidelines
> http://fedoraproject.org/wiki/Packaging/NamingGuidelines

Warren,

He was asking howot name the SRPM to pass to cvs-import.sh a bit strange
question I must admit but nothing something that can be found in the guidelines
AFAIK.

Frank,

I just saw on the cvs-commit that you committed changes to an FC-4 version, I
hope your CVS branch request got handled that quickly, or did you do something
else to get the other branches?

All packages are build perfect for FC4 and FC5. Now the packages are waiting to
be signed.  The changes at the FC4 version are only the parts in the specfile
that for FC4. 

Frank,

That doesn't answer my question. When you import into CVS only the -devel branch
gets created, how did you get the FC-4 FC-5 branches (so fast) Normally you
would edit: http://fedoraproject.org/wiki/Extras/CVSSyncNeeded
And then wait for a human todo the branching. Now I'm assuming that you were
luky and the human did the branching quickly, no problem then. But since you're
still a bit unexperienced it could be you found some other creative way to
create the branches which would be not so good.

Ups. Yes I have edit the wiki page. And ca.10 minirs later I have run cvs co
ctapi-cyberjack ans all was fine.

Good, that was a quick CVs branch, don't get used to it :)

Thanks a lot for your patience with me:)

Package Change Request
======================
Package Name: ctapi-cyberjack
New Branches: F-7

Because the F-7 directory is missing in the CVS repo.

There seems to be a F-7 dir from what I can see...

make sure you use 'cvs update -d' when updating your local directories. 
By default cvs won't make new directories, you have to have -d there to make it
add them. Please re-check and resubmit if you still have issues. 

Yes this have worked.
Thanks.

Package Change Request
======================
Package Name: ctapi-cyberjack
Updated Name: cyberjack

Rename the package to get more clean sub package names.

Upstream is named ctapi-cyberjack isn't it? Or did they rename? 

Wouldn't renaming it confuse users as it's not the same as upstream? 
Also, debian and suse at least ship it with this name. 

It is only an idea, because the PC/SC sub package have an some confuses name.
It will be called ctapi-cyberjack-pcsc... I think this can confuse some users,
because PC/SC and CT-API are to total different API's.

Could you possibly just make the subpackage 'cyberjack-pcsc' and keep the main
package as 'ctapi-cyberjack' ? 

Perhaps it would be worth asking the fedora-packaging-list folks?

I'm going to unset the cvs flag for now until it's determined what needs to be
done here. 

I think we'd want a package named pcsc-lite-cyberjack for the pcsc bits, openct
already does that.  Use "%package -n pcsc-lite-cyberjack" etc in the specfile to
accomplish that.