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

init scripts: revert environment before settings new one, on session loading #5036

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

rjbou
Copy link
Collaborator

@rjbou rjbou commented Feb 2, 2022

fix #4649 + add a test that display init scripts

rjbou added 4 commits February 2, 2022 19:38
… revert it.

Also some rework of writing scripts functions, factorise `if' statement writing, add precondition handling, etc.
@rjbou rjbou added this to the 2.2.0~alpha milestone Feb 2, 2022
@rjbou rjbou requested review from AltGr, kit-ty-kate and dra27 February 2, 2022 18:53
Printf.sprintf "%s\n%s"
(* XXX duplicated with variables.sh *)
(iif "! ${?OPAM_SWITCH_PREFIX}" {|setenv OPAM_SWITCH_PREFIX ""|})
(iif {|"x$OPAM_SWITCH_PREFIX" != "x"|}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same than bash safeguard, needed?

(iif {|"x$OPAM_SWITCH_PREFIX" != "x"|}
"eval `opam env --revert --shell=csh --readonly`")
| SH_fish ->
iif {|test -n "$OPAM_SWITCH_PREFIX"|}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nox safeguard trick, is it needed?

src/state/opamEnv.ml Show resolved Hide resolved
pad icond ithen (ielse_expr ielse) pad
| SH_zsh ->
Printf.sprintf "%sif [[ %s ]]; then\n %s%s%sfi"
pad icond ithen (ielse_expr ielse) pad
Copy link
Member

Choose a reason for hiding this comment

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

Why is the bash / zsh distinction needed here ? I thought both syntaxes existed in both, with [[ allowing more specific built-ins. [ should behave the same for zsh as for bash.

Copy link
Collaborator Author

@rjbou rjbou Feb 3, 2022

Choose a reason for hiding this comment

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

Simply because I took if expression from if_interactive_script, and it set this sh/zsh difference

tests/reftests/scripts.test Show resolved Hide resolved
# Prefix of the current opam switch
OPAM_SWITCH_PREFIX='${BASEDIR}/root/fake'; export OPAM_SWITCH_PREFIX;
# Current opam switch man dir
MANPATH="$MANPATH":'${BASEDIR}/root/fake/man'; export MANPATH;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

too bad, we can't keep man in test, it's not present on some os

 ### cat root/opam-init/variables.sh
 # Prefix of the current opam switch
 OPAM_SWITCH_PREFIX='${BASEDIR}/root/fake'; export OPAM_SWITCH_PREFIX;
-# Current opam switch man dir
-MANPATH="$MANPATH":'${BASEDIR}/root/fake/man'; export MANPATH;
 # Binary dir for opam switch fake
 PATH='${BASEDIR}/root/fake/bin':"$PATH"; export PATH;

@rjbou rjbou added the PR: WIP Not for merge at this stage label Feb 25, 2022
@kit-ty-kate kit-ty-kate removed this from the 2.2.0~alpha milestone Mar 14, 2023
@kit-ty-kate kit-ty-kate removed their request for review May 9, 2024 13:23
@rjbou rjbou marked this pull request as draft July 10, 2024 14:18
@dra27 dra27 added this to the 2.4.0~alpha1 milestone Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: WIP Not for merge at this stage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

global switch remains in PATH when local switch is activated
4 participants