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

use ocaml modern build system #16

Closed
wants to merge 1 commit into from
Closed

Conversation

m4b
Copy link

@m4b m4b commented Aug 20, 2015

This PR sets up the project to use the oasis build system, which is a modern, extensible build system for OCaml. As this project becomes larger, it will become a necessity.

A quick survey is here: https://ocaml.org/learn/tutorials/setting_up_with_oasis.html

A basic .merlin file was added as well, for info on how to use, setup: https://github.com/the-lambda-church/merlin/wiki

Changes:

  • Makefile in ml-proto
  • no building in src anymore
  • make test
  • .merlin file for autocomplete and 😍
  • updated README

@rossberg
Copy link
Member

Thanks for this. But does it work on all platforms? Last time I looked, Opam and Windows was not quite the loving couple. Has that changed?

@m4b
Copy link
Author

m4b commented Aug 20, 2015

@rossberg-chromium opam is the package repo for OCaml; oasis is a build system, which can specify any number of build tools (we can also use a tool in the future like oasis2opam to autogenerate an opam package to distribute, but that's orthogonal). In this case it's calling ocamlbuild behind the scenes, which is no different than the current hand-crafted Makefile.

Here is a link to the github mirror: https://github.com/ocaml/oasis

@kg
Copy link
Contributor

kg commented Aug 20, 2015

This PR sets up the project to use the oasis build system, which is a modern, extensible build system for OCaml. As this project becomes larger, it will become a necessity.

Cool or not, unless this does something useful for us, I don't think we should merge it. Just complicates the prototype. Is it solving an immediate (or nearly immediate, pressing, urgent) problem? Our requirements may change later in a way that makes this a bad choice, so that will be pretty awkward if we don't get anything out of it.

Also:
opam install oasis in the tutorial suggests this won't work on Windows. That's a non-starter. (Sorry.)

@m4b
Copy link
Author

m4b commented Aug 20, 2015

@kg: precisely because requirements may change later is why you have an extensible build system. Otherwise you're manually editing Makefiles, which maybe common in C-land, but seems strange to me when you have all this done automatically. This seems maybe a common mistake for someone who doesn't hack with OCaml very often.

The current PR leaves the Makefile as is, and it can still be built in that manner. It doesn't "complicate" the prototype in anyway, and I'm not even sure what this means. As for solving a problem, extensible build systems are universally considered to be good, it's considered best practice in OCaml circles, and to be building in the src directory is unusual, and seems sloppy. In any event, to not merge seems entirely arbitrary.

Moreover, there seems to be some confusion with opam and oasis. If you took the time to read the tutorial, you'd have noted that oasis is not a build dependency; when you git clone the repo, oasis does not need to be installed, and hence opam install oasis is uncessary. Please see the difference between opam and oasis I outlined in the first response.

Lastly, about windows, from the oasis mirror:

It has been tested on GNU Linux and Windows.

Please read comments before responding hastily.

for WebAssembly
Description: It is written for clarity and simplicity, not speed (although it should be reasonably fast). Hopefully, it can be useful as a playground for trying out ideas and a device for nailing down the exact semantics. For that purpose, the code is written in a fairly declarative, "speccy" way.
Authors: WebAssembly Authors
License: Apache-2.0
Copy link
Member

Choose a reason for hiding this comment

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

I am assuming license stated here is for WebAssembly? Is it Apache 2?

@MikeHolman
Copy link
Member

Our build isn't very complicated (and I don't see it going that way any time soon), so I'm inclined to agree that this seems like a big hammer for a problem we don't have. Also, does this work for lex/yacc too? I couldn't find the build commands for them in the PR.

@rossberg
Copy link
Member

@m4b, please calm down, getting snarky isn't constructive.

I'd be fine with supporting Oasis, but we don't want to make it a requirement. Especially when it introduces extra dependencies, e.g. on findlib. Adding more layers of tooling always adds new and more opaque failure modes as well. Admittedly, I'm also not thrilled about dumping half a dozen more files in the root directory.

At a minimum, both the README and the INSTALL file would need to be written such that they enumerate using Oasis as an alternative, not make it look like a necessity. For hygiene, it would also be preferable if most of the auxiliary files could go to a dedicated oasis subdirectory.

@m4b
Copy link
Author

m4b commented Aug 20, 2015

@MikeHolman:

You just type:

make

in the ml-proto directory (the base-dir) (this was updated in the README.md, instead of make in the src, then polluting the src dir, but as rossberg said, this will have to change/be altered to make it seem less like it's required).

All it does is call: ocaml setup.ml -build which itself calls ocamlbuild and does other dependency resolution, etc., for you.

If you want tests:

./configure --enable-tests && make test

As for license, I just added what is in the ml-proto directory, which is Apache 2.0.

@rossberg-chromium

please calm down, getting snarky isn't constructive.

I'm calm, thanks. Sorry if I came off snarky, I just didn't appreciate being told that this won't work on windows when the time wasn't taken to check whether this was true or not, which seems less constructive and a waste of everyone's time, because here we are still talking about it. But we're done now so it's all ok.

I definitely agree about not being a requirement, hence I left the Makefile intact in the src dir.

Other points:

  • agreed if you support oasis as an alternative build system, the README should be rewritten, or an oasis section added. Again, it's unconventional to have a Makefile in the src dir and build inside there.
  • of course we can easily move the oasis files to another directory, although I'll say this is fairly uncommon in most modern OCaml projects. The _oasis file is a source of both programmatic and semantic information for many people, similar to a bower.json.
  • As for it being a hammer, or etc:
    • for many people, me included, oasis is just easier, and it's easier to add extra hacks/modules if we want to. I use it in every new project now; it's just the best. I understand you don't support opam, but that's almost a given in the OCaml community these days, and if I want to add an extra module, mess around, it's as simple as adding the package name in the BuildDepends and rebuilding the makefiles. This will not be the case at all for hand-crafted Makefiles. If, for example, I wanted to use the base64 package, or the jsonm package for hacking about, it takes no effort whatsoever with oasis, and this is generally considered a good thing for experimentation.
    • agreed it can add extra headaches, but having several OCaml projects (rdr, elf2json, algos, they can become fairly nightmarish to maintain the build, even when you're doing simple stuff only with the standard lib.
    • I'm very interested in working with the OCaml version and using that as an alternative implementation on systems, or even better, using the very nice types and syntax definitions in the prototype as a library, and I'm sure others will too. For example, I'd like to integrate its libs into my rdr project in particular, when things are finalized. But to maintain the implementation as a library with hand-written Makefiles will not be in any way remotely maintainable (I've tried), so another plus for getting a nice, best-practice community supported build system underfoot.

That being said, if there is unanimous desire to not add an extra OCaml friendly build system which does not interfere in any way with the current system, which honestly, I'd find utterly baffling, then I suppose that's all there is to it, and no hard feelings :) I just thought I'd add a feature which I and many others have found extremely useful, but if this is just throwaway code (which would be sad) then we needn't bother.

@rossberg
Copy link
Member

  • agreed if you support oasis as an alternative build system, the
    README should be rewritten, or an oasis section added. Again, it's
    unconventional to have a Makefile in the src dir and build inside there.

I would actually welcome moving the Makefile, and not building inside src.
Just was a bit of extra work that I wasn't keen on doing so far.

Ideally, we would have a Makefile which you can tell what approach to use
(e.g., "make" vs "make with-oasis"? The latter could delegate to the Oasis
Makefile somewhere else).

  • of course we can easily move the oasis files to another directory,
    although I'll say this is fairly uncommon in most modern OCaml projects.
    The _oasis file is a source of both programmatic and semantic
    information for many people, similar to a bower.json.

Yes, I have more issues with the bunch of scripts and (large) auxiliary ML
files.

So, I'd be willing to include Oasis support into the repo if (1) it is not
a requirement, (2) it is minimally intrusive and does not affect anything
else, (3) instructions are clear on pointing out that this is optional, and
not "officially" supported (i.e., we don't have to worry about it, or
update config files ourselves). In other words, if it's guaranteed to not
get in the way and not pollute stuff.

WDYT?

/Andreas

@m4b
Copy link
Author

m4b commented Aug 25, 2015

Sorry for the delay!

So I messed around with getting oasis to build correct build files using a relative, non-standard directory, and didn't try much harder, since it's a manufactured problem, and to steal a term from Jean-Yves Girard, I'm not really interested in the bureaucracy of an invented build system syntax problem.

Nevertheless, I think an easy compromise is to revert to the simple case where the _oasis file sits at the base directory (it being metadata), and that's it.

That way:

(1) there are no cluttering files
(2) it's entirely optional
(3) the README will give instructions for creating an optional, orthogonal, oasis build (oasis setup), for those who want to use oasis, to play around with using external packages, etc.

If this is acceptable, I'll repush these changes.

@rossberg
Copy link
Member

Sounds good to me, thanks!

On 25 August 2015 at 21:08, m4b notifications@github.com wrote:

Sorry for the delay!

So I messed around with getting oasis to build correct build files using a
relative, non-standard directory, and didn't try much harder, since it's a
manufactured problem, and to steal a term from Jean-Yves Girard, I'm not
really interested in the bureaucracy
https://en.wikipedia.org/wiki/Linear_logic#Proof_nets of an invented
build system syntax problem.

Nevertheless, I think an easy compromise is to revert to the simple case
where the _oasis file sits at the base directory (it being metadata), and
that's it.

That way:

(1) there are no cluttering files
(2) it's entirely optional
(3) the README will give instructions for creating an optional,
orthogonal, oasis build (oasis setup), for those who want to use oasis,
to play around with using external packages, etc.

If this is acceptable, I'll repush these changes.


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

@sunfishcode sunfishcode modified the milestone: Meta Mar 14, 2016
@m4b m4b closed this May 17, 2016
littledan pushed a commit to littledan/spec that referenced this pull request Mar 4, 2018
eqrion pushed a commit to eqrion/wasm-spec that referenced this pull request Jul 18, 2019
eqrion pushed a commit to eqrion/wasm-spec that referenced this pull request Sep 18, 2019
ErikMcClure pushed a commit to innative-sdk/spec that referenced this pull request Jun 15, 2020
rossberg referenced this pull request in effect-handlers/wasm-spec Feb 15, 2021
dhil pushed a commit to dhil/webassembly-spec that referenced this pull request Mar 2, 2023
Describe correct tail call behavior across modules
dhil added a commit to dhil/webassembly-spec that referenced this pull request Jan 25, 2024
Merge with WebAssembly/spec, WebAssembly/function-references, and WebAssembly/gc
rossberg pushed a commit that referenced this pull request Mar 7, 2024
rossberg added a commit that referenced this pull request Jul 3, 2024
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