Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

conf-gmp build fails on NetBSD #18128

Closed
azarens-git opened this issue Feb 10, 2021 · 16 comments
Closed

conf-gmp build fails on NetBSD #18128

azarens-git opened this issue Feb 10, 2021 · 16 comments

Comments

@azarens-git
Copy link

azarens-git commented Feb 10, 2021

Report

opam install conf-gmp fails as follows:

<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
[ERROR] The compilation of conf-gmp failed at "/bin/sh -exc cc -c $CFLAGS -I/usr/local/include test.c".

#=== ERROR while compiling conf-gmp.3 =========================================#
# context     2.0.6 | netbsd/x86_64 | ocaml-base-compiler.4.11.1 | https://opam.ocaml.org#4acae195
# path        ~/.opam/ocaml-base-compiler.4.11.1/.opam-switch/build/conf-gmp.3
# command     /bin/sh -exc cc -c $CFLAGS -I/usr/local/include test.c
# exit-code   1
# env-file    ~/.opam/log/conf-gmp-9842-ffb3fd.env
# output-file ~/.opam/log/conf-gmp-9842-ffb3fd.out
### output ###
# + cc -c -I/usr/local/include test.c
# test.c:1:10: fatal error: gmp.h: No such file or directory
#  #include <gmp.h>
#           ^~~~~~~
# compilation terminated.

Analysis

Looking at the corresponding opam definitions:

build: [
  ["sh" "-exc" "cc -c $CFLAGS -I/usr/local/include test.c"] {os != "macos"}
  [
    "sh"
    "-exc"
    "cc -c $CFLAGS -I/opt/homebrew/include -I/opt/local/include -I/usr/local/include test.c"
  ] {os = "macos"}
]

It is rather plain that the author had decided to hardcode include paths, instead of using pkg-config. The pkg-config on this NetBSD host produces the following correct output:

$ pkg-config --cflags gmp
-I/usr/pkg/include

Please fix the issue.

@azarens-git
Copy link
Author

I would like to particularly encourage the use of pkg-config approach with this package instead of hardcoding /usr/pkg/include for another reason as well. It is not unheard of and is certainly not unreasonable for a NetBSD system administrator to change the location of LOCALBASE from /usr/pkg to another value. In fact, if pkgsrc is used to build an environment in some unprivileged location, like e.g. a home directory, then the value of LOCALBASE would certainly be distinct from /usr/pkg.

@kit-ty-kate
Copy link
Member

It is rather plain that the author had decided to hardcode include paths, instead of using pkg-config.

The reason was that gmp didn't ship with its pkg-config file before 6.2.0. For instance Debian 10 (the current stable) still ships gmp 6.1.x and will fail if only pkg-config is tried.
So it was simply a matter of legacy code-base.

I'll have a try at fixing this issue soon.

@kit-ty-kate
Copy link
Member

Also, side-note since you're using NetBSD: opam 2.1.0~beta4 added support for external dependencies (depexts) in NetBSD in ocaml/opam#4396. Since we have very few users using NetBSD, feel free to give us comments about this new feature, whenever you have some time/motivation.

@azarens-git
Copy link
Author

azarens-git commented Feb 11, 2021

Hello Kate,

Please consider the following code:

files/conf-gmp.opam

opam-version: "2.0"
maintainer: "nbraud"
homepage: "http://gmplib.org/"
bug-reports: "https://github.com/ocaml/opam-repository/issues"
license: "GPL-1.0-or-later"
build: [
  ["dune" "build" "-p" name "-j" jobs]
]
depends: [
  "dune-configurator"
]
depopts: [
  "conf-pkg-config"
]
depexts: [
  ["libgmp-dev"] {os-family = "debian"}
  ["gmp"] {os = "macos" & os-distribution = "homebrew"}
  ["gmp"] {os = "macos" & os-distribution = "macports"}
  ["gmp" "gmp-devel"] {os-distribution = "centos"}
  ["gmp" "gmp-devel"] {os-distribution = "fedora"}
  ["gmp" "gmp-devel"] {os-distribution = "ol"}
  ["gmp"] {os = "netbsd"}
  ["gmp"] {os = "openbsd"}
  ["gmp"] {os = "freebsd"}
  ["gmp-dev"] {os-distribution = "alpine"}
  ["gmp-devel"] {os-family = "suse"}
]
synopsis: "Virtual package relying on a GMP lib system installation"
description:
  "This package can only install if the GMP lib is installed on the system."
authors: "nbraud"
flags: conf

files/config/dune

(executable
 (name discover)
 (modules discover)
 (libraries dune-configurator str))

files/config/discover.ml

module C = Configurator.V1
module P = C.Pkg_config

let write_conf ~cflags ~libs =
  ignore libs ;
  C.Flags.write_sexp "c_flags.sexp" cflags

let default_gmp conf =
  let sys_name = C.ocaml_config_var_exn conf "system" in
  if sys_name = "macosx" then
    { P.cflags =
        [ "-I/opt/homebrew/include" ;
          "-I/opt/local/include" ;
          "-I/usr/local/include" ] ;
      libs = []
    }
  else
    { P.cflags =
        [ "-I/usr/local/include" ] ;
      libs = []
    }

let discover_gmp conf =
  let pkg_config = match P.get conf with
    | Some conf_pkgConfig -> begin
        match P.query conf_pkgConfig ~package:"gmp" with
        | Some pkg_config -> pkg_config
        | None -> default_gmp conf
      end
    | None -> default_gmp conf
  in
  let cflags , libs =
    (pkg_config.P.cflags , pkg_config.P.libs) in
  write_conf ~cflags ~libs

let _ =
  C.main ~name:"gmp" discover_gmp

I think that dune should attempt to build a private executable from test. I'm trying to figure out how to specify the build of an executable from C in dune, since the old approach that involves the use of executable stanza along with cflags is being deprecated in recent versions in favor of foreign_stus. If you have a good idea how to accomplish that, I would welcome your input.

@azarens-git
Copy link
Author

Also, please allow me to ask, how would you populate the libs field for macosx case?

@kit-ty-kate
Copy link
Member

I think that dune should attempt to build a private executable from test.

I'm failing to see the advantages here. This is more code to maintain for us for nothing and does not bring any improvements compared to our fix. Furthermore, this brings a lot of dependencies for essentially nothing.

Dune-configurator is helpful for packages themselves, not at all for meta packages here to detect and install external dependencies (conf-* packages). Conf packages are here as a common base from which whatever packages can depend on and not have to maintain their own set of depexts, packages that do not depend on dune are amongst them.

@azarens-git
Copy link
Author

Ok. Let's see if those changes do work on that NetBSD box then. Is there a way to quickly test your changes? I don't see the conf-gmp.4 in the master branch yet, so I doubt I could easily pin. Would you possibly suggest another alternative?

@kit-ty-kate
Copy link
Member

Is there a way to quickly test your changes?

yes: opam remote add tmp git://github.com/kit-ty-kate/opam-repository.git#gmp-pkg-config

@azarens-git
Copy link
Author

Is there a way to do this through opam pin?

@kit-ty-kate
Copy link
Member

I guess opam pin edit conf-gmp + copy/past the opam file from the PR might work, question mark(?)
I'm curious though, why is opam remote not an option?

@azarens-git
Copy link
Author

I'm curious though, why is opam remote not an option?

I'm not 100% sure what's the exact semantics of that command. I see that a new tmp repositary had appeared under the build user's ~/.opam/ tree, with a lot of packages in it.

@azarens-git
Copy link
Author

Weird, I only see conf-gmp.1 in your git repository: https://github.com/kit-ty-kate/opam-repository/tree/master/packages/conf-gmp

@azarens-git
Copy link
Author

Also, looking at your solution, I'm not seeing an attempt to build gmp if it is already listed in pkg-config database. Are you sure this is the correct approach? What is the purpose of conf- packages in general? Is it to test the presence features, or just to verify a system-specific record their presence?

@kit-ty-kate
Copy link
Member

Also, looking at your solution, I'm not seeing an attempt to build gmp if it is already listed in pkg-config database.

correct, because this is not the purpose of the conf-* packages (see #18128 (comment)). The purpose of opam is not to be replacement for system package manager.

Weird, I only see conf-gmp.1 in your git repository: https://github.com/kit-ty-kate/opam-repository/tree/master/packages/conf-gmp

You're not looking at the right branch (see PR or command above)

It's 3am here so I'm going to bed. I'll merge my PR tomorrow at some point unless something is wrong. Good night.

@azarens-git
Copy link
Author

It's 3am here so I'm going to bed. I'll merge my PR tomorrow at some point unless something is wrong. Good night.

Ok. Thank you!

You're not looking at the right branch (see PR or command above)

Ok.

@github-actions
Copy link
Contributor

This issue has been open 90 days with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. If you come across this issue in the future, you may also find it helpful to visit our forum at https://discuss.ocaml.org where queries related to OCaml package management are very welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants