rand00 / conntest

MirageOS unikernel to test networking
GNU Lesser General Public License v2.1
16 stars 7 forks source link

Updated mvar.t to stream.t wherever needed #7

Closed prernadabi23 closed 1 year ago

prernadabi23 commented 1 year ago

Updated the changes required. I tried to solve it, if there's any fault I will try to solve it again. fixes: #4 Signed-off-by: Prerna Dabi prernadabi24@gmail.com @rand00

rand00 commented 1 year ago

Thank you for working on this. I can see from the code changes that this won't compile - but this is a good start.

If you follow the instructions on the README.md to compile the unikernel (e.g. as a unix target to keep it simple), then you will see informative compiler-errors for what is expected/doesn't work.

First, try to see if you can make the compiler happy - and if you have any questions, just ask here (:

0xrotense commented 1 year ago

can I work on this :)?

rand00 commented 1 year ago

can I work on this :)?

@0x0god - @prernadabi23 is already working on this, and I believe I saw you being active on another PR in another repository. Maybe finish one PR at a time

rand00 commented 1 year ago

@prernadabi23 - have you got progress on compilation, or do you have questions on this?

prernadabi23 commented 1 year ago

@rand00 I am working on it. My laptop is having some issues, when I am done with that I'll finish this.

prernadabi23 commented 1 year ago

Screenshot from 2023-03-15 12-23-32

@rand00 I am having issues in running ocaml-platform. I was following this: "https://github.com/mirage/mirage/issues/1402".

rand00 commented 1 year ago

Hmm, I havn't used the ocaml-platform (I've set up my system manually), so don't know what can go wrong with it. Can you try to run opam init yourself and see what happens?

If that succeeds, try to run ocaml-platform again


Edit: I intentionally didn't give opam init the extra arguments, just run it as is

rand00 commented 1 year ago

Ah just had a look at opam init --help - the errorcode 40 means:

Sync error. Could not fetch some remotes from the network. This can be a partial error.

Did you have internet access at the point of writing the command?

rand00 commented 1 year ago

If it's the internet connection that was the problem, just try out running ocaml-platform without opam init first

prernadabi23 commented 1 year ago

okay I'll try it again.

prernadabi23 commented 1 year ago

Screenshot from 2023-03-18 20-06-15

Hello @rand00, I have done those steps successfully. What to do next?😁

0xrotense commented 1 year ago

is this issue fixed now?

prernadabi23 commented 1 year ago

I am working on it @0x0god. I just have to complete the setup.

@0x0god Have you completed the setup? What are the steps to follow after the ss that I have attached above?

prernadabi23 commented 1 year ago

Hello @rand00, I am a little stuck and would appreciate your help.

rand00 commented 1 year ago

Hello @rand00, I am a little stuck and would appreciate your help.

Sorry for the downtime - was sick since the weekend :/

Next step is to compile conntest - you can follow the instructions in the README: https://github.com/rand00/conntest#compiling

prernadabi23 commented 1 year ago

Screenshot from 2023-03-23 18-42-02

I did the commands but I think there is some issue. So I checked this https://github.com/rand00/conntest/issues/1. But still, nothing changed.

rand00 commented 1 year ago

Done! Now, How do I check the compiler errors that you were talking about?

So you earlier transformed the sourcecode a bit by replacing Lwt_mvar with Lwt_stream - this wouldn't compile. So try to compile contest with these changes and see how far you can get with making the compiler happy (:

rand00 commented 1 year ago

I did the commands but I think there is some issue. So I checked this https://github.com/rand00/conntest/issues/1. But still, nothing changed.

So the last line before you cloned the conntest repository said that you should run eval $(opam env). This updates the terminal environment to see the current opam switch - otherwise the installed libraries etc. won't be visible

prernadabi23 commented 1 year ago

Screenshot from 2023-03-23 18-55-17 Screenshot from 2023-03-23 18-56-17

Is this correct?

rand00 commented 1 year ago

It should now have generated a mirage/dist/conntest.spt - if it's there it worked (: Maybe run the whole command again replacing -t spt with -t unix - then you will get a mirage/dist/conntest which you can run directly in the terminal.

When you then begin to develop on conntest you can just run mirage build -f mirage/config.ml to recompile (which is pretty fast relative to reconfiguring the unikernel + compiling)

prernadabi23 commented 1 year ago

Yes, I am getting errors now. @rand00

prernadabi23 commented 1 year ago

Is there any guide or reference that I can use?

rand00 commented 1 year ago

There are several - what are the specific things you want to know about?

rand00 commented 1 year ago

For OCaml here are some good references:

rand00 commented 1 year ago

For finding out what is possible with the code at hand you can do several things:

prernadabi23 commented 1 year ago

Thank you @rand00.

rand00 commented 1 year ago

Remember that there is a deadline april 3. for making the contribution

prernadabi23 commented 1 year ago

@rand00 I was getting syntax errors, so I decided to learn more about the language. I am trying!

prernadabi23 commented 1 year ago

@rand00 I need help, I am just getting endless errors😭.

rand00 commented 1 year ago

I'm sorry you have trouble with it - but it's normal that there is a high learningcurve if you havn't worked with typed functional languages before!

Can you explain what you find hardest?

prernadabi23 commented 1 year ago

I changed the file which needed change - Lwt.async (fun () -> Lwt_mvar.put flow.sink ring_field); to Lwt_stream.push flow.sink ring_field;

then the compiler gave an error then I did some changes again then the error is (snd flow.source)#push data >|= fun () -> ^^^^ Error: This expression has type ([>Data of Cstruct.t ], [> Msg of string ]) result but an expression was expected of type (Cstruct.t Mirage_flow.or_eof, err) Lwt_result.t = (Cstruct.t Mirage_flow.or_eof, err) result Lwt.t

and I now don't know how to solve it. @rand00

rand00 commented 1 year ago

It's smart to start making changes from "one end" of the code, and then make the rest of the code "conform" to that change. So in this case, the type of flow.sink needs to be the right one. With Lwt_stream.t there is both a bounded version and an unbounded version - two type aliases for these are here: https://github.com/rand00/conntest/blob/main/lib/conntest.ml#L163-L164

The difference between these two types is how to push new elements to the stream. Let's take outset in the 'a stream because I think we want an unbounded stream. The ('a option -> unit) part of this type is the push-function for the stream. To use it, one would then call (snd flow.sink) some_new_element.

It's hard to help with the specific error when I don't have the code, but concerning the category of error you are getting, it's about what type of monad you are in. So in MirageOS, the Lwt_result monad is used a lot, which is a stack of two monads: the Result monad and the Lwt monad. You actually see that concrete type in the last error:

... but an expression was expected of type (Cstruct.t Mirage_flow.or_eof, err) Lwt_result.t = (Cstruct.t Mirage_flow.or_eof, err) result Lwt.t

In the error it's complaining that the last value in the expression is missing the Lwt.t part of the type. This can be done in several ways, either by using another Lwt operator (map vs bind) or by wrapping the last returned value in Lwt.return.

I can also understand that this part of the error is confusing: This expression has type ([> Data of Cstruct.t ], [> Msg of string ]) result - as this hasn't got the same name as the later mentioned (Cstruct.t Mirage_flow.or_eof, err) result. What is happening here is that the first type is a subtype of the second - in OCaml there is structural subtyping for polymorphic variants and for objects.

If you go to definition the type Mirage_flow.or_eof on line https://github.com/rand00/conntest/blob/main/lib/conntest.ml#L185 - then you will se that it's a polymorphic variant:

type 'a or_eof = [`Data of 'a | `Eof ]
rand00 commented 1 year ago

Btw. in the top conntest.ml some operators are included in the scope by opening some modules: https://github.com/rand00/conntest/blob/main/lib/conntest.ml#L1-L2

It's possible to go to definition on these.

Lwt.Infix includes e.g. the operators >>= which is Lwt.bind and >|= which is Lwt.map.

Lwt_result.Syntax e.g. introduces let* which is Lwt_result.bind, and let+ which is Lwt_result.map.

The reason why both Infix and Syntax is used is just for convenience of always having operators available for both the Lwt and Lwt_result monads. Another more clean solution would have been to locally open the Syntax modules or call the functions directly when needed - e.g. call Lwt.map instead of >|=.

prernadabi23 commented 1 year ago

@rand00 I am new to Ocaml, It would take time to learn it and to implement it in the code which is basically a hard task. I was reading and changing the code but still it will take time. I am sorry for the delay.

rand00 commented 1 year ago

It is totally understandable that it's hard when you are new. It is really cool that you've been motivated and gotten further towards a PR! Hope you can keep up the motivation, and remember to ask for advice if you get stuck.

prernadabi23 commented 1 year ago

In the final application, what should I write in the timeline? @rand00

rand00 commented 1 year ago

You should just write what you think makes sense - the timeline is not "written in stone", but a suggestion for what will happen. You can look at the existing project description and write down the issues you would like to work on during the internship.

Remember that you need to have comitted something to a PR to be accepted - the PR doesn't have to be finished by the deadline, but there should be some progress.

prernadabi23 commented 1 year ago

Is there anything else an extension or package that we need to have on vscode? @rand00

rand00 commented 1 year ago

What is the issue with VSCode? Do you e.g. have type-checking and autocompletion?

rand00 commented 1 year ago

I havn't tried VSCode, but I can see that there is some extension that seems to be needed: https://marketplace.visualstudio.com/items?itemName=ocamllabs.ocaml-platform

I found that link on this guide for setting up your editor: https://ocaml.org/docs/up-and-running#configuring-your-editor

prernadabi23 commented 1 year ago

https://docs.google.com/document/d/1Ja2gOdcFU5n03FvDP2ZScqSI1lZd1cngk5ObH8BN2UA/edit?usp=sharing

I had made a lot of changes, but I undid them. Now, I am wondering if this was the right way to solve the issue. Therefore, I created a file detailing the changes I have made since the beginning.

I was thinking that there may be some packages that are not installed, as I am getting some errors that I cannot find a solution for.

prernadabi23 commented 1 year ago

I havn't tried VSCode,

Which platform do you recommend then?

rand00 commented 1 year ago

Which platform do you recommend then?

I think VSCode is fine from what I hear, so I recommend that, as it's a simpler editor than e.g. emacs or vim. Personally I use emacs.

Thanks for the document - is well structured - but would make more sense to push the code-changes to this PR instead, as I would be able to comment directly on the code here via Github then (and run the code easily myself). This will also count more towards the internship application (code doesn't need to compile, even though it's preferred).

I'll try to comment here based on the document:

Try to fix these things and it should become less confusing.

Another thing: it's both the sink field of the partial_flow and the t types that need to be changed. (In another issue these two types will later be merged)

prernadabi23 commented 1 year ago

The extension ocamllsp is quite useful, I get to know about the error even without compiling it :)

rand00 commented 1 year ago

Good that you got it working \o/. Was it an error that you closed the PR?

prernadabi23 commented 1 year ago

Yes I don't know why I was not able to commit in this branch.