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

3.1.3 fails to build - new/broken dependency on camlp-streams? #160

Open gdt opened 1 year ago

gdt commented 1 year ago

I am updating pkgsrc from 3.1.2 to 3.1.3 and the build fails with

File "tools/dune", line 22, characters 12-25:
22 |  (libraries camlp-streams))
                 ^^^^^^^^^^^^^
Error: Library "camlp-streams" not found.
Hint: try:
  dune external-lib-deps --missing --no-config --root . --ignore-promoted-rules --default-target @install --always-show-command-line --promote-install-files --release --only-packages lablgtk3,lablgtk3-gtkspell3,lablgtk3-sourceview3 -p lablgtk3,lablgtk3-gtkspell3,lablgtk3-sourceview3 --profile release -j 1 @install
*** Error code 1   

CHANGES.md does not mention a new dependency and README does not describe this as a dependency. it seems likely that this is a maintainer tool that has crept into the build.

pkgsrc is using dune to build.

ejgallego commented 1 year ago

Yes, this is a missing dep, see #159 for a fixed build.

gdt commented 1 year ago

Thanks - will wait for 3.1.4 with the fix.

garrigue commented 1 year ago

I'm a bit confused here. I indeed forgot to add this explicitly to the README, but the opam files are updated with the new dependency. IIUK there is a camlp-streams package for any version of OCaml, but it is just a dummy for versions old than 5.0, which is the first one where streams are not in the standard library. I'm not sure what needs to by fixed in 3.1.4, at least things seem to compile fine with opam.

ejgallego commented 1 year ago

@garrigue the bug here is not about opam, you need to tell the build system (dune in this case) to pick the new library up too.

gdt commented 1 year ago

It needs to be really clear to packagers what is required, because dependencies need to be provided by the packaging system. I commented as I did because README said that streams was a maintainer thing only.

Even if it's just fixing the docs to really state the right deps, I think that deserves a point release once done. We won't run out of integers.

ejgallego commented 1 year ago

Yes, the README got out of date. Note that streams != camlp5 . calmp5 is still developer only. camlp-streams is required for the regular build too.

ejgallego commented 1 year ago

For docs we could just say, "check the opam files", that (once the CI is working again) is guaranteed to work.

garrigue commented 1 year ago

@garrigue the bug here is not about opam, you need to tell the build system (dune in this case) to pick the new library up too.

But I believe I properly updated the dune files. Otherwise, how would it compile with OCaml 5.0 ?

ejgallego commented 1 year ago

But I believe I properly updated the dune files. Otherwise, how would it compile with OCaml 5.0 ?

Yes, they are good now. I got confused as I wrote the CI PR before you did the update.

This bug is just about the confusiong camlp-streams != camlp5