Add integration ci for Monit #1286
Conversation
samuel40791765
force-pushed
the
monit-ci
branch
from
131e896 to
796b3c4
Compare
There was a problem hiding this comment.
Looking at the build output is this line expected:
Compiler flags: -g -O2 -Wextra -fstack-protector-all -D_GNU_SOURCE -Wall -Wunused -std=c11 -D _REENTRANT -I/codebuild/output/src164812706/src/github.com/aws/aws-lc/MONIT_BUILD_ROOT/aws-lc-install/include -I/codebuild/output/src164812706/src/github.com/aws/aws-lc/MONIT_BUILD_ROOT/aws-lc-install/include
Specifically passing in the include directory twice, and here:
Linker flags: -lpam -lz -lpthread -lcrypt -lresolv -lnsl /codebuild/output/src164812706/src/github.com/aws/aws-lc/MONIT_BUILD_ROOT/aws-lc-install/lib/libssl.a /codebuild/output/src164812706/src/github.com/aws/aws-lc/MONIT_BUILD_ROOT/aws-lc-install/lib/libcrypto.a -L/codebuild/output/src164812706/src/github.com/aws/aws-lc/MONIT_BUILD_ROOT/aws-lc-install/lib
Specifically why is it passing the entire path to our libcrypto.a which should tell the linker to link the libcrypto.a and passing in -L/path/to/aws-lc/folder.
Is this just an issue with their makefile, or how we're building it?
There was a problem hiding this comment.
Great attentional to detail. For both issues, I think it's due to their make file configurations.
For the redundant include and linker flag, the first one seems to defined here when --with-ssl-static is used. The second flag is being added here again since with_sslstatic is true.
| # TimeTest will fail on a machine not in CET timezone. | ||
| # https://bitbucket.org/tildeslash/monit/src/def6b462259586358be3c86d76a299c80744df39/libmonit/test/TimeTest.c#lines-24 |
There was a problem hiding this comment.
is there anything important in TimeTest? if so, is it worth configuring this box to be in CET tz within this script?
There was a problem hiding this comment.
Not anything relevant to AWS-LC, they're time function tests to verify things are parsed correctly: https://bitbucket.org/tildeslash/monit/src/master/libmonit/test/TimeTest.c
Issues:
Resolves
CryptoAlg-2167Description of changes:
Adding an integration CI for our recent support for Monit.
Call-outs:
N/A
Testing:
New CI dimension, time tests will fail on a non-CET timezone machine, so I removed the test. It's non-relevant to AWS-LC functionality.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.