Bug 2403018

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

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2403018-userspacevt/fedora-rawhide-x86_64/09676463-userspacevt/fedora-review/review.txt

Found issues:

- Systemd service file(s) in userspacevt
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_scriptlets

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.

Spec URL: https://download.copr.fedorainfracloud.org/results/jfalempe/Userspacevt/fedora-rawhide-x86_64/09676543-userspacevt/userspacevt.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/jfalempe/Userspacevt/fedora-rawhide-x86_64/09676543-userspacevt/userspacevt-0.3.1-1.fc44.src.rpm

Created attachment 2109250 [details]
The .spec file difference from Copr build 9676463 to 9676549

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

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2403018-userspacevt/fedora-rawhide-x86_64/09676549-userspacevt/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.

Taking this review.

Initial spec review:

> %setup -q

This should be "%autosetup -p1"

> pushd %{name}
> cp -a usr %{buildroot}/
> pushd selinux
> make install DESTDIR=%{buildroot}
> popd
> popd

This can be simplified to the following:

cp -a %{name}/usr %{buildroot}/
%make_install -C %{name}/selinux

> %pre selinux
> %selinux_relabel_pre -s %{selinuxtype}
> [...]
> %post selinux
> %selinux_modules_install -s %{selinuxtype} %{_datadir}/selinux/packages/%{selinuxtype}/%{modulename}.pp.bz2
> %selinux_relabel_post -s %{selinuxtype}
> [...]
> %postun selinux
> if [ $1 -eq 0 ]; then
>     %selinux_modules_uninstall -s %{selinuxtype} %{modulename}
>     %selinux_relabel_post -s %{selinuxtype}
> fi
> [...]
> %ghost %verify(not md5 size mode mtime) %{_sharedstatedir}/selinux/%{selinuxtype}/active/modules/200/%{modulename}

Shouldn't this cover all the policy types instead of just targeted, given how core it is?

Also, the changelog entry format isn't quite right.

Cf. https://docs.fedoraproject.org/en-US/packaging-guidelines/manual-changelog/

Spec URL: https://download.copr.fedorainfracloud.org/results/jfalempe/Userspacevt/fedora-rawhide-x86_64/09832312-userspacevt/userspacevt.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/jfalempe/Userspacevt/fedora-rawhide-x86_64/09832312-userspacevt/userspacevt-0.3.1-0.fc44.src.rpm

I've made the required changes, and fixed the changelog too.
I also changed the Release to 0, as I think it should start at 0.

Regarding selinux, I don't have the knowledge, I just followed an how-to that was using targeted policy.
These scripts are a bit experimental, and I expect they may evolve when they get more testing in the field.

Created attachment 2116095 [details]
The .spec file difference from Copr build 9676549 to 9832744

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

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2403018-userspacevt/fedora-rawhide-x86_64/09832744-userspacevt/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.

(In reply to Jocelyn Falempe from comment #7)
> 
> I've made the required changes, and fixed the changelog too.
> I also changed the Release to 0, as I think it should start at 0.
> 

Once you're importing into Fedora, it should start at 1, but starting at 0 makes sense for reviews and COPR builds.

> Regarding selinux, I don't have the knowledge, I just followed an how-to
> that was using targeted policy.
> These scripts are a bit experimental, and I expect they may evolve when they
> get more testing in the field.

Perhaps the better focus is to get the modules into selinux-policy so this is covered properly.

> URL:            https://gitlab.com/kdj0c/%{name}/
> Source:         %{url}-/archive/%{version}/%{name}-%{version}.tar.bz2

Can you drop the trailing dash in URL and put it in Source instead? That way it doesn't look weird.

> Jocelyn Falempe jfalempe

The format here isn't quite right, it's missing <> around the email address.

Spec URL: https://download.copr.fedorainfracloud.org/results/jfalempe/Userspacevt/fedora-rawhide-x86_64/09833398-userspacevt/userspacevt.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/jfalempe/Userspacevt/fedora-rawhide-x86_64/09833398-userspacevt/userspacevt-0.3.1-0.fc44.src.rpm

I've fixed URL/Source

(In reply to Neal Gompa from comment #12)
> > Jocelyn Falempe jfalempe
> 
> The format here isn't quite right, it's missing <> around the email address.

Fixed too, but I should really use the automatic changelog scripts

Created attachment 2116116 [details]
The .spec file difference from Copr build 9832744 to 9833495

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

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2403018-userspacevt/fedora-rawhide-x86_64/09833495-userspacevt/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.