Bug 1326875 - Review Request: keepassx2 - Cross-platform password manager
Summary: Review Request: keepassx2 - Cross-platform password manager
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Mukundan Ragavan
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-04-13 15:33 UTC by Gwyn Ciesla
Modified: 2016-06-24 21:19 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-06-24 21:16:53 UTC
Type: Bug
Embargoed:
nonamedotc: fedora-review+


Attachments (Terms of Use)

Description Gwyn Ciesla 2016-04-13 15:33:43 UTC
Description:
KeePassX is an application for people with extremly high demands on secure
personal data management.
KeePassX saves many different information e.g. user names, passwords, urls,
attachemts and comments in one single database. For a better management
user-defined titles and icons can be specified for each single entry.
Furthermore the entries are sorted in groups, which are customizable as well.
The integrated search function allows to search in a single group or the
complete database.
KeePassX offers a little utility for secure password generation. The password
generator is very customizable, fast and easy to use. Especially someone who
generates passwords frequently will appreciate this feature.
The complete database is always encrypted either with AES (alias Rijndael) or
Twofish encryption algorithm using a 256 bit key. Therefore the saved
information can be considered as quite safe. KeePassX uses a database format
that is compatible with KeePass Password Safe for MS Windows.



SRPM: https://fedorapeople.org/~limb/review/keepassx2/keepassx2-2.0.2-1.fc25.src.rpm
SPEC: https://fedorapeople.org/~limb/review/keepassx2/keepassx2.spec

Comment 1 Mukundan Ragavan 2016-04-14 20:30:58 UTC
I will take this up for review since I use this everyday.

Comment 2 Mukundan Ragavan 2016-04-25 23:06:29 UTC
Sorry for the delay. Mostly minor issues. Can be fixed easily.



Issues:
=======
- Package installs properly.
  Note: Installation errors (see attachment)
  See: https://fedoraproject.org/wiki/Packaging:Guidelines

---> This can be ignored. Local issue.

- Package does not contain duplicates in %files.
  Note: warning: File listed twice:
  /usr/share/keepassx2/translations/keepassx_cs.qm
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#DuplicateFiles


---> I think this is because of %{_datadir}/keepassx2 and correct usage of %find_lang macro.

The %files section could be more descriptive to avoid these clashes.

- Package installs a %{name}.desktop using desktop-file-install or desktop-
  file-validate if there is such a file.


---> Yes, this does seem to be missing (not present in post or posttrans). Please fix this.

===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files in private %_libdir subdirectory (see
     attachment). Verify they are not in ld path.
[x]: Header files in -devel subpackage, if present.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "GPL (v2 or v3)", "LGPL (v2.1 or v3)", "LGPL (v2.1)", "Unknown
     or generated". 13 files have unknown license. Detailed output of
     licensecheck in
     /home/mukundan/ownCloud/misc_pkgs/pkg_reviews/1326875-keepassx2/licensecheck.txt

---> Nothing seems bad.

[x]: License file installed when any subpackage combination is installed.

[?]: Package requires other packages for directories it uses.
     Note: No known owner of /usr/lib64/keepassx2

---> This is perhaps easily fixed.

This - %{_libdir}/keepassx2/libkeepassx*.so - could be changed to 

%{_libdir}/keepassx2/*


[x]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/lib64/keepassx2,
     /usr/share/mime, /usr/share/mimelnk/application, /usr/share/mimelnk,
     /usr/share/mime/packages
[!]: Package does not own files or directories owned by other packages.
     Note: Dirs in package are owned also by:
     /usr/share/icons/hicolor/16x16/mimetypes(hicolor-icon-theme,
     keepassx), /usr/share/icons/hicolor/16x16/actions(hicolor-icon-theme,
     keepassx), /usr/share/icons/hicolor/32x32/apps(hicolor-icon-theme,
     fedora-logos, keepassx), /usr/share/icons/hicolor/24x24/apps(hicolor-
     icon-theme, fedora-logos, keepassx), /usr/share/icons/hicolor/24x24
     (hicolor-icon-theme, fedora-logos, keepassx),
     /usr/share/icons/hicolor/128x128/apps(hicolor-icon-theme, keepassx),
     /usr/share/icons/hicolor/128x128/mimetypes(hicolor-icon-theme,
     keepassx), /usr/share/icons/hicolor/64x64(hicolor-icon-theme,
     keepassx), /usr/share/icons/hicolor/256x256/apps(hicolor-icon-theme,
     fedora-logos, keepassx), /usr/share/icons/hicolor/64x64/mimetypes
     (hicolor-icon-theme, keepassx), /usr/share/icons/hicolor/16x16
     (hicolor-icon-theme, fedora-logos, keepassx),
     /usr/share/icons/hicolor/48x48/apps(hicolor-icon-theme, fedora-logos,
     keepassx, nedit), /usr/share/icons/hicolor/scalable/apps(hicolor-icon-
     theme, fedora-logos, keepassx), /usr/share/icons/hicolor/22x22/actions
     (hicolor-icon-theme, keepassx), /usr/share/icons/hicolor/128x128
     (hicolor-icon-theme, keepassx),
     /usr/share/icons/hicolor/22x22/mimetypes(hicolor-icon-theme,
     keepassx), /usr/share/icons/hicolor/32x32(hicolor-icon-theme, fedora-
     logos, keepassx), /usr/share/icons/hicolor/256x256(hicolor-icon-theme,
     fedora-logos, keepassx), /usr/share/icons/hicolor/16x16/apps(hicolor-
     icon-theme, fedora-logos, keepassx),
     /usr/share/icons/hicolor/64x64/apps(hicolor-icon-theme, keepassx),
     /usr/share/icons/hicolor/22x22(hicolor-icon-theme, fedora-logos,
     keepassx), /usr/share/icons/hicolor/22x22/status(hicolor-icon-theme,
     keepassx), /usr/share/icons/hicolor/48x48(hicolor-icon-theme, fedora-
     logos, keepassx, nedit), /usr/share/icons/hicolor/32x32/mimetypes
     (hicolor-icon-theme, keepassx), /usr/share/icons/hicolor/scalable
     (hicolor-icon-theme, fedora-logos, keepassx)


---> This can also be easily fixed, probably.

Change %{_datadir}/icons/hicolor/*  ---> %{_datadir}/icons/hicolor/*/*/*

[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[x]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.


---> keepassx and keepassx2 can co-exist. Nothing to obsolete.

[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[!]: update-desktop-database is invoked in %post and %postun if package
     contains desktop file(s) with a MimeType: entry.
     Note: desktop file(s) with MimeType entry in keepassx2

---> See above.

[x]: gtk-update-icon-cache is invoked in %postun and %posttrans if package
     contains icons.
     Note: icons in keepassx2


---> This looks good.

[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 20480 bytes in 2 files.


---> This is fine. Does not warrant a subpackage.

[!]: Package complies to the Packaging Guidelines

---> Some minor issues as pointed out.

[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package contains desktop file if it is a GUI application.
[x]: Dist tag is present.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[!]: update-mime-database is invoked in %post and %postun if package stores
     mime configuration in /usr/share/mime/packages.
     Note: mimeinfo files in: keepassx2
     See:
     http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#mimeinfo
[ ]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[ ]: Final provides and requires are sane (see attachments).
[ ]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
     keepassx2-debuginfo
[ ]: Package functions as described.
[ ]: Latest version is packaged.
[ ]: Package does not include license text files separate from upstream.
[ ]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[ ]: Package should compile and build into binary rpms on all supported
     architectures.
[ ]: %check is present and all tests pass.
[ ]: Packages should try to preserve timestamps of original installed
     files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: Mock build failed
     See: http://fedoraproject.org/wiki/Packaging/Guidelines#rpmlint


---> This is fine. I have changed it to [x].

[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
     Note: Arch-ed rpms have a total of 1167360 bytes in /usr/share

---> not an issue in this case.

[x]: Spec file according to URL is the same as in SRPM.


Installation errors
-------------------
INFO: mock.py version 1.2.17 starting (python version = 3.4.3)...
Start: init plugins
INFO: selinux enabled
Finish: init plugins
Start: run
Start: chroot init
INFO: calling preinit hooks
INFO: enabled root cache
INFO: enabled dnf cache
Start: cleaning dnf metadata
Finish: cleaning dnf metadata
Mock Version: 1.2.17
INFO: Mock Version: 1.2.17
Finish: chroot init
INFO: installing package(s): /home/mukundan/ownCloud/misc_pkgs/pkg_reviews/1326875-keepassx2/results/keepassx2-2.0.2-1.fc25.x86_64.rpm /home/mukundan/ownCloud/misc_pkgs/pkg_reviews/1326875-keepassx2/results/keepassx2-debuginfo-2.0.2-1.fc25.x86_64.rpm /home/mukundan/ownCloud/misc_pkgs/pkg_reviews/1326875-keepassx2/results/keepassx2-debuginfo-2.0.2-1.fc25.x86_64.rpm
ERROR: Command failed. See logs for output.
 # /usr/bin/dnf --installroot /var/lib/mock/fedora-rawhide-x86_64/root/ --releasever 24 --disableplugin=local --setopt=deltarpm=false install /home/mukundan/ownCloud/misc_pkgs/pkg_reviews/1326875-keepassx2/results/keepassx2-2.0.2-1.fc25.x86_64.rpm /home/mukundan/ownCloud/misc_pkgs/pkg_reviews/1326875-keepassx2/results/keepassx2-debuginfo-2.0.2-1.fc25.x86_64.rpm /home/mukundan/ownCloud/misc_pkgs/pkg_reviews/1326875-keepassx2/results/keepassx2-debuginfo-2.0.2-1.fc25.x86_64.rpm --setopt=tsflags=nocontexts


Rpmlint
-------
Checking: keepassx2-2.0.2-1.fc25.x86_64.rpm
          keepassx2-debuginfo-2.0.2-1.fc25.x86_64.rpm
          keepassx2-2.0.2-1.fc25.src.rpm
keepassx2.x86_64: W: spelling-error %description -l en_US extremly -> extremely, extreme
keepassx2.x86_64: W: spelling-error %description -l en_US urls -> curls, purls, hurls
keepassx2.x86_64: W: spelling-error %description -l en_US attachemts -> attachments, attache, schemata
keepassx2.x86_64: W: spelling-error %description -l en_US customizable -> customization


---> minor issues.

extremly ---> extremely.
attachemts ---> attachments.

keepassx2.x86_64: W: gzipped-svg-icon /usr/share/icons/hicolor/scalable/apps/keepassx.svgz
keepassx2.x86_64: W: no-manual-page-for-binary keepassx2
keepassx2.src: W: spelling-error %description -l en_US extremly -> extremely, extreme
keepassx2.src: W: spelling-error %description -l en_US urls -> curls, purls, hurls
keepassx2.src: W: spelling-error %description -l en_US attachemts -> attachments, attache, schemata
keepassx2.src: W: spelling-error %description -l en_US customizable -> customization
3 packages and 0 specfiles checked; 0 errors, 10 warnings.




Requires
--------
keepassx2 (rpmlib, GLIBC filtered):
    /bin/sh
    hicolor-icon-theme
    libQtCore.so.4()(64bit)
    libQtGui.so.4()(64bit)
    libX11.so.6()(64bit)
    libXi.so.6()(64bit)
    libXtst.so.6()(64bit)
    libc.so.6()(64bit)
    libgcrypt.so.20()(64bit)
    libgcrypt.so.20(GCRYPT_1.6)(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libz.so.1()(64bit)
    qt
    rtld(GNU_HASH)

keepassx2-debuginfo (rpmlib, GLIBC filtered):



Provides
--------
keepassx2:
    application()
    application(keepassx2.desktop)
    keepassx2
    keepassx2(x86-64)
    libkeepassx-autotype-x11.so()(64bit)
    mimehandler(application/x-keepass2)

keepassx2-debuginfo:
    keepassx2-debuginfo
    keepassx2-debuginfo(x86-64)



Unversioned so-files
--------------------
keepassx2: /usr/lib64/keepassx2/libkeepassx-autotype-x11.so

Source checksums
----------------
https://github.com/keepassx/keepassx/archive/2.0.2.tar.gz#/keepassx-2.0.2.tar.gz :
  CHECKSUM(SHA256) this package     : 204bdcf49c72078cd6f02b4f29b062923cca9e7b2d3551f2bf352763daa236b8
  CHECKSUM(SHA256) upstream package : 204bdcf49c72078cd6f02b4f29b062923cca9e7b2d3551f2bf352763daa236b8


Generated by fedora-review 0.6.0 (3c5c9d7) last change: 2015-05-20
Command line :/usr/bin/fedora-review -m fedora-rawhide-x86_64 -b 1326875
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, Shell-api, C/C++
Disabled plugins: Java, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6

Comment 3 Mukundan Ragavan 2016-05-10 01:14:43 UTC
ping?

Comment 4 Gwyn Ciesla 2016-05-10 14:32:48 UTC
Sorry, see: https://fedorahosted.org/fesco/ticket/1569

This will likely end up being EPEL-only.

Comment 5 srakitnican 2016-05-11 00:12:05 UTC
If I understood correctly, fedora would get 2.0 by default from now on, and no 0.4 version. I am ok with that.

Comment 6 Mukundan Ragavan 2016-05-11 12:10:20 UTC
(In reply to srakitnican from comment #5)
> If I understood correctly, fedora would get 2.0 by default from now on, and
> no 0.4 version. I am ok with that.

That's my understanding as well.


I will review this with epel-7 and post again. The minor issues that cropped up should be the same in epel as well but I'll check anyway.

Comment 7 Mukundan Ragavan 2016-05-15 22:16:33 UTC
Just these two issues as mentioned earlier -

Issues:
=======
- Package installs properly.
  Note: Installation errors (see attachment)
  See: https://fedoraproject.org/wiki/Packaging:Guidelines

---> This can be ignored. Local issue.

- Package does not contain duplicates in %files.
  Note: warning: File listed twice:
  /usr/share/keepassx2/translations/keepassx_cs.qm
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#DuplicateFiles


---> I think this is because of %{_datadir}/keepassx2 and correct usage of %find_lang macro.

The %files section could be more descriptive to avoid these clashes.

- Package installs a %{name}.desktop using desktop-file-install or desktop-
  file-validate if there is such a file.


---> Yes, this does seem to be missing (not present in post or posttrans). Please fix this.

Comment 8 Mukundan Ragavan 2016-05-24 22:11:41 UTC
ping ... Any issues here?

Comment 9 Gwyn Ciesla 2016-05-26 22:05:32 UTC
Sorry, got busy.  I've added desktop-file-validate and fixed the dupes.

SRPM: https://fedorapeople.org/~limb/review/keepassx2/keepassx2-2.0.2-2.fc25.src.rpm
SPEC: https://fedorapeople.org/~limb/review/keepassx2/keepassx2.spec

Comment 10 Mukundan Ragavan 2016-05-27 02:10:24 UTC
I don't have any further problems. Thanks for submitting the review.


Package APPROVED.

Comment 11 Fedora Update System 2016-06-09 14:08:13 UTC
keepassx2-2.0.2-2.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-ffae356c46

Comment 12 Gwyn Ciesla 2016-06-09 14:18:05 UTC
Not building on el6, working on it.

Comment 13 Fedora Update System 2016-06-09 15:23:25 UTC
keepassx2-2.0.2-2.el6 has been submitted as an update to Fedora EPEL 6. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-885487040d

Comment 14 Fedora Update System 2016-06-10 02:49:24 UTC
keepassx2-2.0.2-2.el6 has been pushed to the Fedora EPEL 6 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-885487040d

Comment 15 Fedora Update System 2016-06-10 03:17:16 UTC
keepassx2-2.0.2-2.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-ffae356c46

Comment 16 Fedora Update System 2016-06-24 21:16:50 UTC
keepassx2-2.0.2-2.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.

Comment 17 Fedora Update System 2016-06-24 21:19:05 UTC
keepassx2-2.0.2-2.el6 has been pushed to the Fedora EPEL 6 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.