Bug 2260492

Copr build:
https://copr.fedorainfracloud.org/coprs/build/6963272
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2260492-incus/fedora-rawhide-x86_64/06963272-incus/builder-live.log.gz

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
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.

[fedora-review-service-build]

Created attachment 2011019 [details]
The .spec file difference from Copr build 6963272 to 6967304

Copr build:
https://copr.fedorainfracloud.org/coprs/build/6967304
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2260492-incus/fedora-rawhide-x86_64/06967304-incus/builder-live.log.gz

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
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.

New version.

Spec URL: https://ngompa.fedorapeople.org/for-review/incus.spec
SRPM URL: https://ngompa.fedorapeople.org/for-review/incus-0.6-1.fc39.src.rpm

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

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2260492-incus/fedora-rawhide-x86_64/07092290-incus/fedora-review/review.txt

Please take a look if any issues were found.


---
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.

Skipping the formality of entire review template as there are no glaring errors or problems. That said, a few notes and comments for posterity, mostly/exclusively from rpmlint.

Expected: these are set purposefully.

```
incus.x86_64: E: non-standard-dir-perm /var/cache/incus 700
incus.x86_64: E: non-standard-dir-perm /var/lib/incus 711
incus.x86_64: E: non-standard-dir-perm /var/log/incus 700
```

These seem possibly problematic, but I admit I am not sure if the selinux one is expected, as above.

```
incus-selinux.noarch: E: non-readable /var/lib/selinux/targeted/active/modules/200/incus 0
incus.x86_64: W: log-files-without-logrotate ['/var/log/incus']
incus.x86_64: W: post-without-tmpfile-creation /usr/lib/tmpfiles.d/incus.conf
```

Expected -- comes from macros and are standard:

```
incus-selinux.noarch: W: dangerous-command-in-%pre cp
incus-selinux.noarch: W: dangerous-command-in-%postun rm
incus-selinux.noarch: W: dangerous-command-in-%posttrans rm
incus-selinux.noarch: W: dangerous-command-in-%post rm
```

The rest of the review looks great. Aside from these couple items/questions, this is good to go, from my perspective.

If you want to throw some answers back at me and/or address issues if you think they are in need of fixing, I'm happy to throw an Approved stamp on this.

Regarding the non-default permissions:
That's enforced by the upstream source code anyway so I thought I make it transparent this way.
See: https://discuss.linuxcontainers.org/t/incus-log-file-and-directory-permissions/18520/4

Regarding incus-selinux error:
I think the few SELinux file context definitions should eventually be moved to container-selinux similar to how it's done for Podman or Docker. To have these as a sub-package was convenient on COPR as long as I wasn't sure if this could land in Fedora.  

Regarding log-files-without-logrotate:
In the current default configuration I actually disabled the creation of a daemon log and only depend on journald as a log target. The problem is that the /var/log/incus directory is still "misused" for all kind of per-container runtime configuration and logs that are fully controlled by the Incus daemon. That's why there is no log file that must be managed by logrotate. These runtime state files will be moved to /run in the next few releases so that it might be possible to drop /var/log/incus altogether in the future.
See: https://discuss.linuxcontainers.org/t/incus-0-5-has-been-released/18814#use-of-runincus-for-runtime-data-20

In case you're wondering why I reply instead of Neal Gompa is because he heavily based the submitted spec file on mine that I'm maintaining for the time being on COPR (https://github.com/ganto/copr-lxc4/tree/master/incus). Obviously I'm looking forward to co-maintain this package in Fedora too.

> Regarding the non-default permissions:
> That's enforced by the upstream source code anyway so I thought I make it transparent this way.
> See: https://discuss.linuxcontainers.org/t/incus-log-file-and-directory-permissions/18520/4

Ack.

> Regarding incus-selinux error:
> I think the few SELinux file context definitions should eventually be moved to container-selinux similar to how it's done for Podman or Docker. To have these as a sub-package was convenient on COPR as long as I wasn't sure if this could land in Fedora.

Right--definitely something that can be amended over time to be "correct".

> Regarding log-files-without-logrotate:
> In the current default configuration I actually disabled the creation of a daemon log and only depend on journald as a log target. The problem is that the /var/log/incus directory is still "misused" for all kind of per-container runtime configuration and logs that are fully controlled by the Incus daemon. That's why there is no log file that must be managed by logrotate. These runtime state files will be moved to /run in the next few releases so that it might be possible to drop /var/log/incus altogether in the future.
> See: https://discuss.linuxcontainers.org/t/incus-0-5-has-been-released/18814#use-of-runincus-for-runtime-data-20

Acknowledged, that makes sense. I think good to leave for now, then.

> In case you're wondering why I reply instead of Neal Gompa is because he heavily based the submitted spec file on mine that I'm maintaining for the time being on COPR (https://github.com/ganto/copr-lxc4/tree/master/incus). Obviously I'm looking forward to co-maintain this package in Fedora too.

Right, that makes sense :) I knew the spec was familiar! Very glad to see you here.

New version.

Spec URL: https://ngompa.fedorapeople.org/for-review/incus.spec
SRPM URL: https://ngompa.fedorapeople.org/for-review/incus-0.7-1.fc40.src.rpm

previous reviewer seems to have accepted there are no remaining concerns but never set this as approved, so I've approved to unblock

Thanks Michel. I had this in my queue several times and kept getting hit with login issues with FAS :|

This is indeed fine and ready to move forward. Apologies for the delay.

The Pagure repository was created at https://src.fedoraproject.org/rpms/incus

is it possible to add incus-ui?
https://gist.github.com/vaxvhbe/ce679df15fc521c8aca1ff9ddf537201

also on fedora kinoite incus require another package to work
rpm-ostree install qemu-system-x86

"Instance type not operational	QEMU command not available: exec: "qemu-system-x86_64": executable file not found in $PATH"