Bug 2087174 - Review Request: telepathy-idle - IRC connection manager for Telepathy
Summary: Review Request: telepathy-idle - IRC connection manager for Telepathy
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Linux
unspecified
unspecified
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 2064643
TreeView+ depends on / blocked
 
Reported: 2022-05-17 13:52 UTC by Ali Erdinc Koroglu
Modified: 2022-09-15 08:00 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2022-09-15 06:09:28 UTC
Type: Bug
Embargoed:
mtasaka: fedora-review+


Attachments (Terms of Use)

Comment 1 Ali Erdinc Koroglu 2022-05-17 14:04:22 UTC
This package is runtime dependency for Polari (#2064643).
After the reviewing process I'll open unretire request in pagure.

Comment 2 Mamoru TASAKA 2022-08-29 15:06:10 UTC
So as I happen to be telepathy-glib maintainer, I comment for now.

* Now for new packages License field must use SPDX identifier:
  https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_valid_license_short_names
  https://spdx.org/licenses/

* For BuildRequires: using "BuildRequires: pkgconfig(foo)" is preferred
  than to use "BuildRequires: dbus-glib-devel" or so
  https://docs.fedoraproject.org/en-US/packaging-guidelines/PkgConfigBuildRequires/

* Unneeded BR
  - At least "BR: openssl-devel" seems unneeded
  - Also check if there is any more unneeded BRs left.

* Using "python-unversioned-command" as BR is strongly discouraged.
  Patch out not to use non-version python and use python3.
  https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_dependencies

* scratch build says:
  checking for Python with Twisted and IRC protocol support... no
  https://koji.fedoraproject.org/koji/taskinfo?taskID=91402714
  Looking at configure.ac, this perhaps means that some twisted tests are disabled.
  Please consider to enable this.

* Directory ownership:
  https://docs.fedoraproject.org/en-US/packaging-guidelines/UnownedDirectories/
  With "mock --init" and doint "mock --install telepathy-idle.x86_64.rpm", /usr/share/dbus-1/services/
  is not owned by any packages.
=============================================================
[mockbuild@93f3667e0cdb434cb87064920e66e928 ~]$ rpm -qf /usr/share/dbus-1/services/
file /usr/share/dbus-1/services is not owned by any package
=============================================================
  Make it sure this directory is owned by some package when telepathy-idle is installed.

Comment 3 Mamoru TASAKA 2022-08-29 15:27:52 UTC
Some notes:

* For BR: pkgconfig(foo), you can check for configure.ac, and check
  what is used with PKG_CHECK_MODULES macro or so.

Comment 4 Mamoru TASAKA 2022-09-02 02:01:52 UTC
Setting NEEDINFO.

Comment 6 Mamoru TASAKA 2022-09-07 08:17:07 UTC
Assigning.

Comment 7 Mamoru TASAKA 2022-09-07 15:37:00 UTC
* License
  - Many files (such as src/idle-connection-manager.c or so) explictly says
    the license is under LGPL-2.0 only. Please check this.

* Directory ownership issue
  - /usr/share/dbus-1/ and /usr/share/dbus-1/services/ are unowned by any packages
    when telepathy-idle binary is installed.
    Add: "Requires: dbus-common".

Comment 8 Ali Erdinc Koroglu 2022-09-09 13:13:11 UTC
I checked the code so license is LGPL-2.1-or-later and as you're saying /usr/share/dbus-1/services belongs to dbus-common which is added as "requires" 

SPEC Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/04830536-telepathy-idle/telepathy-idle.spec
SRPM Url: https://download.copr.fedorainfracloud.org/results/aekoroglu/fedora/fedora-rawhide-x86_64/04830536-telepathy-idle/telepathy-idle-0.2.2-1.fc38.src.rpm

Comment 9 Mamoru TASAKA 2022-09-10 14:06:44 UTC
* License
  - For example, this https://gitlab.freedesktop.org/telepathy/telepathy-idle/-/blob/master/src/idle-connection.c#L9-10 says
    "GNU Lesser General Public License version 2.1 as published", this is LGPL-2.1 only.

    For reference, this sentence https://gitlab.freedesktop.org/telepathy/telepathy-idle/-/blob/master/extensions/Connection_Interface_IRC_Command1.xml#L7-8
   is LGPL-2.1-or-later .

Comment 10 Ali Erdinc Koroglu 2022-09-10 14:35:27 UTC
Project's license (https://gitlab.freedesktop.org/telepathy/telepathy-idle/-/blob/master/COPYING) is LGPL-2.1-or-later, probably they forgot
to change that while migrating from SF.net to SVN 15 years ago :)

But we can do like "LGPL-2.1-only AND LGPL-2.1-or-later"

Comment 11 Mamoru TASAKA 2022-09-11 05:27:46 UTC
We judge the license of the software not only from copyright files but also the actual contents of the source file.
Please change the license tag to "LGPL-2.1-only AND LGPL-2.1-or-later".

Comment 13 Mamoru TASAKA 2022-09-11 13:44:48 UTC
* Name legitimate
* EVR correct
* License correct
* Summary clean
* Source matches upstream
* BR / Requires okay
* %prep / %build / %install / %check okay
* %files okay
* rpmlint on srpm clean
* directory ownership okay

* scratch build okay
  https://koji.fedoraproject.org/koji/taskinfo?taskID=91904425
* build logs are sane
* %optflags correctly used
* can be installed
* rpmlint on binary rpm okay

Approving.

==================================================
  This package (telepathy-idle) is APPROVED
  by mtasaka
==================================================

Comment 14 Ali Erdinc Koroglu 2022-09-15 06:09:28 UTC
Thank you Mamoru

https://koji.fedoraproject.org/koji/buildinfo?buildID=2062783

Comment 15 Mamoru TASAKA 2022-09-15 07:22:15 UTC
Please rebuild this package also on F-37, and submit to bodhi (submitting to bodhi is needed for F-37) - unless this,
polari dependency issue on F-37 cannot be resolved.

Comment 16 Ali Erdinc Koroglu 2022-09-15 08:00:23 UTC
Yep I did, thanks

https://bodhi.fedoraproject.org/updates/FEDORA-2022-d23557aec2


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