Skip to content

Recognize MSYS2_ARG_CONV_EXCL#8

Merged
dra27 merged 2 commits intometastack:masterfrom
diskuv:feature-msys2-arg-conv-excl
Sep 6, 2021
Merged

Recognize MSYS2_ARG_CONV_EXCL#8
dra27 merged 2 commits intometastack:masterfrom
diskuv:feature-msys2-arg-conv-excl

Conversation

@jonahbeckford
Copy link
Copy Markdown
Contributor

Prevent MSYS from translating command line switches to paths if and only if MSYS2_ARG_CONV_EXCL has not been set.

When MSYS2_ARG_CONV_EXCL is set (https://www.msys2.org/wiki/Porting/#filesystem-namespaces) then MSYS2 will not interpret the command line.

@jonahbeckford jonahbeckford mentioned this pull request Sep 1, 2021
@dra27
Copy link
Copy Markdown
Contributor

dra27 commented Sep 3, 2021

I don't think this quite works - the prefix can only be relaxed in MSYS2_ARG_CONV_EXCL is * or explicitly has /v:on and /c in it. However, I think there's a simpler fix than I did 2 years ago in 81f4c8f (it's just possible this feature was so new I failed to read the correct docs!):

diff --git a/msvs-detect b/msvs-detect
index 601575c..89691c6 100755
--- a/msvs-detect
+++ b/msvs-detect
@@ -204,13 +204,6 @@ SCAN_ENV=0
 # Various PATH messing around means it's sensible to know where tools are now
 WHICH=$(which which)

-if [[ $(uname --operating-system 2>/dev/null) = "Msys" ]] ; then
-  # Prevent MSYS from translating command line switches to paths
-  SWITCH_PREFIX='//'
-else
-  SWITCH_PREFIX='/'
-fi
-
 # Parse command-line. At the moment, the short option which usefully combines with anything is -d,
 # so for the time being, combining short options is not permitted, as the loop becomes even less
 # clear with getopts. GNU getopt isn't installed by default on Cygwin...
@@ -920,6 +913,8 @@ for i in "${TEST[@]}" ; do
     if [[ $DEBUG -gt 3 ]] ; then
       printf "Scanning %s... " "$(basename "$SCRIPT") $ARCH_SWITCHES $SCRIPT_SWITCHES">&2
     fi
+    # Setting MSYS2_ARG_CONV_EXCL to * inhibits attempts to convert the flags to COMSPEC as
+    # command line parameters.
     num=0
     while IFS= read -r line; do
       case $num in
@@ -933,7 +928,8 @@ for i in "${TEST[@]}" ; do
       ((num++))
     done < <(INCLUDE='' LIB='' PATH="?msvs-detect?:$DIR:$PATH" ORIGINALPATH='' \
              EXEC_SCRIPT="$(basename "$SCRIPT") $ARCH_SWITCHES $SCRIPT_SWITCHES" \
-             $(cygpath "$COMSPEC") ${SWITCH_PREFIX}v:on ${SWITCH_PREFIX}c $COMMAND 2>/dev/null | grep -F XMARKER -A 3 | tr -d '\015' | tail -3)
+             MSYS2_ARG_CONV_EXCL='*' \
+             $(cygpath "$COMSPEC") /v:on /c $COMMAND 2>/dev/null | grep -F XMARKER -A 3 | tr -d '\015' | tail -3)
     if [[ $DEBUG -gt 3 ]] ; then
       echo done>&2
     fi

(i.e. instead of respecting MSYS2_ARG_CONV_EXCL we can control it!)

@dra27
Copy link
Copy Markdown
Contributor

dra27 commented Sep 3, 2021

That diff makes MSYS2_ARG_CONV_EXCL='*' ./msvs-detect --all which before was hanging - is that what you were seeing, too?

@jonahbeckford
Copy link
Copy Markdown
Contributor Author

jonahbeckford commented Sep 3, 2021 via email

Prevent MSYS from translating command line switches to paths _if and
only if_ MSYS2_ARG_CONV_EXCL has not been set.

When `MSYS2_ARG_CONV_EXCL` is set
(https://www.msys2.org/wiki/Porting/#filesystem-namespaces) then MSYS2
will not interpret the command line.
@jonahbeckford
Copy link
Copy Markdown
Contributor Author

That was a great simplification. Tested it and updated the PR.

@dra27 dra27 merged commit 33ddcfe into metastack:master Sep 6, 2021
@dra27
Copy link
Copy Markdown
Contributor

dra27 commented Sep 6, 2021

Thanks for testing it, too!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants