Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Makefile: fix seccomp notify compilation issues #280

Merged
merged 4 commits into from
Jul 16, 2021

Conversation

haircommander
Copy link
Collaborator

on older kernels we need to also check if the proper headers are installed, in addition to the seccomp version

also, unify the syntax of conditional compilation variables

Signed-off-by: Peter Hunt <pehunt@redhat.com>
Signed-off-by: Peter Hunt <pehunt@redhat.com>
@haircommander
Copy link
Collaborator Author

fixes #275

@rhatdan
Copy link
Member

rhatdan commented Jun 23, 2021

LGTM

Copy link
Member

@saschagrunert saschagrunert left a comment

Choose a reason for hiding this comment

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

LGTM

Makefile Outdated
endif

ifeq ($(shell $(PKG_CONFIG) --atleast-version 2.5.0 libseccomp && echo "0" || echo "1"), 0)
ifeq ($(shell $(PKG_CONFIG) --atleast-version 2.5.0 libseccomp && grep -rq "seccomp_notif_sizes" /usr/include && echo "0"), 0)
Copy link
Member

Choose a reason for hiding this comment

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

this could be expensive, and detect the string in other unrelated header files.

What do you think about replacing it with printf "#include <linux/seccomp.h>\nvoid main(){struct seccomp_notif_sizes s;}" | cc -x c - -o /dev/null ?

Copy link
Member

Choose a reason for hiding this comment

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

SGTM

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated as suggested!

Signed-off-by: Peter Hunt <pehunt@redhat.com>
Signed-off-by: Peter Hunt <pehunt@redhat.com>
@rhatdan rhatdan merged commit c79dc94 into containers:main Jul 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants