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

opam var and opam option fixes when no switch is installed #5027

Merged
merged 8 commits into from
Apr 27, 2022

Conversation

rjbou
Copy link
Collaborator

@rjbou rjbou commented Jan 25, 2022

  • Don't error when displaying if switch is not set fix opam var XXX fails when no switch is selected #5025
  • Try to set a variable with option --switch <sw> fails instead of writing a wrong switch-config file
  • When a field is defined in switch and global scope, try to determine the scope also by checking switch selection

the second point need to be backported to 2.1.3

@rjbou rjbou added this to the 2.2.0~alpha milestone Jan 25, 2022
Copy link
Member

@kit-ty-kate kit-ty-kate left a comment

Choose a reason for hiding this comment

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

Looks good!

As a side note, it might be a good idea to remove OpamStateConfig.get_switch in the future. It looks like its design brings issues such as this fairly easily (If i understand what the issue was correctly)

@rjbou
Copy link
Collaborator Author

rjbou commented Jan 25, 2022

Not really, get_switch must be used carefully, but it helps to homogeneise the error & its message. The get_switch_opt function is present, and can be used. But i agree, we need to take a tour and check its use in opam, and replace needed ones.

@rjbou rjbou changed the title some opam var and opam option fixes opam var and opam option fixes when no switch is installed Mar 3, 2022
@rjbou rjbou force-pushed the var-opt-fix branch 2 times, most recently from 0c93862 to 8fe1d4f Compare March 16, 2022 15:00
Copy link
Member

@AltGr AltGr left a comment

Choose a reason for hiding this comment

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

Good to merge.
Remark: the ~no_switch flag is defined but never used.

@rjbou
Copy link
Collaborator Author

rjbou commented Apr 27, 2022

Certainly a kept old implementation, it is not needed.
Just to keep code somewhere in case one day it is needed:

let with_switch ~display gt lock_kind ?no_switch st k =
  match st with
  | Some st -> k st.switch st.switch_config
  | None ->
    match OpamStateConfig.get_switch_opt () with
    | None ->
      (match no_switch with
       | None ->
         OpamConsole.error_and_exit `Configuration_error "No switch selected"
       | Some no_switch ->
         no_switch (OpamSwitch.of_string "no-switch")
           OpamFile.Switch_config.empty)

@rjbou rjbou merged commit 5e544a5 into ocaml:master Apr 27, 2022
@rjbou rjbou mentioned this pull request Apr 28, 2022
4 tasks
rjbou added a commit to rjbou/opam that referenced this pull request Jul 12, 2022
`opam var` and `opam option` fixes when no switch is installed
@rjbou rjbou removed this from the 2.2.0~alpha milestone Jul 4, 2023
@rjbou rjbou added this to the 2.1.3 milestone Jul 4, 2023
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.

opam var XXX fails when no switch is selected
3 participants