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.

Sign up for GitHub

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

Jump to bottom

Configurator: pkg-config plugin uses pkgconf and --personality=TARGET by default #10937

Merged
merged 5 commits into from
Nov 1, 2024
Merged

Configurator: pkg-config plugin uses pkgconf and --personality=TARGET by default #10937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
90202ee
configurator: Allow to specify a path in PKG_CONFIG
pirbo Sep 17, 2024
c29795b
Configurator: try to call pkgconf before failing back to pkg-config
pirbo Sep 18, 2024
22d2e26
Configurator tests: toy pkg-config cleaned and renamed into pkgconf
pirbo Sep 18, 2024
4c1135e
Configurator: add by default --personality=@target@ to pkgconf calls
pirbo Sep 18, 2024
32a403d
Configurator: document new pkg-config plugin behaviors
pirbo Oct 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/changes/10937.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Configurator uses `pkgconf` as pkg-config implementation when available
and forwards it the `target` of `ocamlc -config`. (#10937, @pirbo)
39 changes: 28 additions & 11 deletions otherlibs/configurator/src/v1.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,13 @@ module Find_in_path = struct
;;

let which prog =
List.find_map (get_path ()) ~f:(fun dir ->
let fn = dir ^/ prog ^ exe in
if Filename.is_implicit prog
then
List.find_map (get_path ()) ~f:(fun dir ->
let fn = dir ^/ prog ^ exe in
Option.some_if (Sys.file_exists fn) fn)
else (
let fn = if Filename.check_suffix prog exe then prog else prog ^ exe in
Option.some_if (Sys.file_exists fn) fn)
;;
end
Expand Down Expand Up @@ -653,18 +658,30 @@ module Pkg_config = struct
}

let get c =
let pkg_config_exe_name =
match Sys.getenv "PKG_CONFIG" with
| s -> s
| exception Not_found -> "pkg-config"
in
let pkg_config_args =
let get_pkg_config_args default =
match Sys.getenv "PKG_CONFIG_ARGN" with
| s -> String.split ~on:' ' s
| exception Not_found -> []
| exception Not_found -> default
in
Option.map (which c pkg_config_exe_name) ~f:(fun pkg_config ->
{ pkg_config; pkg_config_args; configurator = c })
match Sys.getenv "PKG_CONFIG" with
| s ->
Option.map (which c s) ~f:(fun pkg_config ->
let pkg_config_args = get_pkg_config_args [] in
{ pkg_config; pkg_config_args; configurator = c })
| exception Not_found ->
(match which c "pkgconf" with
| None ->
Option.map (which c "pkg-config") ~f:(fun pkg_config ->
let pkg_config_args = get_pkg_config_args [] in
{ pkg_config; pkg_config_args; configurator = c })
| Some pkg_config ->
let pkg_config_args =
get_pkg_config_args
(match ocaml_config_var c "target" with
| None -> []
| Some target -> [ "--personality"; target ])
in
Some { pkg_config; pkg_config_args; configurator = c })
;;

type package_conf =
Expand Down
21 changes: 14 additions & 7 deletions otherlibs/configurator/src/v1.mli
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,11 @@ module Pkg_config : sig
type configurator = t
type t

(** Search pkg-config in PATH. Prefers the [PKG_CONFIG_PATH] environment
variable if set. Returns [None] if pkg-config is not found. *)
(** Search a pkg-config implementation in PATH. Use the one
defined in [PKG_CONFIG] environment variable if set else try
[pkgconf] then [pkg-config]. Append the [PKG_CONFIG_PATH]
environment variable to the searched pathes. Returns [None] if
nothing is not found. *)
val get : configurator -> t option

type package_conf =
Expand All @@ -84,9 +87,11 @@ module Pkg_config : sig

(** [query t ~package] query pkg-config for the [package]. The package must
not contain a version constraint. Multiple, unversioned packages are
separated with spaces, for example "gtk+-3.0 gtksourceview-3.0". If set,
the [PKG_CONFIG_ARGN] environment variable specifies a list of arguments
to pass to pkg-config. Returns [None] if [package] is not available *)
separated with spaces, for example "gtk+-3.0 gtksourceview-3.0". By
default, the OCaml compiler [target] is passed to pkgconf as
[--personality] argument. An alternative list of arguments can be
specified by setting the [PKG_CONFIG_ARGN] environment variable.
Returns [None] if [package] is not available *)
val query : t -> package:string -> package_conf option

val query_expr : t -> package:string -> expr:string -> package_conf option
Expand All @@ -95,8 +100,10 @@ module Pkg_config : sig
(** [query_expr_err t ~package ~expr] query pkg-config for the [package].
[expr] may contain a version constraint, for example "gtk+-3.0 >= 3.18".
[package] must be just the name of the package. If [expr] is specified,
[package] must be specified as well. If set, the [PKG_CONFIG_ARGN]
environment variable specifies a list of arguments to pass to pkg-config.
[package] must be specified as well. By default, the OCaml compiler
"target" is passed to pkgconf as [--personality] argument. An
alternative list of arguments can be specified by setting the
[PKG_CONFIG_ARGN] environment variable.
Returns [Error error_msg] if [package] is not available *)
val query_expr_err
: t
Expand Down
8 changes: 4 additions & 4 deletions otherlibs/configurator/test/blackbox-tests/pkg-config-args.t/dune
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
(executable
(name pkg_config)
(modules pkg_config))
(name pkgconf)
(modules pkgconf))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if both dummy pkg_config and pkgconf were both built to be able to demonstrate the fallback behavior in tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put it some thought even before your remark and that fallback is subtle to demonstrate as it requires to remove the pkgconf you have on your system from your PATH else it is favored over the local pkg-config...
If you figure out a way, I can implement it :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point, there is no elegant solution to remove thing from the PATH. That said:

It is a bit of a hassle but you could in the test adjust PATH to only contain a directory you created that contains symlinks to the binaries you expect (e.g. iterate over the PATH that the tests starts with, symlink everything but pkgconf to make sure it still works if pkgconf/pkg-config calls other binaries) and then use that filtered directory as sole PATH for the rest of the test.


(env
(_
(binaries
(./pkg_config.exe as pkg-config))))
(./pkgconf.exe as pkgconf))))

(executable
(name config_test)
Expand All @@ -14,6 +14,6 @@

(rule
(alias default)
(deps %{bin:pkg-config})
(deps %{bin:pkgconf})
(action
(run ./config_test.exe -verbose)))
2 changes: 1 addition & 1 deletion ...box-tests/pkg-config-args.t/pkg_config.ml → ...ackbox-tests/pkg-config-args.t/pkgconf.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ let not_flag x = not ("--print-errors" = x)
let () =
let args = List.tl (Array.to_list Sys.argv) in
let args = List.filter not_flag args in
Format.printf "%a@."
Format.printf "@[<v>%a@]@."
(Format.pp_print_list Format.pp_print_string) args
26 changes: 16 additions & 10 deletions otherlibs/configurator/test/blackbox-tests/pkg-config-args.t/run.t
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,34 +1,40 @@
These tests show that setting `PKG_CONFIG_ARGN` passes extra args to `pkg-config`

$ dune build 2>&1 | awk '/run:.*bin\/pkg-config/{a=1}/stderr/{a=0}a'
run: $TESTCASE_ROOT/_build/default/.bin/pkg-config --print-errors dummy-pkg
$ dune build 2>&1 | awk '/run:.*bin\/pkgconf/{a=1}/stderr/{a=0}a' | sed s/$(ocamlc -config | sed -n "/^target:/ {s/target: //; p; }")/\$TARGET/g
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this rather be added to the BUILD_PATH_PREFIX_MAP variable at the top of this file?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check some of the other tests for how that's done. It feels a bit cleaner to me to do it that way

Copy link
Contributor Author

@pirbo pirbo Oct 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How ? Please help! :-)
This was my first idea of course. I never managed to make it work. That's why I fold back to a sed command...
After your comment, I said to myself to not be lazy and to lead this battle but I'm defeated again and again :-/
I tried all the variant I could imagine of

-  $ dune build 2>&1 | awk '/run:.*bin\/pkgconf/{a=1}/stderr/{a=0}a' | sed s/$(ocamlc -config | sed -n "/^target:/ {s/target: //; p; }")/\$TARGET/g
+  $ target=$(ocamlc -config | sed -n "/^target:/ {s/target: //; p; }")
+  $ printf $target
+  x86_64-pc-linux-gnu
+  $ export BUILD_PATH_PREFIX_MAP="$BUILD_PATH_PREFIX_MAP:TARGET=$target"
+  $ dune build 2>&1 | awk '/run:.*bin\/pkgconf/{a=1}/stderr/{a=0}a'

the outcome keeps being

-  run: $TESTCASE_ROOT/_build/default/.bin/pkgconf --personality $TARGET --cflags dummy-pkg
+  run: $TESTCASE_ROOT/_build/default/.bin/pkgconf --personality x86_64-pc-linux-gnu --cflags dummy-pkg
   -> process exited with code 0
   -> stdout:
    | --personality
-   | $TARGET
+   | x86_64-pc-linux-gnu
    | --cflags
    | dummy-pkg
-  run: $TESTCASE_ROOT/_build/default/.bin/pkgconf --personality $TARGET --libs dummy-pkg
+  run: $TESTCASE_ROOT/_build/default/.bin/pkgconf --personality x86_64-pc-linux-gnu --libs dummy-pkg
   -> process exited with code 0
   -> stdout:
    | --personality
-   | $TARGET
+   | x86_64-pc-linux-gnu
    | --libs
    | dummy-pkg

run: $TESTCASE_ROOT/_build/default/.bin/pkgconf --print-errors dummy-pkg
-> process exited with code 0
-> stdout:
| dummy-pkg
run: $TESTCASE_ROOT/_build/default/.bin/pkg-config --cflags dummy-pkg
run: $TESTCASE_ROOT/_build/default/.bin/pkgconf --personality $TARGET --cflags dummy-pkg
-> process exited with code 0
-> stdout:
| --personality
| $TARGET
| --cflags
| dummy-pkg
run: $TESTCASE_ROOT/_build/default/.bin/pkg-config --libs dummy-pkg
run: $TESTCASE_ROOT/_build/default/.bin/pkgconf --personality $TARGET --libs dummy-pkg
-> process exited with code 0
-> stdout:
| --personality
| $TARGET
| --libs
| dummy-pkg

$ dune clean
$ PKG_CONFIG_ARGN="--static" dune build 2>&1 | awk '/run:.*bin\/pkg-config/{a=1}/stderr/{a=0}a'
run: $TESTCASE_ROOT/_build/default/.bin/pkg-config --print-errors dummy-pkg
$ PKG_CONFIG_ARGN="--static" dune build 2>&1 | awk '/run:.*bin\/pkgconf/{a=1}/stderr/{a=0}a'
run: $TESTCASE_ROOT/_build/default/.bin/pkgconf --print-errors dummy-pkg
-> process exited with code 0
-> stdout:
| dummy-pkg
run: $TESTCASE_ROOT/_build/default/.bin/pkg-config --static --cflags dummy-pkg
run: $TESTCASE_ROOT/_build/default/.bin/pkgconf --static --cflags dummy-pkg
-> process exited with code 0
-> stdout:
| --static--cflags
| --static
| --cflags
| dummy-pkg
run: $TESTCASE_ROOT/_build/default/.bin/pkg-config --static --libs dummy-pkg
run: $TESTCASE_ROOT/_build/default/.bin/pkgconf --static --libs dummy-pkg
-> process exited with code 0
-> stdout:
| --static--libs
| --static
| --libs
| dummy-pkg
2 changes: 1 addition & 1 deletion otherlibs/configurator/test/blackbox-tests/pkg-config-quoting.t/pkg_config.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ let not_flag x = not ("--print-errors" = x)
let () =
let args = List.tl (Array.to_list Sys.argv) in
let args = List.filter not_flag args in
Format.printf "%a@."
Format.printf "@[<v>%a@]@."
(Format.pp_print_list Format.pp_print_string) args
4 changes: 2 additions & 2 deletions otherlibs/configurator/test/blackbox-tests/pkg-config-quoting.t/run.t
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
These tests show how various pkg-config invocations get quotes:
$ dune build 2>&1 | awk '/run:.*bin\/pkg-config/{a=1}/stderr/{a=0}a'
These tests show how various pkg-config invocations get quotes (and test specifying a custom PKG_CONFIG):
$ PKG_CONFIG=$PWD/_build/install/default/bin/pkg-config dune build 2>&1 | awk '/run:.*bin\/pkg-config/{a=1}/stderr/{a=0}a'
run: $TESTCASE_ROOT/_build/install/default/bin/pkg-config --print-errors gtk+-quartz-3.0
-> process exited with code 0
-> stdout:
Expand Down
Loading