ejgallego commented 5 years ago

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

compositional build with other developments, for example CoqIDE,
it allows for an easier layout / library reorganization if wished,
automatic management of install targets, opam install file, META, stubs,
automatic generation of documentation with odoc.

Release / usage instructions are placed in README.dune.md.

The dune files use an "explicit" style. While a bit more verbose than other setup, it is very easy to understand.

Note we pass -Wno-deprecated-declarations to the C compiler as to avoid a super-verbose build. We also disable a few OCaml warnings.

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

Following discussion on caml-list, we provide one opam package for each C library. This is IMO more consistent with what packaging systems do and allows for better dependencies in the OPAM world. For example, Coq can depend now on lablgtk3-sourceview3.

We still depend on the conf-packages as to get depext information, but it is not necessary as the dune-based config script also does C version checks.

Things that would be worth to do in future PRs are:

linting: the large amount of OCaml and C warnings should be addressed
OSX and Win32 support, Travis CI for these platforms
make all examples work again
cleanup makefiles, remove old make system. To be done after we add OSX/Win32 testing
generate varcc / propcc rules using promote

This is based on #14

ejgallego commented 5 years ago

@garrigue IMHO this is a candidate for inclusion if you want; it supports full automated releases to OPAM and CI.

I will do a pass later this week to document / cleanup things better.

garrigue commented 5 years ago

I'm definitely looking forward to merge this. Just let me know when it's ready.

ejgallego commented 5 years ago

@garrigue TTBOMK this is ready for review and merge.

ejgallego commented 5 years ago

Oh, I've now realized that this could be a great opportunity to set (wrapped true) in lablgtk3 [this introduces a top-level wrapper module so Gtk becomes Lablgtk.Gtk etc...]

What do you think?

garrigue commented 5 years ago

Used (wrapped true) would be nice , but it means that we deprecate compiling with make (the build result will be incompatible). Also, how would it behave for the other sub packages (lablgtksourceview, etc...)?

ejgallego commented 5 years ago

We could use the compat mode of wrapped, that should still work with make.

A different question is if make compatibilith is needed. Dune should work fine in other plataforms.

For the sub-packages, wrapped uses the name field so you'd have a Gtksourceview3 wrapper etc...

ejgallego commented 5 years ago

Dependencies fixed, thanks @garrigue ; IMHO we still have three open questions:

Should we enable (wrapped true) (or transition) before merging?
We should definitively build a few examples as to have a sanity check? Which ones should work? I will update the applications / examples dunefile once we reach this.
Windows builds: IMHO we should add a Windows CI job. In Coq we use Azure pipelines and seems to work pretty well. Doing that should allow us to deprecate the makefiles. Could you give me a hint on what is the intended Windows GTK environment for lablgtk3? Is using depext OK?

sacerdot commented 5 years ago

Some examples do work. For example nd.ml works and it uses cairo and pango as well. Also the two examples into glade/ work and they use the glade framework. I have repaired a few more myself. There is however no up-to-date list of what examples work. There is an issue with a check-boxed list, but I think it is not updated. It would have been better to move them to a broken_example directory and restore them one-by-one once they worked.

Note: compiling != working as expected (e.g. good layout), so the test of what examples work or not cannot be automated.

ejgallego commented 5 years ago

Ok, I've added the working examples to the Travis build. Note that indeed some examples compile but don't work, I have a patch addressing this, will submit in a separate PR as requested by @garrigue

garrigue commented 5 years ago

I didn't try installing lablgtk3 on Windows. Do Coq people have some experience with that? For lablgtk2 this involved installing by hand some packages, which is hard to make automatic, and there was a binary distribution, with a special procedure to recompile the ocaml files to keep in sync with the compiler without having to install the full environment needed to compile the C files.

For wrapping, using transition would be fine. At least it allows to write the examples in a style that doesn't require the library to be wrapped. I'm still a bit concerned on how well dune works on windows when you have to cope with lots of C dependencies (but then I'm not sure I'm willing to try very hard to support Windows either :-)

ejgallego commented 5 years ago

In my short experience Dune tends to work very well on Windows; indeed we have experience in the Coq land with GTK + Windows, and there are a couple of options. I guess I will prepare a reasonably standard setup and add Windows CI; this should allow to stop depending on Make I think.

ejgallego commented 5 years ago

We got confirmation from @herbelin that the Dune build works on OSX [in fact the make-based system was failing to properly invoke pkg-config], thus I am adding an Azure job.

Regarding windows I will add a job but I am still thinking what's the preferred way to install GTK https://discuss.ocaml.org/t/preferred-ocaml-windows-environments-mainly-for-continuous-integration/3299

ejgallego commented 5 years ago

CoqIDE master now builds against this branch; we will merge the GTK3 version to master as soon as this is merged and a new opam version is released [with the updated sourceview package name]

OSX and Linux do run fine, Windows support should be easy.

ejgallego commented 5 years ago

c.f. https://github.com/coq/coq/pull/9279

garrigue commented 5 years ago

Should I merge now, or are we still waiting for something?

garrigue commented 5 years ago

Merged. The extra work can go in another PR.

ejgallego commented 5 years ago

Great, thanks! For Coq the only thing remaining is to upload a new version to OPAM, the README.dune file details the workflow, which tends to be pretty straightfoward.

BTW, the PR you merged includes support for OSX testing on Azure, see https://dev.azure.com/ejgallego/ejgallego/_build/results?buildId=7 so maybe you would like to enable Azure for your repos?

garrigue commented 5 years ago

What is azure, and do I enable it?

Also, on my MacOS machine, where I installed gtk3 @3.22.30_0+quartz using macports, I get the following error with dune build:

  ocamlc src/ml_gdk.o (exit 2)

(cd _build/default/src && /Users/garrigue/.opam/4.07.1/bin/ocamlc.opt -g -I /Users/garrigue/.opam/4.07.1/lib/cairo2 -I /Users/garrigue/.opam/4.07.1/lib/ocaml/threads -ccopt -D_REENTRANT -ccopt -I/opt/local/include/gtk-3.0 -ccopt -I/opt/local/include/gio-unix-2.0/ -ccopt -I/opt/local/include/cairo -ccopt -I/opt/local/include -ccopt -I/opt/local/include/pango-1.0 -ccopt -I/opt/local/include/harfbuzz -ccopt -I/opt/local/include -ccopt -I/opt/local/include/pango-1.0 -ccopt -I/opt/local/include/fribidi -ccopt -I/opt/local/include/atk-1.0 -ccopt -I/opt/local/include/cairo -ccopt -I/opt/local/include/pixman-1 -ccopt -I/opt/local/include -ccopt -I/opt/local/include/ossp -ccopt -I/opt/local/include/freetype2 -ccopt -I/opt/local/include/libpng16 -ccopt -I/opt/local/include -ccopt -I/opt/local/include/gdk-pixbuf-2.0 -ccopt -I/opt/local/include/libpng16 -ccopt -I/opt/local/include -ccopt -I/opt/local/include/glib-2.0 -ccopt -I/opt/local/lib/glib-2.0/include -ccopt -I/opt/local/include -ccopt -Wno-deprecated-declarations -o ml_gdk.o ml_gdk.c) ml_gdk.c:32:10: fatal error: 'gdk/gdkx.h' file not found

include <gdk/gdkx.h>

     ^~~~~~~~~~~~

1 error generated.

Here is the command line when I compile with make, which succeeds. Note the -DHAS_GTKQUARTZ, which avoids including gdkx.h.

ocamlc -c -ccopt '-DG_DISABLE_ASSERT -DG_DISABLE_CAST_CHECKS -fno-unwind-tables -D_REENTRANT -I/opt/local/include/gtkspell-3.0 -I/opt/local/include/enchant-2 -I/opt/local/include/gtksourceview-3.0 -I/opt/local/include/libxml2 -I/opt/local/include/gtk-3.0 -I/opt/local/include/gio-unix-2.0/ -I/opt/local/include/cairo -I/opt/local/include -I/opt/local/include/pango-1.0 -I/opt/local/include/harfbuzz -I/opt/local/include -I/opt/local/include/pango-1.0 -I/opt/local/include/fribidi -I/opt/local/include/atk-1.0 -I/opt/local/include/cairo -I/opt/local/include/pixman-1 -I/opt/local/include -I/opt/local/include/ossp -I/opt/local/include/freetype2 -I/opt/local/include/libpng16 -I/opt/local/include -I/opt/local/include/gdk-pixbuf-2.0 -I/opt/local/include/libpng16 -I/opt/local/include -I/opt/local/include/glib-2.0 -I/opt/local/lib/glib-2.0/include -I/opt/local/include -Wno-deprecated-declarations -imacros ml_domain.h -Iocamlfind query cairo2 -O -DHAS_GTKQUARTZ' -verbose ml_gtk.c

ejgallego commented 5 years ago

What is azure, and do I enable it?

This is the build service by MS, it provides pretty nice OSX and Windows runners. You can enable it going to the github marketplace. However I am thinking that maybe using Travis for OSX is a better idea, so please wait on this.

Also, on my MacOS machine, where I installed gtk3 @3.22.30_0+quartz using macports, I get the following error with dune build:

Sorry, I was in the process to document the OSX build when the PR got merged. You have to do (as seen on the CI files):

export LABLGTK_EXTRA_FLAGS="-DHAS_GTKQUARTZ"
dune build

I am thinking on how to automate this, likely having dune-config generate the flags depending on the platform.

garrigue commented 5 years ago

OK. It would be nicer to have the flag automatically added if pkg-config --libs/--cflags gtk+-quartz-3.0 succeeds. And normally you should also use the flags they return.

garrigue commented 5 years ago

Well, actually the flags are the same as for gtk+-3.0, which explains why your hack works.

ejgallego commented 5 years ago

I see, I was not aware such a package existed on OSX. I will sleep over it to see what'd be the best solution.

ejgallego commented 5 years ago

But indeed I guess that we can have dune-config writing a few flags if certain conditions are met, including platform-specific ones.

garrigue commented 5 years ago

I tried doing dune flag -a 3.0.beta4 && dune-release. Two problems:

The generated archive name is lablgtk3-3.0.beta4. This is unfortunate, but I suppose I can live with that.

I get the following error:

dune-release: [ERROR] ./CHANGES: Could not parse change log.
dune-release: [ERROR] run ['dune-release' 'publish' '--verbosity' 'warning']:
                  exited with 3

ejgallego commented 5 years ago

I tried doing dune flag -a 3.0.beta4 && dune-release.

You mean git tag -a right?

The generated archive name is lablgtk3-3.0.beta4. This is unfortunate, but I suppose I can live with that.

Why is that a problem? What should the name be?

I get the following error:

Umm, I cannot reproduce when I do dune-release publish.

ejgallego commented 5 years ago

Can you try to call each step of dune-release separatedly. As outlined in the readme:

dune-release distrib       # Create the distribution archive
dune-release publish       # Publish it on the WWW with its documentation
dune-release opam pkg      # Create an opam package
dune-release opam submit   # Submit it to OCaml's opam repository

garrigue commented 5 years ago

The first line is ok, but I get this error with dune-release publish:

dune-release: [ERROR] ./CHANGES: Could not parse change log.

Despite that dune-release opam pkg works, but get again the same error with dune-release opam submit.

I have pushed all my local changes for you to see.

ejgallego commented 5 years ago

I can't reproduce still, what version of dune-release do you have?

I have 1.1.0

ejgallego commented 5 years ago

Does dune-release publish --dry-run work? Is your github token correctly configured?

For reference that's my output with dry run:

$ dune-release publish --dry-run

=> must exists _build/lablgtk3-3.0.beta4.tbz
=> chdir _build/
   [in _build]
=> exec: tar -xjf lablgtk3-3.0.beta4.tbz
=> chdir _build/lablgtk3-3.0.beta4
   [in _build/lablgtk3-3.0.beta4]
=> exec: dune build -p lablgtk3-sourceview3,lablgtk3 @doc
        odoc _doc/_odoc/lib/lablgtk3/glib.odoc
File "src/glib.mli", line 26, characters 4-11:
unknown tag '@gtkdoc'
[...]
-: exec: git --git-dir .git fetch git@github.com:${user}/${repo}.git gh-pages
-: exec: git --git-dir .git rev-parse FETCH_HEAD
-: exec: git --git-dir .git branch -f gh-pages ${fetch_head}
=> rmdir _build/lablgtk3-3.0.beta4/_build/gh-pages
=> exec:
     git --git-dir .git clone --local .git   _build/lablgtk3-3.0.beta4/_build/gh-pages
=> chdir _build/lablgtk3-3.0.beta4/_build/gh-pages
   [in _build/lablgtk3-3.0.beta4/_build/gh-pages]
-: exec: git --git-dir .git checkout gh-pages
-: rmdir _build/lablgtk3-3.0.beta4/_build/gh-pages/${dir}
[...]
=> read /home/egallego/external/lablgtk/_build/lablgtk3-3.0.beta4/_build/default/_doc/_html/index.html
-: write ${dir}/index.html
-: exec: git --git-dir .git add ${dir}
-: exec: git --git-dir .git commit -m Update lablgtk3 doc to 3.0.beta4.
-: exec: git --git-dir .git push
-: exec:
     git --git-dir .git push git@github.com:${user}/${repo}.git gh-pages:gh-pages
-: rmdir _build/lablgtk3-3.0.beta4/_build/gh-pages
Published documentation for lablgtk3 3.0.beta4 in directory ${dir} of
gh-pages branch
=> must exists _build/lablgtk3-3.0.beta4.tbz
-: exec: git --git-dir .git rev-parse --verify 3.0.beta4
-: exec: git --git-dir .git rev-parse --verify 3.0.beta4
-: exec:
     git --git-dir .git push --force git@github.com:garrigue/lablgtk.git 3.0.beta4
=> read /home/egallego/.config/dune/github.token
=> read /home/egallego/.config/dune/github.token
-: exec:
     curl -L -s -S -K - -D - --data   { "tag_name" : "3.0.beta4", "body" : "CHANGES:\n\nGPack.notebook#append_page and GPack.notebook#insert_page :\n        return int instead of unit\nGAction.action_group#add_action :\n    removed `accel' optional parameter\nGBin.alignment :\n    add `padding' optional parameter\nGEdit.combo_box, combo_box_entry, combo_box_text, combo_box_entry_text :\n    add `active' optional parameter\nGMain.Idle.add, Glib.Idle.add :\n    add a `prio' optional argument\nGMain.Io.add_watch, Glib.Io.add_watch :\n    use a list of `conditions' insted of a single one\nGTree.column_list : \n    replace method `kinds' by `types'\nGWindow.message_dialog : \n    add `use_markup' optional argument\nGlib :\n    remove exception `Critical' and Glib.Message.handle_criticals\n    remove Glib.Message.set_print_handler\n    changed type of Glib.Message.set_log_handler\nGobject.Data :\n    boxed, unsafe_boxed, unsafe_boxed_option : take an additional `g_type' parameter\n    replace `get_fundamental' by `get_type'\nGnoDruid : \n    replace druid_page by druid_page_skel\n" }   https://api.github.com/repos/releases/lablgtk3-3.0.beta4/releases
=> read /home/egallego/.config/dune/github.token
-: exec:
     curl -L -s -S -K - -H Content-Type:application/x-tar --data-binary   @_build/lablgtk3-3.0.beta4.tbz   https://uploads.github.com/repos/releases/lablgtk3-3.0.beta4/releases/0/assets?name=lablgtk3-3.0.beta4.tbz

fined")

garrigue commented 5 years ago

Same version here. I've just done opam update. --dry-run doesn't go very far:

cassis-lablgtk3> dune-release publish --dry-run
=> must exists _build/lablgtk3-3.0.beta4.tbz
dune-release: [ERROR] ./CHANGES: Could not parse change log.

How exactly are you tagging? I just did: git tag -f -a -m "lablgtk3.0.beta4" 3.0.beta4

ejgallego commented 5 years ago

So bizarre, I tagged in the same way as you, and it works here.

I have my ~/config/dune files in order dune-release help files.

I dunno what could be. I think you can workaround this problem using dune-release publish -m "message for the release", but it is very strange that it doesn't fail here :/

ejgallego commented 5 years ago

You can make the changelog parsable easy tho, just add a # at the beginning, such as in https://raw.githubusercontent.com/samoht/dune-release/master/CHANGES.md

garrigue commented 5 years ago

OK, I've now updated all the examples, and I think I'm ready for a release. I'm working on 651a73df73a. Both dune-release distrib and dune-release publish --dry-run work fine, but dune-release publish fails with

dune-release: [ERROR] Could not find release id in response:
                      HTTP/1.1 100 Continue

                      HTTP/1.1 404 Not Found
                      Date: Mon, 11 Feb 2019 06:14:42 GMT
                      Content-Type: application/json;
                      charset=utf-8
                      Content-Length: 120
                      Server: GitHub.com
                      Status: 404 Not Found
                      X-RateLimit-Limit: 5000
                      X-RateLimit-Remaining: 4955
                      X-RateLimit-Reset: 1549865955
                      X-OAuth-Scopes: public_repo
                      X-Accepted-OAuth-Scopes: repo
                      X-GitHub-Media-Type: github.v3;
                      format=json
                      Access-Control-Expose-Headers: ETag, Link, Location,
                      Retry-After, X-GitHub-OTP, X-RateLimit-Limit,
                      X-RateLimit-Remaining, X-RateLimit-Reset,
                      X-OAuth-Scopes, X-Accepted-OAuth-Scopes,
                      X-Poll-Interval,
                      X-GitHub-Media-Type
                      Access-Control-Allow-Origin:
                      *
                      Strict-Transport-Security: max-age=31536000;
                      includeSubdomains; preload
                      X-Frame-Options: deny
                      X-Content-Type-Options: nosniff
                      X-XSS-Protection: 1; mode=block
                      Referrer-Policy: origin-when-cross-origin,
                      strict-origin-when-cross-origin
                      Content-Security-Policy: default-src
                      'none'
                      X-GitHub-Request-Id:
                      C9B0:1E56:313D560:3BE51BA:5C6112D0

                      {
                        "message": "Not Found",
                        "documentation_url":
                      "https://developer.github.com/v3/repos/releases/#create-a-release"
                      }
                      .

Any hint?

ejgallego commented 5 years ago

Last time I saw it was due to your token not being setup correctly.

garrigue commented 5 years ago

I'm at a complete loss. I've generated a new token just in case, to no avail. And, if you read the message, the X-Ratelimit seems to imply that OAuth authorization is successful. Or is there some way to test?

ejgallego commented 5 years ago

Umm, is the token added in ~/.config/dune/github.token ? I had the same error and indeed it drove me crazy but was a problem with the token after all.

The release id is missing because the lack of token means it is not created. It looks like dune-release is missing some error handling code for the RPC call indeed.

garrigue commented 5 years ago

I've checked with a broken token, and I get a "Bad credentials" message in that case. So this is not the problem. It seems that dune-release tries to access a wrong url?

ejgallego commented 5 years ago

The release is not created: https://api.github.com/repos/garrigue/lablgtk/releases

garrigue commented 5 years ago

I should create it by hand? I thought this was automatic.

garrigue commented 5 years ago

Wait a minute. There is a release: https://github.com/garrigue/lablgtk/releases

It seems that it was created this morning. The problem seems to be with the documentation.

ejgallego commented 5 years ago

Oh indeed you may have to remove the release and re-run the script.

ejgallego commented 5 years ago

Oh that's the documentation url indeed :man_facepalming:

ejgallego commented 5 years ago

You can do dune-release publish distrib to skip docs; the URLs seem wrong but I am not sure about the right fix.

garrigue commented 5 years ago

OK. I could do the next two steps successfully (opam pkg and opam submit). So this is released, but the documentation is not available. No idea what to do about it.

ejgallego commented 5 years ago

Note that the release is still not created.

https://api.github.com/repos/garrigue/lablgtk/releases

This can make the opam build fail. Even if github shows tags and releases in the same stop, they are not the same.

I dunno what the problem is, I'm trying myself.

garrigue commented 5 years ago

Also, dune-release opam submit copied the whole CHANGES file to the PR, and it has more than 1700 lines...

garrigue commented 5 years ago

Oops, it tries to get the files from ocamlcore:

[ERROR] The sources of the following couldn't be obtained, aborting:
          - lablgtk3.3.0.beta4:
            http://lablgtk.forge.ocamlcore.org/releases/lablgtk3-3.0.beta4.tbz (curl: code 404 while downloading http://lablgtk.forge.ocamlcore.org/releases/lablgtk3-3.0.beta4.tbz)
          - lablgtk3-gtkspell3.3.0.beta4:
            http://lablgtk.forge.ocamlcore.org/releases/lablgtk3-3.0.beta4.tbz (curl: code 404 while downloading http://lablgtk.forge.ocamlcore.org/releases/lablgtk3-3.0.beta4.tbz)

garrigue commented 5 years ago

Is there a way to fix that without creating a new PR? https://github.com/ocaml/opam-repository/pull/13437

ejgallego commented 5 years ago

Ha, indeed you are right, setting the homepage to something other than the github project is broken in recent versions :( :(

garrigue commented 5 years ago

OK, I think I'm almost there. By editing the files online, I could have lablgtk3 and lablgtk3-gtksourceview3 compile at opal-repository. gtkspell3 lacks some depexts. Looks like I hadn't really ensured it compiled in the previous lablgtk3 release either (it works only if gtkspell3 is already installed). I'll see about that later.

Now, have to find a way to automate that next time, because it's painful. However, not as much as having to update the package itself many times because it won't compile over there...

garrigue / lablgtk

[build] Add support for building and releasing with Dune. #38

include <gdk/gdkx.h>