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

Support Nix depexts with opam env #5982

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
49 changes: 45 additions & 4 deletions .github/scripts/depexts/generate-actions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

set -eu

#for target in alpine archlinux centos debian fedora gentoo opensuse oraclelinux ubuntu; do
#for target in alpine archlinux centos debian fedora gentoo opensuse oraclelinux ubuntu nix; do
target=$1
dir=.github/actions/$target

Expand Down Expand Up @@ -102,6 +102,12 @@ RUN apt install -y $mainlibs $ocaml
RUN apt install -y g++
EOF
;;
nix)
cat >$dir/Dockerfile << EOF
FROM nixos/nix
RUN nix-channel --update
RUN nix-env -i gnum4 git rsync patch bzip2 gnumake wget ocaml ocaml5.1.1-ocaml-compiler-libs unzip gcc diffutils patch getconf-glibc gnused
EOF
esac

OCAML_INVARIANT="\"ocaml\" {>= \"4.09.0\"$OCAML_CONSTRAINT}"
Expand All @@ -121,12 +127,34 @@ RUN echo 'default-invariant: [ $OCAML_INVARIANT ]' > /opam/opamrc
RUN /usr/bin/opam init --no-setup --disable-sandboxing --bare --config /opam/opamrc git+$OPAM_REPO#$OPAM_REPO_SHA
RUN echo 'archive-mirrors: "https://opam.ocaml.org/cache"' >> \$OPAMROOT/config
RUN /usr/bin/opam switch create this-opam --formula='$OCAML_INVARIANT'
EOF

# we can't `nix-env -i binutils`
# https://github.com/NixOS/nix/issues/10587
if [ $target == "nix" ]; then
cat >>$dir/Dockerfile << EOF
RUN nix-shell -p binutils --run "/usr/bin/opam install opam-core opam-state opam-solver opam-repository opam-format opam-client --deps"
EOF
else
cat >>$dir/Dockerfile << EOF
RUN /usr/bin/opam install opam-core opam-state opam-solver opam-repository opam-format opam-client --deps
EOF
fi

cat >>$dir/Dockerfile << EOF
RUN /usr/bin/opam clean -as --logs
COPY entrypoint.sh /opam/entrypoint.sh
ENTRYPOINT ["/opam/entrypoint.sh"]
EOF

if [ $target == "nix" ]; then
cat >>$dir/Dockerfile << EOF
ENTRYPOINT ["nix-shell", "-p", "binutils", "--run", "/opam/entrypoint.sh"]
EOF
else
cat >>$dir/Dockerfile << EOF
ENTRYPOINT ["/opam/entrypoint.sh"]
EOF
fi

### Generate the entrypoint
cat >$dir/entrypoint.sh << EOF
Expand All @@ -142,10 +170,23 @@ cd /github/workspace
#git clone https://github.com/ocaml/opam --single-branch --branch 2.2 --depth 1 local-opam
#cd local-opam

opam install . --deps
eval \$(opam env)
/usr/bin/opam install . --deps
eval \$(/usr/bin/opam env)
./configure
make

EOF

if [ $target == "nix" ]; then
cat >>$dir/entrypoint.sh << EOF
./opam var --global os-family=nixos
./opam var --global os-distribution=nixos

EOF
fi


cat >>$dir/entrypoint.sh << EOF
./opam config report
./opam switch create confs --empty
EOF
Expand Down
18 changes: 18 additions & 0 deletions .github/workflows/depexts.yml
Original file line number Diff line number Diff line change
Expand Up @@ -199,3 +199,21 @@ jobs:
- name: depexts actions ubuntu
uses: ./.github/actions/ubuntu
id: depexts-ubuntu

depexts-nix:
needs: opam-cache
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v3
- name: opam binary cache
uses: actions/cache@v3
with:
path: binary/opam
key: binary-${{ env.OPAMVERSION }}
- name: generate action
run: |
bash .github/scripts/depexts/generate-actions.sh nix
- name: depexts actions nix
uses: ./.github/actions/nix
id: depexts-nix
1 change: 1 addition & 0 deletions master_changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ users)
* Always pass --no-version-check and --no-write-registry to Cygwin setup [#6046 @dra27]
* Use --quiet-mode noinput for the internal Cygwin installation (which is definitely a fully-specified command line) and --quiet-mode unattended for external Cygwin installations (in case the user does need to select something, e.g. a mirror) [#6046 @dra27]
* [BUG] Fix apt/debian lookup for installed packages [#6054 @rjbou]
* [NEW] Support providing external dependencies with Nix [#5982 @RyanGibb]

## Format upgrade

Expand Down
11 changes: 6 additions & 5 deletions src/client/opamClient.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1591,9 +1591,9 @@ let update_with_init_config ?(overwrite=false) config init_config =

let check_for_sys_packages config system_packages =
if system_packages <> [] then
let ((missing, _) as set) =
let (missing, required, available) =
OpamSysInteract.packages_status config
(OpamSysPkg.Set.of_list system_packages)
(OpamSysPkg.Set.of_list system_packages) ~old_packages:OpamSysPkg.Set.empty
in
if not (OpamSysPkg.Set.is_empty missing) then
let vars = OpamFile.Config.global_variables config in
Expand All @@ -1602,8 +1602,8 @@ let check_for_sys_packages config system_packages =
|> OpamVariable.Map.of_list
in
(*Lazy.force header;*)
OpamSolution.print_depext_msg set;
OpamSolution.install_sys_packages ~confirm:true env config missing ()
OpamSolution.print_depext_msg (missing, available);
ignore @@ OpamSolution.install_sys_packages ~confirm:true ~sys_packages:missing ~required env config None

let reinit ?(init_config=OpamInitDefaults.init_config()) ~interactive
?dot_profile ?update_config ?env_hook ?completion ?inplace
Expand Down Expand Up @@ -2299,7 +2299,8 @@ let install_t t ?ask ?(ignore_conflicts=false) ?(depext_only=false)
in
if depext_only then
(OpamSolution.install_depexts ~force_depext:true ~confirm:false t
(OpamSolver.all_packages solution)), None
~new_packages:(OpamSolver.all_packages solution)
~all_packages:t.installed), None
else
let add_roots =
OpamStd.Option.map (function
Expand Down
76 changes: 40 additions & 36 deletions src/client/opamSolution.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1114,11 +1114,12 @@ let print_depext_msg (avail, nf) =

(* Gets depexts from the state, without checking again, unless [recover] is
true. *)
let get_depexts ?(force=false) ?(recover=false) t packages =
if not force && OpamStateConfig.(!r.no_depexts) then OpamSysPkg.Set.empty else
let get_depexts ?(force=false) ?(recover=false) t ~new_packages ~all_packages =
if not force && OpamStateConfig.(!r.no_depexts) then OpamSysPkg.Set.empty, OpamSysPkg.Set.empty else
let sys_packages =
if recover then
OpamSwitchState.depexts_status_of_packages t packages
OpamSwitchState.depexts_status_of_packages t new_packages
~old_packages:(OpamPackage.Set.diff all_packages new_packages)
else
let base = Lazy.force t.sys_packages in
(* workaround: st.sys_packages is not always updated with added
Expand All @@ -1128,37 +1129,38 @@ let get_depexts ?(force=false) ?(recover=false) t packages =
(* dirty heuristic: recompute for all non-canonical packages *)
OpamPackage.Map.find_opt nv t.repos_package_index
<> OpamSwitchState.opam_opt t nv)
packages
new_packages
in
if OpamPackage.Set.is_empty more_pkgs then base else
OpamPackage.Map.union (fun _ x -> x) base
(OpamSwitchState.depexts_status_of_packages t more_pkgs)
(OpamSwitchState.depexts_status_of_packages t more_pkgs ~old_packages:(OpamPackage.Set.diff all_packages more_pkgs))
in
let avail, nf =
OpamPackage.Set.fold (fun pkg (avail,nf) ->
let avail, required, nf =
OpamPackage.Set.fold (fun pkg (avail,req,nf) ->
match OpamPackage.Map.find_opt pkg sys_packages with
| Some sys ->
OpamSysPkg.(Set.union avail sys.s_available),
OpamSysPkg.(Set.union req sys.s_required),
OpamSysPkg.(Set.union nf sys.s_not_found)
| None -> avail, nf)
packages (OpamSysPkg.Set.empty, OpamSysPkg.Set.empty)
| None -> avail, req, nf)
Copy link
Member

Choose a reason for hiding this comment

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

i think it might be worth using a dedicated record instead of a tuple at this point. Otherwise it could become confusing very quickly with all those similar types around

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have opamSysPkg.status?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Though, we're also passing around the couple of available and required a lot, which perhaps warrants a separate record.

all_packages (OpamSysPkg.Set.empty, OpamSysPkg.Set.empty, OpamSysPkg.Set.empty)
in
print_depext_msg (avail, nf);
avail
avail, required

let install_sys_packages ~map_sysmap ~confirm env config sys_packages t =
let rec entry_point t sys_packages =
let install_sys_packages ~map_sysmap ~confirm ~sys_packages ~required env config t =
let rec entry_point t sys_packages required =
if OpamClientConfig.(!r.fake) then
(print_command sys_packages; t)
else if OpamFile.Config.depext_run_installs config then
if confirm then menu t sys_packages else auto_install t sys_packages
if confirm then menu t sys_packages required else auto_install t sys_packages required
else
manual_install t sys_packages
and menu t sys_packages =
and menu t sys_packages required =
let answer =
let pkgman =
OpamConsole.colorise `yellow
(OpamSysInteract.package_manager_name ~env config)
(OpamSysInteract.package_manager_name ~env (Option.map (fun t -> t.switch) t) config)
in
OpamConsole.menu ~unsafe_yes:`Yes ~default:`Yes ~no:`Quit
"opam believes some required external dependencies are missing. opam \
Expand All @@ -1177,7 +1179,7 @@ let install_sys_packages ~map_sysmap ~confirm env config sys_packages t =
in
OpamConsole.msg "\n";
match answer with
| `Yes -> auto_install t sys_packages
| `Yes -> auto_install t sys_packages required
| `No ->
OpamConsole.note "Use 'opam option depext-run-installs=false' \
if you don't want to be prompted again.";
Expand All @@ -1194,7 +1196,7 @@ let install_sys_packages ~map_sysmap ~confirm env config sys_packages t =
if OpamSysPoll.os_distribution env = Some "cygwin" then
OpamSysInteract.Cygwin.check_setup ~update:false;
let commands =
OpamSysInteract.install_packages_commands ~env config sys_packages
OpamSysInteract.install_packages_commands ~env (Option.map (fun t -> t.switch) t) config sys_packages ~required
|> List.map (fun ((`AsAdmin c | `AsUser c), a) -> c::a)
in
OpamConsole.formatted_msg
Expand All @@ -1221,19 +1223,19 @@ let install_sys_packages ~map_sysmap ~confirm env config sys_packages t =
| `Continue -> check_again t sys_packages
| `Ignore -> bypass t
| `Quit -> give_up ()
and auto_install t sys_packages =
and auto_install t sys_packages required =
try
if OpamSysPoll.os_distribution env = Some "cygwin" then
OpamSysInteract.Cygwin.check_setup ~update:true;
OpamSysInteract.install ~env config sys_packages; (* handles dry_run *)
OpamSysInteract.install ~env (Option.map (fun t -> t.switch) t) config sys_packages ~required; (* handles dry_run *)
map_sysmap (fun _ -> OpamSysPkg.Set.empty) t
with Failure msg ->
OpamConsole.error "%s" msg;
check_again t sys_packages
and check_again t sys_packages =
let open OpamSysPkg.Set.Op in
let needed, notfound =
OpamSysInteract.packages_status ~env config sys_packages
let needed, required, notfound =
OpamSysInteract.packages_status ~env config sys_packages ~old_packages:required
in
let still_missing = needed ++ notfound in
let installed = sys_packages -- still_missing in
Expand All @@ -1250,7 +1252,7 @@ let install_sys_packages ~map_sysmap ~confirm env config sys_packages t =
else
(OpamConsole.error "These packages are still missing: %s\n"
(syspkgs_to_string sys_packages);
if OpamStd.Sys.tty_in then entry_point t sys_packages
if OpamStd.Sys.tty_in then entry_point t sys_packages required
else give_up ())
and bypass t =
OpamConsole.note
Expand All @@ -1269,19 +1271,20 @@ let install_sys_packages ~map_sysmap ~confirm env config sys_packages t =
give_up_msg ();
OpamStd.Sys.exit_because `Aborted
in
if OpamSysPkg.Set.is_empty sys_packages ||
if (OpamSysPkg.Set.is_empty sys_packages && OpamSysPkg.Set.is_empty required) ||
OpamClientConfig.(!r.show) ||
OpamClientConfig.(!r.assume_depexts) then
t
else
try
OpamConsole.header_msg "Handling external dependencies";
OpamConsole.msg "\n";
entry_point t sys_packages
entry_point t sys_packages required
with Sys.Break as e -> OpamStd.Exn.finalise e give_up_msg

let install_depexts ?(force_depext=false) ?(confirm=true) t packages =
let install_depexts ?(force_depext=false) ?(confirm=true) t ~new_packages ~all_packages =
let map_sysmap f t =
let t = Option.get t in
Copy link
Member

Choose a reason for hiding this comment

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

could you explain why all these Option.get are needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's because I don't have access to the switch state here edd9030#diff-6426799f367266b2a65325c870931b51df35852e12ddbe8580e74f4eeeb58177R1606 afaik because it hasn't been created at this point. We need to pass it in to OpamSysInteract.install* in order for Nix to write to files in the switch, but we also need to support installing system packages before this switch has been created in the existing OpamClient logic (which will do nothing for the Nix backend).

let sys_packages =
OpamPackage.Set.fold (fun nv sys_map ->
match OpamPackage.Map.find_opt nv sys_map with
Expand All @@ -1291,23 +1294,23 @@ let install_depexts ?(force_depext=false) ?(confirm=true) t packages =
f status.OpamSysPkg.s_available }
sys_map
| None -> sys_map)
packages
new_packages
(Lazy.force t.sys_packages)
in
{ t with sys_packages = lazy sys_packages }
Some { t with sys_packages = lazy sys_packages }
in
let confirm =
confirm && not (OpamSysInteract.Cygwin.is_internal t.switch_global.config)
in
let sys_packages =
get_depexts ~force:force_depext ~recover:force_depext t packages
let sys_packages, required =
get_depexts ~force:force_depext ~recover:force_depext t ~new_packages ~all_packages
in
let env = t.switch_global.global_variables in
let config = t.switch_global.config in
install_sys_packages ~map_sysmap ~confirm env config sys_packages t
Option.get @@ install_sys_packages ~map_sysmap ~confirm env config ~sys_packages ~required (Some t)

let install_sys_packages ~confirm =
install_sys_packages ~map_sysmap:(fun _ () -> ()) ~confirm
install_sys_packages ~map_sysmap:(fun _ t -> t) ~confirm

(* Apply a solution *)
let apply ?ask t ~requested ?print_requested ?add_roots
Expand All @@ -1330,9 +1333,9 @@ let apply ?ask t ~requested ?print_requested ?add_roots
in
let t =
if OpamClientConfig.(!r.show) then
let _ = get_depexts t virt_inst in t
let _ = get_depexts t ~new_packages:virt_inst ~all_packages:t.installed in t
(* Prints the msg about additional depexts to install *)
else install_depexts t virt_inst
else install_depexts t ~new_packages:virt_inst ~all_packages:t.installed
in
t, Nothing_to_do
else (
Expand Down Expand Up @@ -1386,13 +1389,14 @@ let apply ?ask t ~requested ?print_requested ?add_roots
solution0;
);
if OpamClientConfig.(!r.show) then
let _ = get_depexts t new_state0.installed in
let _ = get_depexts t ~new_packages:new_state0.installed ~all_packages:new_state0.installed in
(* Prints the msg about additional depexts to install *)
t, Aborted
else if download_only || confirmation ?ask names solution then (
let t =
install_depexts t @@ OpamPackage.Set.inter
new_state0.installed (OpamSolver.all_packages solution0)
install_depexts t
~new_packages:(OpamPackage.Set.inter new_state0.installed (OpamSolver.all_packages solution0))
~all_packages:new_state0.installed
in
let requested =
OpamPackage.packages_of_names new_state.installed names
Expand Down
6 changes: 3 additions & 3 deletions src/client/opamSolution.mli
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,15 @@ val print_depext_msg : OpamSysPkg.Set.t * OpamSysPkg.Set.t -> unit

(** As {!install_depexts}, but supplied with a set of system packages to be
installed. *)
val install_sys_packages: confirm:bool -> OpamStateTypes.gt_variables ->
OpamFile.Config.t -> OpamSysPkg.Set.t -> unit -> unit
val install_sys_packages: confirm:bool -> sys_packages:OpamSysPkg.Set.t -> required:OpamSysPkg.Set.t ->
OpamStateTypes.gt_variables -> OpamFile.Config.t -> rw switch_state option -> rw switch_state option

(* Install external dependencies of the given package set, according the depext
configuration. If [confirm] is false, install commands are directly
launched, without asking user (used by the `--depext-only` option). If
[force_depext] is true, it overrides [OpamFile.Config.depext] value. *)
val install_depexts: ?force_depext:bool -> ?confirm:bool -> rw switch_state ->
package_set -> rw switch_state
new_packages:package_set -> all_packages:package_set -> rw switch_state

(** {2 Atoms} *)

Expand Down
7 changes: 6 additions & 1 deletion src/format/opamSysPkg.ml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ type status =
s_available : Set.t;
(** Package available but not installed *)

s_required : Set.t;
(** Package installed but also needs to be passed to the installation *)

s_not_found : Set.t;
(** Package unavailable on this system *)
}
Expand All @@ -57,10 +60,12 @@ type status =
let status_empty =
{
s_available = Set.empty;
s_required = Set.empty;
s_not_found = Set.empty;
}

let string_of_status sp =
Printf.sprintf "available: %s; not_found: %s"
Printf.sprintf "available: %s; required: %s, not_found: %s"
(Set.to_string sp.s_available)
(Set.to_string sp.s_required)
(Set.to_string sp.s_not_found)
Loading
Loading