This package built on koji:  https://koji.fedoraproject.org/koji/taskinfo?taskID=109584280

Taking this review.

I would strongly recommend subpackaging out the X11 and Wayland files. Among other things, this also makes it much easier for you to declare the runtime dependencies for X11 and Wayland Gala properly.

For example, you're currently missing the required "xorg-x11-server-Xorg" runtime dependency for gala-x11, and "xorg-x11-server-Xwayland" runtime dependency for gala-wayland.

You are also missing a firstboot Provides for gala itself.

It'll look something like this:

> # http://bugzilla.redhat.com/605675
> Provides: firstboot(windowmanager) = gala

Copr build:
https://copr.fedorainfracloud.org/coprs/build/6695893
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2251581-gala/fedora-rawhide-x86_64/06695893-gala/fedora-review/review.txt

Found issues:

- A package with this name already exists. Please check https://src.fedoraproject.org/rpms/gala
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#_conflicting_package_names

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.

(In reply to Neal Gompa from comment #3)
> I would strongly recommend subpackaging out the X11 and Wayland files. Among
> other things, this also makes it much easier for you to declare the runtime
> dependencies for X11 and Wayland Gala properly.
> 
> For example, you're currently missing the required "xorg-x11-server-Xorg"
> runtime dependency for gala-x11, and "xorg-x11-server-Xwayland" runtime
> dependency for gala-wayland.

To be honest, I'd rather remove the wayland stuff entirely than do something more complicated here.
The wayland support is still rudimentary / experimental, and won't be in a shippable state for some time.

(In reply to Neal Gompa from comment #4)
> You are also missing a firstboot Provides for gala itself.
> 
> It'll look something like this:
> 
> > # http://bugzilla.redhat.com/605675
> > Provides: firstboot(windowmanager) = gala

What the hell is this? I've never seen anything like this.
I don't think gala provides what is necessary for whatever "firstboot" is, and the LightDM greeter for Pantheon uses a different compositor entirely.

(In reply to Fabio Valentini from comment #6)
> (In reply to Neal Gompa from comment #3)
> > I would strongly recommend subpackaging out the X11 and Wayland files. Among
> > other things, this also makes it much easier for you to declare the runtime
> > dependencies for X11 and Wayland Gala properly.
> > 
> > For example, you're currently missing the required "xorg-x11-server-Xorg"
> > runtime dependency for gala-x11, and "xorg-x11-server-Xwayland" runtime
> > dependency for gala-wayland.
> 
> To be honest, I'd rather remove the wayland stuff entirely than do something
> more complicated here.
> The wayland support is still rudimentary / experimental, and won't be in a
> shippable state for some time.
> 

I would prefer the Wayland stuff to be shipped as a package, even if it's not used by default yet. Subpackaging it also makes it easier to not include in a spin image if it's not ready yet.

(In reply to Fabio Valentini from comment #6)
> (In reply to Neal Gompa from comment #4)
> > You are also missing a firstboot Provides for gala itself.
> > 
> > It'll look something like this:
> > 
> > > # http://bugzilla.redhat.com/605675
> > > Provides: firstboot(windowmanager) = gala
> 
> What the hell is this? I've never seen anything like this.
> I don't think gala provides what is necessary for whatever "firstboot" is,
> and the LightDM greeter for Pantheon uses a different compositor entirely.

It's required mostly by initial-setup to use the correct window manager. Strictly speaking, it's not necessary if you've got your own firstboot/initial-setup system to use instead of Anaconda's.

Spec URL: https://decathorpe.fedorapeople.org/gala.spec
SRPM URL: https://decathorpe.fedorapeople.org/gala-7.1.3-1.fc39.src.rpm

Changes:

- x11 subpackage: contains systemd user session unit for gala on X11
- wayland subpackage: contains systemd user session unit for gala on Wayland

Both get a dependency on gnome-session (which is a hard dependency for the systemd units) and on xorg-x11-server-Xorg / xorg-x11-server-Xwayland, respectively.

Note that the session definitions I'm currently using don't use systemd user session support (yet), so in fact, *both* subpackages are unused.

In your noarch subpackages, I see this:

> Requires:       %{name}%{?_isa} = %{version}-%{release}

You need to drop "%{?_isa}" because it will not be properly installable otherwise.

(In reply to Neal Gompa from comment #9)
> In your noarch subpackages, I see this:
> 
> > Requires:       %{name}%{?_isa} = %{version}-%{release}
> 
> You need to drop "%{?_isa}" because it will not be properly installable
> otherwise.

Good point. Local build installed fine, but there's no guarantee that noarch packages will always match arch ...

Files updated.

Review notes:

* Package follows Fedora Packaging Guidelines
* Package builds and installs
* Package licensing is correctly handled
* No serious issues from rpmlint

Note: the devel package has /usr/share/vala and /usr/share/vala/vapi without owners. You should co-own it for now and ask for the vala maintainer to create a -filesystem subpackage that packages can depend on for those directories.

PACKAGE APPROVED.

Thank you for the review!

I will make the package co-own /usr/share/vala and /usr/share/vala/vapi for now.

https://pagure.io/releng/issue/11818

Imported and built:
https://bodhi.fedoraproject.org/updates/FEDORA-2023-ad4be625e8