Bug 2260793 - Review Request: kwin-x11 - KDE Window manager with X11 support
Summary: Review Request: kwin-x11 - KDE Window manager with X11 support
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Sergio Basto
QA Contact: Fedora Extras Quality Assurance
URL: https://userbase.kde.org/KWin
Whiteboard:
Depends On:
Blocks: 2260798
TreeView+ depends on / blocked
 
Reported: 2024-01-28 23:35 UTC by Kevin Kofler
Modified: 2024-03-04 11:28 UTC (History)
9 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2024-03-04 11:28:36 UTC
Type: ---
Embargoed:
sergio: fedora-review+


Attachments (Terms of Use)
The .spec file difference from Copr build 6969124 to 6981267 (1.15 KB, patch)
2024-02-02 03:48 UTC, Fedora Review Service
no flags Details | Diff

Description Kevin Kofler 2024-01-28 23:35:15 UTC
Spec URL: https://repo.calcforge.org/review/kwin-x11.spec
SRPM URL: https://repo.calcforge.org/review/kwin-x11-5.92.0-1.fc40.src.rpm
Description: KDE Window manager with X11 support.
Fedora Account System Username: kkofler

Comment 1 Kevin Kofler 2024-01-28 23:37:04 UTC
This is part one of "unbreak KDE". Sergio, if you want X11 support restored ASAP, then please review this package.

Comment 2 Kevin Kofler 2024-01-28 23:40:43 UTC
Successful Rawhide scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=112520293

Comment 3 Fedora Review Service 2024-01-29 00:20:45 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6969124
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2260793-kwin-x11/fedora-rawhide-x86_64/06969124-kwin-x11/fedora-review/review.txt

Found issues:

- No gcc, gcc-c++ or clang found in BuildRequires
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/
- Systemd user unit service file(s) in kwin-x11
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_user_units

Please know that there can be false-positives.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/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 4 Kevin Kofler 2024-01-29 00:30:43 UTC
The BuildRequires are copied from kwin. The Qt/KDE -devel files drag in gcc-c++ transitively, but I suppose I could add it explicitly.

The systemd user units are shipped the same way (without scriptlets) in the current kwin-wayland and (pre-Rawhide) kwin-x11 subpackages. I am not sure whether it is a good idea to add the scriptlets or not.

Comment 5 Kevin Kofler 2024-01-29 00:33:45 UTC
By the way, I think I could try to use:
%cmake_build --target kwin_x11
(instead of plan %cmake_build) to speed up the build. (I am already filtering what I install in the install step, but in the build step, I build everything so far, except the plugins (decorations and KCMs) that can be disabled through CMake flags.) I am currently trying the equivalent for my plasma-workspace-x11 package.

Comment 6 Sergio Basto 2024-01-29 01:42:22 UTC
is not just set %bcond to 1 on kwin.spec [1] and plasma-workspace.spec [2] ?


[1]
https://src.fedoraproject.org/rpms/kwin/blob/rawhide/f/kwin.spec#_2

[2]
https://src.fedoraproject.org/rpms/plasma-workspace/blob/rawhide/f/plasma-workspace.spec#_2

Comment 7 Kevin Kofler 2024-01-29 01:56:12 UTC
If I just take the spec and set the %bcond to 1, I am going to build the whole thing and conflict with the main kwin package, which is obviously not what we want here. The goal of this package is to build, install, and package only the kwin-x11 package and leave all the other kwin* packages alone.

Of course, if you can get the maintainers of the main kwin package to simply reenable the x11 subpackage, we would not need this package to begin with. But they very stubbornly refuse to do that, so a separate package appears to be the only solution.

Comment 8 Kevin Kofler 2024-01-29 01:58:06 UTC
There is no **technical** reason why this package needs to be separate. It is only necessary to work around a purely political veto.

Comment 9 Fabio Valentini 2024-01-29 09:02:22 UTC
The way this package is set up will break part of this approved Change:
https://fedoraproject.org/wiki/Changes/KDE_Plasma_6
https://pagure.io/fesco/issue/3086

The scope of the proposal lists:

> * Ensure kwin-x11 is obsoleted by kwin-wayland
> * Ensure plasma-workspace-x11 is obsoleted by plasma-workspace-wayland

If this package is reintroduced, it would break part of the Change (due to using a higher Epoch), and make the associated release notes incorrect.

So I don't think introducing this package as-is is OK, *unless* it would be approved by FESCo *and* the Change owners, since it reverts part of their work.

Comment 10 Kevin Kofler 2024-01-29 16:18:21 UTC
As I understand it, since the package in Fedora <= 39 does NOT include the Epoch, the Obsoletes in kwin-wayland (and likewise for plasma-workspace) will still apply on the initial upgrade to Fedora 40. What the Epoch prevents is having the Obsoletes trigger again and again each and every time KWin is updated in Fedora 40, which I hope you agree would not be an acceptable user experience. If the user has opted in to sticking with X11, the choice should be retained on bugfix/security updates. The explicitly installed package must not be removed by regular package updates.

That said, I object to the decision to forcefully remove kwin-x11 and plasma-workspace-x11 on upgrades to Fedora 40 to begin with. IMHO, it is only OK to do something like that if the package is really dropped, which is only OK to do if nobody is willing to maintain it, which is clearly not the case here. But introducing this package does not change this decision, the Obsoletes should still apply on the initial upgrade to Fedora 40 (unless DNF behaves differently from how YUM/DNF used to behave with Obsoletes).

Comment 11 Kevin Kofler 2024-01-29 16:40:50 UTC
I can bring up 2 test Coprs so that you can test DNF behavior:

Copr 1 would have:

testobsoletes-x11 1.2.3-1.fc39 with no Epoch and no Obsoletes
testobsoletes-wayland 1.2.3-1.fc39 with no Epoch and no Obsoletes

Copr 2 would have:

testobsoletes-x11 1:1.2.3-1.fc40 (with no Obsoletes)
testobsoletes-wayland 1.2.3-1.fc40 with Obsoletes: testobsoletes-x11 < 1.2.3-1.fc39 (with no Epoch)

Then first install testobsoletes-x11, with or without testobsoletes-wayland (both should behave the same), from Copr 1, then enable Copr 2 and "dnf upgrade". The way I remember DNF's behavior, you should end up with testobsoletes-x11 removed rather than upgraded.

Shall I create those test Coprs?

And if the intent of the KDE SIG is to block this package based on the Epoch issue only, I can also submit the 2 packages with -X11 suffixes with a capital X (and with no Obsoletes), e.g., kwin-X11, then they will be considered completely different packages by RPM and DNF, and the discussion will be moot. But I do not think this is actually necessary, as explained above.

Comment 12 Kevin Kofler 2024-01-29 16:44:36 UTC
Correction:

> testobsoletes-wayland 1.2.3-1.fc40 with Obsoletes: testobsoletes-x11 < 1.2.3-1.fc39 (with no Epoch)

should read:

testobsoletes-wayland 1.2.3-1.fc40 with Obsoletes: testobsoletes-x11 < 1.2.3-1.fc40 (with no Epoch)

("< 1.2.3-1.fc39" is not going to match the package in Copr 1, would have to be "<=", but the "fc39" part was a typo to begin with.)

Comment 13 Kevin Kofler 2024-01-29 18:10:25 UTC
The Coprs are now set up:
https://copr.fedorainfracloud.org/coprs/kkofler/testobsoletes-1/
https://copr.fedorainfracloud.org/coprs/kkofler/testobsoletes-2/
and DNF behaves pretty much exactly as I expected:

[root@desktop64 ~]# dnf copr enable kkofler/testobsoletes-1
Enabling a Copr repository. Please note that this repository is not part
of the main distribution, and quality may vary.

The Fedora Project does not exercise any power over the contents of
this repository beyond the rules outlined in the Copr FAQ at
<https://docs.pagure.org/copr.copr/user_documentation.html#what-i-can-build-in-copr>,
and packages are not held to any quality or security level.

Please do not file bug reports about these packages in Fedora
Bugzilla. In case of problems, contact the owner of this repository.

Do you really want to enable copr.fedorainfracloud.org/kkofler/testobsoletes-1? [y/N]: y
Die Paketquelle wurde erfolgreich aktiviert.
[root@desktop64 ~]# dnf install testobsoletes-wayland testobsoletes-x11
Copr repo for testobsoletes-1 owned by kkofler  696  B/s | 1.2 kB     00:01    
Last metadata expiration check: 0:00:01 ago on Mon 29 Jan 2024 07:00:04 PM CET.
Dependencies resolved.
================================================================================
 Package
      Arch   Version
                   Repository                                              Size
================================================================================
Installing:
 testobsoletes-wayland
      x86_64 1.2.3-1.fc39
                   copr:copr.fedorainfracloud.org:kkofler:testobsoletes-1 6.8 k
 testobsoletes-x11
      x86_64 1.2.3-1.fc39
                   copr:copr.fedorainfracloud.org:kkofler:testobsoletes-1 6.8 k

Transaction Summary
================================================================================
Install  2 Packages

Total download size: 14 k
Installed size: 0  
Is this ok [y/N]: y
Downloading Packages:
(1/2): testobsoletes-x11-1.2.3-1.fc39.x86_64.rp  31 kB/s | 6.8 kB     00:00    
(2/2): testobsoletes-wayland-1.2.3-1.fc39.x86_6  13 kB/s | 6.8 kB     00:00    
--------------------------------------------------------------------------------
Total                                            27 kB/s |  14 kB     00:00     
Copr repo for testobsoletes-1 owned by kkofler  3.3 kB/s | 1.0 kB     00:00    
Importing GPG key 0x523C8383:
 Userid     : "kkofler_testobsoletes-1 (None) <kkofler#testobsoletes-1.org>"
 Fingerprint: 9021 E66C 8A69 D581 5FD5 F971 05BF 435F 523C 8383
 From       : https://download.copr.fedorainfracloud.org/results/kkofler/testobsoletes-1/pubkey.gpg
Is this ok [y/N]: y
Key imported successfully
Running transaction check
Transaction check succeeded.
Running transaction test
Transaction test succeeded.
Running transaction
  Preparing        :                                                        1/1 
  Installing       : testobsoletes-x11-1.2.3-1.fc39.x86_64                  1/2 
  Installing       : testobsoletes-wayland-1.2.3-1.fc39.x86_64              2/2 
  Verifying        : testobsoletes-wayland-1.2.3-1.fc39.x86_64              1/2 
  Verifying        : testobsoletes-x11-1.2.3-1.fc39.x86_64                  2/2 

Installed:
  testobsoletes-wayland-1.2.3-1.fc39.x86_64                                     
  testobsoletes-x11-1.2.3-1.fc39.x86_64                                         

Complete!
[root@desktop64 ~]# dnf copr enable kkofler/testobsoletes-2
Enabling a Copr repository. Please note that this repository is not part
of the main distribution, and quality may vary.

The Fedora Project does not exercise any power over the contents of
this repository beyond the rules outlined in the Copr FAQ at
<https://docs.pagure.org/copr.copr/user_documentation.html#what-i-can-build-in-copr>,
and packages are not held to any quality or security level.

Please do not file bug reports about these packages in Fedora
Bugzilla. In case of problems, contact the owner of this repository.

Do you really want to enable copr.fedorainfracloud.org/kkofler/testobsoletes-2? [y/N]: y
Repository successfully enabled.
[root@desktop64 ~]# dnf upgrade
Copr repo for testobsoletes-2 owned by kkofler  3.1 kB/s | 1.4 kB     00:00    
Dependencies resolved.
================================================================================
 Package
      Arch   Version
                   Repository                                              Size
================================================================================
Upgrading:
 testobsoletes-wayland
      x86_64 1.2.3-1.fc40
                   copr:copr.fedorainfracloud.org:kkofler:testobsoletes-2 6.9 k
     replacing  testobsoletes-x11.x86_64 1.2.3-1.fc39

Transaction Summary
================================================================================
Upgrade  1 Package

Total download size: 6.9 k
Is this ok [y/N]: y
Downloading Packages:
testobsoletes-wayland-1.2.3-1.fc40.x86_64.rpm    31 kB/s | 6.9 kB     00:00    
--------------------------------------------------------------------------------
Total                                            30 kB/s | 6.9 kB     00:00     
Copr repo for testobsoletes-2 owned by kkofler  1.8 kB/s | 1.0 kB     00:00    
Importing GPG key 0x8B1D5620:
 Userid     : "kkofler_testobsoletes-2 (None) <kkofler#testobsoletes-2.org>"
 Fingerprint: 3470 88B7 EE33 014E 2EA9 629A 6A7D D82C 8B1D 5620
 From       : https://download.copr.fedorainfracloud.org/results/kkofler/testobsoletes-2/pubkey.gpg
Is this ok [y/N]: y
Key imported successfully
Running transaction check
Transaction check succeeded.
Running transaction test
Transaction test succeeded.
Running transaction
  Preparing        :                                                        1/1 
  Upgrading        : testobsoletes-wayland-1.2.3-1.fc40.x86_64              1/3 
  Obsoleting       : testobsoletes-x11-1.2.3-1.fc39.x86_64                  2/3 
  Cleanup          : testobsoletes-wayland-1.2.3-1.fc39.x86_64              3/3 
  Verifying        : testobsoletes-wayland-1.2.3-1.fc40.x86_64              1/3 
  Verifying        : testobsoletes-wayland-1.2.3-1.fc39.x86_64              2/3 
  Verifying        : testobsoletes-x11-1.2.3-1.fc39.x86_64                  3/3 

Upgraded:
  testobsoletes-wayland-1.2.3-1.fc40.x86_64                                     

Complete!
[root@desktop64 ~]# rpm -q testobsoletes-x11
package testobsoletes-x11 is not installed

So this proves that this package does NOT interfere with the quoted parts of the Change Proposal:
> * Ensure kwin-x11 is obsoleted by kwin-wayland
> * Ensure plasma-workspace-x11 is obsoleted by plasma-workspace-wayland

Comment 14 Kevin Kofler 2024-01-29 19:15:03 UTC
FYI, I tested this again with "dnf copr disable kkofler/testobsoletes-1" after "dnf copr enable kkofler/testobsoletes-2", which is a more realistic situation (since the F39 repo will be disabled during the upgrade to F40). The behavior is exactly the same, testobsoletes-x11 gets removed, not upgraded.

The objection is now disproven, so can we please move on with the review?

Comment 15 Stephen Gallagher 2024-01-29 20:37:39 UTC
At today's meeting, FESCo agreed to a preliminary injunction while we consider this issue. Until otherwise notified, these packages (kwin-X11 and plasma-workspace-x11) may not be re-admitted to Fedora. This decision is NOT final, but FESCo needs time to consider what to do here.

Comment 16 Stephen Gallagher 2024-01-29 20:44:41 UTC
Link to the FESCo ticket: https://pagure.io/fesco/issue/3165

Comment 17 Kevin Kofler 2024-01-29 23:55:23 UTC
I count only 4 FESCo members having explicitly agreed to that preliminary injunction in the meeting log, which is not enough for it to pass.

Comment 18 Kevin Kofler 2024-01-29 23:56:19 UTC
(4 including the submitter. FESCo has 9 members, hence 5 members are needed for an absolute majority.)

Comment 19 Kevin Kofler 2024-01-30 16:44:58 UTC
The missing 5th +1 vote for the preliminary injunction was now added in the ticket:
https://pagure.io/fesco/issue/3165#comment-893932

Comment 20 Kevin Kofler 2024-02-02 03:04:50 UTC
(In reply to Kevin Kofler from comment #5)
> By the way, I think I could try to use:
> %cmake_build --target kwin_x11
> (instead of plan %cmake_build) to speed up the build. (I am already
> filtering what I install in the install step, but in the build step, I build
> everything so far, except the plugins (decorations and KCMs) that can be
> disabled through CMake flags.) I am currently trying the equivalent for my
> plasma-workspace-x11 package.

This is now done. It reduces the build time by about ⅜ (i.e., slightly more than ⅓), from almost 32 minutes to only almost 20 minutes. This is not as much as for plasma-workspace where the startkde stack that we need is only a small part of the build and can be built in about 5 minutes (because kwin_x11 actually depends on most of the KWin code, unless I can somehow get it to build against an installed libkwin), but it is still a win.

Updated submission:
Spec URL: https://repo.calcforge.org/review/kwin-x11.spec
SRPM URL: https://repo.calcforge.org/review/kwin-x11-5.92.0-2.fc40.src.rpm
Description: KDE Window manager with X11 support.
Fedora Account System Username: kkofler

Comment 21 Kevin Kofler 2024-02-02 03:06:01 UTC
Successful Rawhide scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=112776667

Comment 22 Fedora Review Service 2024-02-02 03:48:46 UTC
Created attachment 2014519 [details]
The .spec file difference from Copr build 6969124 to 6981267

Comment 23 Fedora Review Service 2024-02-02 03:48:49 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6981267
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2260793-kwin-x11/fedora-rawhide-x86_64/06981267-kwin-x11/fedora-review/review.txt

Found issues:

- No gcc, gcc-c++ or clang found in BuildRequires
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/
- Systemd user unit service file(s) in kwin-x11
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_user_units

Please know that there can be false-positives.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/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 24 Petr Menšík 2024-02-07 12:43:36 UTC
It seems to me %description should be somehow expanded. First, KDE-SIG folks want to make it clear they maintain kwin, but not kwin-x11. Current %description is *very* poor, if it is considered important building block for the second most used desktop environment. I think it should be clear this package is an alternative version of more preferred variant.

I think both original kwin and also this kwin-x11 should expand %description to be more useful and provide more details about what it is for.

Comment 25 Kevin Kofler 2024-02-07 14:15:04 UTC
Something like this?

%description
Alternative version of the KDE Window Manager (KWin) using the legacy X11 window system instead of the default Wayland. This version of KWin is required by plasma-workspace-x11, which provides the "Plasma (X11)" session type. This version is maintained by individual Fedora packagers and NOT supported by the Fedora KDE SIG. (See kwin-wayland for the default version, using Wayland, maintained by the KDE SIG.)

and I can write up something similar for plasma-workspace-x11, which is the package most users will actually want to install. (kwin-x11 is only half of the deal, unless you want to use a semi-supported KWin-only setup, which is a niche thing few people actually want.)

Comment 26 Kevin Kofler 2024-02-07 14:15:31 UTC
(plasma-workspace-x11 requires kwin-x11, not the other way round. It has always been like that.)

Comment 27 Kevin Kofler 2024-02-07 14:21:56 UTC
See https://bugzilla.redhat.com/show_bug.cgi?id=2260798#c8 for the plasma-workspace-x11 %description.

Comment 28 Petr Menšík 2024-02-07 18:05:24 UTC
(In reply to Kevin Kofler from comment #25)
> Something like this?
> 
> %description
> Alternative version of the KDE Window Manager (KWin) using the legacy X11
> window system instead of the default Wayland. This version of KWin is
> required by plasma-workspace-x11, which provides the "Plasma (X11)" session
> type. This version is maintained by individual Fedora packagers and NOT
> supported by the Fedora KDE SIG. (See kwin-wayland for the default version,
> using Wayland, maintained by the KDE SIG.)
> 
> and I can write up something similar for plasma-workspace-x11, which is the
> package most users will actually want to install. (kwin-x11 is only half of
> the deal, unless you want to use a semi-supported KWin-only setup, which is
> a niche thing few people actually want.)

I would start just with what is kwin for. But yes, something like that is much better.
Taken from upstream link, first paragraph. It would omit dependency name, rpm requires says that.
But relation to user visible session type should be included. My idea would be more like:

%description
KWin is the window manager for the KDE Plasma Desktop. It gives you complete
control over your windows, making sure they're not in the way but aid you in
your task. It paints the window decoration, the bar on top of every window
with (configurable) buttons like close, maximize and minimize. It also handles
placing of windows and switching between them.

Alternative to the kwin-wayland maintained by the KDE SIG. This package is
using the legacy X11 window system instead of the default Wayland. It is
required for the "Plasma (X11)" session type. This version is maintained
by individual Fedora packagers and NOT supported by the Fedora KDE SIG.

Comment 29 Josh Stone 2024-02-12 21:10:34 UTC
I want to highlight one suggestion that doesn't appear to have gotten a response on the list:

https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/H43OUABJIIMI24NDB7SM7FHLLLOV4TSL/
> Can we make the x11 packages be named explicitly as compat packages (e.g. prefixed with compat-)

If the new packages have a different name, with no upgrade path from the old (obsoleted) packages, then this will eliminate the uncertainty about epochs and such.

(I appreciate that you tested that upgrade in COPR, but it still makes me uneasy as a technical solution.)

... and I just found this in comment #11:
> I can also submit the 2 packages with -X11 suffixes with a capital X (and with no Obsoletes), e.g., kwin-X11, then they will be considered completely different packages by RPM and DNF, and the discussion will be moot.

I think a mere capital change is too close and will be confusing to users, but otherwise I think this is a good direction.

Comment 30 Kevin Kofler 2024-02-13 01:46:15 UTC
The use of the compat- namespace is pretty clearly documented in Fedora. It is for old sonames of libraries needed to keep applications running. And even there, it is mostly deprecated in favor of just using a version number suffix. Using this for X11 support packages for the latest version of Plasma available in Fedora (possibly even the latest upstream version) would be really confusing to users.

Comment 31 Kevin Kofler 2024-02-13 01:48:37 UTC
(In reply to Stephen Gallagher from comment #15)
> At today's meeting, FESCo agreed to a preliminary injunction while we
> consider this issue. Until otherwise notified, these packages (kwin-X11 and
> plasma-workspace-x11) may not be re-admitted to Fedora. This decision is NOT
> final, but FESCo needs time to consider what to do here.

As I understand it, the FESCo hold was lifted today:

https://pagure.io/fesco/issue/3165#comment-895243

> At today's FESCo meeting, we agreed on the following proposal:
> 
> * AGREED: KDE packages which reintroduce support for X11 are
> allowed in the main Fedora repositories, however they may not be
> included by default on any release-blocking deliverable (ISO, image,
> etc.). The KDE SIG should provide a notice before major changes, but
> is not responsible for ensuring that these packages adapt. Upgrades
> from F38 and F39 will be automatically migrated to Wayland. (+5, 0,
> -1)
> 
> For additional clarification: this means that all users performing upgrades
> MUST be migrated to the Wayland session. They then MAY opt-in to the X11
> session by installing a package for that purpose. We are explicitly not
> providing detailed technical implementation requirements here, but we expect
> all parties to follow the spirit of this decision when making technical
> decisions.

so we can now proceed with this review request.

Comment 32 Kevin Kofler 2024-02-13 01:53:32 UTC
Oh, and:

(In reply to Josh Stone from comment #29)
> (I appreciate that you tested that upgrade in COPR, but it still makes me
> uneasy as a technical solution.)

Upgrades will be routinely tested as part of the release blocker testing process, so we should hopefully notice if people are still stuck with leftover -x11 packages.

Comment 33 Kevin Kofler 2024-02-13 06:49:49 UTC
I would also like to point out again that, as I have already explained (see comment #10), the Epoch: 1 is absolutely necessary to prevent the package from getting obsoleted again and again each time kwin is updated in Fedora 40 (and the same goes for plasma-workspace) and that, as proven above (see comment #13), it does NOT prevent the Obsoletes from kicking in when upgrading from Fedora 38 or 39 to Fedora 40. So the demand to remove the "Epoch: 1" is unreasonable and impossible to satisfy.

As for the Provides: deprecated(), why would it not be up to me as the maintainer to decide whether my package should be deprecated or not? I actively maintain this and see no reason for it to be marked as deprecated. (That said, since this is not going to impact end users, this is at least negotiable. The Epoch is not.)

Comment 34 Stephen Gallagher 2024-02-13 13:06:57 UTC
(In reply to Kevin Kofler from comment #33)
> As for the Provides: deprecated(), why would it not be up to me as the
> maintainer to decide whether my package should be deprecated or not? I
> actively maintain this and see no reason for it to be marked as deprecated.
> (That said, since this is not going to impact end users, this is at least
> negotiable. The Epoch is not.)


The purpose of the `deprecated()` virtual Provides: is to indicate to other packagers that new packages depending on this package are not allowed to enter the distribution (without a dispensation). For example, we don't allow new packages with a dependency on OpenSSL 1.1 and indicate this with the `deprecated()` Provides. It wasn't explicitly discussed in the meeting (and could be raised for official clarification if you desire), but my general feeling was that we were deciding to allow X11 support as roughly equivalent to a compatibility package. So I think the `deprecated()` indicator is sensible here, because we don't necessarily want an entire ecosystem growing out of these two packages.

Comment 35 Kevin Kofler 2024-02-13 13:24:06 UTC
I can readd the Provides: deprecated(), but obviously with the exception that plasma-workspace-x11, which is technically a "new package entering Fedora", is allowed to require kwin-x11, as it did when those were both still subpackages.

I agree that other packages should probably not require kwin-x11. At least as long as it uses the shared libkwin. But if KDE upstream goes ahead with their plans to fork libkwin into 2 separate versions for Wayland and for X11, then people may want to build window decorations against both kwin-wayland and kwin-x11, obviously in separate subpackages with separate Requires.

Why I wanted to remove the Provides: deprecated() is because its purpose is normally to be put on a package that is planned to be dropped from the distribution some time soon (to avoid new dependencies creeping in, slowing down the phaseout). But I am not planning to drop kwin-x11 any time soon.

But if someone wants to submit an application that Requires: kwin-x11 for no good reason, I agree that that would be a very bad idea. (The application should not Require any specific window manager, let alone a specific build of KWin.) So if that is your worry, we can leave the Provides: deprecated() in, as I said with an exception for plasma-workspace-x11.

Comment 36 Stephen Gallagher 2024-02-13 13:28:57 UTC
(In reply to Kevin Kofler from comment #35)
> But if someone wants to submit an application that Requires: kwin-x11 for no
> good reason, I agree that that would be a very bad idea. (The application
> should not Require any specific window manager, let alone a specific build
> of KWin.) So if that is your worry, we can leave the Provides: deprecated()
> in, as I said with an exception for plasma-workspace-x11.

That would work for me. I think we can reasonably assume that the FESCo decision includes a dispensation for plasma-workspace-x11 depending on kwin-x11.

Comment 37 Kevin Kofler 2024-02-13 13:39:50 UTC
As for plasma-workspace-x11, I agree that probably nothing at all should have a Requires: plasma-workspace-x11. Currently, already nothing has that. The only case where I can see it could possibly make sense is if some plasmoid or KCM (either a new one or one split out from the Plasma releases by upstream) specifically only works in an X11 session, but even then we can do without the Requires, or it might even make more sense to just build and ship the component as part of plasma-workspace-x11.

Comment 38 Sergio Basto 2024-02-13 14:39:11 UTC
(In reply to Kevin Kofler from comment #33)
> I would also like to point out again that, as I have already explained (see
> comment #10), the Epoch: 1 is absolutely necessary to prevent the package
> from getting obsoleted again and again each time kwin is updated in Fedora
> 40 (and the same goes for plasma-workspace) and that, as proven above (see
> comment #13), it does NOT prevent the Obsoletes from kicking in when
> upgrading from Fedora 38 or 39 to Fedora 40. So the demand to remove the
> "Epoch: 1" is unreasonable and impossible to satisfy.


of course they have to remove from kwin.spec, the lines that Obsoletes kwin-x11  

https://src.fedoraproject.org/rpms/kwin/blob/rawhide/f/kwin.spec#_148
%if ! %{with x11}	
# Obsolete kwin-x11 as we are dropping the package	
Obsoletes:      %{name}-x11 < %{version}-%{release}
Conflicts:      %{name}-x11 < %{version}-%{release}
%endif

> As for the Provides: deprecated(), 

I also don't see why we should mark a package as deprecated when just one group of people (KDE SIG) mark as deprecated and we don't agree with it

Comment 39 Kevin Kofler 2024-02-13 14:50:50 UTC
> of course they have to remove from kwin.spec, the lines that Obsoletes kwin-x11

I would like them to do that, but alas that is not what FESCo has decided. The ruling is unfortunately pretty clear that the Obsoletes must trigger on the initial upgrade to Fedora 40.

The best we can get is the Epoch: 1 in kwin-x11 in Fedora >= 40, so that the above Obsoletes will only remove the builds from Fedora <= 39, meaning that the Obsoletes will ONLY trigger on the initial upgrade to Fedora 40 and not on every KWin update in Fedora 40 or on upgrades from Fedora 40 to later releases.

Now, if the KDE SIG were to ever add an Epoch to the Obsoletes, then that would be a violation of the agreed compromise and I would definitely take that to FESCo. But as long as the Obsoletes remains without Epoch, this all complies with what FESCo has ruled, so there is nothing to complain about.

> I also don't see why we should mark a package as deprecated when just one group of people (KDE SIG) mark as deprecated and we don't agree with it

I am not convinced that it makes sense either, but I do not want to waste time arguing over that, which will in the end likely have no impact on users of the package.

Comment 40 Steve Cossette 2024-02-13 15:19:54 UTC
Could you make a copr with those 2 packages (The kwin and plasma-workspace) with your current spec? I'd like to test to see how it would work with it enabled while upgrading to f40/rawhide.

Thanks!

Comment 43 Kevin Kofler 2024-02-13 18:01:53 UTC
As per https://bugzilla.redhat.com/show_bug.cgi?id=2260798#c11, the Obsoletes in *-wayland is going to be tightened so that we do not need to bump the Epoch. (They are going to obsolete only versions < 5.92.0 instead of < %{version}-%{release}, so the version 5.92.0 I package here and any newer ones are not covered anymore.) So I am going to submit specfiles without the Epoch, assuming the pull requests get merged.

Comment 44 Josh Stone 2024-02-13 18:04:43 UTC
(In reply to Kevin Kofler from comment #30)
> The use of the compat- namespace is pretty clearly documented in Fedora. It
> is for old sonames of libraries needed to keep applications running. And
> even there, it is mostly deprecated in favor of just using a version number
> suffix. Using this for X11 support packages for the latest version of Plasma
> available in Fedora (possibly even the latest upstream version) would be
> really confusing to users.

I don't care about calling it "compat" in particular. In the FESCo meeting, I
independently suggested that it could be named kwin-alt-x11, and that would be
consistent with the other "alternative" descriptions I see in comments here.

Whatever the new name is, I think this would solve most of the unease on both
sides. Plain upgrades will work as the KDE SIG desires, and their continued
Obsoletes still won't affect your new package name. I could even imagine
introducing the same new name as a metapackage in f38/f39 to opt into the
continued X11 support *before* the upgrade.

Comment 47 Kevin Kofler 2024-02-15 21:55:26 UTC
OK, I will submit new specs without the Epoch as soon as possible.

Comment 48 Sergio Basto 2024-02-16 01:13:51 UTC
from review on 12 Fev 
https://download.copr.fedorainfracloud.org/results/%40fedora-review/fedora-review-2260793-kwin-x11/fedora-40-x86_64/06981267-kwin-x11/fedora-review/review.txt


Issues:
=======
- If your application is a C or C++ application you must list a
  BuildRequires against gcc, gcc-c++ or clang.
  Note: No gcc, gcc-c++ or clang found in BuildRequires
  See: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/
- systemd_user_post is invoked in %post and systemd_user_preun in %preun
  for Systemd user units service files.
  Note: Systemd user unit service file(s) in kwin-x11
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/Scriptlets/#_user_units


(In reply to Kevin Kofler from comment #4)
> The BuildRequires are copied from kwin. The Qt/KDE -devel files drag in
> gcc-c++ transitively, but I suppose I could add it explicitly.

I don't have an opinion it is an detail which doesn't change anything . 

> The systemd user units are shipped the same way (without scriptlets) in the
> current kwin-wayland and (pre-Rawhide) kwin-x11 subpackages. I am not sure
> whether it is a good idea to add the scriptlets or not.

/usr/lib/systemd/user/plasma-kwin_x11.service is a service added user unitdir seems to me correctly 

please remove Epoch before import the package  

no more issues 

PACKAGE APPROVED

Comment 49 Kevin Kofler 2024-02-16 01:42:37 UTC
Spec URL: https://repo.calcforge.org/review/kwin-x11.spec
SRPM URL: https://repo.calcforge.org/review/kwin-x11-5.92.0-4.fc40.src.rpm
Description:
Alternative version of the KDE Window Manager (KWin) using the legacy X11 window
system instead of the default Wayland. This version of KWin is required by
plasma-workspace-x11, which provides the "Plasma (X11)" session type.

This version is maintained by individual Fedora packagers and NOT supported by
the Fedora KDE SIG. (See kwin-wayland for the default version, using Wayland,
maintained by the KDE SIG.)

Fedora Account System Username: kkofler

* Fri Feb 16 2024 Kevin Kofler <…> - 5.92.0-4
- Drop Epoch again, the Obsoletes in -wayland was tightened
- Explicitly BuildRequires: cmake and gcc-c++
- Improve %%description

This addresses the comments that have come up during review.

Comment 50 Fedora Admin user for bugzilla script actions 2024-02-17 22:32:33 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/kwin-x11


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