Bug 2025124 - Review Request: qtile - A pure-Python tiling window manager
Summary: Review Request: qtile - A pure-Python tiling window manager
Status: POST
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Carl George 🤠
QA Contact: Fedora Extras Quality Assurance
URL: http://qtile.org
: 2027073 2036556 (view as bug list)
Depends On: 2157817
TreeView+ depends on / blocked
Reported: 2021-11-20 00:07 UTC by Jakub Kadlčík
Modified: 2023-11-14 08:35 UTC (History)
9 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Last Closed:
Type: ---
carl: fedora-review+

Attachments (Terms of Use)
The .spec file difference from Copr build 6610458 to 6626809 (4.48 KB, patch)
2023-11-12 16:58 UTC, Fedora Review Service
no flags Details | Diff
The .spec file difference from Copr build 6626809 to 6630297 (1.98 KB, patch)
2023-11-13 17:58 UTC, Fedora Review Service
no flags Details | Diff

Description Jakub Kadlčík 2021-11-20 00:07:43 UTC
Spec URL: https://download.copr.fedorainfracloud.org/results/frostyx/qtile/fedora-rawhide-x86_64/02968035-qtile/qtile.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/frostyx/qtile/fedora-rawhide-x86_64/02968035-qtile/qtile-0.18.1-2.fc36.src.rpm
A pure-Python tiling window manager.


    * Simple, small and extensible. It's easy to write your own layouts,
      widgets and commands.
    * Configured in Python.
    * Command shell that allows all aspects of
      Qtile to be managed and inspected.
    * Complete remote scriptability - write scripts to set up workspaces,
      manipulate windows, update status bar widgets and more.
    * Qtile's remote scriptability makes it one of the most thoroughly
      unit-tested window mangers around.

Fedora Account System Username: frostyx

Comment 1 Jakub Kadlčík 2021-11-20 00:11:20 UTC
Qtile was in Fedora from F22 up to F34

Unfortunately, it was orphaned and needs to go through a package review before I can continue maintaining it.
I took the specfile from F34, polished it a little, and updated it to work with the most recent Qtile version.

Comment 2 Jakub Kadlčík 2021-11-20 00:14:54 UTC
FTR: Releng ticket to unretire the qtile package

Comment 3 Jhordy Caceres 2021-11-29 12:02:45 UTC
*** Bug 2027073 has been marked as a duplicate of this bug. ***

Comment 9 lqlarry 2022-04-18 03:01:38 UTC
(In reply to Jakub Kadlčík from comment #8)
> Oops, sorry, wrong link in the previous comment
> Spec URL:
> https://download.copr.fedorainfracloud.org/results/frostyx/qtile/fedora-
> rawhide-x86_64/03814640-qtile/qtile.spec
> https://download.copr.fedorainfracloud.org/results/frostyx/qtile/fedora-
> rawhide-x86_64/03814640-qtile/qtile-0.21.0-1.fc37.src.rpm

(In reply to Jakub Kadlčík from comment #6)
> I bumped a release and added Recommends: for packages, that are needed by
> widgets.
> Spec URL:
> https://download.copr.fedorainfracloud.org/results/frostyx/qtile/fedora-
> rawhide-x86_64/03571096-qtile/qtile.spec
> https://download.copr.fedorainfracloud.org/results/frostyx/qtile/fedora-
> rawhide-x86_64/03571096-qtile/qtile-0.20.0-2.fc37.src.rpm

@jak Thanks for doing thisproject and making it available.  Everything is like a stock Qtile installment.  I was able to slip my dot files in, install programs that I need to personal use and it workded like a charm.

Fedora does need a Qtile install method and now Fedora has one.

Thanks again.

Comment 10 lqlarry 2022-04-18 03:04:12 UTC
(In reply to Jakub Kadlčík from comment #0)
> Spec URL:
> https://download.copr.fedorainfracloud.org/results/frostyx/qtile/fedora-
> rawhide-x86_64/02968035-qtile/qtile.spec
> https://download.copr.fedorainfracloud.org/results/frostyx/qtile/fedora-
> rawhide-x86_64/02968035-qtile/qtile-0.18.1-2.fc36.src.rpm
> Description:
> A pure-Python tiling window manager.
> Features
> ========
>     * Simple, small and extensible. It's easy to write your own layouts,
>       widgets and commands.
>     * Configured in Python.
>     * Command shell that allows all aspects of
>       Qtile to be managed and inspected.
>     * Complete remote scriptability - write scripts to set up workspaces,
>       manipulate windows, update status bar widgets and more.
>     * Qtile's remote scriptability makes it one of the most thoroughly
>       unit-tested window mangers around.
> Fedora Account System Username: frostyx

Fedora does need a Qtile install method and now Fedora has one.

Great work @Jak

Comment 11 Jakub Kadlčík 2022-04-18 22:44:48 UTC
Thank you for the kind words, Larry.
I am happy that the Copr repository for Qtile is useful.

Just to clarify what is the actual blocker (I might have miscommunicated it previously).
We need some Fedora package maintainer to do a formal review

Comment 13 Carl George 🤠 2022-12-18 02:23:36 UTC
*** Bug 2036556 has been marked as a duplicate of this bug. ***

Comment 14 Carl George 🤠 2022-12-18 03:28:04 UTC
I can take this review.


Do you think you can update this spec file to use the current Python packaging guidelines?  At a glance it looks mostly correct for the legacy guidelines (which are still allowed), but since we're re-reviewing it now is a great time to convert it.



The license field needs to be updated to use an SPDX license expression.

-License: MIT and GPLv3+
+License: MIT AND GPL-3.0-or-later

It also appears that the license breakdown comment is out of date.  licensecheck found libqtile/widget/cmus.py and libqtile/widget/moc.py as the only files under the GPL.



Upstream has a test suite, try to run it in %check if possible.  The upstream tox file looks like it tries to pip install stuff, which won't work in the build environment.  You might choose to patch this file and run the tests with %tox.  Alternatively you could run the tests directly with %pytest.



For the %setup macros, the default directory name is %{name}-%{version}, so that flag can be left out.

-%setup -q -n qtile-%{version}
+%setup -q

Optionally you could switch this to %autosetup, which will automatically apply patches, if and when they are needed.

-%setup -q -n qtile-%{version}
+%autosetup -p 1


Comment 15 Jakub Kadlčík 2022-12-21 19:46:55 UTC
I can't believe somebody finally took the review, thank you Carl :-)

I updated the spec to follow the current Python packaging guidelines,
fixed the license, autosetup macro, and added tests. Also, I
remembered that some time ago, I added Wayland support to the package
but it requires dependencies that are not in Fedora yet, so I added
bcond to build the package without Wayland support for the official
Fedora repos but easily allow people to build it for themselves
with Wayland. I intend to add the dependencies into Fedora as well but
it can take some time.

Spec URL: https://download.copr.fedorainfracloud.org/results/frostyx/qtile/fedora-rawhide-x86_64/05160564-qtile/qtile.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/frostyx/qtile/fedora-rawhide-x86_64/05160564-qtile/qtile-0.22.1-2.fc38.src.rpm

Here is the diff, so you can easily see the changes

Comment 17 Carl George 🤠 2022-12-23 05:41:12 UTC
Thanks for the adjustments.  Here are a few more things we can improve.


I can see you had to add another source from the GitHub repo in order to run the tests.  If the PyPI tarball is missing files needed for the tests we can just use the GitHub tarball directly instead.  Also it's optional to use numbered sources now, and really only makes sense if you need to reference the sources independently later in the spec file.

-Source0: %{pypi_source}
-Source1: https://raw.githubusercontent.com/qtile/qtile/v%{pypi_version}/bin/qtile
+Source: https://github.com/qtile/qtile/archive/v%{version}/qtile-%{version}.tar.gz



A quick test build on my end after switching to the GitHub tarball has me run into the warning listed in the "Source files from PyPI" documentation linked above.  We'll also need to export an environment variable in two spots.




Since Name and Version are exactly the same as pypi_name and pypi_version, for readability it would be better to just skip defining the extra macros and use the built in fields.

-%global pypi_name qtile
-%global pypi_version 0.22.1

-Name: %{pypi_name}
-Version: %{pypi_version}
+Name: qtile
+Version: 0.22.1


There are a few buildrequires and requires that are duplicated between the automatic generators and explicit declarations in the spec file.  The explicit ones should be removed in favor of the automatic ones.  The automatic ones are based on the setup_requires and install_requires fields in the setup.cfg file.  Some of the explicit dependencies might not even be necessary if upstream hasn't declared them.  Please review and remove the ones you can.

I'll note that there is also a test_requires field in setup.cfg, but I don't believe that is read by %pyproject_buildrequires.  Even if it were we'd need to patch out the linting dependencies.  Ideally the only explicit Python buildrequires we should have are python3-devel and python3-pytest.  If there are other Python ones we should reach out to upstream and try to get them added to the "test" extra, which we could pull in via `%pyproject_buildrequires -x test`.  Hopefully upstream will also be open to the idea of having separate "test" and "lint" extras so we can skip the linting deps.  Non-Python buildrequirements will probably still need to be listed explicitly.

I'll also point out that once the wayland dependencies are packaged and available, we can include them with `%pyproject_buildrequires -x wayland` without listing them explictly.



The code has a few system libraries that it loads with ffi.dlopen.  These should be listed as explict requires (and buildrequires for the test suite).  It gets a little tricky due to the suffix in the provides, but the following should work for what we need.

+# Some dependencies are loaded with ffi.dlopen, and to declare them properly
+# we'll need this suffix.
+%if 0%{?__isa_bits} == 32
+%global libsymbolsuffix %{nil}
+%global libsymbolsuffix ()(%{__isa_bits}bit)

-BuildRequires:  pango-devel
+BuildRequires: libgobject-2.0.so.0%{libsymbolsuffix}
+BuildRequires: libpango-1.0.so.0%{libsymbolsuffix}
+BuildRequires: libpangocairo-1.0.so.0%{libsymbolsuffix}

+Requires: libgobject-2.0.so.0%{libsymbolsuffix}
+Requires: libpango-1.0.so.0%{libsymbolsuffix}
+Requires: libpangocairo-1.0.so.0%{libsymbolsuffix}


On a related note, I realized that python3-cairocffi also does some dlopen stuff, and doesn't indicate it's requirements properly.  That's why this package appears to need gdk-pixbuf2-devel.  I'll file a bug with that package because `python3 -c 'import cairocffi.pixbuf'` straight up doesn't work unless you have gdk-pixbuf2 installed.  As a temporary fix we can use these lines.

+# missing from python3-cairocffi
+BuildRequires: libgdk_pixbuf-2.0.so.0%{libsymbolsuffix}
+BuildRequires: libglib-2.0.so.0%{libsymbolsuffix}
+BuildRequires: libgdk-3.so.0%{libsymbolsuffix}

+# missing from python3-cairocffi
+Requires: libgdk_pixbuf-2.0.so.0%{libsymbolsuffix}
+Requires: libglib-2.0.so.0%{libsymbolsuffix}
+Requires: libgdk-3.so.0%{libsymbolsuffix}



Since this package doesn't have any subpackages, it's not necessary to define the description in a separate %_description macro.  That's mentioned in the guidelines because usually Python packages need to repeat the same description in the top level %description section and in the python3 subpackage %description section.  That's not the case here.


I see you added the desktop-file-validate commands to %check.  That's a good improvement, but instead of running separate install (in %install) and desktop-file-validate (in %check) commands, we could just run desktop-file-install (in %install) instead and do the installation and validation at the same time.



The upstream setup.cfg file defines the pytest testpaths as "test", so I don't think we need to pass that to %pytest.

-%pytest test


It's not necessary to use `-n qtile` when starting the %files section.  Our Name field is qtile, and %files defaults to that.  That flag is only needed when you need to define it as something else.

-%files -n qtile -f %{pyproject_files}
+%files -f %{pyproject_files}


We can remove the license line outright, because the pyproject macros pick up what the upstream has defined as the license automatically and marks it with the proper RPM metadata.  You can verify that on a built RPM file with the command `rpm -qpL`, which is currently showing me both /usr/lib/python3.11/site-packages/qtile-0.22.1.dist-info/LICENSE and /usr/share/licenses/qtile/LICENSE as license files in the package.  These files are identical so we can skip the non-automatic one.

-%license LICENSE


Not a correction but just a curiosity on my part, what changes when enabling wayland support to switch the package from noarch to being architecture specific?

Comment 18 Carl George 🤠 2022-12-23 21:09:05 UTC
Another thing I found in the upstream docs was that only the x11 backend is tested by default.  To test both we'll need to pass some additional flags to pytest, and we can make the wayland one conditional.

-%pytest test
+%pytest --backend x11 %{?with_wayland:--backend wayland}



Similarly we can pull in the "wayland" extras requirements and not have to specify them manually.

-%if %{with wayland}
-# These packages are not in Fedora yet, however they are packaged in Copr
-# https://copr.fedorainfracloud.org/coprs/frostyx/qtile/
-# So let's temporarily build the official Fedora package without Wayland support
-# but build the Copr package with Wayland support.
-BuildRequires:  python3-pywlroots
-BuildRequires:  python3-pywayland
-BuildRequires:  python3-xkbcommon

+%pyproject_buildrequires %{?with_wayland:-x wayland}

That covers the buildrequires for those, but not the runtime requires.  For that I think the best approach may be to create some subpackages.

* split the libqtile Python library into a python3-libqtile subpackage
* use %pyproject_extras_subpkg to generate a python3-libqtile+wayland subpackage
* recommend python3-libqtile+wayland from the main qtile package


This might be getting a bit too complex for bugzilla comments.  Let me know if you'd prefer I just send a PR to your spec file on GitHub.


I sent a PR upstream to split out the "test" and "lint" dependencies.  If that looks good to you we can include it as a patch and then install the test dependencies with %pyproject_buildrequires.


Comment 19 Jakub Kadlčík 2023-01-01 23:07:57 UTC
Thank you for the feedback,
I am learning some new tricks :-)

I made almost all the requested changes, please take a look.

Spec URL: https://download.copr.fedorainfracloud.org/results/frostyx/qtile/fedora-rawhide-x86_64/05193780-qtile/qtile.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/frostyx/qtile/fedora-rawhide-x86_64/05193780-qtile/qtile-0.22.1-4.fc38.src.rpm
Diff: https://github.com/frostyx/rpmbuild-topdir/commit/9cec169

There were two problems though.

1. When I started removing the explicit BuildRequires, tests started
falling. Do you think, we are installing the generated dependencies
incorrectly? Also, just for the record, I am fine with having explicit
BuildRequires :-)

2. Wayland support requires much more work than I can allocate to it
right now. There are the missing dependencies, and also tests started 
failing for some reason. I would like to keep the Wayland-specific 
changes in a WIP state and iteratively fix them one by one. 

> Not a correction but just a curiosity on my part, what changes when
> enabling Wayland support to switch the package from noarch to being
> architecture specific? 

I am curious about this as well. Builds started failing with a
message, that I use `BuildArch: noarch` for an architecture-specific
package. So I added the condition to fix the issue, but I don't quite
understand why it started building architecture-specific RPMs.

Comment 21 Fedora Review Service 2023-11-07 21:46:27 UTC
Copr build:

Review template:

Please take a look if any issues were found.

This comment was created by the fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 22 Carl George 🤠 2023-11-10 23:22:42 UTC

Comment 23 Carl George 🤠 2023-11-10 23:53:59 UTC
That first PR is the mandatory stuff for the review.

Previously in comment 18 I mentioned implementing python3-libqtile and python3-libqtile+wayland subpackages.  This would prevent the need to manually keep the wayland dependencies in sync with upstream.  Separate from that PR, here is a commit with a possible implementation of those subpackages, as well as a qtile-wayland subpackage to make it obvious for users on how to opt-in.  If you like this, I can send it as a PR as well.  These changes are not mandatory for the review, and we could even leave them out for now until you're ready to enable the wayland conditional.


Comment 25 Fedora Review Service 2023-11-12 16:58:57 UTC
Created attachment 1998876 [details]
The .spec file difference from Copr build 6610458 to 6626809

Comment 26 Fedora Review Service 2023-11-12 16:58:59 UTC
Copr build:

Build log:

Please make sure the package builds successfully at least for Fedora Rawhide.

- If the build failed for unrelated reasons (e.g. temporary network
  unavailability), please ignore it.
- If the build failed because of missing BuildRequires, please make sure they
  are listed in the "Depends On" field

This comment was created by the fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 27 Jakub Kadlčík 2023-11-12 17:15:16 UTC
> Copr build:
> https://copr.fedorainfracloud.org/coprs/build/6626809
> (failed)

I think we can ignore the failure. The tests sometimes fail because of "AssertionError: couldn't connect to xvfb". Simply resubmitting the build usually fixes the issue. I am going to report it to the upstream.

Comment 28 Jakub Kadlčík 2023-11-12 20:26:46 UTC
> I am going to report it to the upstream.

Reported here 

and mentioned in the spec 

Comment 29 Carl George 🤠 2023-11-13 04:24:45 UTC
This is looking good.  I also figured out the wayland test errors, please see this PR:


The fedora-review output is already looking good on the last copr build.  Once that PR is merged let's do one more build and I'll run fedora-review against that and approve the package.

Comment 31 Fedora Review Service 2023-11-13 17:58:24 UTC
Created attachment 1999160 [details]
The .spec file difference from Copr build 6626809 to 6630297

Comment 32 Fedora Review Service 2023-11-13 17:58:27 UTC
Copr build:

Review template:

Please take a look if any issues were found.

This comment was created by the fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 33 Carl George 🤠 2023-11-14 03:01:49 UTC
Package is APPROVED.

Package Review

[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated

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

[x]: Development (unversioned) .so files in -devel subpackage, if present.

[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated", "MIT License", "*No copyright* MIT
     License", "GNU General Public License v3.0 or later". 227 files have
     unknown license. Detailed output of licensecheck in
[x]: License file installed when any subpackage combination is installed.
[x]: If the package is under multiple licenses, the licensing breakdown
     must be documented in the spec.
[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
[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.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[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]: The License field must be a valid SPDX expression.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[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]: Package installs a %{name}.desktop using desktop-file-install or
     desktop-file-validate if there is such a file.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
[x]: Package is named using only allowed ASCII characters.
[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
[x]: File names are valid UTF-8.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 3707 bytes in 1 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

[-]: Binary eggs must be removed in %prep
[x]: Python eggs must not download any dependencies during the build
[-]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Packages MUST NOT have dependencies (either build-time or runtime) on
     packages named with the unversioned python- prefix unless no properly
     versioned package exists. Dependencies on Python packages instead MUST
     use names beginning with python2- or python3- as appropriate.
[x]: Python packages must not contain %{pythonX_site(lib|arch)}/* in %files

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

[-]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
     python3-libqtile , qtile-wayland , python3-libqtile+wayland
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[x]: Package should compile and build into binary rpms on all supported
[x]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
[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
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

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

[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
[x]: Spec file according to URL is the same as in SRPM.

Comment 34 Jakub Kadlčík 2023-11-14 08:35:31 UTC
Thank you very much for the review Carl George <3

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