garrigue / lablgtk

LablGTK 2 and 3: an interface to the GIMP Tool Kit
https://garrigue.github.io/lablgtk
Other
89 stars 40 forks source link

[build] Add support for building with Dune. #14

Closed ejgallego closed 5 years ago

ejgallego commented 5 years ago

We add preliminary support for building with Dune to lablgtk2; this seems to work well and does provide some advantages:

I have used a more "explicit" dune-file style for now, which while a bit more verbose, should be easier to understand.

While the PR is preliminary, it has been tested with CoqIDE and works fine.

One strong motivation to do this PR was to help the GTK 3 porting effort and this provides a seamless developer experience.

Deferred to subsequent PRs:

ejgallego commented 5 years ago

@herbelin this may be of interest to you, as you can do in Coq master:

$ ln -s $path_to_lablgtk2
$ make -f Makefile.dune voboot
$ dune build # ad infinitum

and get a composed build.

garrigue commented 5 years ago

I'm not opposed to add dune support if it doesn't come in the way for my traditional Makefile based approach. Or does dune completely replace all of the Makefiles? This seems surprising, since the logic here is not that simple.

Also, please remove all code changes from this PR. This should be only about dune support, orthogonal to the code base.

ejgallego commented 5 years ago

I'm not opposed to add dune support if it doesn't come in the way for my traditional Makefile based approach.

Thanks @garrigue , indeed it works fully separated from the Makefiles so they shouldn't interfere.

Or does dune completely replace all of the Makefiles? This seems surprising, since the logic here is not that simple.

The current version does build a fully working lablgtk, at least in Linux. So indeed, that's one of the big advantages Dune brings, it can simplify a lot the build system, and even takes care of the generation of .install files [opam in the future, etc..]

Also, please remove all code changes from this PR. This should be only about dune support, orthogonal to the code base.

Sure, I can split them to a different PR; the thing is that the current makefiles don't have logic to build the examples, right? When building the examples I noticed quite a few of them were segfaulting, so I've fixed them.

I will amend the PR. and maybe add Travis support?

garrigue commented 5 years ago

If it can compile everything, then this sounds really interesting. Maybe this should really replace the Makefile. There are still some questions:

ejgallego commented 5 years ago
  • How to integrate with configuration? Note that the current approach of automatically detecting what is already installed is not necessarily good, as it doesn't work well with any package manager. Detection and configuration could be seen as different tasks. And ideally, the extra libraries should be compilable independently.

Dune should be pretty good for this as it was designed for this particular sue case, even if still needs some tweaks; for detection we use Configurator which indeed seems to do a nice job. For legacy reasons, packages are defined by adding a package.opam file at the root [this will be obsoleted soon in favor of dune-project] Then each package is an independent entity, but of course they can be composed.

  • Windows and MacOS support. The current Makefile works on Windows too, and this was important. But maybe it matters less now.

Dune is pretty portable but for sure we need to adapt it to Windows, too bad I am not a windows users. But that should work in the end.

  • Having to write the props rules by hand seems wrong. Isn't there a way to do a better job?

Dune is "more static" than make in this sense, as to avoid some problems related to dynamic rules I think, this will be improved in the future, for now the solution is write a small ocaml program to generate the rules, that's managed pretty well by Dune itself.

  • Also, does it really do the same thing as the Makefile. For instance, there are a few instance in the Makefile where the name of a library and that of its C stubs differ. We have lablgtk.cma and liblablgtk2.a. The dune rules let me think that the generated library will be called lablgtk2.cma.

For sure it takes some different internal linking choices. The library name is controlled by the (name ...) parameter, stubs don't matter so much as Dune is able to infer all the correct dependencies.

I tested vs CoqIDE and it worked pretty well, so at least for that it works. You place Coq master and this PR together and Dune treats them as the same project, that is nice IMHO.

ejgallego commented 5 years ago

@garrigue , I have amended the PR to take your comments into account. See the new TODO list in the description.

IMHO this is ready to merge once we add Travis testing [if you are interested] and we see how to better organize the libraries. It is not entirely yet clear to me how you would like to organized the libraries, Dune does provide two methods:

garrigue commented 5 years ago

For integration with package managers, the mandatory approach seems better.

However, this still leaves us with the problem of how to integrate this with the current build system. Since the result of the build is going to be different from what you obtain with make, this is not just an alternative compilation method, but a different package. Does it mean we only propose it for integrated builds? Or is it going to be completely transparent if one uses ocamlfind?

ejgallego commented 5 years ago

However, this still leaves us with the problem of how to integrate this with the current build system. Since the result of the build is going to be different from what you obtain with make, this is not just an alternative compilation method, but a different package. Does it mean we only propose it for integrated builds? Or is it going to be completely transparent if one uses ocamlfind?

Oh, I see what you mean. We produce lablgtk2.cma. Well, as to ease package maintenance I'd recommend we keep the name lablgtk2 for the directory [and the main cma] but we also ship lablgtk.cma, this is very easy to do with a copy rule, and should make you happy I think.

Indeed ocamlfind clients won't notice that [so that's why CoqIDE works for example] but for some apps such as Unison we may find problems [even if I did a patch some time ago to enable it to use ocamlfind].

I recommend we use lablgtk3.cma for lablgtk3.

ejgallego commented 5 years ago

New version pushed, changes:

IMHO, modulo small nits this seems close to be ready to get in.

garrigue commented 5 years ago

I see nothing about generating the lablgtk2 script in your code. Or is there some hidden trick?

ejgallego commented 5 years ago

I see nothing about generating the lablgtk2 script in your code.

Umm, this script is not needed usually with Dune [you can just do dune utop src]

Anyways, if we have to generate it no problem in adding a rule.

ejgallego commented 5 years ago

Also note that dune exec foo will run foo in the right "install" context [this is a key point of Dune], thus you can just use ocamlfind in the script and don't generate it.

ejgallego commented 5 years ago

Example:

dune exec -- ocamlfind list | grep gtk
lablgtk2            (version: n/a)
lablgtk2-gl         (version: n/a)
lablgtk2-glade      (version: n/a)
lablgtk2-gnome      (version: n/a)
lablgtk2-gnomecanvas (version: n/a)
lablgtk2-gspell     (version: n/a)
lablgtk2-rsvg       (version: n/a)
lablgtk2-sourceview2 (version: n/a)
ejgallego commented 5 years ago

Travis file added, see build here https://travis-ci.org/ejgallego/lablgtk/builds/463469671

In order to see the build here you'll have to enable travis in https://travis-ci.org/garrigue/lablgtk

This should be very useful to test the Dune build in OSX and Windows; I'd be great to hear back from users on these platforms.

ejgallego commented 5 years ago

Last thing remaining would be to update the opam files so they have the correct depexts, etc...

Can do that if there is green light. Note that this would enable an automatic release workflow with dune-release for example.

garrigue commented 5 years ago

OK, I'm getting a bit scared, because I don't have enough knowledge to judge what is right here. My impression was that lablgtk is more or less moribund, and we are only trying to maintain it because a number of projects depend on it. So is it really worth doing extensive changes to the build infrastructure? My impression is that what you do is not going to converge to what is done currently, possibly resulting in incompatible build procedures, so it would probably be better to completely deprecate the Makefile. I'm not necessarily against that, but we currently have another priority which is getting LablGtk3 included in the next release of Debian, mainly for CoqIDE. So my conclusion is that, if you can convince the Coq team and the Debian package managers that they are better off with using dune, then I'm happy with doing the change now, otherwise I would prefer waiting until after this lablgtk3 problem is solved.

ejgallego commented 5 years ago

So is it really worth doing extensive changes to the build infrastructure?

In my opinion yes, moreover when this change is not invasive, and helps a lot with a critical component related to maintenance such as the build system, including automatic opam releases, etc...

My impression was that lablgtk is more or less moribund, and we are only trying to maintain it because a number of projects depend on it.

Well, that was the impression of some too, however some people [including of course @herbelin ] have stepped forward and offered to maintain it, so IMHO this should help them, and I did the porting when I saw @herbelin working on the port as to possibly help him.

My impression is that what you do is not going to converge to what is done currently, possibly resulting in incompatible build procedures,

I am not sure I understand that point, both build systems produce compatible binaries and if not that's a bug. How to do the packaging is a different matter tho, but I have followed what they do in Debian more or less.

we currently have another priority which is getting LablGtk3 included in the next release of Debian, mainly for CoqIDE.

Well this should be quite orthogonal to that, barring the opinion of the Debian devs on the build system. As for CoqIDE, the primary motivation of this patch is indeed to allow seamless development of CoqIDE; also Unison will have to be ported and having an integrated build system does help.

if you can convince the Coq team and the Debian package managers that they are better off with using dune, then I'm happy with doing the change now, otherwise I would prefer waiting until after this lablgtk3 problem is solved.

In Coq I'm pretty sure we don't care as we use ocamlfind and thus in regular builds, we won't notice; as for the integrated build, sure I do prefer it, @herbelin have you tried it, what do you think? [I guess no as the gtk3 patch is not up to date].

As for Debian I cannot say, using Dune should make the packing trivial for them but that's up to them to comment.

garrigue commented 5 years ago

I've just finished release the beta of lablgtk3 on opam. I'm still using the manual approach, even more painful as I'm uploading the tar files on ocamlcore. (Still simple enough; the pain is to have to edit the description page manually.) What took me a long time was getting the compatibility right (ubuntu 16-04 is stuck with gtk-3.18...) Does travis testing give this kind of checking earlier or does one still need to do a pull request on opam-repository to access the farm? For this kind of package with external dependencies, this is more important than checking with multiple versions of ocaml.

ejgallego commented 5 years ago

Does travis testing give this kind of checking earlier or does one still need to do a pull request on opam-repository to access the farm?

Sure, Travis can check against any distro/os combo we want, including OSX and Windows.

ejgallego commented 5 years ago

Does travis testing give this kind of checking earlier or does one still need to do a pull request on opam-repository to access the farm?

Sure, Travis can check against any distro/os combo we want, including OSX and Windows.

It can even deploy the packages automatically if we want.

I'm uploading the tar files on ocamlcore.

dune-release / topkg do upload to github and take care of that.

herbelin commented 5 years ago

My impression was that lablgtk is more or less moribund, and we are only trying to maintain it because a number of projects depend on it.

I can confirm what @ejgallego said. As far as I'm concerned, I'm a priori willing to contribute to maintain lablgtk3 as long as it is needed for developing CoqIDE (and thus at least as long as there is no better alternative to CoqIDE). At some time I was afraid that gtk would be inherently limited in terms of look-and-feel but I'm now discovering e.g. gtksourceview style files, which seem to be convenient for providing "modern" UI styles.

Incidentally, when talking about lablgtk2 around me, I realized that it is not uncommon in our community to hear about people who developed for private applications a lablgtk2-based interface (e.g. last in time, Adrien G. from IRIF).

sacerdot commented 5 years ago

Matita is also heavily based on lablgtk and I also volunteer to contribute to maintain lablgtk3 for the time being.

garrigue commented 5 years ago

Then, couldn't we start by merging the dune support in the lablgtk3 branch. This is actually easier, since there are fewer external dependencies, and the users at this point are only experienced developers? Sorry to seem hesitating, @ejgallego, but I'm trying to maximize the usefulness for developers while minimizing incompatibilities for old packages.

ejgallego commented 5 years ago

The nice thing about adding Dune support for GTK2 [and the reason I didn't directly go to GTK3] was indeed to allow a composable workflow for porting, which allows to work both in matita and gtk [for example].

That being said, I will rebase the Dune lablgtk3 version when I get a free minute. [I did submit a PR but to Hugo's repo]

Note that the Dune build should be 100% compatible unless a bug is here.

ejgallego commented 5 years ago

I've added dune-release support, so that should allow us to do automatic testing for example wrt revdeps of the OCaml packages.

IMHO this is still useful for lablgtk2, but gtk3 patch incoming just in case.

ejgallego commented 5 years ago

I am closing this PR, as dune support for gtk3 has landed.

The motivation of this PR was to ease porting applications, but I am not sure it is worth the effort anymore. Anyways, if someone could get a benefit from this please ping me here and I'll reopen and backport the improvements of the gtk3 branch.