Skip to content

Add apparmor#21906

Merged
clalancette merged 2 commits intoros:masterfrom
ruffsl:apparmor
Oct 22, 2019
Merged

Add apparmor#21906
clalancette merged 2 commits intoros:masterfrom
ruffsl:apparmor

Conversation

@ruffsl
Copy link
Copy Markdown
Contributor

@ruffsl ruffsl commented Aug 2, 2019

debian
ubuntu

Not found for:
fedora

Copy link
Copy Markdown
Member

@tfoote tfoote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like Fedora doesn't support apparmor without a custom kernel: https://serverfault.com/questions/339842/apparmor-on-fedora-rhel-centos

@cottsay any suggestions?

@clalancette
Copy link
Copy Markdown
Contributor

@ruffsl This has been in draft state for almost 2 months. Any thoughts on whether this is going to move forward? Thanks.

@ruffsl ruffsl marked this pull request as ready for review October 8, 2019 16:47
@ruffsl ruffsl requested a review from a team as a code owner October 8, 2019 16:47
@ruffsl
Copy link
Copy Markdown
Contributor Author

ruffsl commented Oct 8, 2019

Sorry, this fell off my backlog. I don't have any further changes for this, and is ready.

@clalancette
Copy link
Copy Markdown
Contributor

Sorry, this fell off my backlog. I don't have any further changes for this, and is ready.

Thanks, appreciated.

ubuntu: [apache2-mpm-prefork]
apparmor:
debian: [apparmor]
ubuntu: [apparmor]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed above, this won't realistically ever be available for Fedora. I'm thinking we should put an explicit fedora: null here, to indicate that it can't be available for Fedora (rather than just not being done, or something that could be done later).

Copy link
Copy Markdown
Contributor Author

@ruffsl ruffsl Oct 8, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I never realized rosdistro was using null to explicitly state unavailability, as it seems the null set is arbitrarily large. If this is common practice, then I can update the PR accordingly.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to be a relatively new practice, and to be honest, the tools don't support it all that well (rosdep just throws a stacktrace if you try to install a null key). But I think it should be straightforward enough to enhance the tools to make it nicer.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I've added 61989c3

to explicitly indicate that it isn't available for Fedora

Signed-off-by: ruffsl <roxfoxpox@gmail.com>
@nuclearsandwich nuclearsandwich added the rosdep Issue/PR is for a rosdep key label Oct 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rosdep Issue/PR is for a rosdep key

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants