Win32: fix `Win32OperatingSystem class >> pathOfCommand:` jv
authorJan Vrany <jan.vrany@fit.cvut.cz>
Fri, 18 Jan 2019 11:57:14 +0000
branchjv
changeset 23614 1c6f89bd0de6
parent 23613 27c3cc6f5371
child 23754 41e4acf3b3f6
child 23758 8fcc24554bc7
Win32: fix `Win32OperatingSystem class >> pathOfCommand:` The original implementation was bogus - in some caes it uses hardcoded extension list with typos, in some other it used `#executableFileExtensions`, inconsistent handling of absolute and relative paths and so on. This commit sanitize the code, making it correct (I hope) and robust.
PCFilename.st
Win32OperatingSystem.st
--- a/PCFilename.st	Wed Dec 12 16:28:57 2018 +0100
+++ b/PCFilename.st	Fri Jan 18 11:57:14 2019 +0000
@@ -618,7 +618,7 @@
         ^ true.
     ].
 
-    (#('bat' 'cmd') includes:self suffix asLowercase) ifTrue:[
+    (OperatingSystem executableFileExtensions includes:self suffix asLowercase) ifTrue:[
         ^ (OperatingSystem isValidPath:osName)      
             and:[(OperatingSystem isDirectory:osName) not].
     ].
@@ -634,10 +634,9 @@
         OperatingSystem getBinaryType:'c:\'  
     "
 
-
     "Created: / 16-10-1997 / 13:19:10 / cg"
     "Modified: / 09-09-1998 / 20:17:52 / cg"
-    "Modified: / 23-08-2011 / 21:24:57 / jv"
+    "Modified: / 18-01-2019 / 11:08:51 / jv"
 !
 
 isExplicitRelative
@@ -992,6 +991,11 @@
     ^ '$Header$'
 !
 
+version_HG
+
+    ^ '$Changeset: <not expanded> $'
+!
+
 version_SVN
     ^ '$Id$'
 ! !
--- a/Win32OperatingSystem.st	Wed Dec 12 16:28:57 2018 +0100
+++ b/Win32OperatingSystem.st	Fri Jan 18 11:57:14 2019 +0000
@@ -20,7 +20,7 @@
 AbstractOperatingSystem subclass:#Win32OperatingSystem
 	instanceVariableNames:''
 	classVariableNames:'CurrentDirectory DomainName HostName Initialized LastOsTimeHi
-		LastOsTimeLow LastTimeInfo LastTimeInfoIsLocal'
+		LastOsTimeLow LastTimeInfo LastTimeInfoIsLocal PathExtensions'
 	poolDictionaries:'Win32Constants'
 	category:'OS-Windows'
 !
@@ -3923,79 +3923,81 @@
     self primitiveFailed
 !
 
-pathOfCommand:aCommand
+pathOfCommand:aString
     "find where aCommand's executable file is;
      return its full pathName if there is such a command, otherwise
      return nil."
 
-    |cmdFile path rentry rpath hasSuffix|
-
-    cmdFile := aCommand asFilename.
-    cmdFile isAbsolute ifTrue:[
-	cmdFile isExecutableProgram ifTrue:[
-	    ^ aCommand
-	].
-        cmdFile suffix isEmpty ifTrue:[ 
-            ((path := cmdFile withSuffix: 'com') exists 
-                or:[ (path := cmdFile withSuffix: 'exe') exists 
-                or:[ (path := cmdFile withSuffix: 'bar') exists ]]) ifTrue:[ 
-                    path isExecutable ifTrue:[
-                        ^ path pathName
-                    ]. 
+    |path ext search rentry|
+
+    (aString includes: Filename separator) ifTrue:[
+        "/ If `aString` contains path separator, it is either
+        "/ relative or absolute path. In that case:
+        "/ * first check whether there's file named `aString`.
+        "/ * second, if `aString` has no extension, try same name  with 
+        "/   all executable file extensions. 
+        "/ If anything is found, return the absolute path.
+        path := aString asFilename.
+        ext := path suffix.
+        ext isEmptyOrNil ifTrue:[ 
+            self executableFileExtensions do:[:e | 
+                path := path withSuffix: e.
+                (path exists and:[ path isRegularFile ]) ifTrue:[ 
+                    ^ path asAbsoluteFilename pathName.
                 ].
+            ].
+        ] ifFalse:[ 
+            (self executableFileExtensions includes: ext) ifTrue:[ 
+                (path exists and:[ path isRegularFile ]) ifTrue:[ 
+                    ^ path asAbsoluteFilename pathName.
+                ]. 
+            ].
         ].
-	^ nil
-    ].
-
-    (aCommand includes:Filename separator) ifTrue:[
-	path := Filename currentDirectory construct:aCommand.
-	path isExecutableProgram ifTrue:[
-	    ^ path pathName.
-	].
-	^ nil
-    ].
-
-    "search in all directories of PATH.
-     If there no extension, add the known extensions."
-    path := (self getEnvironment:'PATH') ? ''.
-    (rentry := self registryEntry key: 'HKEY_CURRENT_USER\Environment') notNil ifTrue:[
-	rpath := rentry valueNamed: 'PATH'.
-	rpath notNil ifTrue:[
-	    path := path , self pathSeparator , rpath
-	].
-    ].
-    path := '.;', path.
-    hasSuffix := cmdFile suffix notEmpty.
-
-    (path asCollectionOfSubstringsSeparatedBy:self pathSeparator) do:[:eachDirectory |
-	|file|
-
-	eachDirectory isEmpty ifTrue:[
-	    file := cmdFile
-	] ifFalse:[
-	    file := eachDirectory asFilename construct:aCommand.
-	].
-	hasSuffix ifTrue:[
-	    file isExecutableProgram ifTrue:[
-		^ file pathName.
-	    ].
-	] ifFalse:[
-	    self executableFileExtensions do:[:ext |
-		|fExt|
-
-		fExt := file withSuffix:ext.
-		fExt isExecutableProgram ifTrue:[
-		    ^ fExt pathName.
-		].
-	    ].
-	].
-    ].
+    ] ifFalse:[
+        "/ If it does not contain path separator, then it's
+        "/ 'simple' command name that is to be looked up in PATH.
+
+        "/ First, construct the PATH. 
+
+        "/ Start with current working directory..."
+        search := Array with: Filename currentDirectory.
+
+        "/ ...then add all directories in PATH environment variable...
+        search := search , (((self getEnvironment:'PATH') ? '') splitBy: $;).
+
+        "/ ...and finally get PATH from registry."
+        (rentry := self registryEntry key: 'HKEY_CURRENT_USER\Environment') notNil ifTrue:[
+            search := search , (((rentry valueNamed: 'PATH') ? '') splitBy: $;).
+        ].
+
+        "/ Second, search each directory for given command:
+        search do:[:directoryName | 
+            | directory |
+
+            directory := directoryName asFilename.
+            (directory isAbsolute and:[ directory isDirectory ]) ifTrue:[ 
+                "/ If such directory exists, construct the program name
+                "/ as if it were in that `directory` and see if it is.
+
+                "/ This is a trick: we call this method recursively passing
+                "/ an ABSOLUTE path so it would fall into code in first part of 
+                "/ this method. Note the ^ nil there...
+                path := self pathOfCommand: (directory pathName , Filename separator , aString).
+                "/ ...so if this method returns something, we found it and
+                "/ just return it.
+                path notNil ifTrue:[ ^ path ].
+            ].
+        ].
+    ].
+
+    "/ If we reach this point, no such executable was found
+    "/ along the path, so give up and return nil.
     ^ nil
 
     "
      OperatingSystem pathOfCommand:'bcc32'
      OperatingSystem pathOfCommand:'diff'
-     OperatingSystem pathOfCommand:'cvs'
+     OperatingSystem pathOfCommand:'cvs'    
      OperatingSystem pathOfCommand:'cvs.exe'
      OperatingSystem pathOfCommand:'stx.exe'
      OperatingSystem pathOfCommand:'stx'
@@ -4004,7 +4006,7 @@
 
     "Modified: / 20-01-2012 / 13:32:55 / cg"
     "Modified: / 01-07-2015 / 06:04:56 / Jan Vrany <jan.vrany@fit.cvut.cz>"
-
+    "Modified: / 18-01-2019 / 11:46:23 / jv"
 !
 
 primExec:commandPath commandLine:commandLine environment:environmentOrNil
@@ -8334,10 +8336,40 @@
     "return a collection of extensions for executable program files.
      Only req'd for msdos like systems ..."
 
-    ^ #('com' 'exe' 'bat' 'cmd')
+    | pathext |
+
+    PathExtensions isNil ifTrue:[
+        "/ First, initialize to some (safe) default in case
+        "/ PATHEXT is not available.
+        "/ 
+        "/ Note: maybe we should also add .ps1 these days.
+        PathExtensions := #('com' 'exe' 'bat' 'cmd').
+
+        "/ If PATHEXT environment is available, use all extensions
+        "/ in that one but make sure defaults aboce are there
+        "/ in case of malformed PATHEXT.
+
+        pathext := self getEnvironment:'PATHEXT'.  
+        pathext notNil ifTrue:[ 
+            pathext := pathext asLowercase.
+            (pathext splitBy: $;) do:[:dotext |
+                | ext |
+
+                ext := dotext copyFrom: 2.
+                (PathExtensions includes: ext) ifFalse:[ 
+                    PathExtensions := PathExtensions copyWith: ext.
+                ].
+            ].
+        ].
+    ].
+    ^ PathExtensions 
+
+    "
+    OperatingSystem executableFileExtensions
+    "
 
     "Created: / 02-05-1997 / 11:42:29 / cg"
-    "Modified: / 23-08-2011 / 21:14:45 / jv"
+    "Modified: / 18-01-2019 / 11:06:55 / jv"
 !
 
 expandEnvironmentStrings:aString