-
Notifications
You must be signed in to change notification settings - Fork 11
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
Pass security flags and check gcc version #525
base: all-citus
Are you sure you want to change the base?
Conversation
We were not passing security flags for citus community packages, which we are for enterprise. Also this adds the check for gcc version to make sure we are compliant with security.
9fcd572
to
514a929
Compare
@@ -45,6 +45,11 @@ if [ "$(printf '%s\n' "$requiredgccver" "$currentgccver" | sort -V | tail -n1)" | |||
fi | |||
fi | |||
|
|||
gccgte8=$(expr `gcc -dumpversion | cut -f1 -d.` \>= 8) | |||
ifeq "$(gccgte8)" "1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't ifeq
a Makefile
thing? I believe this spec file is (ba)sh.
fi | ||
fi | ||
|
||
gccgte8=$(expr `gcc -dumpversion | cut -f1 -d.` \>= 8) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer if this used the same type of check as the one above for consistency, i.e. using sort -V
(version sort).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually thinking about that a bit, we would enter that if it the version is equal to "4.8.2" but we don't want to, isn't that if check wrong in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you are right, maybe a better way would be to use head -n1
and check against currentgccver
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if currentgccver and requiredgccver are the same, then what is the difference between head -n1 or tail -n1, or checking against currentgccver
or requiredgccver
. I would say that we can switch to gte
structure here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if ! [ "$(printf '%s\n' "$requiredgccver" "$currentgccver" | sort -V | head -n1)" = "$requiredgccver" ]; then
echo WARNING: Using slower security flags because of outdated compiler
fi
or something like the above maybe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes that seems good, but maybe use !=
instead of ! [ ... = ... ]
And then use
if [ "$(printf '%s\n' "8.0.0" "$currentgccver" | sort -V | head -n1)" = "8.0.0" ]; then
# add extra security flag
fi
9fa64d6
to
6ace573
Compare
We were not passing security flags for citus community packages, which
we are for enterprise.
Also this adds the check for gcc version to make sure we are compliant
with security.