On Tuesday 21 October 2014 06:28 PM, Maximiliano Curia wrote:
Totally understand and appreciate this. I didn't think that a package in
Ubuntu mainstream would need so much review.
The review process involves checking and fixing the packaging, and checking
upstream code for possible errors/incompatibilities with the way things are
done in the distribution. It takes time from both of us.
Agreed. But it would be great if we can have this in Debian Jessie. Is
it still possible?
My ultra motive to offer you to review the package is to have more members
engaged in the team, not to push things that are not up to the quality expected
There are a couple of fixes in the upstream git, last commit is 2014-05-08,
you might want to include those.
To be under the kde team umbrella the package should be something like:
Maintainer: Debian/Kubuntu Qt/KDE Maintainers <debian-qt-...@lists.debian.org>
Maintainer: Debian KDE Extras Team <pkg-kde-ext...@lists.alioth.debian.org>
Maintainer: Debian Krap Maintainers <debian-qt-...@lists.debian.org>
The field: XSBC-Original-Maintainer is not considered valid in Debian
Add add yourself to the Uploaders list.
In the debian/copyright file:
Please update the template to point to the upstream git repository.
Also in the debian/copyright file, the debian/* path is licensed under a more
restrictive license than the upstream code (GPL, and LGPL respectively), this
kind of licensing could block patches in the debian package from ever be
applied upstream and should be avoided. I pinged Rohan about this.
In Debian the pam modules are named libpam-$module, please rename the binary
Done. I also added a README file describing the prerequisites and
The description provides almost no information, please extend it. Consider
using the kwalletmanager description, and adding a paragraph about the pam
module (ala libpam-gnome-keyring).
It's a good idea to set the build dependencies versions to (at least) the ones
listed in the CMakeLists.txt, in this case cmake (>= 2.8.8) and
libgcrypt11-dev (>= 1.5.0).
I have not reviewed the upstream code (not sure if I'll be able to
understand it also). Also, I prefer to leave upstream code unchanged
unless it breaks something or has some security or performance issues.
In the code I don't see any obvious errors, but I'm not an expert in pam
modules, some comments though:
In kwallet_hash, after the call to error = gcry_kdf_derive(..) it's not
checking in error returned something.
In prompt_for_password, the memset in the lines:
struct pam_response *response = NULL;
memset (&response, 0, sizeof(response));
Also, the normal review process is done via mentors.debian.net, where you
could upload the package and send a RFS, I prefer using a git repository where
I can see the changes made, and afterwards integrate the changes in a repository
for the package, either one is fine, or even an uri where I can fetch the
package source (I don't care about the binary file).
You can get the source at *https://github.com/amaramrahul/pam-kwallet*
In any case, I would prefer not to have the packages as attachments, specially
in bugs and the team mailing lists, so, unless you can't publish the files
somewhere else, please avoid sending them like so. And if you really have to
send the files as attachments, please send them via direct mail, without
Looking forward to your response.