Skip to content

Smart HTTP support#49

Merged
samoht merged 9 commits into
mirage:masterfrom
samoht:master
Jan 11, 2015
Merged

Smart HTTP support#49
samoht merged 9 commits into
mirage:masterfrom
samoht:master

Conversation

@samoht
Copy link
Copy Markdown
Member

@samoht samoht commented Jan 10, 2015

It was more painful than expected, especially bounding Lwt_io channels and Cohttp bodies together. But it seems to work ...

samoht added 8 commits January 9, 2015 00:47
also, update references only if asked to (default is false)
This was a bit more painful than what was planned, as:
- the the "smart" HTTP mode and in the Git/SSH ones have minor differences
- it was quite involved to wrap Lwt_io.{input,output}_channel and Cohttp
  response and request bodies. But surpisingly, it seems to work well now.

Very limited testing so far but this works:

ogit clone https://github.com/samoht/test.git

(And it's using conduit, so `export CONDUIT_TLS=native` works as well:
OCaml everywhere!)
@samoht
Copy link
Copy Markdown
Member Author

samoht commented Jan 10, 2015

Note: currently support clone and fetch only, not push. Remaining patches coming tomorrow.

Comment thread lib/unix/git_unix.ml
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hm, I wonder if this would be easier with a Cohttp_lwt_cstruct backend instead? @djs55 wanted this for the xapi socket backend too

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

not sure. the idea here is just to consider the body as a normal channel (and do the chunk encoding automatically). Actually the "hard" bit is http_ic.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ahh that's a nice API -- i'll think about if it would fit into either conduit or cohttp

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

it's also parsing/skipping the HTTP headers, so yea would fit into cohttp better. The only issue is that is seems not possible to start reading the response while the the post body is not completely sent. Not sure if it is an issue with cohttp or with the Git servers.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@avsm I think it could help if conduit close function didn't close both input and output channels.

@samoht samoht force-pushed the master branch 4 times, most recently from 6876454 to 520ffda Compare January 10, 2015 14:35
@samoht
Copy link
Copy Markdown
Member Author

samoht commented Jan 10, 2015

Need a new release of cohttp to support lwt 2.4.7 to fix the Travis runs (see mirage/ocaml-cohttp#230)

@avsm
Copy link
Copy Markdown
Member

avsm commented Jan 10, 2015

will do shortly

On 10 Jan 2015, at 15:48, Thomas Gazagnaire notifications@github.com wrote:

Need a new release of cohttp to support lwt 2.4.7 to fix the Travis scripts.


Reply to this email directly or view it on GitHub #49 (comment).

samoht added a commit that referenced this pull request Jan 11, 2015
@samoht samoht merged commit 87850fb into mirage:master Jan 11, 2015
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.

2 participants