%if %{defined el7}
Requires:       systemd-udev
%else
Requires:       systemd
%endif

This looks like reversed logic.

I am committing the following logic instead to both cover more versions, and to fix the reversed logic.

@@ -65,7 +65,11 @@ Requires(pre):  shadow-utils
 # /etc/modprobe.d/
 Requires:       kmod
 # /etc/udev/rules.d/  and  /usr/lib/udev/rules.d/
+%if 0%{?rhel} <= 7
+Requires:       systemd
+%else
 Requires:       systemd-udev
+%endif


 %description

Thank you for your contribution. This saved me from finding out which package does provide the needed rules.d directories.

This looks like reversed logic.

Indeed, I accidentally copy/pasted that wrong when setting up the conditional. It should have been a default else condition for systemd-udev with a special case of systemd on el7 only.

I am committing the following logic instead to both cover more versions, and to fix the reversed logic.

This is actually wrong also, for two reasons.

• 0%{?rhel} <= 7 is true on Fedora. You can see in your recent Fedora builds that the package requires systemd instead of systemd-udev as intended.
• RHEL versions earlier than 7 don't have systemd at all.

Ranged conditionals based on %rhel and %fedora are difficult to get correct. It's much more robust to just use the number conditionals like %el7. I'm reopening the pull request with the fixup.

I can see you fixed this another way in a new commit. I'm not sure why you weren't able to just merge this pull request. Are pull requests to this package not welcome?

Good catches.

As beep.spec already contains

%{!?el7:BuildRequires:  libubsan}

it makes sense to follow that pattern, so I have committed and test-built the following two conditional Requires: lines:

# /etc/udev/rules.d/  and  /usr/lib/udev/rules.d/
%{?el7:Requires:  systemd}
%{!?el7:Requires: systemd-udev}

I have verified the Requires: systemd for el7 and Requires: systemd-udev for everything else with a series of fedpkg mockbuild with subsequent rpm -q --requires and by looking at the koji built RPMs in koji.

I can see you fixed this another way in a new commit. I'm not sure why you weren't able to just merge this pull request. Are pull requests to this package not welcome?

Pull Requests are always welcome. I am sorry that I made it appear to be otherwise, especially with the race conditions which happend in our communication between comments here, comments on bugzilla, and commits on the git repo presenting an unintended picture.

I simply find the two self contained lines are easier to read than five lines which establish conditional context without block indentation which could help with reading which conditional context applies to which spec file lines.

As beep.spec already contains a similar self-contained conditional requirement line this does not introduce a new bit of spec file syntax to the file either, so I think the two self contained lines implement the logic in a better way without any drawbacks, and that is why I deemed that a better solution for beep.spec and committed that.

Thank you for the initial PR with the inverted logic, thank you for finding my erroneous logic, thank you for your fixed PR, thank you for your general interest in and input into this bug and this package, and sorry for appearing to be snubbing you.