Patch BoringSSL files for C tests #1692
Conversation
|
@andres-erbsen I'm going to set this to squash and merge so we can (relatively) quickly unblock the other failing PRs, but feel free to revert and implement some other preferred alternative. |
JasonGross
force-pushed
the
patch-boring
branch
from
5ad9efc to
f4f6951
Compare
JasonGross
force-pushed
the
patch-boring
branch
from
f4f6951 to
3378f57
Compare
|
I don't understand what you're trying to do here. IIUC the point of this integration test is to exercise the bedrock2-generated C code, so we probably shouldn't prefer assembly implementations? |
JasonGross
force-pushed
the
patch-boring
branch
from
77ed9ba to
df5c158
Compare
| @@ -40,7 +40,7 @@ echo "::group::Building BoringSSL" | |||
| set -ex | |||
| mkdir build | |||
| cd build | |||
| cmake -GNinja .. -DCMAKE_CXX_FLAGS="-Wno-error=unused-function ${EXTRA_CFLAGS}" -DCMAKE_C_FLAGS="-Wno-error=unused-function ${EXTRA_CFLAGS}" || exit $? | |||
| cmake -GNinja .. -DOPENSSL_NO_ASM=1 -DCMAKE_CXX_FLAGS="-Wno-error=unused-function ${EXTRA_CFLAGS}" -DCMAKE_C_FLAGS="-Wno-error=unused-function ${EXTRA_CFLAGS}" || exit $? | |||
There was a problem hiding this comment.
@andres-erbsen you suggested
env CC=clang CXX=clang++ cmake -GNinja -B build-noasm -DOPENSSL_NO_ASM=1 -DCMAKE_BUILD_TYPE=Releaseseems to work for me
What's the relationship between .. and -B build-noasm (if any)? What's the importance of -B build-noasm?
And what are your thoughts on -DCMAKE_BUILD_TYPE=Release vs -DCMAKE_CXX_FLAGS="-Wno-error=unused-function ${EXTRA_CFLAGS}" -DCMAKE_C_FLAGS="-Wno-error=unused-function ${EXTRA_CFLAGS}"? (EXTRA_CFLAGS gets set to -Wno-error=unused-but-set-variable by CI)
|
Apparently build time for C tests jumped from ~13m on the last successful run before this change to ~30m after this change, did BoringSSL get much slower to build around this time? @davidben @andres-erbsen |
|
Can't think of anything. We have been adjusting the build, but I don't see anything obviously relevant around October. I dunno, if it's a huge problem, maybe try building it at different versions and see what changed? |
See #1684 (comment)