search

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

No generated files #89

Closed olafhering closed 4 years ago

olafhering commented 4 years ago

What is the advantage of doing this?

Sorry, I was under the impression that no generated files in SCM is common knowledge.

As I see it it only causes pain to users. Also, CI is broken.

You are right. I missed a few places where camlp4 was referenced, not sure why. Also the opam files are no update to refer to camlp5.

The only failure I have seen seems to be unrelated to this change. There is also no push notification about CI failures, one has to poll for results.

olafhering commented 4 years ago

Not sure how to resolve the "change requested" thing. From my side the commit is done.

ejgallego commented 4 years ago

Hi @olafherring,

Sorry, I was under the impression that no generated files in SCM is common knowledge.

Actually this is far from an universal law; it is not uncommon to commit configure files, parsers, etc... In this case I do support the current solution of having the generated parser in the source tree:

Thus my opinion is that they should remain in the SCM, and the default opam packages shall not depend on camlp5. Note that the CI tests the build of these files [broken by the way after 3a7f9c47e7da83402fb6484484d7173f0345d0bf ]

Of course distributions are free to package lablgtk3 and require a rebuild of the files using dune --root . --only-packages PKG --no-config --profile release as documented in dune's man page.

olafhering commented 4 years ago

camlp5 is a very special dependency and maybe not be available in newer OCaml versions, so it is reasonable to allow building lablgtk without it,

camlp4 was the tool that can be used only up to 4.08. camlp5 is maintained. If there is an OCaml, there is also a dune and a camlp5.

dune rebuilds the missing files, but only if mode promote is removed.

ejgallego commented 4 years ago

dune rebuilds the missing files, but only if mode promote is removed.

Not exactly, it will also rebuild them when using the developer profile.

XVilka commented 4 years ago

@olafhering there are no guarantees that camlp5 will be maintained for some future OCaml versions. Ideally, it should be rewritten into the more modern tools, see https://github.com/garrigue/lablgtk/pull/67#issuecomment-506601088

garrigue commented 4 years ago

@XVilka The author of camlp5 has no plan to discontinue it, so I do not see why we should preempt his decisions. Ideally the functionality of camlp5 we are using here (stream parsers) should be separated or go back to ocaml, as it was part of ocaml since its inception (with no dependency on camlp4). The use of camlp5 for the documentation is another question, and it should be changed anyway in order to switch to odoc.

@olafhering I already switched the lablgtk3 branch to camlp5. I will do so for lablgtk2 on the next occasion. Not making it a dependency of the opam package is essential, as camlp5 may not be updated immediately after an ocaml release. This is also the case when checking the developer sources with upcoming releases.

ejgallego commented 4 years ago

Not making it a dependency of the opam package is essential, as camlp5 may not be updated immediately after an ocaml release. This is also the case when checking the developer sources with upcoming releases.

Indeed the 4.10 target cannot depend on camlp5; IMVHO @garrigue and I agree on that this PR could create some problems and the current situation is OK [the files are rebuilt on the dev setup and on the CI] thus closing. Thanks for the submission tho!