space after = ;)

Please follow semantic line breaks: http://sembr.org/

IMO this should not be in guidelines.

Please provide such makefile as part of selinux-devel or such package so that packagers do not have to copy it over and over

So after reading this, I think different structure should be used, probably something like:

• BuildRequires
• %build section, which commands to execute in order to build policy (note the makefile comment above, so plain commands + example with using makefile, not the makefile itself)
• %install section, where to install, some small example which files to install and where
• Scriptlets
  ** applying policy
  ** port labelling
• Example spec
• Links to some examples which mention how to write a policy

probably %build, %install and scriptlets should be a subsections of port labels / other policies

None of this "git repository setup" section belongs in the packaging guidelines. I would completely remove lines 50 through 131.

I believe this needs to be converted to prescriptive language. If we're saying that manipulating booleans in this way is a requirement, then use "MUST" and such. Right now I can't tell if "it is necessary" means that the macro is the way to do this kind of thing, or if the package has to take these steps.

What is a "product" in this context? It's not a term we use elsewhere in the guidelines.

Please follow semantic line breaks: http://sembr.org/

Sorry, I'll fix that.

What is a "product" in this context? It's not a term we use elsewhere in the guidelines.

Application or daemon. As specified in the first sentence of the section.

IMO this should not be in guidelines.

I believe the "Extraction process" is important. Especially for people who are new to writing SELinux policy. Should we move it to our wiki page and reference in this document, or what would be the best approach?

None of this "git repository setup" section belongs in the packaging guidelines. I would completely remove lines 50 through 131.

This part is in place to suggest keeping the SELinux policy sources in a separate repository to minimize synchronization issues with distribution policy (during the transitional period when the policy module is in both "selinux-policy-{targeted|mls}" and "foo-selinux" packages).

Please provide such makefile as part of selinux-devel or such package so that packagers do not have to copy it over and over

The makefile is not necessary, but it's nice to have. The idea here is that the maintainer will customize the makefile with all that is necessary for the module installation (switching booleans, adding port mappings, etc.).
The module compilation is actually done using a makefile provided by selinux-policy-devel (make -f ${SHAREDIR}/selinux/devel/Makefile $@).

I believe this needs to be converted to prescriptive language. If we're saying that manipulating booleans in this way is a requirement, then use "MUST" and such. Right now I can't tell if "it is necessary" means that the macro is the way to do this kind of thing, or if the package has to take these steps.

I'll reword the section. It only describes the behaviour of the macros.

Updated. Please see https://pagure.io/packaging-committee/pull-request/814#comment-66657 for more details.

as said earlier, this doesn't belong to Packaging Guidelines.

why is this makefile needed? Just create necessary RPM macro for it.

why is this makefile needed? Just create necessary RPM macro for it.

The makefile makes it possible to easily use the policy outside of an RPM. Also, it is consistent with policies generated by "sepolicy generate".
However, an RPM macro is a good idea. We could let maintainers decide what they want to use.

Updated:
- Describe Makefile as optional
- Remove "Git Repository setup" section
- Change section label syntax to work with "asciidoc" properly

Could someone please have a look at the updated document?

you did not close ] :)

also, you can drop square brackets since there is no good replacement.

use *SELinux` Fedora mailing list

I'm not happy with this wording. Probably something like create a build script (e.g. Makefile)?

I don't think this chapter is relevant to guidelines

But this is definitely important note. Probably annotate it with CAUTION: and move somewhere above where text is about whether to write from scratch or extract it from selinux-policy? I know it is obvious that people should look at license of the project they are taking code from, but it won't hurt anybody to repeat it again.

Example Makefile?

If sepolicy generate generates it, let's remove this section and put somewhere above something like You can use sepolicy generate to start new project or something like that?

Especially because it is already distributed by selinux. Let's just say that packagers can use this command to compile their policies.

this should be executed by brp-* script in RPM, mind opening RFE or sending patch on https://github.com/rpm-software-management/rpm ?

You can annotate file names in asciidoctor syntax (see Rust.adoc for some example).

pretty sure this is not necessary.

Let's remove this section entirely. Guidelines is not the place where you describe how to set up git repo and archive files.

Just use install -D to create directory so that previous line is not needed

As I said, guidelines is not a tutorial. Just put one complete spec file in Expamles section on this page

I would prefer if it would be .pp* so that if we ever change archive format we won't have to redo all specs. Does selinux support different compression formats?

would appreciate if you could tell what this "200" means

hi @vmojzis, I took a quick look on page. It does look better, but I left some comments.

Basically guidelines is the page where you describe things like "you MUST call %selinux_foo_bar in %post", "you MUST NOT depend on custom interfaces blah" and so on.

It definitely should have some examples and some quick guide how to actually package selinux policies, but not how to create tarball and create license file.

Thank you. Updated and rebased.

would appreciate if you could tell what this "200" means

That is the priority of the new module. As described in the "SELinux Policy module priorities" section, all custom policy modules are shipped with this priority.

Thank you for the review. Updated and rebased.

Could someone please review the latest changes?

I don't understand, why this guildeline appears to suggest to create independent "myapp-selinux" packages, while the typical use case should be just "-slinux" subpackages IMO.

On Thursday, September 26, 2019 11:55:16 AM CEST V=EDt Ondruch wrote:

=20
vondruch commented on the pull-request: Add SELinux Independent Policy G= uidelines that you are following:
I don't understand, why this guildeline appears to suggest to create independent "myapp-selinux" packages, while the typical use case should be just "-slinux" subpackages IMO.

I think that separate package is better because - if one eventually decides=
to
maintain the separate policy - usually there would be more packages ruled=
by
one selinux package. E.g. postgresql-selinux would be usable not only by
postgresql.spec but also third party modules (and other higher level
stuff built on top of postgresql-server e.g).

Then, it is much easier to have separate selinux package with separate life=
cycle
(it is easier and less risky to update the policy without rebuilding any ot=
her
package). Subpackage sounds like a valid option, but I guess that would be=
less
frequent.

I don't understand, why this guildeline appears to suggest to create independent "myapp-selinux" packages, while the typical use case should be just "-slinux" subpackages IMO.

Both are possible (and SELinux team has no preference), but we expect that subpackages will be more common.
The example spec file can be easily used to add a -selinux subpackage to an existing spec file, while containing all the information necessary for a standalone package.

based on the real life conversation with @vmojzis I'm assigning this to him for now. He is planning to update the proposal to be more guidelines-like.

We discussed this at this weeks meeting (https://meetbot-raw.fedoraproject.org/fedora-meeting-1/2019-11-21/fpc.2019-11-21-17.00.txt):

• #814 PR: Add SELinux Independent Policy Guidelines (geppetto,
  17:28:05)
• LINK: https://pagure.io/packaging-committee/pull-request/814
  (geppetto, 17:28:11)
• mhroncok spoke to vmojzis and lvrabec, hopefully get an updated PR
  early next year. (geppetto, 17:32:15)

I spoke to @lvrabec, not lbrabec, sorry about that. I've edited your comment to correct that.

Updated and rebased. Hopefully I addressed all that we talked about :)

I added an alternative example spec file and a section advising how to treat file context equivalency rules.

@churchyard any issues after the update?

Sorry, I wasn't able to look into this yet. I was on PTO for a week and my TODO list is overflowing. Will try to do this soon.

I understand, thank you.

Rebased and updated:
- Add "Backwards compatibility" and "Moving type/attribute/alias definitions" sections
- Improve "Setting Booleans During Installation"
- Fix minor issues in example spec files

@churchyard is the "meeting" tag still valid?

Yes, but our meetings tend to get cancelled for no quorum. See also https://pagure.io/packaging-committee/issue/977

We talked about this at this weeks meeting (https://meetbot-raw.fedoraproject.org/fedora-meeting-1/2020-09-03/fpc.2020-09-03-16.00.txt):

• #pr-814 Add SELinux Independent Policy Guidelines. (geppetto,
  16:08:14)
• LINK: https://pagure.io/packaging-committee/pull-request/814
  (geppetto, 16:08:14)
• LINK:
  https://pagure.io/packaging-committee/pull-request/814#request_diff
  I guess (tibbs, 16:13:11)
• ACTION: mhroncok to speak to authors again, having a working example
  might help a lot. (geppetto, 16:34:35)

Rebased and updated based on experience with latest policy "adopters".
- minor changes to example spec file
- new section regarding testing
- rewritten interface file handling
- add link to multi-mode policy package guide

This is not necessary.

This can just be selinux-policy

selinux-policy-%{selinuxtype} is redundant with %{?selinux_requires}

Remove /%{selinuxtype} from the path to install the module.

/distributed doesn't exist in the selinux-policy hierarchy, and I've generally seen it put in /contrib instead, which does exist.

Drop -s %{selinuxtype}

Drop -s %{selinuxtype} and /%{selinuxtype}

Drop -s %{selinuxtype}

Drop -s %{selinuxtype}

Drop -s %{selinuxtype}

Drop -s %{selinuxtype} and /%{selinuxtype}

Drop -s %{selinuxtype}

Drop -s %{selinuxtype}

Drop -s %{selinuxtype}

Drop /%{selinuxtype}

You probably want to use /contrib rather than /distributed here, as the latter doesn't exist as far as I can tell.

%verify() is redundant with %ghost and this should be macroized so %ghost entries exist for all available SELinux policy variants provided by the selinux-policy package, with the correct override number in the file path set by the other macros.

This seems to assume people won't use different policy types, which is not required, nor a good idea.

When I wrote the SELinux policy for snapd, I was able to avoid those pitfalls.

• https://src.fedoraproject.org/rpms/snapd/blob/ff9639a57941497bac4272d06a90a0bdaa52b9d6/f/snapd.spec#_207-219
• https://src.fedoraproject.org/rpms/snapd/blob/ff9639a57941497bac4272d06a90a0bdaa52b9d6/f/snapd.spec#_630-632
• https://src.fedoraproject.org/rpms/snapd/blob/ff9639a57941497bac4272d06a90a0bdaa52b9d6/f/snapd.spec#_852-856
• https://src.fedoraproject.org/rpms/snapd/blob/ff9639a57941497bac4272d06a90a0bdaa52b9d6/f/snapd.spec#_922-936

Moving this to needinfo.
mhroncok kind of volunteered to speak to everyone and try to get the changes needed in, but I don't think he's had time and it isn't moving forward in other ways.

Just as a housekeeping note/reminder, in addition to the other changes requested here by reviewers, if this were to be revived all of the links that use URLs like:

https://src.fedoraproject.org/rpms/foo/tree/master

would need to be changed to

https://src.fedoraproject.org/rpms/foo/tree/rawhide

To avoid the ugly redirect-to-default-branch banner on arrival.

This seems to assume people won't use different policy types, which is not required, nor a good idea.

When I wrote the SELinux policy for snapd, I was able to avoid those pitfalls.

• https://src.fedoraproject.org/rpms/snapd/blob/ff9639a57941497bac4272d06a90a0bdaa52b9d6/f/snapd.spec#_207-219
• https://src.fedoraproject.org/rpms/snapd/blob/ff9639a57941497bac4272d06a90a0bdaa52b9d6/f/snapd.spec#_630-632
• https://src.fedoraproject.org/rpms/snapd/blob/ff9639a57941497bac4272d06a90a0bdaa52b9d6/f/snapd.spec#_852-856
• https://src.fedoraproject.org/rpms/snapd/blob/ff9639a57941497bac4272d06a90a0bdaa52b9d6/f/snapd.spec#_922-936
  

Actually, "selinuxtype" is important because the policy module needs to be compiled separately for different policy types (binary policy module compiled against targeted policy will not work with mls policy). Most of the time we really only want to deploy the policy module for targeted policy, but the path we save the module files should still contain selinuxtype if this was to change (if the package started deploying versions for other policy types).

You did not avoid the pitfalls. Your policy module will not install on an "mls" or "minimum" system because of the policytype check in %selinux_modules_install. And as explained above, it would fail to install even if you used "semodule -i" directly. Also, %verify() is not redundant with %ghost. Try removing it and verifying the package after installation.

I'm sorry, but this PR has taken way too much of everyone's time and the wiki page seems to work just fine for this purpose so I'm closing it.
Thanks everyone for your reviews and notes, they have been important in finalizing the document.