Conversation
Removes the .merlin from the tree, use the ones generated by dune
|
Am using this right now, it is quite convenient. A few issues/remarks:
|
| (with-stdout-to %{targets} | ||
| (progn | ||
| (bash "echo \\# 1 \\\"`cat contains-input-name`\\\"") | ||
| (bash "%{dep:../tools/cvt_emit.exe} < `cat contains-input-name`")))))) |
There was a problem hiding this comment.
Another way to do this would be to modify cvt_emit to take a filename and include a line directive in the output.
There was a problem hiding this comment.
Yes I agree. But I tried to minimize the changes in this PR, to limit the amount of review required. I'll change that in a follow-up PR I think.
| (rule | ||
| (targets ocaml.byte) | ||
| (action (run %{ocaml_where}/expunge topstart.exe %{targets} | ||
| ; FIXME: inlined $(STDLIB_MODULES) ... minus Labels ones ... |
There was a problem hiding this comment.
Discussed with @trefis, we have some code in utop to avoid writing the list of modules explicitly:
https://github.com/ocaml-community/utop/blob/master/src/top/expunge/expunge.ml
Called as follow: https://github.com/ocaml-community/utop/blob/master/src/top/expunge/expunge.ml
We could modify expunge to do that directly.
There was a problem hiding this comment.
I'll try to do that in a follow-up PR.
Indeed, that's me doing some last minute cleanups of the dune files without testing properly. It should now be fixed.
We've discussed this on another medium, but to answer publicly: indeed it would, and it would also be nice to be able to run the testsuite for example. We might have some sort of bootstrap procedure with dune in the future (based on workspaces?), but until then, dune is really only usable for building things, not for actual testing.
Fair point, I pushed a commit fixing this. |
|
FTR check-typo is currently unhappy because there are no header in the dune files. Should I add some? Could I opt-out dune files of that check? |
|
Makefiles have headers, so why not Dune files? |
|
Well, the .merlin file didn't have a header. I considered dune files to be in the same scope as the .merlin was, that is: convenient for developers, but unessential. But you're right: let's look towards the (unavoidable) future where dune finally takes over. I'll add some headers. |
|
I don't understand the title of this PR, perhaps because I am missing a I am a bit puzzled by the removal of the .merlin file because that will Was it on purpose that you kept the mention of Inria in the headers of the I think Same for |
|
The title was indeed a reference to dune. But sure, I'll change it.
Merlin is completely unusable on trunk right now, and was also completely unusable for a few weeks/months before 4.07.0 was released. And no, we can't keep the the .merlin file commited.
Yes, but it might be a mistake. I have no idea.
Indeed! I'll push a fix.
Fair enough. I had just copied it with the actual rule from the main Makefile. But I'll edit it.
Sure!
You guessed right. Will do. Thanks for the review @shindere ! |
otherlibs/str/dune
Outdated
| (library | ||
| (name str) | ||
| (modes byte) | ||
| (wrapped false) |
There was a problem hiding this comment.
Does this need wrapped to be false? There is only one str module in the library currently.
There was a problem hiding this comment.
I guess it doesn't!
I was on auto-pilot at this point :p
|
Thomas Refis (2018/10/10 01:50 -0700):
> Was it on purpose that you kept the mention of Inria in the headers of the Dune file?
Yes, but it might be a mistake. I have no idea.
I think you take a classical header like those in OCamltest, you replace
my name by yours and Institut national etc by Jane Street and you remove
Inria and you'll be okay.
|
|
I pushed fixes for all of your comments! |
|
I think all the headers are now correct, and the CI is happy. |
|
Thomas Refis (2018/10/10 14:57 +0000):
I think all the headers are now correct, and the CI is happy.
Are people OK if I (squash and) merge this now?
OK with me, just change the title (again), please. It's still not precise
enough, IMO. It should be something like
Provide a way to build the bytecode compiler using Dune
Or something like that.
|
|
Thank you again for the review @shindere! I set a more descriptive name (reusing your suggestion) for the commit, I don't think the PR itself needs anything more descriptive now that the discussion is over. |
|
Thomas Refis (2018/10/10 08:17 -0700):
I set a more descriptive name (reusing your suggestion) for the
commit, I don't think the PR itself needs anything more descriptive
now that the discussion is over.
Okay, thanks.
But now this commit has broken the compiler's bootstrap, as reported by
Inria's CI. Can you look into this @trefis? You should be able to
execute the bootstrap script locally, or at least to follow it and to
kind of replay it.
|
|
I'll have a look now. |
|
@shindere I managed to reproduce the bootstrap failure without my commit: I wanted to look further, but I am absolutely incapable of decyphering the content of |
|
Thomas Refis (2018/10/11 02:42 -0700):
@shindere I managed to reproduce the bootstrap failure *without* my commit:
```
$ git reset --hard origin/trunk
$ make disclean
Minor, but missing t, here
$ git clean -dfx
$ git reset --hard HEAD^
$ make world
You didn't ./configure ?
$ sed -i 's/\x23define \+EXEC_MAGIC \+\x22Caml1999X023\x22/#define EXEC_MAGIC "toto"/' runtime/caml/exec.h
$ sed -i 's/let \+exec_magic_number \+= \+\x22Caml1999X023\x22/let
exec_magic_number = "toto"/' utils/config.mlp
The magic number is expected to be 12 characters
$ patch -p1 < tools/ci/inria/remove-sinh-primitive.patch
$ make coreall
$ make bootstrap
../boot/ocamlrun ../boot/ocamlc -g -nostdlib -I ../boot -use-prims ../runtime/primitives -I .. make_opcodes.ml -o make_opcodes
make[2]: Leaving directory `/usr/local/home/trefis/ocaml/trunk/tools'
runtime/ocamlrun tools/make_opcodes -opcodes < runtime/caml/instruct.h > bytecomp/opcodes.ml
Fatal error: the file 'tools/make_opcodes' has not the right magic number: expected toto, got
make[1]: *** [bytecomp/opcodes.ml] Error 2
```
Well I don't know where exactly the problem comes from, but as far as I
know your PR is the first for which the CI job failed.
|
I did, you can't build anything if you don't
Ah! The error indeed disappears when I use a magic number of the right length. Fantastic. I'll keep digging, thank you @shindere! |
|
Thomas Refis (2018/10/11 03:38 -0700):
> You didn't ./configure ?
I did, you can't build anything if you don't `./configure`.
Yep I know but still wanted to make sure.
Sorry for the omission.
NP.
> The magic number is expected to be 12 characters
Ah! The error indeed disappears when I use a magic number of the right
length. Fantastic.
Good.
I'll keep digging, thank you @shindere!
Thanks.
FWIW, the bootstrap is very sensitive to the order in which things are
done so if I were you I would log the output of the different
invocations of make beofre and after your patch and try to figure out
which commands are performed in a different order because of your patch.
|
|
@trefis Two questions: (a) At current head of trunk, I believe Dune is using my OPAM ocaml 4.07.01, though it's hard for me to be certain (I'm not sure how to get Dune to tell me. EDIT: I just realized this is almost certainly what I'm using as it gives the command line above, woops!) Is this a KI? Should dune build @world succeed? (b) Related: I was getting a bunch more errors before, when I without realizing it had a old 4.05 install of Ocaml from OPAM. Dune gave me no warning. Is there a way to change the dune file to specify a required compiler version? |
|
The first is a known issue with the dune build: because it uses your current OCaml install, and not the bootstrapped compiler of the compiler distribution, the build will break whenever the two are not compatible (during the development, some changes create incompatibilities, changing the type of format in certain ways is one of them). With the current way the dune build system for the compiler works, you can just assume that the build of |
Steps:
$ dune build -w @worldThe last command currently builds bytecode versions of ocamlc, ocamlopt, ocamldebug, ocamldoc, ocamltest, the toplevel.
The idea is that dune can be used for development (that is why we only build bytecode executables), to get fast feedback and a working merlin environment.
It is not meant to replace the usual build process.
I extracted a couple of things from the makefiles so they can be more easily reused by dune. I'm guessing @shindere will probably want to have a look.
It probably doesn't work on windows, I'm sure @dra27 will send a follow-up PR.