Make demos build-able from top level makefile and add them to CI #24047
Conversation
nhorman
force-pushed
the
351
branch
4 times, most recently
from
6f7e707
to
7248b99
Compare
nhorman
force-pushed
the
351
branch
3 times, most recently
from
420dd08
to
09a17ec
Compare
|
@paulidale thanks, not done yet of course, need to fix up all the build errors, and figure out how to link to external libraries using our build system for the http3 demo |
|
I like what I see, and it makes sense, but I am wondering if you thought of a way to do this without removing the Make files that end users could use in the "here is a simple self-contained demo openssl application that you can try to get started" setting. |
nhorman
force-pushed
the
351
branch
9 times, most recently
from
6ce1eb0
to
d8050ed
Compare
|
@romen this big issue I've run into there is the makefiles themselves. The makefiles currently use several features that POSIX/GNUmake only. Windows uses nmake in our build environment, which is, to say the least, limited. There is no good way I can find to write a Makefile that is compatible with both, that doesn't amount to just writing two parallel makefiles (one for windows, one for Gnu Make), which to me doesn't seem like a much better solution. I think, given that we only distribute our demos with our source tree, and the current build (which only works on unix systems at the moment), explicitly link with the libraries that are built from the same source tree, I feel like using the standard build system to enable windows compatibility is a step forward. I'm open to suggestions of course. One possibility might be to augment the readme file in each demo directory with instructions on how to build these demos (i.e. what include/library paths are required), to guide users in writing their own independent makefiles specific to their platform. Would that be sufficient to enable independent builds? |
|
Maybe an OTC discussion? |
|
I can't speak to the history of how/why they were created, but I can say they currently only work on unix systems (i.e. windows users are left writing their own name files for these demos at the moment). But yeah, we can discuss on OTC |
nhorman
added
branch: master
Windows doesn't support getline, so we need to use fgets here
cygwin caught a signedness difference in this pointer.
|
Ok, still working on getting the windows builds solid, but I've restored all the makefiles. They are currently unused in our build system (we use the perl build system for that), but they exist as examples for guidance |
Thanks. We should add something in |
The external nghttp3 library seems to have a linking issue on windows (several missing symbols). Disable that build in windows for now until its fixed
Note that they are available but only meant as a guide to self building, and are not used expressly to build as part of the overall openssl build
|
I like the final suggestion 👍 |
t-j-h
removed
the
approval: otc review pending
t8m
added
approval: done
|
merged |
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> (Merged from #24047)
Fix up the warnings in the demos and make them configurable with enable-demos Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> (Merged from #24047)
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> (Merged from #24047)
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> (Merged from #24047)
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> (Merged from #24047)
The platform doesn't support it Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> (Merged from #24047)
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> (Merged from #24047)
Windows doesn't support getline, so we need to use fgets here Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> (Merged from #24047)
cygwin caught a signedness difference in this pointer. Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> (Merged from #24047)
The external nghttp3 library seems to have a linking issue on windows (several missing symbols). Disable that build in windows for now until its fixed Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> (Merged from #24047)
Note that they are available but only meant as a guide to self building, and are not used expressly to build as part of the overall openssl build Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Paul Dale <ppzgs1@gmail.com> Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com> (Merged from #24047)
We should be ensuring our demos in the source tree are buildable on a regular basis. Add a target to build them from the top level makefile and add a step to the appropriate CI jobs to check that