-
Notifications
You must be signed in to change notification settings - Fork 140
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
Adding test for the Visual Studio compile, plus some fixes #53
Conversation
Adding basic test in the Windows port
Thank you very much for working on this. I think that I can merge this. OTOH, have you looked at (or is the windows port already) building and running t/openssl.c? It runs a series of unit tests including TLS handshakes in various modes (e.g., full handshake, abbreviated, 0-RTT). IMO running that as well would help us find issues at an early stage. |
Yes, I should try running the tests. But I also wanted to do something very simple to check that I understand the way to use the libraries. I plan to use it in my implementation of QUIC, https://github.com/private-octopus/picoquic, and so I am actually interested in the "in memory configuration". |
Thank you for the clarification! That makes sense, and I am looking forward to seeing picotls used in picoquic! I have merged the PR with little tweaks. Regarding the changes, I saw I am fine with the PR (hence merged), but would like to understand the requirements so that we could have less issues when adding new features. |
On 6/28/2017 9:15 AM, Kazuho Oku wrote:
Thank you for the clarification! That makes sense, and I am looking
forward to seeing picotls used in picoquic!
I have merged the PR with little tweaks.
Regarding the changes, I saw |static| in array size specifiers (i.e.
|[static NN]|) being removed from header files but not from the source
files. Is your intention to improve C89 compatibility with your code,
while compiling picotls using C99?
I was getting a compiler error for the headers with the [static NN]
declaration. I seems that the VS 2017 compiler did not like them. But I
just did the minimal fixes that allowed compilation to run. You are
right, this is inconsistent. I will investigate further in due time --
although right now my priority is to get picoquic in shape for the
hackathon at the Prague IETF.
I am fine with the PR (hence merged), but would like to understand the
requirements so that we could have less issues when adding new features.
I should write a paper about this porting issues. That would be good for
everybody...
…--
Christian Huitema
|
Thank you for the response. No need to hurry. I am perfectly fine with fixing things as issues arise; we have no need to ship code that successfully compiles everywhere.
I would appreciate that. OTOH I wonder if we could have a CI running on Windows. Having that would help me (or any other people with no access to Visual Studio) see and fix the issues myself. |
On 6/28/2017 12:08 PM, Kazuho Oku wrote:
You are right, this is inconsistent. I will investigate further in
due time -- although right now my priority is to get picoquic in
shape for the hackathon at the Prague IETF.
Thank you for the response. No need to hurry. I am perfectly fine with
fixing things as issues arise; we have no need to ship code that
successfully compiles everywhere.
I should write a paper about this porting issues. That would be
good for everybody...
I would appreciate that. OTOH I wonder if we could have a CI running
on Windows. Having that would help me (or any other people with no
access to Visual Studio) see and fix the issues myself.
You mean a binary version that people can use on Windows without having
to compile it themselves? It is a bit difficult, since Windows libraries
come in many flavors. For real portability you need to package as a DLL
and expose something minimal.
Anyhow, I have been thinking about that for other projects. A simpler
goal would be to have the same kind of install for Windows as Open DNS.
They have a dependency on the Visual Studio compiler, but that is much
simpler than a dependency on the whole IDE. But I have to spend some
time understanding the Perl script that they use to generate the Windows
environment.
…--
Christian Huitema
|
Thank you for the response. I was considering of something like Travis CI that supports running tests on Windows. We currently run Travis for every commit we make (either by directly pushing it to the repo or through pull requests; please refer to https://travis-ci.org/h2o/picotls/). However Travis only supports running tests on Linux or Mac. Doing some googling tells me that AppVeyor might be providing such service. But I am not sure if actually usable in our use-case or not. |
I understand why only the header file were connected. My test were in "open ssl" mode, without reference to the cifra library. I am now running the t/OpenSSL test, and sure enough there are lots of issues. Expect a new PR soon!
…-- Christian Huitema
On Jun 28, 2017, at 4:14 PM, Kazuho Oku ***@***.***> wrote:
Thank you for the response.
I was considering of something like Travis CI that supports running tests on Windows.
We currently run Travis for every commit we make (either by directly pushing it to the repo or through pull requests; please refer to https://travis-ci.org/h2o/picotls/). However Travis only supports running tests on Linux or Mac.
Doing some googling tells me that AppVeyor might be providing such service. But I am not sure if actually usable in our use-case or not.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
The Visual Studio project includes a basic test: a console application that performs a handshake "in memory", thus exercising both server and client functions.