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

Fix angstrom #280

Merged
merged 5 commits into from
Mar 5, 2018
Merged

Fix angstrom #280

merged 5 commits into from
Mar 5, 2018

Conversation

dinosaure
Copy link
Member

Update ocaml-git to the last API of angstrom.

This was referenced Mar 1, 2018
@dinosaure
Copy link
Member Author

We have a problem about the tcpip package on 4.04.0:

tcpi

@dinosaure
Copy link
Member Author

I just tried to compile mirage-tcpip.3.4.0 on ocaml.4.04.2 with pin of mirage-protocols.1.3.0 and mirage-stack.1.3.0 and all works on my side (chemoisamarsh). So, I don't understand why Travis fails - I restart the compilation process however.

@dinosaure
Copy link
Member Author

See mirage/mirage-tcpip#349

@samoht
Copy link
Member

samoht commented Mar 1, 2018

I am not sure the warning is the real error. Would be great to print more things (using ALCOTEST_VERBOSE=1) but I'll try to reproduce.

@dinosaure
Copy link
Member Author

Catched!

catched

@samoht
Copy link
Member

samoht commented Mar 2, 2018

It's a very weird error. @yomimono any idea what this Tcpv4_socket error is?

@yomimono
Copy link
Contributor

yomimono commented Mar 2, 2018

I think this is mirage/mirage-tcpip@39a3c08 . 3.4.0 needs jbuilder 17 at highest; Rudi's fix didn't quite make the release. I thought there was a constraint on this in opam-repository but there isn't, so I'll add one. (ed: ocaml/opam-repository#11515)

@dinosaure
Copy link
Member Author

I just cleaned commit history. I don't if we wait results or merge this PR.

@dinosaure
Copy link
Member Author

On my side, I catched the same error than Travis:

External library "tcpip.tcpv4-socket" is not available because it depends on the following libraries that are not available:
- tcp_socket_options -> not found
Hint: try: jbuilder external-lib-deps --missing --dev @runtest

jbuilder external-lib-deps --missing --dev @runtest just fails (cc @rgrinberg here) with jbuilder.1.0+beta17. Then, my old reflex is to check what ocamlfind knows:

$ ocamlfind list|grep tcpip.tcpv4-socket
tcpip.tcpv4-socket (version: 3.4.0)

So, I don't understand ... 😞

@yomimono
Copy link
Contributor

yomimono commented Mar 2, 2018

If you pin tcpip to the current master, do you get a successful build (either with jbuilder 18 or earlier, it shouldn't matter)? We can release a 3.4.1 if that fixes things.

@dinosaure
Copy link
Member Author

tcpip.master can not compile with jbuilder.1.0+beta18 on my side. Then, I have the same error with tcpip.master.

It seems than in the META file generated, the tcpip package expects a tcp_socket_options library which is not exposed but exists in src/stack-unix/. I tried on my side to delete (wrapped false) but it does not change anything on the META generated.

@yomimono
Copy link
Contributor

yomimono commented Mar 2, 2018

If you're keen to get these tests moving ASAP, pinning the test-socket-stack in my fork of tcpip seems to work for other stuff that builds against tcpip's socket stack. I'll release a 3.4.1 once that PR (or another fix) is merged.

This commit downgrade for a moment the tcpip package and waiting for a new update of tcpip which will fix linking.
@dinosaure
Copy link
Member Author

Hmmhmm it's seems possible to downgrade tcpip to 3.3.1 which seems to compile and link and waiting a new version of it then instead to pin in travis.yml your version, sorry. This PR is ready to merge 👍 !

@dinosaure
Copy link
Member Author

About appveyor, it seems to angstrom is not yet available as a package on the mirror.

@rgrinberg
Copy link
Member

ocaml/dune#580

This is a bug in jbuilder guys. The workaround is to give this tcp_stack_options library a public_name.

@samoht
Copy link
Member

samoht commented Mar 5, 2018

Ok merging for now on as the CI issues are unrelated.

@samoht samoht merged commit c034faf into mirage:master Mar 5, 2018
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