From 8bb17929d09a121a1ae11bebd2eba85cda6fc168 Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Sun, 28 Apr 2024 10:57:10 +0100 Subject: [PATCH] Replace PATH-splitting implementation in OpamEnv Existing implementation was not correct w.r.t. separators which were themselves quoted. Use OpamStd.String.split_quoted instead. However, this causes superfluous quotes in variables to be removed. Note that this fixes the MANPATH issue of #5838 but the quoting-handling is still broken. --- src/state/opamEnv.ml | 31 +------------------------------ tests/reftests/env.unix.test | 20 ++++++++++---------- tests/reftests/env.win32.test | 23 ++++++++++++----------- 3 files changed, 23 insertions(+), 51 deletions(-) diff --git a/src/state/opamEnv.ml b/src/state/opamEnv.ml index c44a1c4b5a2..2df7cb4ff43 100644 --- a/src/state/opamEnv.ml +++ b/src/state/opamEnv.ml @@ -153,36 +153,7 @@ let split_var ~(sepfmt:sep_path_format) var value = | Target | Host -> OpamStd.String.split value sep | Target_quoted | Host_quoted -> - (* we suppose that it is in the form: - - "quoted":unquoted - - unquoted:"quoted" - - "quoted":unquoted:"quoted" - - unquoted:"quoted":unquoted - - "quoted" - - unquoted - *) - let rec aux remaining acc = - match String.get remaining 0 with - | '"' -> - (let remaining = - String.sub remaining 1 (String.length remaining - 1) - in - match OpamStd.String.cut_at remaining '"' with - | Some (quoted, rest) -> - aux rest (("\""^quoted^"\"")::acc) - | None -> remaining::acc) - | _ -> - let remaining = - if Char.equal (String.get remaining 0) sep then - String.sub remaining 1 (String.length remaining - 1) - else remaining in - (match OpamStd.String.cut_at remaining sep with - | Some (unquoted, rest) -> - aux rest (unquoted::acc) - | None -> remaining::acc) - | exception Invalid_argument _ -> acc - in - List.rev @@ aux value [] + OpamStd.String.split_quoted value sep let join_var ~(sepfmt:sep_path_format) var values = let separator = diff --git a/tests/reftests/env.unix.test b/tests/reftests/env.unix.test index eaa9395e32c..417e063d93c 100644 --- a/tests/reftests/env.unix.test +++ b/tests/reftests/env.unix.test @@ -115,7 +115,7 @@ Processing 2/3: [col-target-quoted: sh env | grep RCTQ_ENV | sort] + sh "-c" "env | grep RCTQ_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/col-target-quoted.1) - RCTQ_ENVBUILD=/a/given/path - RCTQ_ENVBUILD_ADD=/a/given/path:a/path/to -- RCTQ_ENVBUILD_ADD_WITH_COL="a:/nother/gi;ven/path":/a/path/to:"t:/his/is/quoted" +- RCTQ_ENVBUILD_ADD_WITH_COL="a:/nother/gi;ven/path":/a/path/to:t:/his/is/quoted - RCTQ_ENVBUILD_STR=something - RCTQ_ENVBUILD_WITH_COL="s:mething" - RCTQ_ENVSET_ADD=a/path/to @@ -126,7 +126,7 @@ Done. ### opam env | sort | grep "RCTQ_ENV" RCTQ_ENVSET='/a/given/path'; export RCTQ_ENVSET; RCTQ_ENVSET_ADD='/a/given/path:a/path/to'; export RCTQ_ENVSET_ADD; -RCTQ_ENVSET_ADD_WITH_COL='"a:/nother/gi;ven/path":/a/path/to:"t:/his/is/quoted"'; export RCTQ_ENVSET_ADD_WITH_COL; +RCTQ_ENVSET_ADD_WITH_COL='"a:/nother/gi;ven/path":/a/path/to:t:/his/is/quoted'; export RCTQ_ENVSET_ADD_WITH_COL; RCTQ_ENVSET_STR='something'; export RCTQ_ENVSET_STR; RCTQ_ENVSET_WITH_COL='"s:mething"'; export RCTQ_ENVSET_WITH_COL; ### cat OPAM/rewriting/.opam-switch/environment | grep RCTQ_ENV @@ -249,7 +249,7 @@ Processing 2/3: [col-host-quoted: sh env | grep RCHQ_ENV | sort] + sh "-c" "env | grep RCHQ_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/col-host-quoted.1) - RCHQ_ENVBUILD=/a/given/path - RCHQ_ENVBUILD_ADD=/a/given/path:a/path/to -- RCHQ_ENVBUILD_ADD_WITH_COL="a:/nother/gi;ven/path":/a/path/to:"t:/his/is/quoted" +- RCHQ_ENVBUILD_ADD_WITH_COL="a:/nother/gi;ven/path":/a/path/to:t:/his/is/quoted - RCHQ_ENVBUILD_STR=something - RCHQ_ENVBUILD_WITH_COL="s:mething" - RCHQ_ENVSET_ADD=a/path/to @@ -260,7 +260,7 @@ Done. ### opam env | sort | grep "RCHQ_ENV" RCHQ_ENVSET='/a/given/path'; export RCHQ_ENVSET; RCHQ_ENVSET_ADD='/a/given/path:a/path/to'; export RCHQ_ENVSET_ADD; -RCHQ_ENVSET_ADD_WITH_COL='"a:/nother/gi;ven/path":/a/path/to:"t:/his/is/quoted"'; export RCHQ_ENVSET_ADD_WITH_COL; +RCHQ_ENVSET_ADD_WITH_COL='"a:/nother/gi;ven/path":/a/path/to:t:/his/is/quoted'; export RCHQ_ENVSET_ADD_WITH_COL; RCHQ_ENVSET_STR='something'; export RCHQ_ENVSET_STR; RCHQ_ENVSET_WITH_COL='"s:mething"'; export RCHQ_ENVSET_WITH_COL; ### cat OPAM/rewriting/.opam-switch/environment | grep RCHQ_ENV @@ -380,7 +380,7 @@ Processing 2/3: [semicol-target-quoted: sh env | grep RSTQ_ENV | sort] + sh "-c" "env | grep RSTQ_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/semicol-target-quoted.1) - RSTQ_ENVBUILD=/a/given/path - RSTQ_ENVBUILD_ADD=/a/given/path;a/path/to -- RSTQ_ENVBUILD_ADD_WITH_COL="a:/nother/gi;ven/path";/a/path/to;"t:/his/is/quoted" +- RSTQ_ENVBUILD_ADD_WITH_COL="a:/nother/gi;ven/path";/a/path/to;t:/his/is/quoted - RSTQ_ENVBUILD_STR=something - RSTQ_ENVBUILD_WITH_COL=s:mething - RSTQ_ENVSET_ADD=a/path/to @@ -391,7 +391,7 @@ Done. ### opam env | sort | grep "RSTQ_ENV" RSTQ_ENVSET='/a/given/path'; export RSTQ_ENVSET; RSTQ_ENVSET_ADD='/a/given/path;a/path/to'; export RSTQ_ENVSET_ADD; -RSTQ_ENVSET_ADD_WITH_COL='"a:/nother/gi;ven/path";/a/path/to;"t:/his/is/quoted"'; export RSTQ_ENVSET_ADD_WITH_COL; +RSTQ_ENVSET_ADD_WITH_COL='"a:/nother/gi;ven/path";/a/path/to;t:/his/is/quoted'; export RSTQ_ENVSET_ADD_WITH_COL; RSTQ_ENVSET_STR='something'; export RSTQ_ENVSET_STR; RSTQ_ENVSET_WITH_COL='s:mething'; export RSTQ_ENVSET_WITH_COL; ### cat OPAM/rewriting/.opam-switch/environment | grep RSTQ_ENV @@ -514,7 +514,7 @@ Processing 2/3: [semicol-host-quoted: sh env | grep RSHQ_ENV | sort] + sh "-c" "env | grep RSHQ_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/semicol-host-quoted.1) - RSHQ_ENVBUILD=/a/given/path - RSHQ_ENVBUILD_ADD=/a/given/path;a/path/to -- RSHQ_ENVBUILD_ADD_WITH_COL="a:/nother/gi;ven/path";/a/path/to;"t:/his/is/quoted" +- RSHQ_ENVBUILD_ADD_WITH_COL="a:/nother/gi;ven/path";/a/path/to;t:/his/is/quoted - RSHQ_ENVBUILD_STR=something - RSHQ_ENVBUILD_WITH_COL=s:mething - RSHQ_ENVSET_ADD=a/path/to @@ -525,7 +525,7 @@ Done. ### opam env | sort | grep "RSHQ_ENV" RSHQ_ENVSET='/a/given/path'; export RSHQ_ENVSET; RSHQ_ENVSET_ADD='/a/given/path;a/path/to'; export RSHQ_ENVSET_ADD; -RSHQ_ENVSET_ADD_WITH_COL='"a:/nother/gi;ven/path";/a/path/to;"t:/his/is/quoted"'; export RSHQ_ENVSET_ADD_WITH_COL; +RSHQ_ENVSET_ADD_WITH_COL='"a:/nother/gi;ven/path";/a/path/to;t:/his/is/quoted'; export RSHQ_ENVSET_ADD_WITH_COL; RSHQ_ENVSET_STR='something'; export RSHQ_ENVSET_STR; RSHQ_ENVSET_WITH_COL='s:mething'; export RSHQ_ENVSET_WITH_COL; ### cat OPAM/rewriting/.opam-switch/environment | grep RSHQ_ENV @@ -651,7 +651,7 @@ Processing 2/3: [rewrite: sh env | grep RO_ENV | sort] - RO_ENVBUILD=/another/given/path - RO_ENVBUILD_COL=s:mething - RO_ENVBUILD_COL_TARGET=a:/nother/gi;ven/path;a/path/to -- RO_ENVBUILD_COL_TARGET_QUOTED="a:/nother/gi;ven/path":a/path/to:"this/i:s/quoted" +- RO_ENVBUILD_COL_TARGET_QUOTED="a:/nother/gi;ven/path":a/path/to:this/i:s/quoted - RO_ENVBUILD_STR=something - RO_ENVSET_COL_TARGET=a/path/to - RO_ENVSET_COL_TARGET_QUOTED=a/path/to:"this/i:s/quoted" @@ -662,7 +662,7 @@ Done. RO_ENVSET='/a/given/path'; export RO_ENVSET; RO_ENVSET_COL='"s:mething"'; export RO_ENVSET_COL; RO_ENVSET_COL_TARGET='a:/nother/gi;ven/path;a/path/to'; export RO_ENVSET_COL_TARGET; -RO_ENVSET_COL_TARGET_QUOTED='"a:/nother/gi;ven/path":a/path/to:"this/i:s/quoted"'; export RO_ENVSET_COL_TARGET_QUOTED; +RO_ENVSET_COL_TARGET_QUOTED='"a:/nother/gi;ven/path":a/path/to:this/i:s/quoted'; export RO_ENVSET_COL_TARGET_QUOTED; RO_ENVSET_STR='something'; export RO_ENVSET_STR; RO_ENVSET_STR_WS='something/else'; export RO_ENVSET_STR_WS; ### cat OPAM/rewriting/.opam-switch/environment | grep RO_ENV diff --git a/tests/reftests/env.win32.test b/tests/reftests/env.win32.test index 76e3571377f..58f70752565 100644 --- a/tests/reftests/env.win32.test +++ b/tests/reftests/env.win32.test @@ -26,11 +26,12 @@ Done. set "MANPATH=:"${BASEDIR}/OPAM/rewriting/man"" set PATH=${BASEDIR}/OPAM/rewriting/bin;C:\Devel\bin1;C:\Devel\bin2;"C:\Devel\bin3;";C:\Devel\bin4;ZZZ:\; ### opam exec -- opam env --shell=cmd --revert | grep 'ZZZ:' | 'ZZZ:\\.*' -> 'ZZZ:\' | 'set "P' -> 'set P' -set PATH=C:\Devel\bin1;C:\Devel\bin2;"C:\Devel\bin3;";C:\Devel\bin4;ZZZ:\ +set PATH=C:\Devel\bin1;C:\Devel\bin2;C:\Devel\bin3;;C:\Devel\bin4;ZZZ:\ ### : Test for #5838 ### opam env | grep MANPATH MANPATH=':"${BASEDIR}/OPAM/rewriting/man"'; export MANPATH; ### opam exec -- opam env --revert | grep MANPATH +MANPATH=''; export MANPATH; ### : Tests forward and backslash rewriting on revert ### : This sequence of updates is what presently happens with the compiler ### @@ -171,7 +172,7 @@ Processing 2/3: [col-target-quoted: sh env | grep RCTQ_ENV | sort] + sh "-c" "env | grep RCTQ_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/col-target-quoted.1) - RCTQ_ENVBUILD=\a\given\path - RCTQ_ENVBUILD_ADD=\a\given\path:a/path/to -- RCTQ_ENVBUILD_ADD_WITH_COL="a:\nother\gi;ven\path":/a/path/to:"t:/his/is/quoted" +- RCTQ_ENVBUILD_ADD_WITH_COL="a:\nother\gi;ven\path":/a/path/to:t:/his/is/quoted - RCTQ_ENVBUILD_STR=something - RCTQ_ENVBUILD_WITH_COL="s:mething" - RCTQ_ENVSET_ADD=a/path/to @@ -182,7 +183,7 @@ Done. ### opam env | sort | grep "RCTQ_ENV" RCTQ_ENVSET='\a\given\path'; export RCTQ_ENVSET; RCTQ_ENVSET_ADD='\a\given\path:a/path/to'; export RCTQ_ENVSET_ADD; -RCTQ_ENVSET_ADD_WITH_COL='"a:\nother\gi;ven\path":/a/path/to:"t:/his/is/quoted"'; export RCTQ_ENVSET_ADD_WITH_COL; +RCTQ_ENVSET_ADD_WITH_COL='"a:\nother\gi;ven\path":/a/path/to:t:/his/is/quoted'; export RCTQ_ENVSET_ADD_WITH_COL; RCTQ_ENVSET_STR='something'; export RCTQ_ENVSET_STR; RCTQ_ENVSET_WITH_COL='"s:mething"'; export RCTQ_ENVSET_WITH_COL; ### cat OPAM/rewriting/.opam-switch/environment | grep RCTQ_ENV @@ -307,7 +308,7 @@ Processing 2/3: [col-host-quoted: sh env | grep RCHQ_ENV | sort] + sh "-c" "env | grep RCHQ_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/col-host-quoted.1) - RCHQ_ENVBUILD=/a/given/path - RCHQ_ENVBUILD_ADD=/a/given/path:a/path/to -- RCHQ_ENVBUILD_ADD_WITH_COL=/cygdrive/a/nother/gi;ven/path:/a/path/to:"t:/his/is/quoted" +- RCHQ_ENVBUILD_ADD_WITH_COL=/cygdrive/a/nother/gi;ven/path:/a/path/to:t:/his/is/quoted - RCHQ_ENVBUILD_STR=something - RCHQ_ENVBUILD_WITH_COL="s:mething" - RCHQ_ENVSET_ADD=a/path/to @@ -318,7 +319,7 @@ Done. ### opam env | sort | grep "RCHQ_ENV" RCHQ_ENVSET='/a/given/path'; export RCHQ_ENVSET; RCHQ_ENVSET_ADD='/a/given/path:a/path/to'; export RCHQ_ENVSET_ADD; -RCHQ_ENVSET_ADD_WITH_COL='/cygdrive/a/nother/gi;ven/path:/a/path/to:"t:/his/is/quoted"'; export RCHQ_ENVSET_ADD_WITH_COL; +RCHQ_ENVSET_ADD_WITH_COL='/cygdrive/a/nother/gi;ven/path:/a/path/to:t:/his/is/quoted'; export RCHQ_ENVSET_ADD_WITH_COL; RCHQ_ENVSET_STR='something'; export RCHQ_ENVSET_STR; RCHQ_ENVSET_WITH_COL='"s:mething"'; export RCHQ_ENVSET_WITH_COL; ### cat OPAM/rewriting/.opam-switch/environment | grep RCHQ_ENV @@ -443,7 +444,7 @@ Processing 2/3: [semicol-target-quoted: sh env | grep RSTQ_ENV | sort] + sh "-c" "env | grep RSTQ_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/semicol-target-quoted.1) - RSTQ_ENVBUILD=\a\given\path - RSTQ_ENVBUILD_ADD=\a\given\path;a/path/to -- RSTQ_ENVBUILD_ADD_WITH_COL="a:\nother\gi;ven\path";/a/path/to;"t:/his/is/quoted" +- RSTQ_ENVBUILD_ADD_WITH_COL="a:\nother\gi;ven\path";/a/path/to;t:/his/is/quoted - RSTQ_ENVBUILD_STR=something - RSTQ_ENVBUILD_WITH_COL=s:mething - RSTQ_ENVSET_ADD=a/path/to @@ -454,7 +455,7 @@ Done. ### opam env | sort | grep "RSTQ_ENV" RSTQ_ENVSET='\a\given\path'; export RSTQ_ENVSET; RSTQ_ENVSET_ADD='\a\given\path;a/path/to'; export RSTQ_ENVSET_ADD; -RSTQ_ENVSET_ADD_WITH_COL='"a:\nother\gi;ven\path";/a/path/to;"t:/his/is/quoted"'; export RSTQ_ENVSET_ADD_WITH_COL; +RSTQ_ENVSET_ADD_WITH_COL='"a:\nother\gi;ven\path";/a/path/to;t:/his/is/quoted'; export RSTQ_ENVSET_ADD_WITH_COL; RSTQ_ENVSET_STR='something'; export RSTQ_ENVSET_STR; RSTQ_ENVSET_WITH_COL='s:mething'; export RSTQ_ENVSET_WITH_COL; ### cat OPAM/rewriting/.opam-switch/environment | grep RSTQ_ENV @@ -579,7 +580,7 @@ Processing 2/3: [semicol-host-quoted: sh env | grep RSHQ_ENV | sort] + sh "-c" "env | grep RSHQ_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/semicol-host-quoted.1) - RSHQ_ENVBUILD=/a/given/path - RSHQ_ENVBUILD_ADD=/a/given/path;a/path/to -- RSHQ_ENVBUILD_ADD_WITH_COL="/cygdrive/a/nother/gi;ven/path";/a/path/to;"t:/his/is/quoted" +- RSHQ_ENVBUILD_ADD_WITH_COL="/cygdrive/a/nother/gi;ven/path";/a/path/to;t:/his/is/quoted - RSHQ_ENVBUILD_STR=something - RSHQ_ENVBUILD_WITH_COL=s:mething - RSHQ_ENVSET_ADD=a/path/to @@ -590,7 +591,7 @@ Done. ### opam env | sort | grep "RSHQ_ENV" RSHQ_ENVSET='/a/given/path'; export RSHQ_ENVSET; RSHQ_ENVSET_ADD='/a/given/path;a/path/to'; export RSHQ_ENVSET_ADD; -RSHQ_ENVSET_ADD_WITH_COL='"/cygdrive/a/nother/gi;ven/path";/a/path/to;"t:/his/is/quoted"'; export RSHQ_ENVSET_ADD_WITH_COL; +RSHQ_ENVSET_ADD_WITH_COL='"/cygdrive/a/nother/gi;ven/path";/a/path/to;t:/his/is/quoted'; export RSHQ_ENVSET_ADD_WITH_COL; RSHQ_ENVSET_STR='something'; export RSHQ_ENVSET_STR; RSHQ_ENVSET_WITH_COL='s:mething'; export RSHQ_ENVSET_WITH_COL; ### cat OPAM/rewriting/.opam-switch/environment | grep RSHQ_ENV @@ -721,7 +722,7 @@ Processing 2/3: [rewrite: sh env | grep RO_ENV | sort] - RO_ENVBUILD=\another\given\path - RO_ENVBUILD_COL=s:mething - RO_ENVBUILD_COL_TARGET=/cygdrive/a/nother/gi;ven/path;a/path/to -- RO_ENVBUILD_COL_TARGET_QUOTED=/cygdrive/a/nother/gi;ven/path:a/path/to:"this/i:s/quoted" +- RO_ENVBUILD_COL_TARGET_QUOTED=/cygdrive/a/nother/gi;ven/path:a/path/to:this/i:s/quoted - RO_ENVBUILD_STR=something - RO_ENVSET_COL_TARGET=a/path/to - RO_ENVSET_COL_TARGET_QUOTED=a/path/to:"this/i:s/quoted" @@ -732,7 +733,7 @@ Done. RO_ENVSET='\a\given\path'; export RO_ENVSET; RO_ENVSET_COL='"s:mething"'; export RO_ENVSET_COL; RO_ENVSET_COL_TARGET='/cygdrive/a/nother/gi;ven/path;a/path/to'; export RO_ENVSET_COL_TARGET; -RO_ENVSET_COL_TARGET_QUOTED='/cygdrive/a/nother/gi;ven/path:a/path/to:"this/i:s/quoted"'; export RO_ENVSET_COL_TARGET_QUOTED; +RO_ENVSET_COL_TARGET_QUOTED='/cygdrive/a/nother/gi;ven/path:a/path/to:this/i:s/quoted'; export RO_ENVSET_COL_TARGET_QUOTED; RO_ENVSET_STR='something'; export RO_ENVSET_STR; RO_ENVSET_STR_WS2='something/else'; export RO_ENVSET_STR_WS2; RO_ENVSET_STR_WS='something/else'; export RO_ENVSET_STR_WS;