search

colis-anr / morbig

A static parser for POSIX Shell
Other
189 stars 8 forks source link

Broken installation when following instructions #189

Open anadon opened 1 month ago

anadon commented 1 month ago

When following the instructions outlined for an example project and setup for morbig, I recieve the following:

anadon@botamon:~/Documents/code/detect_programs_called_from_script$ dune build
File "bin/dune", line 4, characters 52-58:
4 |  (libraries detect_programs_called_from_script Pcre morbig))
                                                        ^^^^^^
Error: Library "morbig" not found.
-> required by _build/default/bin/main.exe
-> required by alias bin/all
-> required by alias default

Those in the Librairc #ocaml chat suspect that this library is broken somehow.

anadon commented 1 month ago 
(04:11:14 PM) discocaml: <leviroth> I don't think this is an issue with opam setup. I can reproduce this on my own, definitely functional opam installation.
(04:11:55 PM) discocaml: <leviroth> It's not actually installing any findlib package
leviroth commented 2 weeks ago

Ultimately, I think this happens because the make build executed by the build: field in opam-repository (https://github.com/ocaml/opam-repository/blob/b6de4bb8dee70812d424cc3121b00a1e03b2b447/packages/morbig/morbig.0.11.0/opam) does not use -p, and thus does not promote the morbig.install file at the end of the build.

yawaramin commented 2 weeks ago

Interestingly, the opam file in this repo itself does use the -p flag: https://github.com/colis-anr/morbig/blob/b97a5be622a7b5539f43617e74bf9cccad1d58b9/morbig.opam#L39

leviroth commented 1 week ago

Right, it was intentionally changed (presumably with unintended consequences) when packaged for opam-repository.

Niols commented 1 week ago

Hi all, thanks for the report and the work put into understand what's going on!

Indeed, the package on opam-repository relies on make build instead of dune build -p .... The proper fix for this would be to push all the building steps into Dune and to only rely on dune build -p ...:

https://github.com/colis-anr/morbig/blob/b97a5be622a7b5539f43617e74bf9cccad1d58b9/Makefile#L11-L15

In particular, one would need to manage to express L12 in pure Dune build files. At the time of writing Morbig, this was not possible, but it might be today; I haven't followed the latest improvements to Dune. In the meantime, would it suffice to add -p morbig to L13?

leviroth commented 1 week ago

It appears so: I downloaded and unpacked the v0.11.0 tarball, added -p, and pinned opam to that directory. I saw that the package is now installed, in the sense that e.g. ocamlfind query morbig prints a path.

However, I'm not sure if src/c/dune is having the desired effect. It doesn't appear that libmorbigc is being installed, as judged by opam show morbig --list-files | grep libmorbig.

Niols commented 5 days ago

After all, I think what's missing from the opam-repository opam files is a field install: [make "install"]; no need for -p morbig. I am not really sure why it was not working before.

Could you confirm?

  1. Download the v0.11.0 tarball from Morbig's GitHub, unarchive it.
  2. Download the v0.11.0 opam file from opam-repository
  3. Remove the url { lines, add install: [make "install"] instead
  4. Pin the directory.
  5. Check that it installs.
  6. Check that the morbig utility has been installed. (type morbig should find a location in your .opam directory).
  7. Check that the C library has been installed. (opam show morbig --list-files | grep libmorbig should find a .h and a .o files).
leviroth commented 3 days ago

Ah, yes, that works - everything in steps 5-7 checks out. And I think I understand why things weren't working in various configurations before. For example, a silly one is that I was previously starting with the opam file from the tarball, instead of the opam-repository one in your instructions.

Niols commented 1 day ago

I have opened a PR on opam-repository to propose this fix. Let us see how it unfolds.