Project

Profile

Help

Issue #2121

Requires: selinux-policy is broken in spec files

Added by milligana over 4 years ago. Updated almost 2 years ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
Normal
Assignee:
Category:
-
Sprint/Milestone:
-
Start date:
Due date:
Estimated time:
Severity:
2. Medium
Version:
2.9.0
Platform Release:
2.9.3
OS:
Triaged:
Yes
Groomed:
No
Sprint Candidate:
No
Tags:
Easy Fix, Pulp 2
Sprint:
Quarter:

Description

In pulp.spec, we have this travesty:

%define selinux_policyver %(sed -e's,.*selinux-policy-\\([^/]*\\)/.*,\\1,'/usr/share/selinux/devel/policyhelp 2> /dev/null)

Which supposedly fires up xdg (ie X in mock) and groks the policy version out of a help browser footer.

But it doesn't actually work in my setup as there does not seem to be a version shipped in selinux policy help ...

How about something less ambitious - perhaps like:

%define selinux_policyver %(rpm --qf "%{version}" -q selinux-policy)

I bring this up because I can't actually build vanilla pulp from src as you ship it.

Associated revisions

Revision 8c152a68 View on GitHub
Added by bmbouter over 4 years ago

Fixes selinux-policy spec file macro

Adjusts the selinux_policyver macro in the pulp spec file to not be an empty string. The new field value includes both version and release of selinux-policy.

Also removes an unecessary if statement for F19 which is not longer built.

https://pulp.plan.io/issues/2121 re #2121

Revision 8c152a68 View on GitHub
Added by bmbouter over 4 years ago

Fixes selinux-policy spec file macro

Adjusts the selinux_policyver macro in the pulp spec file to not be an empty string. The new field value includes both version and release of selinux-policy.

Also removes an unecessary if statement for F19 which is not longer built.

https://pulp.plan.io/issues/2121 re #2121

Revision 3f7e3698 View on GitHub
Added by semyers over 4 years ago

Revert "Fixes selinux-policy spec file macro"

This reverts commit 8c152a681ecd1e577c1ef6b7b56d3e5bf506a899.

re #2121 https://pulp.plan.io/issues/2121#note-8

Revision 3f7e3698 View on GitHub
Added by semyers over 4 years ago

Revert "Fixes selinux-policy spec file macro"

This reverts commit 8c152a681ecd1e577c1ef6b7b56d3e5bf506a899.

re #2121 https://pulp.plan.io/issues/2121#note-8

Revision 3f7195f9 View on GitHub
Added by bmbouter over 4 years ago

Fixes selinux-policy Requires statement

This is the same fix as 8c152a68 except it correctly escapes the %{version} and %{release} fields using %%.

Now when you run:

rpmspec -q pulp.spec --requires|grep selinux-policy I get the following output:

selinux-policy >= 3.13.1-158.21.fc23

https://pulp.plan.io/issues/2121 closes #2121

Revision 3f7195f9 View on GitHub
Added by bmbouter over 4 years ago

Fixes selinux-policy Requires statement

This is the same fix as 8c152a68 except it correctly escapes the %{version} and %{release} fields using %%.

Now when you run:

rpmspec -q pulp.spec --requires|grep selinux-policy I get the following output:

selinux-policy >= 3.13.1-158.21.fc23

https://pulp.plan.io/issues/2121 closes #2121

History

#1 Updated by bmbouter over 4 years ago

  • Description updated (diff)

#2 Updated by bmbouter over 4 years ago

  • Description updated (diff)

#3 Updated by bmbouter over 4 years ago

On my Fedora machine I also get this error when running the `sed` portion of the bad statement:

[vagrant@dev ~]$ sudo sed -e 's,.*selinux-policy-\\([^/]*\\)/.*,\\1,' /usr/share/selinux/devel/policyhelp
sed: can't read /usr/share/selinux/devel/policyhelp: No such file or directory

I suspect in the actual build environment it is always being returned as "" because further down there is a `Requires:` statement which branches on that case.

https://github.com/pulp/pulp/blob/fd627ea382299981bd5def2d7892ee2dd4b8c035/pulp.spec#L953-L955

That is also a problem because that effectively means our spec file is required this for all platforms:

Requires: selinux-policy >= ""

I'm surprised this works at all.

#4 Updated by bmbouter over 4 years ago

  • Status changed from NEW to ASSIGNED
  • Platform Release set to 2.9.2

The suggested replacement is good as it produces:

rpm --qf "%{version}" -q selinux-policy
3.13.1

There can be important bugfixes in the release field of selinux-policy and it is run per-distro so instead I'm going to make a PR that uses:

[vagrant@dev ~]$ rpm --qf "%{version}-%{release}" -q selinux-policy
3.13.1-191.5.fc24[vagrant@dev ~]$

#5 Updated by bmbouter over 4 years ago

  • Assignee set to bmbouter

#6 Updated by bmbouter over 4 years ago

  • Status changed from ASSIGNED to POST

#7 Updated by bmbouter over 4 years ago

  • Subject changed from pulp.spec crapulous %selinux_policyver macro definition to Requires: selinux-policy is broken in spec files
  • Status changed from POST to MODIFIED

#8 Updated by semyers over 4 years ago

  • Status changed from MODIFIED to ASSIGNED
  • Platform Release deleted (2.9.2)

Apologies Brian, but I just discovered something nasty with this while doing some testing of my 2.9.2 build:

$ rpmspec -q pulp.spec --requires|grep selinux-policy
selinux-policy >= 2.9.2-0.1.beta.fc23

Here's what's happening, I think:

$ rpm --qf "%{version}-%{release}" -q selinux-policy
3.13.1-158.11.fc23
$ rpm --qf "2.9.2-0.1.beta" -q selinux-policy
2.9.2-0.1.beta

The %{version}and %{release} are being expanded in the spec file before the command is run, so the output format just outputs the value for version and release in the current spec file being processed.

I assume there's a way to escape those template strings in the spec, but I definitely don't know what that looks like.

#9 Updated by semyers over 4 years ago

I'm going to revert this commit on 2.9-dev and rebuild 2.9.2 without this change.

#10 Updated by bmbouter over 4 years ago

Ugh I see what you mean. Reverting this is the right thing to do and I plan to pick this up after my current bugfix.

#11 Updated by amacdona@redhat.com over 4 years ago

  • Triaged changed from No to Yes

#12 Updated by bmbouter over 4 years ago

  • Status changed from ASSIGNED to MODIFIED
  • % Done changed from 0 to 100

#13 Updated by bmbouter over 4 years ago

The new PR that correctly escapes the macro definition is available at: https://github.com/pulp/pulp/pull/2685

#14 Updated by bmbouter over 4 years ago

  • Platform Release set to 2.9.3

#15 Updated by rmcgover over 4 years ago

This caused an "interesting" issue in development environment for me, see https://github.com/pulp/pulp/pull/2722 .

#16 Updated by semyers over 4 years ago

  • Status changed from MODIFIED to 5

#17 Updated by semyers over 4 years ago

  • Status changed from 5 to CLOSED - CURRENTRELEASE

#18 Updated by bmbouter almost 2 years ago

  • Tags Pulp 2 added

Please register to edit this issue

Also available in: Atom PDF