Skip to content
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

Port to CMake #4

Open
wants to merge 48 commits into
base: master
Choose a base branch
from
Open

Port to CMake #4

wants to merge 48 commits into from

Conversation

jirihnidek
Copy link

Hi,
I ported Wslay to use CMake as build system. I changed the API a little. You have to include wslay.h using #include <wslay.h> not #include <wslay/wslay.h>. I consider to have wslay.h in special directory unnecessary, because there will be only one .h file. I also fixed some compile warnings and I ported patch from https://github.com/benvanik (it should be possible to compile Wslay at Windows). I tested this build system only at Linux. I didn't do any changes of indentation.

Jiri

benvanik and others added 21 commits February 8, 2013 22:08
* WIP: Removed support for autotools
* Removed some build warnings
* Added support for building CUnit tests
 - It is optional
 - Tests are compiled to binary ./bin/tests
 - You can run test using make test
* Improved CMakeLists.txt in root dir
Signed-off-by: Jiri Hnidek <jiri.hnidek@tul.cz>
* Fixed some build warnings

Signed-off-by: Jiri Hnidek <jiri.hnidek@tul.cz>
Signed-off-by: Jiri Hnidek <jiri.hnidek@tul.cz>
Signed-off-by: Jiri Hnidek <jiri.hnidek@tul.cz>
Signed-off-by: Jiri Hnidek <jiri.hnidek@tul.cz>
using this library

Signed-off-by: Jiri Hnidek <jiri.hnidek@tul.cz>
* Fixed small compile warning, when Release target is used
 - Added simple find module for sphinx
@jirihnidek
Copy link
Author

I did some small changes in README, NEWS, etc and I also deleted some useless files.

@jirihnidek
Copy link
Author

I created some issues for my wslay branch: https://github.com/jirihnidek/wslay/issues?state=open

* Added optional debug print to fork-echoserv.c
@tatsuhiro-t
Copy link
Owner

Thank you for the patches.
While I understand the usefulness of cmake, but I want to keep autotools files.
Also I think storing libraries source files in lib folder is no so uncommon.
Creating dedicated directory for header files has an advantage, for example, we can put header file autogenerated by build tools. I don't want to auto-generate wslay.h.

@jirihnidek
Copy link
Author

Hi,
Thanks for your response. First, I want to say that I really appreciate your Wslay library. it seems that it will help me a lot. 👍

When you claim that you understand the usefulness of CMake, then I don't understand why you want to keep Autotools files. Does it mean, you do not want to add support for CMake at all? BTW: I have experience with big projects and it is not wise to have more then one build system. Neither one build system is not well supported then.

CMake is IMHO much more better build system than Autotools is; e.g. generating Makefiles with autotools takes more time, then generating Makefiles and building whole project using CMake. On the other side CMake is more simple and flexible then Autotools, etc. From my point of view Autotools is like steam technology. :-)

May by it is not uncommon to have sources in lib folder, but is not to much descriptive. Anyway this is not anything essential for me. I can switch it back to lib.

I don't understand your claim about dedicated folder for wslay.h. Do you want or do you not want to autogenerate wslay.h? This is not anything essential for me too. It is easy to switch it back to <wslay/wslay.h>

BTW: there wasn't much activity for last few months in this project. Do you have any plans for this project?

Best regards,

Jiri

@tatsuhiro-t
Copy link
Owner

I am OK to add cmake support. But I'd like to keep autotools, just like curl project does. I'm not fully determined to get rid of autotools yet.

Choosing source directory name is subjective and is a good example of bikeshed. I have no reason to change lib to src at the moment, so leave it unchanged.

Actually, we have 2 header files in wslay directory. wslayver.h is generated from wslayver.h.in by build system, so that it contains correct version. You hardcoded version name in wslay.h, which is easily missed to update it.
Since we have 2 files already, it is an obvious choice to use dedicated directory.

We have no update recently because there are nothing to do. No spec change in RFC, no interesting extension ATM.

functions are NULL. Added check for pointer at callback functions.

May be, there should be some check during initialization of server
context for basic callback functions.

Signed-off-by: Jiri Hnidek <jiri.hnidek@tul.cz>
@jirihnidek
Copy link
Author

Jiri Hnidek added 2 commits July 12, 2013 19:23
* Use <wslay/wslay.h> again
* Autogenerate wslayver.h from template

Signed-off-by: Jiri Hnidek <jiri.hnidek@tul.cz>
Signed-off-by: Jiri Hnidek <jiri.hnidek@tul.cz>
@jirihnidek
Copy link
Author

Hi, pushed some changes to git (#include <wslay/wslay.h> and renaming src back to lib).

My standpoint about autotools: I'm not going to add back support for autotools, because I really do not like this technology.

Nikita Nerkar and others added 20 commits October 10, 2013 11:10
Expected behavior:
	'Ref RFC 6455'
	A fragmented message consists of a single frame with the FIN bit
	clear and an opcode other than 0, followed by zero or more frames
	with the FIN bit clear and the opcode set to 0, and terminated by
	a single frame with the FIN bit set and an opcode of 0
Current Behavior:
	When server sends data in fragmented frames,
	for some platforms, the data received is not always equal to the payload length
	and hence the original frame is received in multiple receives due to which the
	final bit is set even if the received fragmented subframe is not the
	terminating one.
	Consider if server sends the terminating frame of size 1008 bytes with final bit set.
	On client side, it further gets divided into two chunks of size 532 bytes
	and 476 bytes. For both the frames the final bit is set which violates the
	protocol.
	This behavior is observed as the final bit value received from server is
	directly assigned to the client side frame without checking if thats the
	terminating frame or not.
Expected behavior:
        'Ref RFC 6455'
        A fragmented message consists of a single frame with the FIN bit
        clear and an opcode other than 0, followed by zero or more frames
        with the FIN bit clear and the opcode set to 0, and terminated by
        a single frame with the FIN bit set and an opcode of 0
Current Behavior:
        When server sends data in fragmented frames,
        for some platforms, the data received is not always equal to the payload
	length and hence the original frame is received in multiple receives due to
	which the opcode bit is set even if the received fragmented subframe is not
	the first one.
        Consider if server sends the first frame of size 1008 bytes with opcode bit set.
        On client side, it further gets divided into two chunks of size 532 bytes
        and 476 bytes. For both the frames the opcode is set which violates the
	protocol.
	This behavior is observed as the opcode bit value received from server is
        directly assigned to the client side frame without checking if thats the
        first frame or not.

Signed-off-by: Nikita Nerkar <nikitan@marvell.com>
Signed-off-by: Jiri Hnidek <jiri.hnidek@tul.cz>
 - Examples using Nettle are located now in ./examples/nettle
 - Added Option to find OpenSSL
 - Added epmty example of ./example/openssl; there will be example
   of server using OpenSSL.
 - Modified CMakeFiles.txt

Signed-off-by: Jiri Hnidek <jiri.hnidek@tul.cz>
Pull request from nikita-n's fork
Signed-off-by: Jiri Hnidek <jiri.hnidek@tul.cz>
 * OpenSSL library is used for computing SHA1 and BASE64
 * It also uses select() instead of poll()
 - Unit tests for functions in wslay_stack.c were not called.
 - TODO: This code is not used anywhere (dead code/WIP code?).
@wonder-mice
Copy link
Contributor

I'm also interested in CMake support :)
Just to make it clear, since I'm going to work on that:
@tatsuhiro-t As I understand, your are OK with adding CMake support in addition to autotools. Is that right?

@tatsuhiro-t
Copy link
Owner

@wonder-mice yes. We accept cmake support patch, with the condition that autotools support is still in tact.

Jiri Hnidek added 2 commits November 30, 2016 16:10
Conflicts:
	.gitignore
	CMakeLists.txt
	NEWS
	README.rst
	configure.ac
	doc/Makefile.am
	doc/sphinx/conf.py.in
	lib/CMakeLists.txt
	tests/CMakeLists.txt
	tests/main.c
	tests/wslay_event_test.c
	tests/wslay_event_test.h
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants