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

Add ocaml-mdx verification of the tutorials #7

Merged
merged 7 commits into from
Sep 17, 2019

Conversation

craigfe
Copy link
Member

@craigfe craigfe commented Aug 7, 2019

This adds opam/dune configuration to run ocaml-mdx pp over the tutorials (and then run the result). As a side-effect, also verifies that the main example on the homepage compiles.

@craigfe craigfe force-pushed the mdx-verification branch 7 times, most recently from 2db678b to 8ebf4fb Compare August 8, 2019 10:17
data/tutorial/dune Outdated Show resolved Hide resolved
@craigfe craigfe force-pushed the mdx-verification branch 5 times, most recently from c4e77a3 to 931683c Compare August 13, 2019 12:20
@craigfe
Copy link
Member Author

craigfe commented Sep 4, 2019

Rebased. If realworldocaml/mdx#161 is merged, we can use it as a temporary solution to avoid fixing the network issues with mdx verification inside Travis CI.

@samoht
Copy link
Member

samoht commented Sep 4, 2019

realworldocaml/mdx#161 is merged -- can also add our custom CI instead of Travis to make the tests quicker.

@craigfe
Copy link
Member Author

craigfe commented Sep 4, 2019

The Travis CI here is doing JS linting via npm and such, but it would definitely be nice to get rid of the OCaml travis runner in favour of ocurrent ASAP. (fortunately, JS on Travis is much better cached than OCaml).

@craigfe craigfe force-pushed the mdx-verification branch 4 times, most recently from 5b7d0ac to b245a6c Compare September 4, 2019 16:48
irmin-website.opam Outdated Show resolved Hide resolved
@craigfe craigfe force-pushed the mdx-verification branch 4 times, most recently from 7ff1afa to d9d93ce Compare September 16, 2019 13:00
@craigfe
Copy link
Member Author

craigfe commented Sep 16, 2019

CI is now green. Build times are ~16mins; would be nice to get the new CI on here ASAP 🙂

@samoht
Copy link
Member

samoht commented Sep 16, 2019

would be nice to get the new CI on here ASAP

done ;-)

@talex5
Copy link
Contributor

talex5 commented Sep 16, 2019

The new CI fails the build with xargs: unrecognized option: max-args=1, due to running the build under Alpine (maybe use -n instead?).

@craigfe
Copy link
Member Author

craigfe commented Sep 16, 2019

Done; looks like it's still failing for some reason however...

#          mdx alias data/tutorial/runtest (got signal SEGV)

???

@talex5
Copy link
Contributor

talex5 commented Sep 16, 2019

bash-5.0$ cd _build/default/data/tutorial
bash-5.0$ ./mdx.exe 
Segmentation fault (core dumped)

bash-5.0$ sudo apk add gdb
[...]
bash-5.0$ gdb ./mdx.exe 
GNU gdb (GDB) 8.3
[...]
Reading symbols from ./mdx.exe...
(gdb) run
Starting program: /src/_build/default/data/tutorial/mdx.exe 
warning: Error disabling address space randomization: Operation not permitted
[Detaching after fork from child process 3026]

Program received signal SIGSEGV, Segmentation fault.
0x00007fbc918b5242 in stpncpy () from /lib/ld-musl-x86_64.so.1
(gdb) bt
#0  0x00007fbc918b5242 in stpncpy () from /lib/ld-musl-x86_64.so.1
#1  0x00007ffc6340ce90 in ?? ()
#2  0x00007fbc918b57bc in strncpy () from /lib/ld-musl-x86_64.so.1
#3  0x00007ffc6340ce90 in ?? ()
#4  0x000055c3de73ac12 in strncpy (__n=127, __s=<optimized out>, __d=0x7ffc6340ce90 "Connection refused")
    at /usr/include/fortify/string.h:139
#5  __redisSetErrorFromErrno (c=c@entry=0x55c3df73f720, prefix=prefix@entry=0x0, type=1) at src/hiredis/net.c:74
#6  0x000055c3de73ae10 in redisCheckSocketError (c=0x55c3df73f720) at src/hiredis/net.c:246
#7  0x000055c3de73aee7 in redisContextWaitReady (c=c@entry=0x55c3df73f720, msec=msec@entry=-1)
    at src/hiredis/net.c:224
#8  0x000055c3de73b42d in _redisContextConnectTcp (c=0x55c3df73f720, addr=<optimized out>, port=<optimized out>, 
    timeout=<optimized out>, source_addr=<optimized out>) at src/hiredis/net.c:392
#9  0x000055c3de73a05e in redisConnect (ip=0x55c3de8af640 "127.0.0.1", port=6379) at src/hiredis/hiredis.c:682
#10 0x000055c3de7377f5 in redis_context_connect (host=<optimized out>, port=<optimized out>, 
    nonblock=<optimized out>) at src/hiredis_stubs.c:145
#11 0x000055c3de3235bb in camlHiredis_client__connect_inner_924 () at src/hiredis_client.ml:121
#12 0x000055c3de31d326 in camlMdx__v_4297 () at backend.md:52
#13 0x000055c3de6174ec in camlIrmin__v_6475 () at src/irmin/irmin.ml:96
#14 0x000055c3de619598 in camlIrmin__v_10580 () at src/irmin/irmin.ml:255
#15 0x000055c3de321038 in camlMdx__entry () at backend.md:300
#16 0x000055c3de318ff9 in caml_program ()
#17 0x000055c3de7988b0 in caml_start_program ()
#18 0x000055c3de77b1d5 in caml_startup_common (argv=0x7ffc6340d348, pooling=<optimized out>, pooling@entry=0)
    at startup_nat.c:160
#19 0x000055c3de77b21b in caml_startup_exn (argv=<optimized out>) at startup_nat.c:170
#20 caml_startup (argv=<optimized out>) at startup_nat.c:170
#21 0x000055c3de31738c in main (argc=<optimized out>, argv=<optimized out>) at main.c:44

@craigfe craigfe force-pushed the mdx-verification branch 2 times, most recently from c67f431 to ba93fc4 Compare September 16, 2019 14:04
@craigfe
Copy link
Member Author

craigfe commented Sep 16, 2019

Can be 'fixed' by not exercising the Redis backend, as in ba93fc4, although clearly this is a bad idea.

I'm not familiar with the Redis backend being constructed here. Perhaps @icristescu or @zshipko will have ideas?

@icristescu
Copy link
Contributor

Can be 'fixed' by not exercising the Redis backend, as in ba93fc4, although clearly this is a bad idea.

I'm not familiar with the Redis backend being constructed here. Perhaps @icristescu or @zshipko will have ideas?

If we want to run the code in the Redis backend then there are some bits missing, like instantiating the KV functor or calling the server_start function. I think @zshipko left out some parts because they are a repetition w.r.t to other parts in the tutorial.

@craigfe
Copy link
Member Author

craigfe commented Sep 16, 2019

If we want to run the code in the Redis backend then there are some bits missing, like instantiating the KV functor or calling the server_start function. I think @zshipko left out some parts because they are a repetition w.r.t to other parts in the tutorial.

We could add more testing, but this would likely require some external unit tests because, as you say, the code isn't very interesting from a tutorial perspective. I am interested in keeping the blocks that currently exist, which I believe construct an instance of the store and show how to start/stop a server. These run just fine on Travis CI, but something about them causes a segfault when running on the new CI.

@samoht
Copy link
Member

samoht commented Sep 17, 2019

All green! @craigfe could you remove wip in your last commit and remove travis CI configuration files so that we can merge this and have a fast CI ? :-)

@craigfe
Copy link
Member Author

craigfe commented Sep 17, 2019

It's all green but with two skipped blocks (which are skipped only because they fail to run on the new CI). Can merge as-is, but I think we should at least make new issues tracking that these blocks are currently untested.

@craigfe
Copy link
Member Author

craigfe commented Sep 17, 2019

Also, we still need the JS section of the Travis configuration file, but this shouldn't be a problem as this only takes around 1 min to run.

craigfe added a commit to craigfe/irmin.io that referenced this pull request Sep 17, 2019
These blocks fail to run on ocaml-ci:
mirage#7 (comment).
This issue should be investigated in future.
craigfe and others added 7 commits September 17, 2019 13:44
This block fails to run on Travis CI, presumably due to issues
with the network configuration. The quick-and-dirty solution
is just to skip it; this requires using the dev version of `mdx`:
realworldocaml/mdx#161.
The markdown parser reads "```ocaml skip" as using the "ocamlskip"
language. This alias is a quick hack to retain syntax highlighting
for skipped code blocks.
These blocks fail to run on ocaml-ci:
mirage#7 (comment).
This issue should be investigated in future.
@craigfe
Copy link
Member Author

craigfe commented Sep 17, 2019

Issues #23 and #24 opened to track the skipped blocks. I think this PR is ready to merge now!

@samoht samoht merged commit 9f12574 into mirage:master Sep 17, 2019
@samoht
Copy link
Member

samoht commented Sep 17, 2019

Great, many thanks for all the fixes!

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.

5 participants