Bug 2120119

Summary: Review Request: waydroid - run android applications on wayland
Product: [Fedora] Fedora Reporter: Alessandro Astone <ales.astone>
Component: Package ReviewAssignee: Neal Gompa <ngompa13>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: rawhideCC: davide, decathorpe, dustymabe, ego.cordatus, iela_milfsnaked, nerijus, ngompa13, package-review, pbrobinson, robatino, s.i.v.892, yajo.sk8, zawertun
Target Milestone: ---Flags: ngompa13: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2023-01-03 01:24:51 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 2120129, 2120130    
Bug Blocks: 2157810    

Description Alessandro Astone 2022-08-21 20:32:00 UTC
Spec URL: https://pagure.io/waydroid/waydroid/raw/main/f/waydroid.spec
SRPM URL: https://pagure.io/waydroid/waydroid
Description: A container-based approach to boot a full Android system on a regular GNU/Linux system.
Fedora Account System Username: aleasto

I already contribute to fedora in the form of pull-requests and dev chat in IRC, mostly with the KDE SIG. However, this is my first package submission so I do need a sponsor.
I'm also a waydroid contributor and maintainer at https://github.com/waydroid

This package requires the following new dependencies:
https://pagure.io/waydroid/libglibutil
https://pagure.io/waydroid/libgbinder
https://pagure.io/waydroid/python-gbinder
https://pagure.io/waydroid/python-pyclip

For the best user experience (namely being able to use waydroid without ever touching the terminal and becoming root), this package should have its systemd unit automatically *enabled* and *started* on install. I understand this requires being allow-listed in /usr/lib/systemd/system-preset/

I currently provide working builds through copr: https://copr.fedorainfracloud.org/coprs/aleasto/waydroid/
For the copr purpose I currently include a systemd preset file in the waydroid rpm. This will be removed for the official package.

Comment 1 Alessandro Astone 2022-08-21 20:42:25 UTC
A note on legal:
On first launch waydroid needs to download the Android image files. These are source built from LineageOS plus some patches. LineageOS contains various projects with different permissive licenses, however it also includes FFMPEG to enable media consumption. For this reason, I believe this package is not allowed to contain a link to such image files, as per point 3 of https://fedoraproject.org/wiki/Licensing:SoftwareTypes#Emulators

In the specfile I am thus removing these links and leaving it to the user to insert the correct links when launching waydroid, as they can be found on the waydroid webpage and documentation.

Comment 2 Peter Robinson 2022-08-21 21:21:54 UTC
> This package requires the following new dependencies:
> https://pagure.io/waydroid/libglibutil
> https://pagure.io/waydroid/libgbinder
> https://pagure.io/waydroid/python-gbinder
> https://pagure.io/waydroid/python-pyclip

These need to be done as separate reviews and linked a blockers to this review as it's unusable with out them so they need to go first.

> For the best user experience (namely being able to use waydroid without ever
> touching the terminal and becoming root), this package should have its
> systemd unit automatically *enabled* and *started* on install. I understand
> this requires being allow-listed in /usr/lib/systemd/system-preset/

That's a different process that can be dealt with once it's actually in Fedora.

Comment 3 Fabio Valentini 2022-08-21 21:34:59 UTC
(In reply to Alessandro Astone from comment #1)
> A note on legal:
> On first launch waydroid needs to download the Android image files.

This sounds like waydroid will not be functional without these images?

If that is the case, it might run afoul of this section of the packaging guidelines:
https://docs.fedoraproject.org/en-US/packaging-guidelines/what-can-be-packaged/#_packages_which_are_not_useful_without_external_code

In particular:

> This also means that packages which are not functional or useful without code or packages from third-party sources are not acceptable for inclusion in Fedora.

Comment 4 Peter Robinson 2022-08-21 21:38:56 UTC
> > This also means that packages which are not functional or useful without code or packages from third-party sources are not acceptable for inclusion in Fedora.

There's other examples of this in Fedora like some of the games eg I don't believe Doom/Quake engines ship with all the game files (not 100% sure of this).

Comment 5 Alessandro Astone 2022-08-21 21:41:22 UTC
(In reply to Fabio Valentini from comment #3)
> (In reply to Alessandro Astone from comment #1)
> > A note on legal:
> > On first launch waydroid needs to download the Android image files.
> 
> This sounds like waydroid will not be functional without these images?
> 
> If that is the case, it might run afoul of this section of the packaging
> guidelines:
> https://docs.fedoraproject.org/en-US/packaging-guidelines/what-can-be-
> packaged/#_packages_which_are_not_useful_without_external_code
> 
> In particular:
> 
> > This also means that packages which are not functional or useful without code or packages from third-party sources are not acceptable for inclusion in Fedora.

How is any game console emulator allowed then?

One could also argue that you can self compile android images and configure waydroid to use those.

Comment 6 Fabio Valentini 2022-08-21 21:52:38 UTC
(In reply to Alessandro Astone from comment #5)
> How is any game console emulator allowed then?

I think the difference is that they don't need third-party (non-packaged) code *to run*, but they are able to *run* third-party code OOTB.

> One could also argue that you can self compile android images and configure
> waydroid to use those.

This is probably a high burden for most users :(

Comment 7 Alessandro Astone 2022-08-21 21:55:47 UTC
(In reply to Fabio Valentini from comment #6)
> (In reply to Alessandro Astone from comment #5)
> > How is any game console emulator allowed then?
> 
> I think the difference is that they don't need third-party (non-packaged)
> code *to run*, but they are able to *run* third-party code OOTB.
> 

What does an emulator run without rom files?
If showing a GUI telling the user to download some rom files counts, then that's exactly what waydroid does.

Comment 8 Fabio Valentini 2022-08-21 22:05:34 UTC
(In reply to Alessandro Astone from comment #7)
> What does an emulator run without rom files?
> If showing a GUI telling the user to download some rom files counts, then
> that's exactly what waydroid does.

The purpose of an emulator is to run user-supplied ROM files.
This functionality works OOTB.

The purpose of waydroid, as I understand, is to run user-supplied Android apps.
This will not work OOTB, as it first requires an additional download of third-party content (other than the actual Android app).

This might sound pedantic, but it's a possibly important difference.

Comment 9 Alessandro Astone 2022-08-21 22:13:09 UTC
(In reply to Fabio Valentini from comment #8)
> (In reply to Alessandro Astone from comment #7)
> > What does an emulator run without rom files?
> > If showing a GUI telling the user to download some rom files counts, then
> > that's exactly what waydroid does.
> 
> The purpose of an emulator is to run user-supplied ROM files.
> This functionality works OOTB.
> 
> The purpose of waydroid, as I understand, is to run user-supplied Android
> apps.
> This will not work OOTB, as it first requires an additional download of
> third-party content (other than the actual Android app).
> 
> This might sound pedantic, but it's a possibly important difference.

How about the purpose of waydroid is to boot an android system as a linux container? ^^
Waydroid is merely a python script that spawns an LXC container that uses an android system image as chroot. By default it will boot to the android homescreen. It then has some utilities to install and launch individual apps.

Comment 10 Fabio Valentini 2022-08-21 22:19:24 UTC
(In reply to Alessandro Astone from comment #9)
>
> How about the purpose of waydroid is to boot an android system as a linux
> container? ^^
> Waydroid is merely a python script that spawns an LXC container that uses an
> android system image as chroot. By default it will boot to the android
> homescreen. It then has some utilities to install and launch individual apps.

If you present it like that, it certainly sounds more acceptable ;)

I didn't want to deter you from packaging this - I think having it in Fedora would be a great addition.
I just wanted to make you aware that there are rules that *might* be interpreted in a way that could prevent waydroid from being added as an official Fedora package.

Comment 11 Alessandro Astone 2022-08-28 14:11:57 UTC
In case anyone is currently trying waydroid out there's a kernel bug with the binder driver that prevents waydroid from working on kernels 5.18.18 and up, and 5.19.2 and up.
Patch is pending merge...

Comment 12 Alessandro Astone 2022-09-03 19:05:05 UTC
(In reply to Alessandro Astone from comment #11)
> In case anyone is currently trying waydroid out there's a kernel bug with
> the binder driver that prevents waydroid from working on kernels 5.18.18 and
> up, and 5.19.2 and up.
> Patch is pending merge...

This is resolved now with kernel 5.19.6

Comment 16 Neal Gompa 2022-12-24 15:17:37 UTC
Taking this review.

Comment 17 Neal Gompa 2022-12-24 15:22:34 UTC
Initial spec review:

> %global debug_package %{nil}

Why are we doing this?

> Summary:        waydroid

This is not a valid summary

> Patch0:         setup-firealld.patch

Typo?

> Requires:       (%{name}-selinux = %{version}-%{release} if selinux-policy-targeted)

Shouldn't this be "selinux-policy-%{selinuxtype}"?

> Requires:       nftables iproute dnsmasq

Please put these on their own lines

> Recommends:     python-pyclip

This doesn't exist?

> waydroid upgrade -o >/dev/null

What happens when this fails? We can't have package transactions failing because of failed commands.

Comment 18 Alessandro Astone 2022-12-24 16:05:56 UTC
(In reply to Neal Gompa from comment #17)
> > %global debug_package %{nil}
> 
> Why are we doing this?
"""
Processing files: waydroid-debugsource-1.3.4-2.fc37.x86_64
error: Empty %files file /builddir/build/BUILD/waydroid-1.3.4/debugsourcefiles.list
RPM build errors:
    Empty %files file /builddir/build/BUILD/waydroid-1.3.4/debugsourcefiles.list
"""
> > Recommends:     python-pyclip
> 
> This doesn't exist?

This is one of the dependencies i packaged and i'm now maintaining: https://packages.fedoraproject.org/pkgs/python-pyclip/
Do you mean it should be Recommends: python3-pyclip instead or..?

> > waydroid upgrade -o >/dev/null
> 
> What happens when this fails? We can't have package transactions failing
> because of failed commands.

Right, i'll add `|| :`

Comment 19 Neal Gompa 2022-12-25 03:16:53 UTC
(In reply to Alessandro Astone from comment #18)
> (In reply to Neal Gompa from comment #17)
> > > %global debug_package %{nil}
> > 
> > Why are we doing this?
> """
> Processing files: waydroid-debugsource-1.3.4-2.fc37.x86_64
> error: Empty %files file
> /builddir/build/BUILD/waydroid-1.3.4/debugsourcefiles.list
> RPM build errors:
>     Empty %files file
> /builddir/build/BUILD/waydroid-1.3.4/debugsourcefiles.list
> """

It seems like this package should be noarch entirely? It doesn't have any arch-specific stuff.

> > > Recommends:     python-pyclip
> > 
> > This doesn't exist?
> 
> This is one of the dependencies i packaged and i'm now maintaining:
> https://packages.fedoraproject.org/pkgs/python-pyclip/
> Do you mean it should be Recommends: python3-pyclip instead or..?
> 

Yeah, it should be python3-pyclip.

Comment 21 Neal Gompa 2022-12-25 19:49:48 UTC
> Summary:        A container-based approach to boot a full Android system on a regular GNU/Linux system.

Drop the article adjective at the beginning and the period at the end.

Comment 23 Neal Gompa 2022-12-26 00:15:08 UTC
> Waydroid uses Linux namespaces to run a full Android system in a container and provide Android applications on any GNU/Linux-based platform.
> The Android system inside the container has direct access to needed hardware through LXC and the binder interface.

Text needs to be wrapped at 79 columns.

> %{_systemd_util_dir}/system-preset/95-waydroid.preset

Presets are not allowed in any package except fedora-release. You'll need to make a request for fedora-release to include it.

Cf. https://docs.fedoraproject.org/en-US/packaging-guidelines/DefaultServices/#_how_to_enable_a_service_by_default

> if [ $1 -eq 1 ] && [ -x /usr/bin/systemctl ]; then
>   /usr/bin/systemctl start waydroid-container.service || :
> fi

This should also check if the service is enabled before attempting to start it. Here's an example: https://src.fedoraproject.org/rpms/snapd/blob/rawhide/f/snapd.spec#_877-885

> %triggerin -- %{name} < 1.2.1-13
> /usr/lib/systemd/systemd-update-helper mark-restart-system-units waydroid-container.service || :

What is this about? Also, this will probably never trigger since no package entering Fedora predates this version.

Comment 24 Alessandro Astone 2022-12-26 00:55:00 UTC
(In reply to Neal Gompa from comment #23)
> > %{_systemd_util_dir}/system-preset/95-waydroid.preset
> 
> Presets are not allowed in any package except fedora-release. You'll need to
> make a request for fedora-release to include it.
> 
> Cf.
> https://docs.fedoraproject.org/en-US/packaging-guidelines/DefaultServices/
> #_how_to_enable_a_service_by_default

I'm aware. If you don't mind i'll keep it in the spec until there's no other remarks for the purpose of distributing via copr, without having to fork my git tree

> > %triggerin -- %{name} < 1.2.1-13
> > /usr/lib/systemd/systemd-update-helper mark-restart-system-units waydroid-container.service || :
> 
> What is this about? Also, this will probably never trigger since no package
> entering Fedora predates this version.

I had to get creative to restart the service when upgrading from an older release that did not include %systemd_postun_with_restart, to avoid an issue with having rewritten the whole sepolicy. Since then I've also added %systemd_postun_with_restart so yes, this will serve no purpose if you first install the package from @fedora

Comment 25 Neal Gompa 2022-12-26 01:06:32 UTC
(In reply to Alessandro Astone from comment #24)
> (In reply to Neal Gompa from comment #23)
> > > %{_systemd_util_dir}/system-preset/95-waydroid.preset
> > 
> > Presets are not allowed in any package except fedora-release. You'll need to
> > make a request for fedora-release to include it.
> > 
> > Cf.
> > https://docs.fedoraproject.org/en-US/packaging-guidelines/DefaultServices/
> > #_how_to_enable_a_service_by_default
> 
> I'm aware. If you don't mind i'll keep it in the spec until there's no other
> remarks for the purpose of distributing via copr, without having to fork my
> git tree
> 

I cannot approve a package that violates the policy in this manner, so you'll need to address it soon.

> > > %triggerin -- %{name} < 1.2.1-13
> > > /usr/lib/systemd/systemd-update-helper mark-restart-system-units waydroid-container.service || :
> > 
> > What is this about? Also, this will probably never trigger since no package
> > entering Fedora predates this version.
> 
> I had to get creative to restart the service when upgrading from an older
> release that did not include %systemd_postun_with_restart, to avoid an issue
> with having rewritten the whole sepolicy. Since then I've also added
> %systemd_postun_with_restart so yes, this will serve no purpose if you first
> install the package from @fedora


Right.

Comment 27 Neal Gompa 2022-12-27 20:28:34 UTC
Package Review
==============

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


Issues:
=======
- Package installs a %{name}.desktop using desktop-file-install or desktop-
  file-validate if there is such a file.
- systemd_post is invoked in %post, systemd_preun in %preun, and
  systemd_postun in %postun for Systemd service files.
  Note: Systemd service file(s) in waydroid
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/Scriptlets/#_scriptlets


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

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: "Unknown or generated", "GNU General Public License, Version
     3", "BSD 3-Clause License GNU General Public License v3.0 or later".
     36 files have unknown license. Detailed output of licensecheck in
     /home/ngompa/2120119-waydroid/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[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.
[-]: 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.
[x]: Package contains systemd file(s) if in need.
[-]: Package is not known to require an ExcludeArch tag.
[-]: 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.
[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]: 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]: 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
     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

Python:
[x]: Python eggs must not download any dependencies during the build
     process.
[x]: 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
[x]: Binary eggs must be removed in %prep

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

Generic:
[-]: 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).
[!]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name} = %{version}-%{release} in
     waydroid-selinux
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[!]: Patches link to upstream bugs/comments/lists or are otherwise
     justified.
[!]: Scriptlets must be sane, if used.
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[-]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[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]: 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 =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: waydroid-1.3.4-2.fc38.noarch.rpm
          waydroid-selinux-1.3.4-2.fc38.noarch.rpm
          waydroid-1.3.4-2.fc38.src.rpm
=========================================================================================================== rpmlint session starts ===========================================================================================================
rpmlint: 2.4.0
configuration:
    /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-legacy-licenses.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
rpmlintrc: [PosixPath('/tmp/tmpbx4lc8t1')]
checks: 31, packages: 3

waydroid.noarch: E: summary-too-long Container-based approach to boot a full Android system on a regular GNU/Linux system
waydroid.src: E: summary-too-long Container-based approach to boot a full Android system on a regular GNU/Linux system
waydroid.spec: W: patch-not-applied Patch0: setup-firewalld.patch
waydroid.spec: W: patch-not-applied Patch1: mount-secontext.patch
waydroid.noarch: W: only-non-binary-in-usr-lib
waydroid.noarch: W: no-manual-page-for-binary waydroid
waydroid.noarch: W: desktopfile-without-binary /usr/share/applications/Waydroid.desktop waydroid
waydroid.noarch: W: desktopfile-without-binary /usr/share/applications/waydroid.market.desktop waydroid
waydroid-selinux.noarch: W: dangerous-command-in-%pre cp
waydroid-selinux.noarch: W: dangerous-command-in-%posttrans rm
============================================================================ 3 packages and 0 specfiles checked; 2 errors, 8 warnings, 2 badness; has taken 0.3 s ============================================================================




Rpmlint (installed packages)
----------------------------
/bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
/bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
/bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
/bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
/bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
/bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
/bin/sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
============================ rpmlint session starts ============================
rpmlint: 2.4.0
configuration:
    /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-legacy-licenses.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
checks: 31, packages: 2

waydroid.noarch: E: summary-too-long Container-based approach to boot a full Android system on a regular GNU/Linux system
waydroid.noarch: W: only-non-binary-in-usr-lib
waydroid.noarch: W: no-manual-page-for-binary waydroid
waydroid-selinux.noarch: W: dangerous-command-in-%pre cp
waydroid-selinux.noarch: W: dangerous-command-in-%posttrans rm
 2 packages and 0 specfiles checked; 1 errors, 4 warnings, 1 badness; has taken 0.1 s 



Source checksums
----------------
https://github.com/waydroid/waydroid/archive/1.3.4/waydroid-1.3.4.tar.gz :
  CHECKSUM(SHA256) this package     : 32e30e6c7384a514649daeac68b1326e2781362692ae143927628ff2f1387c4e
  CHECKSUM(SHA256) upstream package : 32e30e6c7384a514649daeac68b1326e2781362692ae143927628ff2f1387c4e


Requires
--------
waydroid (rpmlib, GLIBC filtered):
    (waydroid-selinux = 1.3.4-2.fc38 if selinux-policy-targeted)
    /bin/sh
    /usr/bin/python3
    /usr/bin/sh
    dnsmasq
    gtk3
    iproute
    lxc
    nftables
    python3-gbinder
    python3-gobject

waydroid-selinux (rpmlib, GLIBC filtered):
    /bin/sh
    container-selinux
    libselinux-utils
    policycoreutils
    policycoreutils-python-utils
    selinux-policy
    selinux-policy-base



Provides
--------
waydroid:
    application()
    application(Waydroid.desktop)
    application(waydroid.market.desktop)
    metainfo()
    metainfo(id.waydro.waydroid.metainfo.xml)
    mimehandler(x-scheme-handler/market)
    waydroid

waydroid-selinux:
    waydroid-selinux



Generated by fedora-review 0.9.0 (6761b6c) last change: 2022-08-23
Command line :/usr/bin/fedora-review -b 2120119 -m fedora-rawhide-x86_64
Buildroot used: fedora-rawhide-x86_64
Active plugins: Shell-api, Python, Generic
Disabled plugins: fonts, C/C++, Ocaml, SugarActivity, PHP, Java, Haskell, R, Perl
Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH

Comment 28 Neal Gompa 2022-12-27 20:37:48 UTC
Issues:
=======
> - Package installs a %{name}.desktop using desktop-file-install or desktop-
>   file-validate if there is such a file.

I think this is not having desktop-file-validate and appstream-util validate in %check.

Cf. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_desktop_file_install_usage

Cf. https://docs.fedoraproject.org/en-US/packaging-guidelines/AppData/#_app_data_validate_usage

> - systemd_post is invoked in %post, systemd_preun in %preun, and
>   systemd_postun in %postun for Systemd service files.
>   Note: Systemd service file(s) in waydroid
>   See: https://docs.fedoraproject.org/en-US/packaging-
>   guidelines/Scriptlets/#_scriptlets

This is because "%systemd_post" is run in %posttrans rather than %post. I don't see a reason that this can't be moved to %post.

> [!]: Fully versioned dependency in subpackages if applicable.
>      Note: No Requires: %{name} = %{version}-%{release} in
>      waydroid-selinux

This should be an easy fix.

> [!]: Patches link to upstream bugs/comments/lists or are otherwise
>      justified.

Should be an easy fix.

> waydroid.noarch: E: summary-too-long Container-based approach to boot a full Android system on a regular GNU/Linux system
> waydroid.src: E: summary-too-long Container-based approach to boot a full Android system on a regular GNU/Linux system

This needs to be shrunk to 79 columns.

Once these are resolved, I can approve this.

Comment 29 Alessandro Astone 2022-12-27 20:53:42 UTC
(In reply to Neal Gompa from comment #28)

> > - systemd_post is invoked in %post, systemd_preun in %preun, and
> >   systemd_postun in %postun for Systemd service files.
> >   Note: Systemd service file(s) in waydroid
> >   See: https://docs.fedoraproject.org/en-US/packaging-
> >   guidelines/Scriptlets/#_scriptlets
> 
> This is because "%systemd_post" is run in %posttrans rather than %post. I
> don't see a reason that this can't be moved to %post.

My memory is not super fresh but this is what i remember:
The reasoning was that I needed the systemd service to (re)start *after* the selinux changes take place. Now, I see that my %selinux_relabel_post is also in %posttrans which I probably took from the first code snippet here https://fedoraproject.org/wiki/SELinux/IndependentPolicy#The_%post_Section , so I put the %systemd_post in %posttrans as well because %post would be too soon.

However I see that the second code snippet in the same link above prefers putting them both in %post instead. As well as %systemd_postun_with_restart in `%post selinux` for when selinux is only installed after the main package.
I'll verify whether it keeps the right behaviour and update the spec.

Comment 31 Neal Gompa 2022-12-27 22:56:37 UTC
Looks great!

PACKAGE APPROVED.

Comment 32 Kevin Fenzi 2022-12-29 18:13:40 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/waydroid

Comment 33 Fedora Update System 2022-12-31 11:31:26 UTC
FEDORA-2022-9360cde92d has been submitted as an update to Fedora 37. https://bodhi.fedoraproject.org/updates/FEDORA-2022-9360cde92d

Comment 34 Fedora Update System 2022-12-31 11:31:27 UTC
FEDORA-2022-d5c219f4bc has been submitted as an update to Fedora 36. https://bodhi.fedoraproject.org/updates/FEDORA-2022-d5c219f4bc

Comment 35 Fedora Update System 2023-01-01 01:51:58 UTC
FEDORA-2022-9360cde92d has been pushed to the Fedora 37 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2022-9360cde92d \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2022-9360cde92d

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 36 Fedora Update System 2023-01-01 02:00:05 UTC
FEDORA-2022-d5c219f4bc has been pushed to the Fedora 36 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2022-d5c219f4bc \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2022-d5c219f4bc

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 37 Fedora Update System 2023-01-03 01:24:51 UTC
FEDORA-2022-9360cde92d has been pushed to the Fedora 37 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 38 Fedora Update System 2023-01-07 02:21:35 UTC
FEDORA-2023-c1e1ab321a has been pushed to the Fedora 36 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf upgrade --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-c1e1ab321a`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-c1e1ab321a

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 39 Fedora Update System 2023-01-15 01:35:44 UTC
FEDORA-2023-c1e1ab321a has been pushed to the Fedora 36 stable repository.
If problem still persists, please make note of it in this bug report.