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

Add --keep-default-environment flag to admin list #5184

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

balsoft
Copy link

@balsoft balsoft commented Jul 15, 2022

Add a flag which allows to keep the default environment available even
if --environment is specified. Useful for overriding some parts of the
default environment while keeping others.

For example, this is useful when we want to figure out packages&depexts available for some OS/distribution, without removing default availability constraints. Compare:

$ opam admin list --resolve=ocaml-system --environment='os=linux;os-family=debian;os-distribution=debian'
# Packages matching: available & solution(ocaml-system)
[ERROR] No solution for ocaml-system:   * Missing dependency:
            - ocaml-system
            unmet availability conditions, e.g. 'sys-ocaml-version = "4.14.0"'
$ opam admin list --resolve=ocaml-system --environment='os=linux;os-family=debian;os-distribution=debian' --keep-default-environment
# Packages matching: available & solution(ocaml-system)
# Name        # Installed # Synopsis
base-bigarray --
base-threads  --
base-unix     --
ocaml         --          The OCaml compiler (virtual package)
ocaml-config  --          OCaml Switch Configuration
ocaml-system  --          The OCaml compiler (system version, from outside of opam)

Add a flag which allows to keep the default environment available even
if --environment is specified. Useful for overriding some parts of the
default environment while keeping others.
@rjbou
Copy link
Collaborator

rjbou commented Jul 18, 2022

Thanks for the PR. What is your usecase that need that cli flag?

@balsoft
Copy link
Author

balsoft commented Jul 18, 2022

Sorry for not being quite clear about it in the initial PR. I'm the author and maintainer of a tool called opam-nix, which is used to build opam packages as Nix derivations. It uses opam admin list to resolve the dependencies of the user-requested packages, to be later turned into the opam-nix "package set" (the alternative to opam's "switch"). Due to debian being the most common Linux distribution family, and therefore most packages listing it when specifying depexts or availability conditions, it is beneficial to set os=linux;os-family=debian;os-distribution=debian as the environment for Linux builds. However, without being able to keep the default environment, this also requires setting other environment variables, for which what I really want to do is to just keep them default. This PR allows to do just that.

@rjbou rjbou self-requested a review July 19, 2022 10:20
@rjbou
Copy link
Collaborator

rjbou commented Aug 9, 2022

Ok, I better understand, thanks for explanation.

I'm not sure that it needs a flag addition. In fact, there is a typo in the documentation

   OpamArg.mk_opt ~cli OpamArg.cli_original ["environment"]
-   "VAR=VALUE[;VAR=VALUE]"
+   "VAR=VALUE[,VAR=VALUE]"

If we take the command

$ opam admin list --resolve=ocaml-system --environment='os=linux;os-family=debian;os-distribution=debian'

It does not resolve correctly because opam adds in the enviromnent the binding [ 'os', 'linux;os-family=debian;os-distribution=debian].

Now if we take the good syntax, it does not resolve neither

$ opam admin list --resolve=ocaml-system --environment='os=linux,os-family=debian,os-distribution=debian'
# Packages matching: available & solution(ocaml-system)
            - ocaml-system
            unmet availability conditions, e.g. 'sys-ocaml-version = "4.14.0"'

This is because opam admin runs environment-less, all environment is given in the commandline. ocaml-system is a special package that needs sys-ocaml-version variable defined (it is defined in the default config file, see here or with opam init --show-default-opamrc). So this variable need to be given to opam admin too:

opam admin list --resolve=ocaml-system --environment='os=linux,os-family=debian,os-distribution=debian,sys-ocaml-version=4.14.0'
# Packages matching: available & solution(ocaml-system)
# Name        # Installed # Synopsis
base-bigarray --
base-threads  --
base-unix     --
ocaml         --          The OCaml compiler (virtual package)
ocaml-config  --          OCaml Switch Configuration
ocaml-system  --          The OCaml compiler (system version, from outside of opam)

@rjbou
Copy link
Collaborator

rjbou commented Aug 9, 2022

cf #5235

@balsoft
Copy link
Author

balsoft commented Aug 22, 2022

@rjbou Thanks! But it's still unclear to me if there's a way I can set os, os-family and os-distribution without explicitly setting sys-ocaml-version and just letting opam accept all ocaml versions regardless of this constraint (since my tool will take the ocaml version from nixpkgs, and there are multiple different ocaml versions available from there).

@kit-ty-kate
Copy link
Member

@rjbou Thanks! But it's still unclear to me if there's a way I can set os, os-family and os-distribution without explicitly setting sys-ocaml-version and just letting opam accept all ocaml versions regardless of this constraint (since my tool will take the ocaml version from nixpkgs, and there are multiple different ocaml versions available from there).

What about:

opam admin list --resolve=ocaml-system --environment="os=linux,os-family=debian,os-distribution=debian,sys-ocaml-version=$(ocamlc -vnum)"

@balsoft
Copy link
Author

balsoft commented Sep 1, 2022

Once again, this forces a certain ocaml version. What I would like to see is that sys-ocaml-version is treated as it is without --environment, so that opam chooses the latest one (which I think is what happens with --environment, correct me if I'm wrong). Then I'll take the version which opam has chosen from nixpkgs.

@rjbou
Copy link
Collaborator

rjbou commented Sep 1, 2022

Is there a case where nix packaged ocaml version is not in ocaml-system versions?

@balsoft
Copy link
Author

balsoft commented Sep 1, 2022

No, all Nix-packaged versions are an opam-system version, but the problem is inverse: I don't want to force an ocaml version. I want to let opam choose the ocaml version, so that it chooses the latest possible, and then pick that version from nixpkgs.

@kit-ty-kate
Copy link
Member

No, all Nix-packaged versions are an opam-system version, but the problem is inverse: I don't want to force an ocaml version. I want to let opam choose the ocaml version, so that it chooses the latest possible, and then pick that version from nixpkgs.

Ah, i see. I thought you wanted to get the list of all installable packages for each installable ocaml version in nix.

In that case what about:

opam admin list --resolve=ocaml-system --environment="os=linux,os-family=debian,os-distribution=debian,sys-ocaml-version=$(opam admin list -s ocaml-system --columns=version | tail -n 1)"

@balsoft
Copy link
Author

balsoft commented Sep 1, 2022

This is a problem in case a package in the resolve has an upper constraint on ocaml. The resolution will fail, even though it should succeed (by just using an older version of ocaml).

@kit-ty-kate
Copy link
Member

In that case I'm not sure to understand how --keep-default-environment would help. Wouldn't that just default to the ocaml version currently installed? (would do the same as #5184 (comment))

@rjbou
Copy link
Collaborator

rjbou commented Sep 1, 2022

The name is measleading. The option set the default value on variables to true, so all non defined variables with --environment will be set to true. Without --environment, it is se to true.

@kit-ty-kate
Copy link
Member

So if it were to be added it should be called --ignore-undefined-variables (which, if i understand correctly, is not really what @balsoft is after either)

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

Successfully merging this pull request may close these issues.

None yet

3 participants