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

build: Vendor CocoaAsyncSocket, CocoaHTTPServer and RoutingHTTPServer #348

Merged
merged 23 commits into from
Jun 11, 2020

Conversation

qmfrederik
Copy link

CocoaHTTPServer and RoutingHTTPServer are not being actively maintained and have been forked by the Appium team.

Instead of maintaining code in three separate repositories, consolidate the CocoaHTTPServer and RoutingHTTPServer in the WebDriverAgent repository.

CocoaHTTPServer contains a source-level copy of CocoaAsyncSocket, so this code has been included, too.

This PR:

  • Copies the CocoaAsyncCode, CocoaHTTPServer and RoutingHTTPServer to the WebDriverAgentLib/Vendor folder
  • Adds the BSD-style licenses for these projects
  • Disables certain compiler warnings the sources which have been copied
  • Removes unused code (File responses, multi-part forms, HTTP authentication, HTTPS)
  • Updates the README with an attribution notice

This PR contains individual commits to keep the history easy to digest. Feel free to squash these commits if you want.

This PR should compile and all tests should pass. There's some work that can still be done, preferably in follow-up PRs:

  • Upgrade to the latest version of CocoaAsyncSockets
  • Take a dependency on the upstream CocoaAsyncSockets project (which is still being maintained) instead of copying the sources
  • Fix compiler warnings instead of silencing them

@qmfrederik qmfrederik changed the title Vendor CocoaAsyncSocket, CocoaHTTPServer and RoutingHTTPServer [build] Vendor CocoaAsyncSocket, CocoaHTTPServer and RoutingHTTPServer Jun 11, 2020
@qmfrederik qmfrederik changed the title [build] Vendor CocoaAsyncSocket, CocoaHTTPServer and RoutingHTTPServer build: Vendor CocoaAsyncSocket, CocoaHTTPServer and RoutingHTTPServer Jun 11, 2020
@mykola-mokhnach
Copy link

mykola-mokhnach commented Jun 11, 2020

Would it be possible to autoformat the lib files, so they follow the common WDA indentation style (2 spaces)?

@qmfrederik
Copy link
Author

Sure. I didn't reformat CocoaAsyncSocket because they come from a maintained upstream (see PR description).

@mykola-mokhnach
Copy link

Sure. I didn't reformat CocoaAsyncSocket because they come from a maintained upstream (see PR description).

What if we include CocoaAsyncSocket to the carthage dependencies then? If you say it is maintained properly then it makes sense to use that snapshot rather than copy it here. Or RoutingHTTPServer cannot work with the recent version of it?

@qmfrederik
Copy link
Author

What if we include CocoaAsyncSocket to the carthage dependencies then?

Yes, I think that's the proper way to fix it. I've tried upgrading it to the latest version (by copying the source), and it worked just fine on iOS. There were some build errors on tvOS but nothing fundamental, I think.

That said, this PR in its current state is "pure refactoring" - almost no source code changes. Upgrading CocoaAsyncSockets will include a significant amount of code change, and that's why it's perhaps better to do it on a separate PR?

What do you think?

@mykola-mokhnach
Copy link

yep, that makes sense. Lets do it in separate PR.

@@ -1,5 +1,2 @@
# Used for HTTP routing
github "appium/RoutingHTTPServer"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@KazuCocoa KazuCocoa merged commit f2f23e3 into appium:master Jun 11, 2020
@qmfrederik qmfrederik deleted the consolidation/import-http-server branch June 12, 2020 09:52
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.

None yet

3 participants